fix(auth): try JWT validation before password in token exchange
In password-auth mode, the /auth/token endpoint only tried ValidatePassword, rejecting JWTs issued by gordon auth login. Now ValidateToken is tried first regardless of auth type, falling back to password validation. Fixes push/preview 401 on password-auth servers.
- commit
- 2b4c159e2aee1c4e5b8d9a01a5fcda8ec7f63fb7
- author
- brice · · 2026-03-19 06:44 UTC
- parent
- b72aa77
- signature
- unsigned
2 files · +43 -10
M internal/adapters/in/http/auth/handler.go +12 -7
| @@ -275,13 +275,18 @@ func (h *Handler) authenticateTokenCredentials(ctx context.Context, r *http.Requ | |||
| 275 | 275 | return true, nil | |
| 276 | 276 | } | |
| 277 | 277 | ||
| 278 | - | switch h.authSvc.GetAuthType() { | |
| 279 | - | case domain.AuthTypePassword: | |
| 280 | - | return h.authSvc.ValidatePassword(ctx, username, password), nil | |
| 281 | - | case domain.AuthTypeToken: | |
| 282 | - | claims, err := h.authSvc.ValidateToken(ctx, password) | |
| 283 | - | if err == nil && claims.Subject == username { | |
| 284 | - | return true, claims | |
| 278 | + | // Always try JWT token validation first — even in password-auth mode the | |
| 279 | + | // server issues JWTs on login, and the CLI sends them back via Basic Auth | |
| 280 | + | // for registry token exchange. | |
| 281 | + | claims, err := h.authSvc.ValidateToken(ctx, password) | |
| 282 | + | if err == nil && claims.Subject == username { | |
| 283 | + | return true, claims | |
| 284 | + | } | |
| 285 | + | ||
| 286 | + | // Fall back to password validation for direct username/password credentials. | |
| 287 | + | if h.authSvc.GetAuthType() == domain.AuthTypePassword { | |
| 288 | + | if h.authSvc.ValidatePassword(ctx, username, password) { | |
| 289 | + | return true, nil | |
| 285 | 290 | } | |
| 286 | 291 | } | |
| 287 | 292 | ||
| @@ -275,13 +275,18 @@ func (h *Handler) authenticateTokenCredentials(ctx context.Context, r *http.Requ | |||||
| 275 | return true, nil | 275 | return true, nil | ||
| 276 | } | 276 | } | ||
| 277 | 277 | ||||
| 278 | - | switch h.authSvc.GetAuthType() { | 278 | + | // Always try JWT token validation first — even in password-auth mode the |
| 279 | - | case domain.AuthTypePassword: | 279 | + | // server issues JWTs on login, and the CLI sends them back via Basic Auth |
| 280 | - | return h.authSvc.ValidatePassword(ctx, username, password), nil | 280 | + | // for registry token exchange. |
| 281 | - | case domain.AuthTypeToken: | 281 | + | claims, err := h.authSvc.ValidateToken(ctx, password) |
| 282 | - | claims, err := h.authSvc.ValidateToken(ctx, password) | 282 | + | if err == nil && claims.Subject == username { |
| 283 | - | if err == nil && claims.Subject == username { | 283 | + | return true, claims |
| 284 | - | return true, claims | 284 | + | } |
| 285 | + | ||||
| 286 | + | // Fall back to password validation for direct username/password credentials. | |||
| 287 | + | if h.authSvc.GetAuthType() == domain.AuthTypePassword { | |||
| 288 | + | if h.authSvc.ValidatePassword(ctx, username, password) { | |||
| 289 | + | return true, nil | |||
| 285 | } | 290 | } | ||
| 286 | } | 291 | } | ||
| 287 | 292 | ||||
M internal/adapters/in/http/auth/handler_test.go +31 -3
| @@ -3,6 +3,7 @@ package auth | |||
| 3 | 3 | import ( | |
| 4 | 4 | "bytes" | |
| 5 | 5 | "encoding/json" | |
| 6 | + | "fmt" | |
| 6 | 7 | "net/http" | |
| 7 | 8 | "net/http/httptest" | |
| 8 | 9 | "testing" | |
| @@ -158,6 +159,8 @@ func TestHandler_Token_Success(t *testing.T) { | |||
| 158 | 159 | authSvc := mocks.NewMockAuthService(t) | |
| 159 | 160 | ||
| 160 | 161 | authSvc.EXPECT().IsEnabled().Return(true) | |
| 162 | + | // ValidateToken is tried first; plain password fails JWT parsing | |
| 163 | + | authSvc.EXPECT().ValidateToken(mock.Anything, "secret").Return(nil, fmt.Errorf("not a token")) | |
| 161 | 164 | authSvc.EXPECT().GetAuthType().Return(domain.AuthTypePassword) | |
| 162 | 165 | authSvc.EXPECT().ValidatePassword(mock.Anything, "admin", "secret").Return(true) | |
| 163 | 166 | authSvc.EXPECT().GenerateAccessToken(mock.Anything, "admin", []string{"repository:myrepo:pull"}, 5*time.Minute). | |
| @@ -215,6 +218,7 @@ func TestHandler_Token_InvalidCredentials(t *testing.T) { | |||
| 215 | 218 | authSvc := mocks.NewMockAuthService(t) | |
| 216 | 219 | ||
| 217 | 220 | authSvc.EXPECT().IsEnabled().Return(true) | |
| 221 | + | authSvc.EXPECT().ValidateToken(mock.Anything, "wrongpassword").Return(nil, fmt.Errorf("not a token")) | |
| 218 | 222 | authSvc.EXPECT().GetAuthType().Return(domain.AuthTypePassword) | |
| 219 | 223 | authSvc.EXPECT().ValidatePassword(mock.Anything, "admin", "wrongpassword").Return(false) | |
| 220 | 224 | ||
| @@ -261,7 +265,6 @@ func TestHandler_Token_TokenAuth(t *testing.T) { | |||
| 261 | 265 | authSvc := mocks.NewMockAuthService(t) | |
| 262 | 266 | ||
| 263 | 267 | authSvc.EXPECT().IsEnabled().Return(true) | |
| 264 | - | authSvc.EXPECT().GetAuthType().Return(domain.AuthTypeToken) | |
| 265 | 268 | authSvc.EXPECT().ValidateToken(mock.Anything, "existing-jwt-token").Return(&domain.TokenClaims{ | |
| 266 | 269 | Subject: "myuser", | |
| 267 | 270 | Scopes: []string{"repository:*:pull"}, | |
| @@ -289,7 +292,6 @@ func TestHandler_Token_TokenAuthScopeIntersection(t *testing.T) { | |||
| 289 | 292 | authSvc := mocks.NewMockAuthService(t) | |
| 290 | 293 | ||
| 291 | 294 | authSvc.EXPECT().IsEnabled().Return(true) | |
| 292 | - | authSvc.EXPECT().GetAuthType().Return(domain.AuthTypeToken) | |
| 293 | 295 | authSvc.EXPECT().ValidateToken(mock.Anything, "existing-jwt-token").Return(&domain.TokenClaims{ | |
| 294 | 296 | Subject: "myuser", | |
| 295 | 297 | Scopes: []string{"repository:myrepo:pull"}, | |
| @@ -316,7 +318,6 @@ func TestHandler_Token_TokenAuthScopeDenied(t *testing.T) { | |||
| 316 | 318 | authSvc := mocks.NewMockAuthService(t) | |
| 317 | 319 | ||
| 318 | 320 | authSvc.EXPECT().IsEnabled().Return(true) | |
| 319 | - | authSvc.EXPECT().GetAuthType().Return(domain.AuthTypeToken) | |
| 320 | 321 | authSvc.EXPECT().ValidateToken(mock.Anything, "existing-jwt-token").Return(&domain.TokenClaims{ | |
| 321 | 322 | Subject: "myuser", | |
| 322 | 323 | Scopes: []string{"repository:myrepo:pull"}, | |
| @@ -334,6 +335,33 @@ func TestHandler_Token_TokenAuthScopeDenied(t *testing.T) { | |||
| 334 | 335 | assert.Contains(t, rec.Body.String(), "insufficient scope") | |
| 335 | 336 | } | |
| 336 | 337 | ||
| 338 | + | func TestHandler_Token_PasswordAuthWithJWT(t *testing.T) { | |
| 339 | + | authSvc := mocks.NewMockAuthService(t) | |
| 340 | + | ||
| 341 | + | // Auth type is password, but the client sends a JWT (from gordon auth login) | |
| 342 | + | authSvc.EXPECT().IsEnabled().Return(true) | |
| 343 | + | authSvc.EXPECT().ValidateToken(mock.Anything, "jwt-from-login").Return(&domain.TokenClaims{ | |
| 344 | + | Subject: "brissou", | |
| 345 | + | Scopes: []string{"*"}, | |
| 346 | + | }, nil) | |
| 347 | + | authSvc.EXPECT().GenerateAccessToken(mock.Anything, "brissou", []string{"repository:*:push,pull"}, 5*time.Minute). | |
| 348 | + | Return("short-lived-token", nil) | |
| 349 | + | ||
| 350 | + | handler := NewHandler(authSvc, InternalAuth{}, testLogger()) | |
| 351 | + | ||
| 352 | + | req := httptest.NewRequest(http.MethodGet, "/auth/token?scope=repository:*:push,pull", nil) | |
| 353 | + | req.SetBasicAuth("brissou", "jwt-from-login") | |
| 354 | + | rec := httptest.NewRecorder() | |
| 355 | + | ||
| 356 | + | handler.ServeHTTP(rec, req) | |
| 357 | + | ||
| 358 | + | assert.Equal(t, http.StatusOK, rec.Code) | |
| 359 | + | var resp map[string]any | |
| 360 | + | err := json.NewDecoder(rec.Body).Decode(&resp) | |
| 361 | + | assert.NoError(t, err) | |
| 362 | + | assert.Equal(t, "short-lived-token", resp["token"]) | |
| 363 | + | } | |
| 364 | + | ||
| 337 | 365 | func TestHandler_Token_MethodNotAllowed(t *testing.T) { | |
| 338 | 366 | handler := NewHandler(nil, InternalAuth{}, testLogger()) | |
| 339 | 367 | ||
| @@ -3,6 +3,7 @@ package auth | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "bytes" | 4 | "bytes" | ||
| 5 | "encoding/json" | 5 | "encoding/json" | ||
| 6 | + | "fmt" | |||
| 6 | "net/http" | 7 | "net/http" | ||
| 7 | "net/http/httptest" | 8 | "net/http/httptest" | ||
| 8 | "testing" | 9 | "testing" | ||
| @@ -158,6 +159,8 @@ func TestHandler_Token_Success(t *testing.T) { | |||||
| 158 | authSvc := mocks.NewMockAuthService(t) | 159 | authSvc := mocks.NewMockAuthService(t) | ||
| 159 | 160 | ||||
| 160 | authSvc.EXPECT().IsEnabled().Return(true) | 161 | authSvc.EXPECT().IsEnabled().Return(true) | ||
| 162 | + | // ValidateToken is tried first; plain password fails JWT parsing | |||
| 163 | + | authSvc.EXPECT().ValidateToken(mock.Anything, "secret").Return(nil, fmt.Errorf("not a token")) | |||
| 161 | authSvc.EXPECT().GetAuthType().Return(domain.AuthTypePassword) | 164 | authSvc.EXPECT().GetAuthType().Return(domain.AuthTypePassword) | ||
| 162 | authSvc.EXPECT().ValidatePassword(mock.Anything, "admin", "secret").Return(true) | 165 | authSvc.EXPECT().ValidatePassword(mock.Anything, "admin", "secret").Return(true) | ||
| 163 | authSvc.EXPECT().GenerateAccessToken(mock.Anything, "admin", []string{"repository:myrepo:pull"}, 5*time.Minute). | 166 | authSvc.EXPECT().GenerateAccessToken(mock.Anything, "admin", []string{"repository:myrepo:pull"}, 5*time.Minute). | ||
| @@ -215,6 +218,7 @@ func TestHandler_Token_InvalidCredentials(t *testing.T) { | |||||
| 215 | authSvc := mocks.NewMockAuthService(t) | 218 | authSvc := mocks.NewMockAuthService(t) | ||
| 216 | 219 | ||||
| 217 | authSvc.EXPECT().IsEnabled().Return(true) | 220 | authSvc.EXPECT().IsEnabled().Return(true) | ||
| 221 | + | authSvc.EXPECT().ValidateToken(mock.Anything, "wrongpassword").Return(nil, fmt.Errorf("not a token")) | |||
| 218 | authSvc.EXPECT().GetAuthType().Return(domain.AuthTypePassword) | 222 | authSvc.EXPECT().GetAuthType().Return(domain.AuthTypePassword) | ||
| 219 | authSvc.EXPECT().ValidatePassword(mock.Anything, "admin", "wrongpassword").Return(false) | 223 | authSvc.EXPECT().ValidatePassword(mock.Anything, "admin", "wrongpassword").Return(false) | ||
| 220 | 224 | ||||
| @@ -261,7 +265,6 @@ func TestHandler_Token_TokenAuth(t *testing.T) { | |||||
| 261 | authSvc := mocks.NewMockAuthService(t) | 265 | authSvc := mocks.NewMockAuthService(t) | ||
| 262 | 266 | ||||
| 263 | authSvc.EXPECT().IsEnabled().Return(true) | 267 | authSvc.EXPECT().IsEnabled().Return(true) | ||
| 264 | - | authSvc.EXPECT().GetAuthType().Return(domain.AuthTypeToken) | |||
| 265 | authSvc.EXPECT().ValidateToken(mock.Anything, "existing-jwt-token").Return(&domain.TokenClaims{ | 268 | authSvc.EXPECT().ValidateToken(mock.Anything, "existing-jwt-token").Return(&domain.TokenClaims{ | ||
| 266 | Subject: "myuser", | 269 | Subject: "myuser", | ||
| 267 | Scopes: []string{"repository:*:pull"}, | 270 | Scopes: []string{"repository:*:pull"}, | ||
| @@ -289,7 +292,6 @@ func TestHandler_Token_TokenAuthScopeIntersection(t *testing.T) { | |||||
| 289 | authSvc := mocks.NewMockAuthService(t) | 292 | authSvc := mocks.NewMockAuthService(t) | ||
| 290 | 293 | ||||
| 291 | authSvc.EXPECT().IsEnabled().Return(true) | 294 | authSvc.EXPECT().IsEnabled().Return(true) | ||
| 292 | - | authSvc.EXPECT().GetAuthType().Return(domain.AuthTypeToken) | |||
| 293 | authSvc.EXPECT().ValidateToken(mock.Anything, "existing-jwt-token").Return(&domain.TokenClaims{ | 295 | authSvc.EXPECT().ValidateToken(mock.Anything, "existing-jwt-token").Return(&domain.TokenClaims{ | ||
| 294 | Subject: "myuser", | 296 | Subject: "myuser", | ||
| 295 | Scopes: []string{"repository:myrepo:pull"}, | 297 | Scopes: []string{"repository:myrepo:pull"}, | ||
| @@ -316,7 +318,6 @@ func TestHandler_Token_TokenAuthScopeDenied(t *testing.T) { | |||||
| 316 | authSvc := mocks.NewMockAuthService(t) | 318 | authSvc := mocks.NewMockAuthService(t) | ||
| 317 | 319 | ||||
| 318 | authSvc.EXPECT().IsEnabled().Return(true) | 320 | authSvc.EXPECT().IsEnabled().Return(true) | ||
| 319 | - | authSvc.EXPECT().GetAuthType().Return(domain.AuthTypeToken) | |||
| 320 | authSvc.EXPECT().ValidateToken(mock.Anything, "existing-jwt-token").Return(&domain.TokenClaims{ | 321 | authSvc.EXPECT().ValidateToken(mock.Anything, "existing-jwt-token").Return(&domain.TokenClaims{ | ||
| 321 | Subject: "myuser", | 322 | Subject: "myuser", | ||
| 322 | Scopes: []string{"repository:myrepo:pull"}, | 323 | Scopes: []string{"repository:myrepo:pull"}, | ||
| @@ -334,6 +335,33 @@ func TestHandler_Token_TokenAuthScopeDenied(t *testing.T) { | |||||
| 334 | assert.Contains(t, rec.Body.String(), "insufficient scope") | 335 | assert.Contains(t, rec.Body.String(), "insufficient scope") | ||
| 335 | } | 336 | } | ||
| 336 | 337 | ||||
| 338 | + | func TestHandler_Token_PasswordAuthWithJWT(t *testing.T) { | |||
| 339 | + | authSvc := mocks.NewMockAuthService(t) | |||
| 340 | + | ||||
| 341 | + | // Auth type is password, but the client sends a JWT (from gordon auth login) | |||
| 342 | + | authSvc.EXPECT().IsEnabled().Return(true) | |||
| 343 | + | authSvc.EXPECT().ValidateToken(mock.Anything, "jwt-from-login").Return(&domain.TokenClaims{ | |||
| 344 | + | Subject: "brissou", | |||
| 345 | + | Scopes: []string{"*"}, | |||
| 346 | + | }, nil) | |||
| 347 | + | authSvc.EXPECT().GenerateAccessToken(mock.Anything, "brissou", []string{"repository:*:push,pull"}, 5*time.Minute). | |||
| 348 | + | Return("short-lived-token", nil) | |||
| 349 | + | ||||
| 350 | + | handler := NewHandler(authSvc, InternalAuth{}, testLogger()) | |||
| 351 | + | ||||
| 352 | + | req := httptest.NewRequest(http.MethodGet, "/auth/token?scope=repository:*:push,pull", nil) | |||
| 353 | + | req.SetBasicAuth("brissou", "jwt-from-login") | |||
| 354 | + | rec := httptest.NewRecorder() | |||
| 355 | + | ||||
| 356 | + | handler.ServeHTTP(rec, req) | |||
| 357 | + | ||||
| 358 | + | assert.Equal(t, http.StatusOK, rec.Code) | |||
| 359 | + | var resp map[string]any | |||
| 360 | + | err := json.NewDecoder(rec.Body).Decode(&resp) | |||
| 361 | + | assert.NoError(t, err) | |||
| 362 | + | assert.Equal(t, "short-lived-token", resp["token"]) | |||
| 363 | + | } | |||
| 364 | + | ||||
| 337 | func TestHandler_Token_MethodNotAllowed(t *testing.T) { | 365 | func TestHandler_Token_MethodNotAllowed(t *testing.T) { | ||
| 338 | handler := NewHandler(nil, InternalAuth{}, testLogger()) | 366 | handler := NewHandler(nil, InternalAuth{}, testLogger()) | ||
| 339 | 367 | ||||