mirror of
https://tangled.org/evan.jarrett.net/at-container-registry
synced 2026-08-29 04:06:58 +00:00
auth: serve the OAuth2 POST form at /auth/token
The route was registered GET-only, so containerd and Docker, which try the OAuth2 POST endpoint first whenever they hold a secret, ate a 405 and retried on the GET form. Every authenticated pull paid two auth round trips, and in the production logs the POST share of token traffic grew from 0.5% to 38% over six weeks as more clients pulled from k8s with basic-auth imagePullSecrets. Serve both specs on the same path. After credentials and scope are extracted the two paths are identical, so this is an extraction branch plus a form-shaped error writer. Only grant_type=password is supported and no refresh token is issued: the registry JWT's lifetime is pinned to the AppView<->hold service-auth, 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 already hold. The refresh grant is refused with 401 rather than the 400 that RFC 6749 5.2 prescribes. containerd sends that grant only 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 and routes them to the GET form, where a device secret authenticates off the password alone. That shape previously had no working path at all. resolveService now takes the requested service as an argument, since it arrives in the query string on GET and in the form body on POST. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
05b856bf4e
commit
b25aee336b
@@ -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)
|
||||
|
||||
+96
-28
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user