fix: address CodeRabbit review — internal auth scope restriction, IPv6 redirect, TTL cap, docs
- commit
- e4d9ea04729634c774811d4dced2f12a632b2d1e
- author
- brice · · 2026-03-22 18:30 UTC
- parent
- c8a82f0
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +14 -5
M docs/upgrading.md +2 -2
| @@ -26,10 +26,10 @@ Gordon v2.30.0 removes password-based authentication entirely. Only token-based | |||
| 26 | 26 | 1. **Before upgrading**, generate a token on your current Gordon instance: | |
| 27 | 27 | ||
| 28 | 28 | ```bash | |
| 29 | - | gordon auth token generate --subject deploy --scopes "push,pull,admin:*:*" --expiry 0 | |
| 29 | + | gordon auth token generate --subject deploy --scopes "push,pull" --expiry 0 | |
| 30 | 30 | ``` | |
| 31 | 31 | ||
| 32 | - | Save this token securely. You will need it after upgrading. | |
| 32 | + | Save this token securely. You will need it after upgrading. Admin scopes (`admin:*:*`) are only available after upgrading to v2.30.0 — regenerate your token with admin scopes after the upgrade if needed. | |
| 33 | 33 | ||
| 34 | 34 | 2. **Update remotes** to use the generated token: | |
| 35 | 35 | ||
| @@ -26,10 +26,10 @@ Gordon v2.30.0 removes password-based authentication entirely. Only token-based | |||||
| 26 | 1. **Before upgrading**, generate a token on your current Gordon instance: | 26 | 1. **Before upgrading**, generate a token on your current Gordon instance: | ||
| 27 | 27 | ||||
| 28 | ```bash | 28 | ```bash | ||
| 29 | - | gordon auth token generate --subject deploy --scopes "push,pull,admin:*:*" --expiry 0 | 29 | + | gordon auth token generate --subject deploy --scopes "push,pull" --expiry 0 |
| 30 | ``` | 30 | ``` | ||
| 31 | 31 | ||||
| 32 | - | Save this token securely. You will need it after upgrading. | 32 | + | Save this token securely. You will need it after upgrading. Admin scopes (`admin:*:*`) are only available after upgrading to v2.30.0 — regenerate your token with admin scopes after the upgrade if needed. |
| 33 | 33 | ||||
| 34 | 2. **Update remotes** to use the generated token: | 34 | 2. **Update remotes** to use the generated token: | ||
| 35 | 35 | ||||
M internal/adapters/in/http/auth/handler.go +4 -1
| @@ -168,7 +168,10 @@ func (h *Handler) handleToken(w http.ResponseWriter, r *http.Request) { | |||
| 168 | 168 | func (h *Handler) authenticateTokenCredentials(ctx context.Context, r *http.Request, username, password string, log zerowrap.Logger) (bool, *domain.TokenClaims) { | |
| 169 | 169 | if httputil.IsLocalhostRequest(r) && h.isInternalAuth(username, password) { | |
| 170 | 170 | log.Debug().Str("username", username).Msg("internal registry auth accepted") | |
| 171 | - | return true, nil | |
| 171 | + | return true, &domain.TokenClaims{ | |
| 172 | + | Subject: username, | |
| 173 | + | Scopes: []string{"push", "pull"}, | |
| 174 | + | } | |
| 172 | 175 | } | |
| 173 | 176 | ||
| 174 | 177 | // Validate JWT token sent via Basic Auth password field. | |
| @@ -168,7 +168,10 @@ func (h *Handler) handleToken(w http.ResponseWriter, r *http.Request) { | |||||
| 168 | func (h *Handler) authenticateTokenCredentials(ctx context.Context, r *http.Request, username, password string, log zerowrap.Logger) (bool, *domain.TokenClaims) { | 168 | func (h *Handler) authenticateTokenCredentials(ctx context.Context, r *http.Request, username, password string, log zerowrap.Logger) (bool, *domain.TokenClaims) { | ||
| 169 | if httputil.IsLocalhostRequest(r) && h.isInternalAuth(username, password) { | 169 | if httputil.IsLocalhostRequest(r) && h.isInternalAuth(username, password) { | ||
| 170 | log.Debug().Str("username", username).Msg("internal registry auth accepted") | 170 | log.Debug().Str("username", username).Msg("internal registry auth accepted") | ||
| 171 | - | return true, nil | 171 | + | return true, &domain.TokenClaims{ |
| 172 | + | Subject: username, | |||
| 173 | + | Scopes: []string{"push", "pull"}, | |||
| 174 | + | } | |||
| 172 | } | 175 | } | ||
| 173 | 176 | ||||
| 174 | // Validate JWT token sent via Basic Auth password field. | 177 | // Validate JWT token sent via Basic Auth password field. | ||
M internal/app/run.go +6 -0
| @@ -1098,6 +1098,9 @@ func buildAuthConfig(ctx context.Context, cfg Config, authType domain.AuthType, | |||
| 1098 | 1098 | if parsed <= 0 { | |
| 1099 | 1099 | return auth.Config{}, fmt.Errorf("auth.access_token_ttl must be positive") | |
| 1100 | 1100 | } | |
| 1101 | + | if parsed > auth.MaxAccessTokenLifetime { | |
| 1102 | + | return auth.Config{}, fmt.Errorf("auth.access_token_ttl must not exceed %v", auth.MaxAccessTokenLifetime) | |
| 1103 | + | } | |
| 1101 | 1104 | accessTokenTTL = parsed | |
| 1102 | 1105 | } | |
| 1103 | 1106 | authConfig.AccessTokenTTL = accessTokenTTL | |
| @@ -1812,6 +1815,9 @@ func runServers(ctx context.Context, v *viper.Viper, cfg Config, registryHandler | |||
| 1812 | 1815 | } | |
| 1813 | 1816 | if tlsPort != 443 { | |
| 1814 | 1817 | host = net.JoinHostPort(host, strconv.Itoa(tlsPort)) | |
| 1818 | + | } else if strings.Contains(host, ":") { | |
| 1819 | + | // Re-bracket bare IPv6 literal for the URL. | |
| 1820 | + | host = "[" + host + "]" | |
| 1815 | 1821 | } | |
| 1816 | 1822 | target := "https://" + host + r.RequestURI | |
| 1817 | 1823 | http.Redirect(w, r, target, http.StatusPermanentRedirect) | |
| @@ -1098,6 +1098,9 @@ func buildAuthConfig(ctx context.Context, cfg Config, authType domain.AuthType, | |||||
| 1098 | if parsed <= 0 { | 1098 | if parsed <= 0 { | ||
| 1099 | return auth.Config{}, fmt.Errorf("auth.access_token_ttl must be positive") | 1099 | return auth.Config{}, fmt.Errorf("auth.access_token_ttl must be positive") | ||
| 1100 | } | 1100 | } | ||
| 1101 | + | if parsed > auth.MaxAccessTokenLifetime { | |||
| 1102 | + | return auth.Config{}, fmt.Errorf("auth.access_token_ttl must not exceed %v", auth.MaxAccessTokenLifetime) | |||
| 1103 | + | } | |||
| 1101 | accessTokenTTL = parsed | 1104 | accessTokenTTL = parsed | ||
| 1102 | } | 1105 | } | ||
| 1103 | authConfig.AccessTokenTTL = accessTokenTTL | 1106 | authConfig.AccessTokenTTL = accessTokenTTL | ||
| @@ -1812,6 +1815,9 @@ func runServers(ctx context.Context, v *viper.Viper, cfg Config, registryHandler | |||||
| 1812 | } | 1815 | } | ||
| 1813 | if tlsPort != 443 { | 1816 | if tlsPort != 443 { | ||
| 1814 | host = net.JoinHostPort(host, strconv.Itoa(tlsPort)) | 1817 | host = net.JoinHostPort(host, strconv.Itoa(tlsPort)) | ||
| 1818 | + | } else if strings.Contains(host, ":") { | |||
| 1819 | + | // Re-bracket bare IPv6 literal for the URL. | |||
| 1820 | + | host = "[" + host + "]" | |||
| 1815 | } | 1821 | } | ||
| 1816 | target := "https://" + host + r.RequestURI | 1822 | target := "https://" + host + r.RequestURI | ||
| 1817 | http.Redirect(w, r, target, http.StatusPermanentRedirect) | 1823 | http.Redirect(w, r, target, http.StatusPermanentRedirect) | ||
M internal/usecase/auth/service_test.go +2 -2
| @@ -743,7 +743,7 @@ func TestExtendTokenSlidesExpiry(t *testing.T) { | |||
| 743 | 743 | ||
| 744 | 744 | ctx := testContext() | |
| 745 | 745 | ||
| 746 | - | // Generate a token with a short remaining life (2h) so ExtendToken produces a different exp | |
| 746 | + | // Generate a token with a short remaining life (MaxAccessTokenLifetime+1m) so ExtendToken produces a different exp | |
| 747 | 747 | var capturedToken *domain.Token | |
| 748 | 748 | tokenStore.EXPECT(). | |
| 749 | 749 | SaveToken(mock.Anything, mock.Anything, mock.Anything). | |
| @@ -815,7 +815,7 @@ func TestExtendTokenDebounce(t *testing.T) { | |||
| 815 | 815 | ||
| 816 | 816 | ctx := testContext() | |
| 817 | 817 | ||
| 818 | - | // Generate a short-lived token (2h) so the first extension produces a different JWT | |
| 818 | + | // Generate a short-lived token (MaxAccessTokenLifetime+1m) so the first extension produces a different JWT | |
| 819 | 819 | var capturedToken *domain.Token | |
| 820 | 820 | tokenStore.EXPECT(). | |
| 821 | 821 | SaveToken(mock.Anything, mock.Anything, mock.Anything). | |
| @@ -743,7 +743,7 @@ func TestExtendTokenSlidesExpiry(t *testing.T) { | |||||
| 743 | 743 | ||||
| 744 | ctx := testContext() | 744 | ctx := testContext() | ||
| 745 | 745 | ||||
| 746 | - | // Generate a token with a short remaining life (2h) so ExtendToken produces a different exp | 746 | + | // Generate a token with a short remaining life (MaxAccessTokenLifetime+1m) so ExtendToken produces a different exp |
| 747 | var capturedToken *domain.Token | 747 | var capturedToken *domain.Token | ||
| 748 | tokenStore.EXPECT(). | 748 | tokenStore.EXPECT(). | ||
| 749 | SaveToken(mock.Anything, mock.Anything, mock.Anything). | 749 | SaveToken(mock.Anything, mock.Anything, mock.Anything). | ||
| @@ -815,7 +815,7 @@ func TestExtendTokenDebounce(t *testing.T) { | |||||
| 815 | 815 | ||||
| 816 | ctx := testContext() | 816 | ctx := testContext() | ||
| 817 | 817 | ||||
| 818 | - | // Generate a short-lived token (2h) so the first extension produces a different JWT | 818 | + | // Generate a short-lived token (MaxAccessTokenLifetime+1m) so the first extension produces a different JWT |
| 819 | var capturedToken *domain.Token | 819 | var capturedToken *domain.Token | ||
| 820 | tokenStore.EXPECT(). | 820 | tokenStore.EXPECT(). | ||
| 821 | SaveToken(mock.Anything, mock.Anything, mock.Anything). | 821 | SaveToken(mock.Anything, mock.Anything, mock.Anything). | ||