fix: address authorizer review comments

- Normalize ghcr.io hostname using net.SplitHostPort + strings.EqualFold
- Save and restore ForceAttemptOAuth2 via defer so it is always false
  after Do() returns, fixing regression tests
- Explicitly switch to basic auth (ForceAttemptOAuth2=false) before
  retrying after a 401/403 error so the second attempt uses a different
  auth mode
- Fix require.NoError called from goroutine in concurrent test; use
  t.Errorf+return instead (testify require is not goroutine-safe)
- Replace TestAuthorizer_Do_StatusCodeErrorChecking with tests that
  exercise Authorizer.Do directly: RetriesOn401 and NoRetryOn404
- Restore TestLogin_ResetsForceAttemptOAuth2_OnSuccess and
  TestLogin_ResetsForceAttemptOAuth2_OnFailure regression tests that
  were removed; these now pass with the defer-based restore

Signed-off-by: Terry Howe <terrylhowe@gmail.com>

pkg/registry/authorizer.go

@@ -18,6 +18,7 @@ package registry
 
 import (
 	"context"
+	"net"
 	"net/http"
 	"strings"
 	"sync"
@@ -34,6 +35,15 @@ type Authorizer struct {
 	attemptBearerAuthentication bool
 }
 
+// isGHCR reports whether host refers to ghcr.io, normalizing port and case.
+func isGHCR(host string) bool {
+	h, _, err := net.SplitHostPort(host)
+	if err != nil {
+		h = host
+	}
+	return strings.EqualFold(h, "ghcr.io")
+}
+
 func NewAuthorizer(httpClient *http.Client, credentialsStore credentials.Store, username, password string) *Authorizer {
 	authorizer := Authorizer{
 		Client: auth.Client{
@@ -82,15 +92,18 @@ func (a *Authorizer) setForceAttemptOAuth2(value bool) {
 	a.ForceAttemptOAuth2 = value
 }
 
-// Do This method wraps auth.Client.Do in attempt to retry authentication
+// Do wraps auth.Client.Do to retry with fallback authentication on 401/403 errors.
 func (a *Authorizer) Do(originalReq *http.Request) (*http.Response, error) {
 	if a.getAttemptBearerAuthentication() {
 		needsAuthentication := originalReq.Header.Get("Authorization") == ""
 		if needsAuthentication {
-			a.setForceAttemptOAuth2(true)
-			if originalReq.Host == "ghcr.io" {
+			if isGHCR(originalReq.Host) {
 				a.setForceAttemptOAuth2(false)
 				a.setAttemptBearerAuthentication(false)
+			} else {
+				prev := a.getForceAttemptOAuth2()
+				a.setForceAttemptOAuth2(true)
+				defer a.setForceAttemptOAuth2(prev)
 			}
 			resp, err := a.Client.Do(originalReq)
 			if err == nil {
@@ -101,6 +114,8 @@ func (a *Authorizer) Do(originalReq *http.Request) (*http.Response, error) {
 				!strings.Contains(err.Error(), "response status code 403") {
 				return nil, err
 			}
+			// Switch to basic auth fallback before retrying
+			a.setForceAttemptOAuth2(false)
 		}
 	}
 	return a.Client.Do(originalReq)

pkg/registry/authorizer_test.go

@@ -21,7 +21,6 @@ import (
 	"errors"
 	"net/http"
 	"net/http/httptest"
-	"strings"
 	"sync"
 	"testing"
 
@@ -136,7 +135,7 @@ func TestAuthorizer_Do(t *testing.T) {
 			host:                  "registry.example.com",
 			authHeader:            "",
 			serverStatus:          200,
-			expectForceOAuth2:     true,
+			expectForceOAuth2:     false,
 			expectBearerAuthAfter: false,
 		},
 		{
@@ -267,7 +266,10 @@ func TestAuthorizer_ConcurrentAccess(t *testing.T) {
 			defer wg.Done()
 			for j := 0; j < numRequests; j++ {
 				req, err := http.NewRequest(http.MethodGet, server.URL, nil)
-				require.NoError(t, err)
+				if err != nil {
+					t.Errorf("failed to create request: %v", err)
+					return
+				}
 				req.Host = "registry.example.com"
 
 				resp, err := authorizer.Do(req)
@@ -298,48 +300,64 @@ func TestAuthorizer_ConcurrentAccess(t *testing.T) {
 	wg.Wait()
 }
 
-func TestAuthorizer_Do_StatusCodeErrorChecking(t *testing.T) {
-	tests := []struct {
-		name        string
-		errorMsg    string
-		shouldRetry bool
-		description string
-	}{
-		{
-			name:        "retry on 401 error",
-			errorMsg:    "response status code 401",
-			shouldRetry: true,
-			description: "401 errors should trigger retry logic",
-		},
-		{
-			name:        "retry on 403 error",
-			errorMsg:    "response status code 403",
-			shouldRetry: true,
-			description: "403 errors should trigger retry logic",
-		},
-		{
-			name:        "no retry on 404 error",
-			errorMsg:    "response status code 404",
-			shouldRetry: false,
-			description: "404 errors should not trigger retry logic",
-		},
-		{
-			name:        "no retry on 500 error",
-			errorMsg:    "response status code 500",
-			shouldRetry: false,
-			description: "500 errors should not trigger retry logic",
-		},
-	}
+// roundTripFunc is an http.RoundTripper backed by a function.
+type roundTripFunc func(*http.Request) (*http.Response, error)
+
+func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { return f(req) }
+
+func TestAuthorizer_Do_RetriesOn401(t *testing.T) {
+	var mu sync.Mutex
+	callCount := 0
+
+	transport := roundTripFunc(func(req *http.Request) (*http.Response, error) {
+		mu.Lock()
+		n := callCount
+		callCount++
+		mu.Unlock()
+		if n == 0 {
+			// Simulate ORAS auth failure with 401
+			return nil, errors.New("response status code 401")
+		}
+		return &http.Response{
+			StatusCode: http.StatusOK,
+			Body:       http.NoBody,
+			Header:     make(http.Header),
+		}, nil
+	})
+
+	authorizer := NewAuthorizer(&http.Client{Transport: transport}, &mockCredentialsStore{}, "", "")
+
+	req, err := http.NewRequest(http.MethodGet, "http://registry.example.com/v2/", nil)
+	require.NoError(t, err)
+	req.Host = "registry.example.com"
 
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			err := errors.New(tt.errorMsg)
+	resp, err := authorizer.Do(req)
+	require.NoError(t, err)
+	require.NotNil(t, resp)
 
-			should401Retry := strings.Contains(err.Error(), "response status code 401")
-			should403Retry := strings.Contains(err.Error(), "response status code 403")
-			actualShouldRetry := should401Retry || should403Retry
+	assert.Equal(t, 2, callCount, "Authorizer.Do should retry once after 401 error")
+	assert.False(t, authorizer.getForceAttemptOAuth2(), "ForceAttemptOAuth2 must be false after Do()")
+}
 
-			assert.Equal(t, tt.shouldRetry, actualShouldRetry, tt.description)
-		})
-	}
+func TestAuthorizer_Do_NoRetryOn404(t *testing.T) {
+	var mu sync.Mutex
+	callCount := 0
+
+	transport := roundTripFunc(func(req *http.Request) (*http.Response, error) {
+		mu.Lock()
+		callCount++
+		mu.Unlock()
+		return nil, errors.New("response status code 404")
+	})
+
+	authorizer := NewAuthorizer(&http.Client{Transport: transport}, &mockCredentialsStore{}, "", "")
+
+	req, err := http.NewRequest(http.MethodGet, "http://registry.example.com/v2/", nil)
+	require.NoError(t, err)
+	req.Host = "registry.example.com"
+
+	_, err = authorizer.Do(req)
+	assert.Error(t, err, "404 error should propagate without retry")
+	assert.Equal(t, 1, callCount, "Authorizer.Do must not retry on non-401/403 errors")
+	assert.False(t, authorizer.getForceAttemptOAuth2(), "ForceAttemptOAuth2 must be false after Do()")
 }