mirror of
https://tangled.org/evan.jarrett.net/at-container-registry
synced 2026-09-20 01:04:15 +00:00
appview: keep the port in the registry host printed to users
On the dev stack (registry_domains: ["127.0.0.1:5000"]) the /auth/token failure guidance said `docker login 127.0.0.1`, dropping the port docker needs. The message exists to tell a stuck developer what to run, so a command that cannot work is the whole defect. Production is unaffected, both registry domains being port-free. The stripping itself is deliberate and stays: NormalizeService removes the port so JWT audiences match the port-stripped routing host. Removing it would desynchronize the service key from the routing key. The unstripped form turned out to survive in only one place. resolveService returns a key from h.services, which SetServices normalizes, and cfg.Auth.Services is normalized too by deriveServices, so neither holds the original. Only cfg.Server.RegistryDomains, straight off the YAML, does, and it was never reaching the token package. Adds a display-only map keyed by the same NormalizeService function the other two key on, so a resolved service always maps back to the entry it came from, and a multi-domain deployment prints the domain the client is authenticating against rather than the primary. Collisions take the first configured entry, matching deriveServices' own first-wins dedupe. A miss falls through to the normalized name, which is today's behaviour. Audiences and routing are untouched, proven by a test that mints a real token, parses the JWT and asserts aud is still the normalized host. The wiring line landed in the previous commit, both changes having been made in server.go at the same time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9
This commit is contained in:
co-authored by
Claude Opus 5
parent
3589473feb
commit
e542dd12f6
+79
-11
@@ -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 <r.Host>" 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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user