diff --git a/pkg/appview/server.go b/pkg/appview/server.go index 61fdc74..375d154 100644 --- a/pkg/appview/server.go +++ b/pkg/appview/server.go @@ -608,7 +608,12 @@ func NewAppViewServer(cfg *Config, branding *BrandingOverrides) (*AppViewServer, return nil }) + // Both Docker token specs are served on the same path: GET is the + // original Basic-auth form, POST is the OAuth2 form that containerd and + // Docker try first whenever they hold a secret. Serving only GET made + // every such client eat a 405 and retry. mainRouter.Get("/auth/token", tokenHandler.ServeHTTP) + mainRouter.Post("/auth/token", tokenHandler.ServeHTTP) // Device authorization endpoints (public) routes.RegisterDeviceEndpoints(mainRouter, s.DeviceStore, baseURL) diff --git a/pkg/auth/token/handler.go b/pkg/auth/token/handler.go index 6a6f470..d53fc7d 100644 --- a/pkg/auth/token/handler.go +++ b/pkg/auth/token/handler.go @@ -2,6 +2,7 @@ package token import ( "context" + "encoding/json" "errors" "fmt" "log/slog" @@ -134,16 +135,18 @@ func (h *Handler) SetServices(services []string) { // resolveService picks the registry domain to stamp as the JWT's audience. // -// The ?service= query parameter is the primary signal: Docker echoes back -// whatever the WWW-Authenticate challenge advertised, and that challenge is -// built per front door. The request's own host is the fallback, which covers -// clients that reach /auth/token directly on a registry domain rather than via -// the realm. Both are client-influenced, so both are only honoured when they -// name a configured registry domain; anything else falls back to the issuer's -// service. That makes the worst case a token scoped to the primary domain, not -// a caller-chosen audience. -func (h *Handler) resolveService(r *http.Request) string { - if s := NormalizeService(r.URL.Query().Get("service")); h.services[s] { +// The requested service is the primary signal: Docker echoes back whatever the +// WWW-Authenticate challenge advertised, and that challenge is built per front +// door. It arrives in the ?service= query parameter on the GET form and in the +// service form field on the POST form, so callers extract it per method and +// pass it in. The request's own host is the fallback, which covers clients that +// reach /auth/token directly on a registry domain rather than via the realm. +// Both are client-influenced, so both are only honoured when they name a +// configured registry domain; anything else falls back to the issuer's service. +// That makes the worst case a token scoped to the primary domain, not a +// caller-chosen audience. +func (h *Handler) resolveService(r *http.Request, requested string) string { + if s := NormalizeService(requested); h.services[s] { return s } if s := NormalizeService(r.Host); h.services[s] { @@ -219,35 +222,100 @@ func sendOAuthSessionExpiredError(w http.ResponseWriter, r *http.Request) { // before the first PDS call, and these phase timings exist to attribute them. const slowPhaseThreshold = 5 * time.Second -// ServeHTTP handles the token request +// oauthError is the RFC 6749 section 5.2 error body used by the POST form. +type oauthError struct { + Error string `json:"error"` + ErrorDescription string `json:"error_description,omitempty"` +} + +// writeOAuthError responds in the shape the OAuth2 token spec defines. Only the +// POST form uses it; the GET form keeps its plain-text guidance, which is what +// users actually see when they run docker login by hand. +func writeOAuthError(w http.ResponseWriter, status int, code, description string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + if err := json.NewEncoder(w).Encode(oauthError{Error: code, ErrorDescription: description}); err != nil { + slog.Warn("failed to write OAuth error response", "error", err) + } +} + +// ServeHTTP handles the token request. +// +// Both Docker token specs land here. The original spec is GET with HTTP Basic +// and the scope in the query string. The OAuth2 spec is POST with a form-encoded +// body; containerd and Docker try it first whenever they hold a secret and only +// fall back to the GET form on 404/401/405, so answering it saves every such +// client a wasted round trip. Once credentials and scope are extracted the two +// paths are identical. func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { phaseStart := time.Now() slog.Debug("Received token request", "method", r.Method, "path", r.URL.Path) - // Only accept GET requests (per Docker spec) - if r.Method != http.MethodGet { + var username, password, scopeParam, requestedService string + + switch r.Method { + case http.MethodGet: + var ok bool + username, password, ok = r.BasicAuth() + if !ok { + slog.Debug("No Basic auth credentials provided") + sendAuthError(w, r, "authentication required") + return + } + scopeParam = r.URL.Query().Get("scope") + requestedService = r.URL.Query().Get("service") + + case http.MethodPost: + if err := r.ParseForm(); err != nil { + writeOAuthError(w, http.StatusBadRequest, "invalid_request", "malformed request body") + return + } + // Only the password grant is supported, and no refresh token is issued. + // The registry JWT's lifetime is deliberately pinned to the AppView<->hold + // service-auth (see SetServiceAuthFetcher), so a refresh token would be a + // fourth long-lived credential with its own storage and revocation. + // Clients handle its absence by continuing to use the credential they hold. + // + // The status here is deliberately 401 rather than the 400 that RFC 6749 + // section 5.2 prescribes. containerd only sends grant_type=refresh_token + // when it has no username, which is the same condition that disables its + // 405 fallback, so a 400 would hard-fail those clients. 401 is on its + // retry list, sending them to the GET form, where a device secret + // authenticates off the password alone and succeeds. + if grant := r.PostFormValue("grant_type"); grant != "" && grant != "password" { + writeOAuthError(w, http.StatusUnauthorized, "unsupported_grant_type", + fmt.Sprintf("grant_type %q is not supported, use password", grant)) + return + } + username = r.PostFormValue("username") + password = r.PostFormValue("password") + if username == "" || password == "" { + // 401 is one of the statuses clients retry on the GET form, where the + // plain-text guidance in sendAuthError is waiting for them. + writeOAuthError(w, http.StatusUnauthorized, "invalid_client", + "username and password are required") + return + } + scopeParam = r.PostFormValue("scope") + requestedService = r.PostFormValue("service") + + default: http.Error(w, "method not allowed", http.StatusMethodNotAllowed) return } - // Extract Basic auth credentials - username, password, ok := r.BasicAuth() - if !ok { - slog.Debug("No Basic auth credentials provided") - sendAuthError(w, r, "authentication required") - return - } - - // Reconstruct DID usernames that were mangled by BasicAuth's colon split + // Reconstruct DID usernames that were mangled by BasicAuth's colon split. + // Form bodies carry the username as a discrete field so only the + // hyphen-encoding case can fire there, but running both keeps the two paths + // accepting exactly the same set of usernames. username, password = parseBasicAuthDID(username, password) - slog.Debug("Got Basic auth credentials", "username", username, "passwordLength", len(password)) + slog.Debug("Got credentials", "username", username, "passwordLength", len(password)) - // Parse query parameters. The service names the front door the client is - // authenticating against and becomes the JWT's audience; resolveService - // validates it against the configured registry domains. - service := h.resolveService(r) - scopeParam := r.URL.Query().Get("scope") + // The service names the front door the client is authenticating against and + // becomes the JWT's audience; resolveService validates it against the + // configured registry domains. + service := h.resolveService(r, requestedService) // Parse scopes var scopes []string diff --git a/pkg/auth/token/handler_test.go b/pkg/auth/token/handler_test.go index 80dc7c0..f42f389 100644 --- a/pkg/auth/token/handler_test.go +++ b/pkg/auth/token/handler_test.go @@ -9,6 +9,7 @@ import ( "errors" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "strings" @@ -166,8 +167,8 @@ func TestHandler_ServeHTTP_WrongMethod(t *testing.T) { handler := NewHandler(issuer, nil) - // Try POST instead of GET - req := httptest.NewRequest(http.MethodPost, "/auth/token", nil) + // GET and POST are both real token specs; anything else is not. + req := httptest.NewRequest(http.MethodPut, "/auth/token", nil) w := httptest.NewRecorder() handler.ServeHTTP(w, req) @@ -177,6 +178,115 @@ func TestHandler_ServeHTTP_WrongMethod(t *testing.T) { } } +// newOAuthTokenRequest builds a POST request in the OAuth2 token spec's +// form-encoded shape. +func newOAuthTokenRequest(form url.Values) *http.Request { + req := httptest.NewRequest(http.MethodPost, "/auth/token", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return req +} + +func TestHandler_ServeHTTP_OAuthPost_UnsupportedGrant(t *testing.T) { + keyPath := getSharedTestKey(t) + + issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) + if err != nil { + t.Fatalf("NewIssuer() error = %v", err) + } + + handler := NewHandler(issuer, nil) + + // We issue no refresh tokens, so the refresh grant is rejected. The status + // must stay 401 and not the RFC's 400: clients that send this grant have no + // username, so 401 is the only answer that still routes them to the GET form + // instead of failing them outright. + req := newOAuthTokenRequest(url.Values{ + "grant_type": {"refresh_token"}, + "refresh_token": {"whatever"}, + "service": {"registry"}, + }) + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("Expected status %d, got %d", http.StatusUnauthorized, w.Code) + } + + var body oauthError + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("response body is not an OAuth error object: %v", err) + } + if body.Error != "unsupported_grant_type" { + t.Errorf("error = %q, want unsupported_grant_type", body.Error) + } +} + +func TestHandler_ServeHTTP_OAuthPost_MissingCredentials(t *testing.T) { + keyPath := getSharedTestKey(t) + + issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) + if err != nil { + t.Fatalf("NewIssuer() error = %v", err) + } + + handler := NewHandler(issuer, nil) + + // 401 is one of the statuses clients retry on the GET form, so an empty + // body must not come back as anything else. + req := newOAuthTokenRequest(url.Values{"grant_type": {"password"}}) + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Fatalf("Expected status %d, got %d", http.StatusUnauthorized, w.Code) + } +} + +func TestHandler_ServeHTTP_OAuthPost_DeviceAuth_Valid(t *testing.T) { + keyPath := getSharedTestKey(t) + + issuer, err := NewIssuer(keyPath, "atcr.io", "registry", 15*time.Minute) + if err != nil { + t.Fatalf("NewIssuer() error = %v", err) + } + + deviceStore, database := setupTestDeviceStore(t) + deviceSecret := createTestDevice(t, deviceStore, database, "did:plc:user123", "alice.bsky.social") + + handler := NewHandler(issuer, deviceStore) + + // Same credentials and scope as the GET device-auth test, delivered in the + // form body instead. The two paths must issue equivalent tokens. + req := newOAuthTokenRequest(url.Values{ + "grant_type": {"password"}, + "username": {"alice.bsky.social"}, + "password": {deviceSecret}, + "service": {"registry"}, + "scope": {"repository:alice.bsky.social/myapp:pull,push"}, + }) + w := httptest.NewRecorder() + + handler.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("Expected status %d, got %d (body: %s)", http.StatusOK, w.Code, w.Body.String()) + } + + var resp TokenResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("Failed to unmarshal response: %v", err) + } + if resp.Token == "" { + t.Error("Expected non-empty token") + } + // Docker's OAuth2 client reads access_token, not the legacy token field. + if resp.AccessToken != resp.Token { + t.Error("access_token must mirror token for OAuth2 clients") + } +} + func TestHandler_ServeHTTP_DeviceAuth_Valid(t *testing.T) { keyPath := getSharedTestKey(t) diff --git a/pkg/auth/token/service_test.go b/pkg/auth/token/service_test.go index 7079eea..04238e6 100644 --- a/pkg/auth/token/service_test.go +++ b/pkg/auth/token/service_test.go @@ -99,7 +99,7 @@ func TestHandlerResolveService(t *testing.T) { req.URL.RawQuery = q.Encode() } - if got := h.resolveService(req); got != tt.want { + if got := h.resolveService(req, req.URL.Query().Get("service")); got != tt.want { t.Errorf("resolveService() = %q, want %q (%s)", got, tt.want, tt.description) } })