diff --git a/docs/server/docs.go b/docs/server/docs.go index 1a5734c59e..bd79ea1bee 100644 --- a/docs/server/docs.go +++ b/docs/server/docs.go @@ -1462,6 +1462,32 @@ const docTemplate = `{ "RegistryTypeDefault" ] }, + "pkg_api_v1.UpdateRegistryAuthRequest": { + "description": "OAuth authentication configuration (optional)", + "properties": { + "audience": { + "description": "OAuth audience (optional)", + "type": "string" + }, + "client_id": { + "description": "OAuth client ID", + "type": "string" + }, + "issuer": { + "description": "OIDC issuer URL", + "type": "string" + }, + "scopes": { + "description": "OAuth scopes (optional)", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + } + }, + "type": "object" + }, "pkg_api_v1.UpdateRegistryRequest": { "description": "Request containing registry configuration updates", "properties": { @@ -1473,6 +1499,9 @@ const docTemplate = `{ "description": "MCP Registry API URL", "type": "string" }, + "auth": { + "$ref": "#/components/schemas/pkg_api_v1.UpdateRegistryAuthRequest" + }, "local_path": { "description": "Local registry file path", "type": "string" @@ -1772,6 +1801,14 @@ const docTemplate = `{ "pkg_api_v1.getRegistryResponse": { "description": "Response containing registry details", "properties": { + "auth_status": { + "description": "AuthStatus is one of: \"none\", \"configured\", \"authenticated\".\nIntentionally omits omitempty — see registryInfo for rationale.", + "type": "string" + }, + "auth_type": { + "description": "AuthType is \"oauth\", \"bearer\" (future), or empty string when no auth.\nIntentionally omits omitempty — see registryInfo for rationale.", + "type": "string" + }, "last_updated": { "description": "Last updated timestamp", "type": "string" @@ -2022,6 +2059,14 @@ const docTemplate = `{ "pkg_api_v1.registryInfo": { "description": "Basic information about a registry", "properties": { + "auth_status": { + "description": "AuthStatus is one of: \"none\", \"configured\", \"authenticated\".\nIntentionally omits omitempty so clients always receive the field,\neven when the value is \"none\" (the zero-value equivalent).", + "type": "string" + }, + "auth_type": { + "description": "AuthType is \"oauth\", \"bearer\" (future), or empty string when no auth.\nIntentionally omits omitempty so clients can distinguish \"no auth\nconfigured\" (empty string) from \"field missing\" without extra logic.", + "type": "string" + }, "last_updated": { "description": "Last updated timestamp", "type": "string" diff --git a/docs/server/swagger.json b/docs/server/swagger.json index 4c347eed6b..044c0ee718 100644 --- a/docs/server/swagger.json +++ b/docs/server/swagger.json @@ -1455,6 +1455,32 @@ "RegistryTypeDefault" ] }, + "pkg_api_v1.UpdateRegistryAuthRequest": { + "description": "OAuth authentication configuration (optional)", + "properties": { + "audience": { + "description": "OAuth audience (optional)", + "type": "string" + }, + "client_id": { + "description": "OAuth client ID", + "type": "string" + }, + "issuer": { + "description": "OIDC issuer URL", + "type": "string" + }, + "scopes": { + "description": "OAuth scopes (optional)", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + } + }, + "type": "object" + }, "pkg_api_v1.UpdateRegistryRequest": { "description": "Request containing registry configuration updates", "properties": { @@ -1466,6 +1492,9 @@ "description": "MCP Registry API URL", "type": "string" }, + "auth": { + "$ref": "#/components/schemas/pkg_api_v1.UpdateRegistryAuthRequest" + }, "local_path": { "description": "Local registry file path", "type": "string" @@ -1765,6 +1794,14 @@ "pkg_api_v1.getRegistryResponse": { "description": "Response containing registry details", "properties": { + "auth_status": { + "description": "AuthStatus is one of: \"none\", \"configured\", \"authenticated\".\nIntentionally omits omitempty — see registryInfo for rationale.", + "type": "string" + }, + "auth_type": { + "description": "AuthType is \"oauth\", \"bearer\" (future), or empty string when no auth.\nIntentionally omits omitempty — see registryInfo for rationale.", + "type": "string" + }, "last_updated": { "description": "Last updated timestamp", "type": "string" @@ -2015,6 +2052,14 @@ "pkg_api_v1.registryInfo": { "description": "Basic information about a registry", "properties": { + "auth_status": { + "description": "AuthStatus is one of: \"none\", \"configured\", \"authenticated\".\nIntentionally omits omitempty so clients always receive the field,\neven when the value is \"none\" (the zero-value equivalent).", + "type": "string" + }, + "auth_type": { + "description": "AuthType is \"oauth\", \"bearer\" (future), or empty string when no auth.\nIntentionally omits omitempty so clients can distinguish \"no auth\nconfigured\" (empty string) from \"field missing\" without extra logic.", + "type": "string" + }, "last_updated": { "description": "Last updated timestamp", "type": "string" diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml index 741cedbe4a..b7570f441d 100644 --- a/docs/server/swagger.yaml +++ b/docs/server/swagger.yaml @@ -1365,6 +1365,25 @@ components: - RegistryTypeURL - RegistryTypeAPI - RegistryTypeDefault + pkg_api_v1.UpdateRegistryAuthRequest: + description: OAuth authentication configuration (optional) + properties: + audience: + description: OAuth audience (optional) + type: string + client_id: + description: OAuth client ID + type: string + issuer: + description: OIDC issuer URL + type: string + scopes: + description: OAuth scopes (optional) + items: + type: string + type: array + uniqueItems: false + type: object pkg_api_v1.UpdateRegistryRequest: description: Request containing registry configuration updates properties: @@ -1374,6 +1393,8 @@ components: api_url: description: MCP Registry API URL type: string + auth: + $ref: '#/components/schemas/pkg_api_v1.UpdateRegistryAuthRequest' local_path: description: Local registry file path type: string @@ -1588,6 +1609,16 @@ components: pkg_api_v1.getRegistryResponse: description: Response containing registry details properties: + auth_status: + description: |- + AuthStatus is one of: "none", "configured", "authenticated". + Intentionally omits omitempty — see registryInfo for rationale. + type: string + auth_type: + description: |- + AuthType is "oauth", "bearer" (future), or empty string when no auth. + Intentionally omits omitempty — see registryInfo for rationale. + type: string last_updated: description: Last updated timestamp type: string @@ -1775,6 +1806,18 @@ components: pkg_api_v1.registryInfo: description: Basic information about a registry properties: + auth_status: + description: |- + AuthStatus is one of: "none", "configured", "authenticated". + Intentionally omits omitempty so clients always receive the field, + even when the value is "none" (the zero-value equivalent). + type: string + auth_type: + description: |- + AuthType is "oauth", "bearer" (future), or empty string when no auth. + Intentionally omits omitempty so clients can distinguish "no auth + configured" (empty string) from "field missing" without extra logic. + type: string last_updated: description: Last updated timestamp type: string diff --git a/pkg/api/server.go b/pkg/api/server.go index eb30306524..2f9a8840a8 100644 --- a/pkg/api/server.go +++ b/pkg/api/server.go @@ -287,7 +287,7 @@ func (b *ServerBuilder) setupDefaultRoutes(r *chi.Mux) { standardRouters := map[string]http.Handler{ "/health": v1.HealthcheckRouter(b.containerRuntime), "/api/v1beta/version": v1.VersionRouter(), - "/api/v1beta/registry": v1.RegistryRouter(), + "/api/v1beta/registry": v1.RegistryRouter(true), "/api/v1beta/discovery": v1.DiscoveryRouter(), "/api/v1beta/clients": v1.ClientRouter(b.clientManager, b.workloadManager, b.groupManager), "/api/v1beta/secrets": v1.SecretsRouter(), diff --git a/pkg/api/v1/registry.go b/pkg/api/v1/registry.go index 7d14424046..ab4f3ec72e 100644 --- a/pkg/api/v1/registry.go +++ b/pkg/api/v1/registry.go @@ -17,8 +17,119 @@ import ( "github.com/stacklok/toolhive-core/registry/types" "github.com/stacklok/toolhive/pkg/config" regpkg "github.com/stacklok/toolhive/pkg/registry" + "github.com/stacklok/toolhive/pkg/registry/auth" + "github.com/stacklok/toolhive/pkg/secrets" ) +// RegistryAuthRequiredCode is the machine-readable error code returned in the +// structured JSON 503 response when registry authentication is missing. +// Desktop clients (Studio) match on this value to display the correct UI. +const RegistryAuthRequiredCode = "registry_auth_required" + +// registryAuthErrorResponse is the JSON body for HTTP 503 auth-required errors. +// Studio uses the "code" field to detect this specific condition and prompt the user. +type registryAuthErrorResponse struct { + Code string `json:"code"` + Message string `json:"message"` +} + +// writeRegistryAuthRequiredError writes a structured JSON 503 response. +// HTTP 503 is correct: the incoming client (Studio) is authenticated to the thv serve API, +// but thv serve itself lacks a valid registry credential. This is a server-side dependency +// issue, not a client auth failure (which would be 401). +func writeRegistryAuthRequiredError(w http.ResponseWriter) { + body := registryAuthErrorResponse{ + Code: RegistryAuthRequiredCode, + Message: "Registry authentication required. Run 'thv registry login' to authenticate.", + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusServiceUnavailable) + _ = json.NewEncoder(w).Encode(body) +} + +// resolveAuthStatus returns the auth_status and auth_type strings for API responses +// by delegating to the AuthManager. +func (rr *RegistryRoutes) resolveAuthStatus() (authStatus, authType string) { + authMgr := regpkg.NewAuthManager(rr.configProvider) + return authMgr.GetAuthStatus() +} + +// isRegistryAuthError checks if an error is a registry auth required error. +func isRegistryAuthError(err error) bool { + return errors.Is(err, auth.ErrRegistryAuthRequired) +} + +// newSecretsProvider creates a secrets provider from the given config provider. +func newSecretsProvider(configProvider config.Provider) (secrets.Provider, error) { + cfg, err := configProvider.LoadOrCreateConfig() + if err != nil { + return nil, fmt.Errorf("loading config: %w", err) + } + providerType, err := cfg.Secrets.GetProviderType() + if err != nil { + return nil, fmt.Errorf("getting secrets provider type: %w", err) + } + return secrets.CreateSecretProvider(providerType) +} + +// registryAuthLogin handles POST /registry/auth/login. +// It triggers an interactive OAuth flow that opens the user's browser. +// This endpoint is only available in serve mode and is designed for desktop +// clients (e.g. Studio) where the user has a local browser. Headless or +// remote deployments should pre-configure credentials via the CLI instead. +func (rr *RegistryRoutes) registryAuthLogin(w http.ResponseWriter, r *http.Request) { + secretsProvider, err := newSecretsProvider(rr.configProvider) + if err != nil { + slog.Error("failed to create secrets provider", "error", err) + http.Error(w, "Failed to create secrets provider", http.StatusInternalServerError) + return + } + + if err := auth.Login(r.Context(), rr.configProvider, secretsProvider, auth.LoginOptions{}); err != nil { + if isRegistryAuthError(err) { + http.Error(w, "Registry OAuth not configured; use 'thv config set-registry-auth' first", http.StatusBadRequest) + return + } + slog.Error("registry login failed", "error", err) + http.Error(w, "Login failed", http.StatusInternalServerError) + return + } + + // Reset the singleton provider so subsequent registry calls pick up the new token. + regpkg.ResetDefaultProvider() + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{"status": "authenticated"}) +} + +// registryAuthLogout handles POST /registry/auth/logout. +// It clears cached OAuth tokens for the configured registry. +// This endpoint is only available in serve mode. +func (rr *RegistryRoutes) registryAuthLogout(w http.ResponseWriter, r *http.Request) { + secretsProvider, err := newSecretsProvider(rr.configProvider) + if err != nil { + slog.Error("failed to create secrets provider", "error", err) + http.Error(w, "Failed to create secrets provider", http.StatusInternalServerError) + return + } + + if err := auth.Logout(r.Context(), rr.configProvider, secretsProvider); err != nil { + if isRegistryAuthError(err) { + http.Error(w, "Registry OAuth not configured; use 'thv config set-registry-auth' first", http.StatusBadRequest) + return + } + slog.Error("registry logout failed", "error", err) + http.Error(w, "Logout failed", http.StatusInternalServerError) + return + } + + // Reset the singleton provider so subsequent registry calls reflect the logged-out state. + regpkg.ResetDefaultProvider() + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{"status": "logged_out"}) +} + const ( // defaultRegistryName is the name of the default registry defaultRegistryName = "default" @@ -94,10 +205,20 @@ func (rr *RegistryRoutes) getRegistryInfo() (RegistryType, string) { return RegistryType(registryType), source } -// getCurrentProvider returns the current registry provider using the injected config +// getCurrentProvider returns the current registry provider using the injected config. +// In serve mode, the provider is created with non-interactive auth to prevent +// browser-based OAuth flows from being triggered by API requests. func (rr *RegistryRoutes) getCurrentProvider(w http.ResponseWriter) (regpkg.Provider, bool) { - provider, err := regpkg.GetDefaultProviderWithConfig(rr.configProvider) + var opts []regpkg.ProviderOption + if rr.serveMode { + opts = append(opts, regpkg.WithInteractive(false)) + } + provider, err := regpkg.GetDefaultProviderWithConfig(rr.configProvider, opts...) if err != nil { + if isRegistryAuthError(err) { + writeRegistryAuthRequiredError(w) + return nil, false + } http.Error(w, "Failed to get registry provider", http.StatusInternalServerError) slog.Error("failed to get registry provider", "error", err) return nil, false @@ -109,6 +230,7 @@ func (rr *RegistryRoutes) getCurrentProvider(w http.ResponseWriter) (regpkg.Prov type RegistryRoutes struct { configProvider config.Provider configService regpkg.Configurator + serveMode bool } // NewRegistryRoutes creates a new RegistryRoutes with the default config provider @@ -128,9 +250,26 @@ func NewRegistryRoutesWithProvider(provider config.Provider) *RegistryRoutes { } } +// NewRegistryRoutesForServe creates RegistryRoutes configured for serve mode. +// In serve mode, the registry provider uses non-interactive auth (no browser OAuth). +func NewRegistryRoutesForServe() *RegistryRoutes { + return &RegistryRoutes{ + configProvider: config.NewDefaultProvider(), + configService: regpkg.NewConfigurator(), + serveMode: true, + } +} + // RegistryRouter creates a new router for the registry API. -func RegistryRouter() http.Handler { - routes := NewRegistryRoutes() +// When serveMode is true, the registry provider uses non-interactive auth, +// ensuring browser-based OAuth flows are never triggered from API requests. +func RegistryRouter(serveMode bool) http.Handler { + routes := func() *RegistryRoutes { + if serveMode { + return NewRegistryRoutesForServe() + } + return NewRegistryRoutes() + }() r := chi.NewRouter() r.Get("/", routes.listRegistries) @@ -144,6 +283,17 @@ func RegistryRouter() http.Handler { r.Get("/", routes.listServers) r.Get("/{serverName}", routes.getServer) }) + + // Auth routes (serve mode only). + // This static route takes priority over the /{name} parameter in Chi, + // so it does not conflict with a registry named "auth". + if serveMode { + r.Route("/auth", func(r chi.Router) { + r.Post("/login", routes.registryAuthLogin) + r.Post("/logout", routes.registryAuthLogout) + }) + } + return r } @@ -163,12 +313,18 @@ func (rr *RegistryRoutes) listRegistries(w http.ResponseWriter, _ *http.Request) reg, err := provider.GetRegistry() if err != nil { + if isRegistryAuthError(err) { + writeRegistryAuthRequiredError(w) + return + } http.Error(w, "Failed to get registry", http.StatusInternalServerError) return } registryType, source := rr.getRegistryInfo() + regAuthStatus, regAuthType := rr.resolveAuthStatus() + registries := []registryInfo{ { Name: defaultRegistryName, @@ -177,6 +333,8 @@ func (rr *RegistryRoutes) listRegistries(w http.ResponseWriter, _ *http.Request) ServerCount: len(reg.Servers), Type: registryType, Source: source, + AuthStatus: regAuthStatus, + AuthType: regAuthType, }, } @@ -229,12 +387,18 @@ func (rr *RegistryRoutes) getRegistry(w http.ResponseWriter, r *http.Request) { reg, err := provider.GetRegistry() if err != nil { + if isRegistryAuthError(err) { + writeRegistryAuthRequiredError(w) + return + } http.Error(w, "Failed to get registry", http.StatusInternalServerError) return } registryType, source := rr.getRegistryInfo() + regAuthStatus, regAuthType := rr.resolveAuthStatus() + response := getRegistryResponse{ Name: defaultRegistryName, Version: reg.Version, @@ -242,6 +406,8 @@ func (rr *RegistryRoutes) getRegistry(w http.ResponseWriter, r *http.Request) { ServerCount: len(reg.Servers), Type: registryType, Source: source, + AuthStatus: regAuthStatus, + AuthType: regAuthType, Registry: reg, } @@ -289,29 +455,50 @@ func (rr *RegistryRoutes) updateRegistry(w http.ResponseWriter, r *http.Request) return } - // Process the registry update - responseType, err := rr.processRegistryUpdate(&req) - if err != nil { - // Check if it's a connectivity error - return 504 Gateway Timeout - var connErr *connectivityError - if errors.As(err, &connErr) { - http.Error(w, connErr.Error(), http.StatusGatewayTimeout) + // Process the registry URL/path update. + // Call processRegistryUpdate when source fields are present, or when no fields + // at all are provided (which resets the registry to defaults). + hasSourceFields := req.URL != nil || req.APIURL != nil || req.LocalPath != nil + hasSourceUpdate := hasSourceFields || req.Auth == nil + var responseType string + if hasSourceUpdate { + var err error + responseType, err = rr.processRegistryUpdate(&req) + if err != nil { + // Check if it's a connectivity error - return 504 Gateway Timeout + var connErr *connectivityError + if errors.As(err, &connErr) { + http.Error(w, connErr.Error(), http.StatusGatewayTimeout) + return + } + // Check if it's a validation error - return 502 Bad Gateway + if isValidationError(err) { + http.Error(w, err.Error(), http.StatusBadGateway) + return + } + // Other errors - return 400 Bad Request + http.Error(w, err.Error(), http.StatusBadRequest) return } - // Check if it's a validation error - return 502 Bad Gateway - if isValidationError(err) { - http.Error(w, err.Error(), http.StatusBadGateway) + } + + // Process auth configuration if provided + if req.Auth != nil { + if err := rr.processAuthUpdate(req.Auth); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) return } - // Other errors - return 400 Bad Request - http.Error(w, err.Error(), http.StatusBadRequest) - return } // Reset the registry provider cache to pick up configuration changes - // Note: The config singleton is already reset by the service regpkg.ResetDefaultProvider() + // If no source update was performed, resolve the current type from config + if responseType == "" { + currentType, _ := rr.getRegistryInfo() + responseType = string(currentType) + } + response := UpdateRegistryResponse{ Type: responseType, } @@ -334,6 +521,18 @@ func validateRegistryRequest(req *UpdateRegistryRequest) error { return nil } +// processAuthUpdate validates and applies OAuth configuration for registry auth. +func (rr *RegistryRoutes) processAuthUpdate(authReq *UpdateRegistryAuthRequest) error { + if authReq.Issuer == "" || authReq.ClientID == "" { + return fmt.Errorf("auth.issuer and auth.client_id are required") + } + authMgr := regpkg.NewAuthManager(rr.configProvider) + if err := authMgr.SetOAuthAuth(authReq.Issuer, authReq.ClientID, authReq.Audience, authReq.Scopes); err != nil { + return fmt.Errorf("failed to configure registry auth: %w", err) + } + return nil +} + // processRegistryUpdate processes the registry update based on request type func (rr *RegistryRoutes) processRegistryUpdate(req *UpdateRegistryRequest) (string, error) { // Handle registry reset (unset) @@ -430,6 +629,10 @@ func (rr *RegistryRoutes) listServers(w http.ResponseWriter, r *http.Request) { // Get the full registry to access both container and remote servers reg, err := provider.GetRegistry() if err != nil { + if isRegistryAuthError(err) { + writeRegistryAuthRequiredError(w) + return + } slog.Error("failed to get registry", "error", err) http.Error(w, "Failed to get registry", http.StatusInternalServerError) return @@ -546,6 +749,14 @@ type registryInfo struct { Type RegistryType `json:"type"` // Source of the registry (URL, file path, or empty string for built-in) Source string `json:"source"` + // AuthStatus is one of: "none", "configured", "authenticated". + // Intentionally omits omitempty so clients always receive the field, + // even when the value is "none" (the zero-value equivalent). + AuthStatus string `json:"auth_status"` + // AuthType is "oauth", "bearer" (future), or empty string when no auth. + // Intentionally omits omitempty so clients can distinguish "no auth + // configured" (empty string) from "field missing" without extra logic. + AuthType string `json:"auth_type"` } // registryListResponse represents the response for listing registries @@ -572,6 +783,12 @@ type getRegistryResponse struct { Type RegistryType `json:"type"` // Source of the registry (URL, file path, or empty string for built-in) Source string `json:"source"` + // AuthStatus is one of: "none", "configured", "authenticated". + // Intentionally omits omitempty — see registryInfo for rationale. + AuthStatus string `json:"auth_status"` + // AuthType is "oauth", "bearer" (future), or empty string when no auth. + // Intentionally omits omitempty — see registryInfo for rationale. + AuthType string `json:"auth_type"` // Full registry data Registry *registry.Registry `json:"registry"` } @@ -610,6 +827,20 @@ type UpdateRegistryRequest struct { LocalPath *string `json:"local_path,omitempty"` // Allow private IP addresses for registry URL or API URL AllowPrivateIP *bool `json:"allow_private_ip,omitempty"` + // OAuth authentication configuration (optional) + Auth *UpdateRegistryAuthRequest `json:"auth,omitempty"` +} + +// UpdateRegistryAuthRequest contains OAuth configuration fields for registry auth. +type UpdateRegistryAuthRequest struct { + // OIDC issuer URL + Issuer string `json:"issuer"` + // OAuth client ID + ClientID string `json:"client_id"` + // OAuth audience (optional) + Audience string `json:"audience,omitempty"` + // OAuth scopes (optional) + Scopes []string `json:"scopes,omitempty"` } // UpdateRegistryResponse represents the response for updating a registry diff --git a/pkg/registry/auth_manager.go b/pkg/registry/auth_manager.go index bc63c9d0e8..994b2a356d 100644 --- a/pkg/registry/auth_manager.go +++ b/pkg/registry/auth_manager.go @@ -11,6 +11,16 @@ import ( "github.com/stacklok/toolhive/pkg/registry/auth" ) +// Auth status constants for API responses. +const ( + // AuthStatusNone indicates no registry auth is configured. + AuthStatusNone = "none" + // AuthStatusConfigured indicates auth is configured but no cached tokens exist. + AuthStatusConfigured = "configured" + // AuthStatusAuthenticated indicates auth is configured with cached tokens from a prior login. + AuthStatusAuthenticated = "authenticated" +) + // AuthManager provides operations for managing registry authentication configuration. type AuthManager interface { // SetOAuthAuth configures OAuth/OIDC authentication for the registry. @@ -22,6 +32,11 @@ type AuthManager interface { // GetAuthInfo returns the current auth type and whether tokens are cached. GetAuthInfo() (authType string, hasCachedTokens bool) + + // GetAuthStatus returns the auth status and auth type for API responses. + // Status is one of AuthStatusNone, AuthStatusConfigured, or AuthStatusAuthenticated. + // AuthType is "oauth" or empty string when no auth is configured. + GetAuthStatus() (status, authType string) } // DefaultAuthManager is the default implementation of AuthManager. @@ -65,3 +80,15 @@ func (c *DefaultAuthManager) GetAuthInfo() (string, bool) { return cfg.RegistryAuth.Type, hasCachedTokens } + +// GetAuthStatus returns the auth status and auth type for API responses. +func (c *DefaultAuthManager) GetAuthStatus() (string, string) { + authType, hasCachedTokens := c.GetAuthInfo() + if authType == "" { + return AuthStatusNone, "" + } + if hasCachedTokens { + return AuthStatusAuthenticated, authType + } + return AuthStatusConfigured, authType +} diff --git a/pkg/registry/auth_manager_test.go b/pkg/registry/auth_manager_test.go index c237c2597b..6fee85979d 100644 --- a/pkg/registry/auth_manager_test.go +++ b/pkg/registry/auth_manager_test.go @@ -145,6 +145,77 @@ func TestDefaultAuthManager_GetAuthInfo(t *testing.T) { } } +func TestDefaultAuthManager_GetAuthStatus(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + registryAuth config.RegistryAuth + wantStatus string + wantAuthType string + }{ + { + name: "returns none when no auth configured", + registryAuth: config.RegistryAuth{}, + wantStatus: AuthStatusNone, + wantAuthType: "", + }, + { + name: "returns configured when OAuth set but no cached tokens", + registryAuth: config.RegistryAuth{ + Type: config.RegistryAuthTypeOAuth, + OAuth: &config.RegistryOAuthConfig{ + Issuer: "https://auth.example.com", + ClientID: "my-client", + }, + }, + wantStatus: AuthStatusConfigured, + wantAuthType: config.RegistryAuthTypeOAuth, + }, + { + name: "returns authenticated when OAuth set with cached tokens", + registryAuth: config.RegistryAuth{ + Type: config.RegistryAuthTypeOAuth, + OAuth: &config.RegistryOAuthConfig{ + Issuer: "https://auth.example.com", + ClientID: "my-client", + CachedRefreshTokenRef: "REGISTRY_OAUTH_aabbccdd", + }, + }, + wantStatus: AuthStatusAuthenticated, + wantAuthType: config.RegistryAuthTypeOAuth, + }, + { + name: "returns configured when OAuth section is nil", + registryAuth: config.RegistryAuth{ + Type: config.RegistryAuthTypeOAuth, + OAuth: nil, + }, + wantStatus: AuthStatusConfigured, + wantAuthType: config.RegistryAuthTypeOAuth, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockProvider := configmocks.NewMockProvider(ctrl) + + mockProvider.EXPECT(). + GetConfig(). + Return(&config.Config{RegistryAuth: tt.registryAuth}) + + mgr := NewAuthManager(mockProvider) + status, authType := mgr.GetAuthStatus() + + require.Equal(t, tt.wantStatus, status) + require.Equal(t, tt.wantAuthType, authType) + }) + } +} + // errUpdateFailed is a sentinel error for testing UpdateConfig failure paths. var errUpdateFailed = errSentinel("UpdateConfig failed") diff --git a/pkg/registry/factory.go b/pkg/registry/factory.go index 82358e6c50..c15c5d5fcc 100644 --- a/pkg/registry/factory.go +++ b/pkg/registry/factory.go @@ -85,16 +85,18 @@ func GetDefaultProvider() (Provider, error) { return GetDefaultProviderWithConfig(config.NewDefaultProvider()) } -// GetDefaultProviderWithConfig returns a registry provider using the given config provider -// This allows tests to inject their own config provider -func GetDefaultProviderWithConfig(configProvider config.Provider) (Provider, error) { +// GetDefaultProviderWithConfig returns a registry provider using the given config provider. +// This allows tests to inject their own config provider. +// The interactive flag controls whether browser-based OAuth flows are allowed. +// Pass true for CLI contexts, false for headless/serve mode. +func GetDefaultProviderWithConfig(configProvider config.Provider, opts ...ProviderOption) (Provider, error) { defaultProviderOnce.Do(func() { cfg, err := configProvider.LoadOrCreateConfig() if err != nil { defaultProviderErr = err return } - defaultProvider, defaultProviderErr = NewRegistryProvider(cfg) + defaultProvider, defaultProviderErr = NewRegistryProvider(cfg, opts...) }) return defaultProvider, defaultProviderErr diff --git a/pkg/registry/provider_api.go b/pkg/registry/provider_api.go index 8f8780ea3c..2b063c0570 100644 --- a/pkg/registry/provider_api.go +++ b/pkg/registry/provider_api.go @@ -5,6 +5,7 @@ package registry import ( "context" + "errors" "fmt" "time" @@ -60,6 +61,14 @@ func NewAPIRegistryProvider(apiURL string, allowPrivateIp bool, tokenSource auth // Try to list servers with a small limit to verify API functionality _, err = client.ListServers(ctx, &api.ListOptions{Limit: 1}) if err != nil { + if errors.Is(err, api.ErrRegistryUnauthorized) { + return nil, fmt.Errorf( + "registry at %s returned 401 Unauthorized\n\n"+ + "If this registry requires authentication, configure it with:\n"+ + " thv config set-registry-auth --issuer --client-id : %w", + apiURL, auth.ErrRegistryAuthRequired, + ) + } return nil, fmt.Errorf("API endpoint not functional: %w", err) } } @@ -77,6 +86,16 @@ func (p *APIRegistryProvider) GetRegistry() (*types.Registry, error) { // Fetch all servers from the API servers, err := p.client.ListServers(ctx, nil) if err != nil { + // Propagate auth errors so API handlers can return structured responses. + // ErrRegistryAuthRequired: no token available locally (never tried the registry). + // ErrRegistryUnauthorized: token was sent but rejected by the registry (401/403). + // Both are wrapped with ErrRegistryAuthRequired so the API layer returns 503. + if errors.Is(err, auth.ErrRegistryAuthRequired) { + return nil, fmt.Errorf("no registry credentials available: %w", err) + } + if errors.Is(err, api.ErrRegistryUnauthorized) { + return nil, fmt.Errorf("registry rejected credentials: %w", auth.ErrRegistryAuthRequired) + } return nil, fmt.Errorf("failed to list servers from API: %w", err) }