diff --git a/pkg/auth/token/handler.go b/pkg/auth/token/handler.go index 658a16a..9bc4c43 100644 --- a/pkg/auth/token/handler.go +++ b/pkg/auth/token/handler.go @@ -107,6 +107,12 @@ type Handler struct { // normalized hostname. Nil means single-domain: the lookups in // resolveService miss and every token gets the issuer's own service. services map[string]bool + + // serviceDisplay maps a normalized service back to the registry domain as + // it was configured, port and all. Only the human-readable guidance in + // sendAuthError reads it; audiences and routing keep using the normalized + // key. Nil (or a miss) means the normalized form is printed as-is. + serviceDisplay map[string]string } // NewHandler creates a new token handler @@ -174,6 +180,55 @@ func (h *Handler) SetServices(services []string) { h.services = set } +// SetServiceDisplayNames records the registry domains as configured, before +// NormalizeService lowercased them and stripped their ports, so the plain-text +// guidance can print a host `docker login` will actually accept. +// +// SetServices is fed the normalized list on purpose: a JWT audience has to +// match the port-stripped r.Host that DomainRoutingMiddleware routes on, so a +// dev stack configured as "127.0.0.1:5000" is the service "127.0.0.1". That is +// right for the audience and wrong for a printed command, which needs the port +// back. This is the only place the unstripped form survives, and it feeds +// nothing but the message. +// +// Pass the raw server.registry_domains list. Entries are keyed by their +// normalized form; when two entries collide (say "atcr.io" and "atcr.io:443") +// the first wins, matching the first-wins dedupe in the AppView's +// deriveServices and the "first entry is primary" rule. Unset, or a service +// with no configured entry, prints the normalized name as before. +func (h *Handler) SetServiceDisplayNames(domains []string) { + if len(domains) == 0 { + h.serviceDisplay = nil + return + } + display := make(map[string]string, len(domains)) + for _, d := range domains { + d = strings.TrimSpace(d) + n := NormalizeService(d) + if n == "" { + continue + } + if _, ok := display[n]; ok { + continue + } + display[n] = d + } + h.serviceDisplay = display +} + +// displayService renders a normalized service for human consumption, restoring +// the port the configured registry domain carried. Audience and routing never +// call this; it exists only so the guidance names a host docker can reach. +func (h *Handler) displayService(service string) string { + if service == "" { + return "" + } + if d, ok := h.serviceDisplay[service]; ok && d != "" { + return d + } + return service +} + // resolveService picks the registry domain to stamp as the JWT's audience. // // The requested service is the primary signal: Docker echoes back whatever the @@ -224,10 +279,17 @@ func getBaseURL(r *http.Request) string { // sendAuthError sends the plain-text guidance a user sees when docker login fails. // // registryHost is the registry domain the token is being issued for, as -// resolved by resolveService. It is deliberately not r.Host: /auth/token is -// served on the UI domain as well as on every registry domain (the realm below -// points at the UI domain's copy, and DomainRoutingMiddleware serves the -// endpoint directly on registry domains so the Authorization header survives). +// resolved by resolveService and then run back through displayService, which +// restores whatever port server.registry_domains configured. The normalized +// service key has its port stripped because a JWT audience has to match the +// port-stripped routing host, but a command a human is told to run does not: +// on the dev stack the audience is "127.0.0.1" and the command is +// "docker login 127.0.0.1:5000". +// +// It is deliberately not r.Host: /auth/token is served on the UI domain as +// well as on every registry domain (the realm below points at the UI domain's +// copy, and DomainRoutingMiddleware serves the endpoint directly on registry +// domains so the Authorization header survives). // On a split-domain deployment the UI host refuses /v2/* with an OCI // UNSUPPORTED error, so "docker login " printed there names a host // where the handshake cannot succeed. resolveService falls back to the @@ -418,6 +480,12 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // configured registry domains. service := h.resolveService(r, requestedService) + // The same domain, rendered for a human: resolveService returns the + // normalized (port-stripped) key the audience needs, which is not + // necessarily something `docker login` can dial. Only the guidance below + // uses this. + loginHost := h.displayService(service) + access, err := auth.ParseScope(scopes) if err != nil { http.Error(w, fmt.Sprintf("invalid scope: %v", err), http.StatusBadRequest) @@ -449,11 +517,11 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } slog.Debug("Anonymous pull denied: no requested repository admits anonymous reads") - sendAuthError(w, r, service, "authentication required") + sendAuthError(w, r, loginHost, "authentication required") return } slog.Debug("No Basic auth credentials provided") - sendAuthError(w, r, service, "authentication required") + sendAuthError(w, r, loginHost, "authentication required") return } @@ -467,7 +535,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { device, err := h.deviceStore.ValidateDeviceSecret(password) if err != nil { slog.Debug("Device secret validation failed", "error", err) - sendAuthError(w, r, service, "authentication failed") + sendAuthError(w, r, loginHost, "authentication failed") return } @@ -496,16 +564,16 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Log at WARN level with specific error type if errors.Is(err, auth.ErrIdentityResolution) { slog.Warn("Identity resolution failed", "error", err, "username", username) - sendAuthError(w, r, service, "authentication failed: could not resolve handle") + sendAuthError(w, r, loginHost, "authentication failed: could not resolve handle") } else if errors.Is(err, auth.ErrInvalidCredentials) { slog.Warn("Invalid credentials", "username", username) - sendAuthError(w, r, service, "authentication failed: invalid credentials") + sendAuthError(w, r, loginHost, "authentication failed: invalid credentials") } else if errors.Is(err, auth.ErrPDSUnavailable) { slog.Warn("PDS unavailable", "error", err, "username", username) - sendAuthError(w, r, service, "authentication failed: PDS unavailable") + sendAuthError(w, r, loginHost, "authentication failed: PDS unavailable") } else { slog.Warn("Authentication failed", "error", err, "username", username) - sendAuthError(w, r, service, "authentication failed") + sendAuthError(w, r, loginHost, "authentication failed") } return } diff --git a/pkg/auth/token/handler_auth_error_test.go b/pkg/auth/token/handler_auth_error_test.go index e8aeeda..c51abca 100644 --- a/pkg/auth/token/handler_auth_error_test.go +++ b/pkg/auth/token/handler_auth_error_test.go @@ -6,6 +6,8 @@ import ( "strings" "testing" "time" + + "github.com/golang-jwt/jwt/v5" ) // The plain-text guidance sent with a 401 has to name a host `docker login` @@ -24,6 +26,9 @@ func TestSendAuthError_NamesRegistryDomainNotRequestHost(t *testing.T) { host string // service parameter the client echoes back, if any. requested string + // registry_domains as configured, before normalization. Empty leaves + // the display list unset, which is the pre-existing behaviour. + displayNames []string wantLogin string // hostname step 2 must name wantAbsent []string // hostnames step 2 must not name @@ -71,6 +76,68 @@ func TestSendAuthError_NamesRegistryDomainNotRequestHost(t *testing.T) { host: "localhost:5000", wantLogin: "localhost", }, + { + // The dev stack as actually configured: registry_domains is + // ["127.0.0.1:5000"], which normalizes to the audience "127.0.0.1". + // The command has to keep the port or it dials the wrong place. + name: "dev stack keeps the configured port", + services: []string{"127.0.0.1"}, + displayNames: []string{"127.0.0.1:5000"}, + host: "127.0.0.1:5000", + wantLogin: "127.0.0.1:5000", + wantAbsent: []string{"docker login 127.0.0.1\n"}, + }, + { + // Production shape: port-free domains print exactly as before. + name: "production domain unchanged by the display list", + services: []string{"seamark.cr"}, + displayNames: []string{"seamark.cr"}, + host: "seamark.dev", + wantLogin: "seamark.cr", + wantAbsent: []string{"docker login seamark.dev"}, + }, + { + // The checked-in seamark deployment fronts three registry domains. + // The guidance must name the one the client is authenticating + // against, resolved back to its own configured entry. + name: "multi-domain picks the entry the service came from", + services: []string{"buoy.cr", "bouy.cr", "seamark.cr"}, + displayNames: []string{"buoy.cr", "bouy.cr", "seamark.cr"}, + host: "seamark.dev", + requested: "seamark.cr", + wantLogin: "seamark.cr", + wantAbsent: []string{"docker login buoy.cr", "docker login bouy.cr"}, + }, + { + // Same, with ports: each normalized service maps back to its own + // configured entry, not to the primary's. + name: "multi-domain with ports maps each service to its own entry", + services: []string{"127.0.0.1", "localhost"}, + displayNames: []string{"127.0.0.1:5000", "localhost:5001"}, + host: "127.0.0.1:5000", + requested: "localhost:5001", + wantLogin: "localhost:5001", + wantAbsent: []string{"docker login 127.0.0.1"}, + }, + { + // Two configured entries that normalize to the same host: first + // wins, matching deriveServices' first-wins dedupe. + name: "colliding entries print the first configured form", + services: []string{"atcr.io"}, + displayNames: []string{"atcr.io:443", "atcr.io"}, + host: "atcr.io", + wantLogin: "atcr.io:443", + }, + { + // A service with no configured entry (the issuer's fallback on a + // deployment whose display list does not cover it) prints the + // normalized name, exactly as before this existed. + name: "service absent from the display list falls back to normalized", + services: []string{"seamark.cr"}, + displayNames: []string{"buoy.cr:8443"}, + host: "seamark.dev", + wantLogin: "seamark.cr", + }, } for _, tt := range tests { @@ -81,6 +148,7 @@ func TestSendAuthError_NamesRegistryDomainNotRequestHost(t *testing.T) { } handler := NewHandler(issuer, nil) handler.SetServices(tt.services) + handler.SetServiceDisplayNames(tt.displayNames) // A push-only scope with no credentials has no anonymous // component, so it draws the plain-text challenge. @@ -143,3 +211,107 @@ func TestSendAuthError_NoServiceOmitsDockerLoginStep(t *testing.T) { t.Errorf("guidance names a docker login host with no service configured.\nBody:\n%s", body) } } + +// The display list must not leak into the JWT. Audiences stay on the +// normalized, port-stripped service, because that is what the registry +// compares against the port-stripped r.Host it routes on. +func TestServiceDisplayNames_DoNotChangeTokenAudience(t *testing.T) { + keyPath := getSharedTestKey(t) + + tests := []struct { + name string + services []string + displayNames []string + host string + requested string + wantAudience string + }{ + { + name: "dev stack", + services: []string{"127.0.0.1"}, + displayNames: []string{"127.0.0.1:5000"}, + host: "127.0.0.1:5000", + wantAudience: "127.0.0.1", + }, + { + name: "dev stack, service echoed back with its port", + services: []string{"127.0.0.1"}, + displayNames: []string{"127.0.0.1:5000"}, + host: "127.0.0.1:5000", + requested: "127.0.0.1:5000", + wantAudience: "127.0.0.1", + }, + { + name: "multi-domain with ports", + services: []string{"127.0.0.1", "localhost"}, + displayNames: []string{"127.0.0.1:5000", "localhost:5001"}, + host: "127.0.0.1:5000", + requested: "localhost:5001", + wantAudience: "localhost", + }, + { + name: "production shape", + services: []string{"seamark.cr", "buoy.cr"}, + displayNames: []string{"seamark.cr", "buoy.cr"}, + host: "seamark.dev", + requested: "buoy.cr", + wantAudience: "buoy.cr", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + issuer, err := NewIssuer(keyPath, "atcr.io", tt.services[0], 15*time.Minute) + if err != nil { + t.Fatalf("NewIssuer() error = %v", err) + } + handler := NewHandler(issuer, nil) + handler.SetServices(tt.services) + handler.SetServiceDisplayNames(tt.displayNames) + + // An anonymous pull actually mints a token, so the audience is + // observable end to end rather than through resolveService alone. + target := "/auth/token?scope=repository:bob.bsky.social/myapp:pull" + if tt.requested != "" { + target += "&service=" + tt.requested + } + req := httptest.NewRequest(http.MethodGet, target, nil) + req.Host = tt.host + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want %d. Body: %s", w.Code, http.StatusOK, w.Body.String()) + } + + claims := parseTokenClaims(t, decodeToken(t, w), issuer) + if len(claims.Audience) != 1 || claims.Audience[0] != tt.wantAudience { + t.Errorf("audience = %v, want [%q]", claims.Audience, tt.wantAudience) + } + + // And the resolved service key itself, which is what routing and + // the access controller compare against. + if got := handler.resolveService(req, tt.requested); got != tt.wantAudience { + t.Errorf("resolveService() = %q, want %q", got, tt.wantAudience) + } + }) + } +} + +// parseTokenClaims verifies the JWT against the issuer's own key and returns +// its claims. +func parseTokenClaims(t *testing.T, tokenString string, issuer *Issuer) *Claims { + t.Helper() + parsed, err := jwt.ParseWithClaims(tokenString, &Claims{}, func(*jwt.Token) (any, error) { + return issuer.publicKey, nil + }) + if err != nil { + t.Fatalf("parse token: %v", err) + } + claims, ok := parsed.Claims.(*Claims) + if !ok { + t.Fatalf("claims type = %T, want *Claims", parsed.Claims) + } + return claims +}