diff --git a/packages/auth/pkg/auth/internal/middleware/middleware.go b/packages/auth/pkg/auth/internal/middleware/middleware.go index c0a0cd89af..44b14f3730 100644 --- a/packages/auth/pkg/auth/internal/middleware/middleware.go +++ b/packages/auth/pkg/auth/internal/middleware/middleware.go @@ -148,6 +148,59 @@ func adminValidationFunction(adminToken string) func(ctx context.Context, ginCtx } } +// AuthenticatorConfig describes a header-token security scheme. +// +// The constructors below cover the schemes this package defines, each with +// its scheme name and context key fixed. This exists for a service that +// defines its own — one that verifies a token under a scheme of its own name, +// or records something other than a user or a team. +// +// Without it such a service reimplements the header handling and the 401 +// stamping below, which is how a scheme ends up subtly different from every +// other one rather than merely differently named. +type AuthenticatorConfig[T any] struct { + // SchemeName must match the securityScheme in the service's OpenAPI + // document; the validator dispatches on it. + SchemeName string + + // Header is the header carrying the token. + Header string + + // RequiredPrefix, when set, is a prefix the token must carry for this + // scheme to apply. A token without it is left to another authenticator. + RequiredPrefix string + + // StrippedPrefix is removed before validation, e.g. "Bearer ". + StrippedPrefix string + + // Validate turns a token into whatever the scheme establishes, or an + // APIError carrying the status the caller should see. + Validate func(ctx context.Context, ginCtx *gin.Context, token string) (T, *APIError) + + // SetContext records the result for handlers. Optional: a scheme that + // only proves the caller may proceed has nothing to record. + SetContext func(ginCtx *gin.Context, value T) + + // ErrorMessage prefixes the failure returned to the validator. + ErrorMessage string +} + +// NewAuthenticator builds an Authenticator for a scheme this package does not +// name itself. +func NewAuthenticator[T any](config AuthenticatorConfig[T]) Authenticator { + return &commonAuthenticator[T]{ + schemeName: config.SchemeName, + header: headerKey{ + name: config.Header, + prefix: config.RequiredPrefix, + removePrefix: config.StrippedPrefix, + }, + validationFunc: config.Validate, + setContextFunc: config.SetContext, + errorMessage: config.ErrorMessage, + } +} + // NewApiKeyAuthenticator creates an authenticator for the ApiKeyAuth security scheme (X-API-Key header, e2b_ prefix). func NewApiKeyAuthenticator(validationFunc func(ctx context.Context, ginCtx *gin.Context, token string) (*types.Team, *APIError)) Authenticator { return &commonAuthenticator[*types.Team]{ @@ -205,7 +258,7 @@ func NewAuthProviderTeamAuthenticator(validationFunc func(ctx context.Context, g } // NewAdminJWTAuthenticator creates an authenticator for the AdminJWTAuth security scheme. -func NewAdminJWTAuthenticator(verifier *token.AdminVerifier) Authenticator { +func NewAdminJWTAuthenticator(verifier *token.JWKSVerifier) Authenticator { return &commonAuthenticator[struct{}]{ schemeName: "AdminJWTAuth", header: headerKey{ diff --git a/packages/auth/pkg/auth/internal/middleware/middleware_test.go b/packages/auth/pkg/auth/internal/middleware/middleware_test.go index 2062f7d6ef..59e49f9d60 100644 --- a/packages/auth/pkg/auth/internal/middleware/middleware_test.go +++ b/packages/auth/pkg/auth/internal/middleware/middleware_test.go @@ -79,3 +79,115 @@ func TestAdminTeamAuthenticatorSetsTeamContext(t *testing.T) { t.Errorf("authcontext.GetTeamInfo(ginCtx).Team.ID = %s, want %s", got.Team.ID, teamID) } } + +// A service defining its own scheme gets the same header handling and the +// same 401 stamping as the named ones, and can record something other than a +// user or a team. +func TestNewAuthenticatorAppliesTheConfiguredScheme(t *testing.T) { + t.Parallel() + + const subjectKey = "provider_subject" + + authenticator := NewAuthenticator(AuthenticatorConfig[string]{ + SchemeName: "CustomBearerAuth", + Header: HeaderAuthorization, + StrippedPrefix: PrefixBearer, + Validate: func(_ context.Context, _ *gin.Context, token string) (string, *APIError) { + if token != "good-token" { + return "", &APIError{Err: ErrInvalidAuthHeader, ClientMsg: "nope", Code: http.StatusUnauthorized} + } + + return "subject-1", nil + }, + SetContext: func(c *gin.Context, subject string) { c.Set(subjectKey, subject) }, + ErrorMessage: "Invalid custom token.", + }) + + require.Equal(t, "CustomBearerAuth", authenticator.SecuritySchemeName()) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil) + req.Header.Set(HeaderAuthorization, PrefixBearer+"good-token") + ginCtx, _ := gin.CreateTestContext(httptest.NewRecorder()) + + require.NoError(t, authenticator.Authenticate(t.Context(), ginCtx, &openapi3filter.AuthenticationInput{ + RequestValidationInput: &openapi3filter.RequestValidationInput{Request: req}, + })) + + subject, ok := ginCtx.Get(subjectKey) + require.True(t, ok) + require.Equal(t, "subject-1", subject) +} + +// The prefix is stripped before validation, so a scheme sharing the +// Authorization header does not have to strip it again. +func TestNewAuthenticatorStripsTheConfiguredPrefix(t *testing.T) { + t.Parallel() + + var seen string + authenticator := NewAuthenticator(AuthenticatorConfig[struct{}]{ + SchemeName: "CustomBearerAuth", + Header: HeaderAuthorization, + StrippedPrefix: PrefixBearer, + Validate: func(_ context.Context, _ *gin.Context, token string) (struct{}, *APIError) { + seen = token + + return struct{}{}, nil + }, + }) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil) + req.Header.Set(HeaderAuthorization, PrefixBearer+"raw-token") + ginCtx, _ := gin.CreateTestContext(httptest.NewRecorder()) + + require.NoError(t, authenticator.Authenticate(t.Context(), ginCtx, &openapi3filter.AuthenticationInput{ + RequestValidationInput: &openapi3filter.RequestValidationInput{Request: req}, + })) + require.Equal(t, "raw-token", seen) +} + +// A missing header stamps 401 rather than leaving the validator's 400 +// fallback to win, which is what makes an auth failure look like one. +func TestNewAuthenticatorStamps401OnAMissingHeader(t *testing.T) { + t.Parallel() + + authenticator := NewAuthenticator(AuthenticatorConfig[struct{}]{ + SchemeName: "CustomBearerAuth", + Header: HeaderAuthorization, + Validate: func(context.Context, *gin.Context, string) (struct{}, *APIError) { + return struct{}{}, nil + }, + }) + + recorder := httptest.NewRecorder() + ginCtx, _ := gin.CreateTestContext(recorder) + + err := authenticator.Authenticate(t.Context(), ginCtx, &openapi3filter.AuthenticationInput{ + RequestValidationInput: &openapi3filter.RequestValidationInput{ + Request: httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil), + }, + }) + require.Error(t, err) + require.Equal(t, http.StatusUnauthorized, ginCtx.Writer.Status()) +} + +// SetContext is optional: a scheme that only proves the caller may proceed +// has nothing to record, and must not require a setter to say so. +func TestNewAuthenticatorAllowsNoContextSetter(t *testing.T) { + t.Parallel() + + authenticator := NewAuthenticator(AuthenticatorConfig[struct{}]{ + SchemeName: "CustomBearerAuth", + Header: HeaderAdminToken, + Validate: func(context.Context, *gin.Context, string) (struct{}, *APIError) { + return struct{}{}, nil + }, + }) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil) + req.Header.Set(HeaderAdminToken, "anything") + ginCtx, _ := gin.CreateTestContext(httptest.NewRecorder()) + + require.NoError(t, authenticator.Authenticate(t.Context(), ginCtx, &openapi3filter.AuthenticationInput{ + RequestValidationInput: &openapi3filter.RequestValidationInput{Request: req}, + })) +} diff --git a/packages/auth/pkg/auth/internal/service/service.go b/packages/auth/pkg/auth/internal/service/service.go index abbebb203e..4583648526 100644 --- a/packages/auth/pkg/auth/internal/service/service.go +++ b/packages/auth/pkg/auth/internal/service/service.go @@ -51,7 +51,7 @@ type Service interface { type AuthService struct { store authStore teamCache *authCache - authProviderVerifier *token.ProviderVerifier + authProviderVerifier *token.LinkedOIDCVerifier } // Compile-time assertion that *AuthService satisfies the Service interface. @@ -82,7 +82,7 @@ func NewAuthService( // OIDC bootstrap writes identity rows on the primary immediately before the // next authenticated request; using the read replica here races replication lag. identityLookup := newAuthIdentityLookup(authDB.Queries) - v, err := token.NewProviderVerifier(ctx, providerConfig, httpClient, identityLookup) + v, err := token.NewLinkedOIDCVerifier(ctx, providerConfig, httpClient, identityLookup) if err != nil { return nil, fmt.Errorf("initializing auth provider JWT verifier: %w", err) } @@ -188,7 +188,7 @@ func (s *AuthService) ValidateAuthProviderToken(ctx context.Context, ginCtx *gin return s.validateJWTWithProvider(ctx, ginCtx, s.authProviderVerifier, token, "auth provider") } -func (s *AuthService) validateJWTWithProvider(ctx context.Context, ginCtx *gin.Context, v *token.ProviderVerifier, token string, tokenSource string) (uuid.UUID, *APIError) { +func (s *AuthService) validateJWTWithProvider(ctx context.Context, ginCtx *gin.Context, v *token.LinkedOIDCVerifier, token string, tokenSource string) (uuid.UUID, *APIError) { userID, _, err := v.Verify(ctx, token) if err != nil { return uuid.UUID{}, &APIError{ diff --git a/packages/auth/pkg/auth/internal/token/admin.go b/packages/auth/pkg/auth/internal/token/admin.go deleted file mode 100644 index 54352e5565..0000000000 --- a/packages/auth/pkg/auth/internal/token/admin.go +++ /dev/null @@ -1,69 +0,0 @@ -package token - -import ( - "context" - "errors" - "fmt" - "net/http" - "time" - - "github.com/golang-jwt/jwt/v5" - - "github.com/e2b-dev/infra/packages/auth/pkg/auth/internal/token/jwks" -) - -// adminJWTClockSkew is the leeway applied to time-based claims of admin -// service JWTs. -const adminJWTClockSkew = 30 * time.Second - -// AdminVerifier verifies admin service JWTs against one or more configured -// issuers and returns the first successful verification. -type AdminVerifier struct { - verifiers []*jwks.Verifier -} - -// NewAdminVerifier builds the verifier for the AdminJWTAuth security -// scheme from the same ProviderConfig shape used for AUTH_PROVIDER_CONFIG: -// short-lived service tokens whose signing methods are declared by JWKS keys. -// It returns nil when the config has no issuers, leaving the scheme unconfigured. -func NewAdminVerifier(ctx context.Context, config ProviderConfig, httpClient *http.Client) (*AdminVerifier, error) { - normalized := config.normalize() - if !normalized.enabled() { - return nil, nil - } - - verifiers := make([]*jwks.Verifier, 0, len(normalized.JWT)) - for i, entry := range normalized.JWT { - verifier, err := jwks.NewVerifierFromIssuerJWKS(ctx, entry, httpClient, - jwks.WithParserOptions(jwt.WithLeeway(adminJWTClockSkew)), - ) - if err != nil { - return nil, fmt.Errorf("admin JWT jwt[%d]: %w", i, err) - } - verifiers = append(verifiers, verifier) - } - - return &AdminVerifier{verifiers: verifiers}, nil -} - -// Verify iterates over the configured issuers and returns the claims of the -// first successful verification. -func (v *AdminVerifier) Verify(ctx context.Context, tokenString string) (jwt.MapClaims, error) { - if v == nil || len(v.verifiers) == 0 { - return nil, errors.New("admin JWT verifier is not configured") - } - - errs := make([]error, 0, len(v.verifiers)) - for _, verifier := range v.verifiers { - claims, err := verifier.Verify(ctx, tokenString) - if err != nil { - errs = append(errs, err) - - continue - } - - return claims, nil - } - - return nil, fmt.Errorf("failed to verify admin JWT: %w", errors.Join(errs...)) -} diff --git a/packages/auth/pkg/auth/internal/token/jwks_verifier.go b/packages/auth/pkg/auth/internal/token/jwks_verifier.go new file mode 100644 index 0000000000..1a78af02a5 --- /dev/null +++ b/packages/auth/pkg/auth/internal/token/jwks_verifier.go @@ -0,0 +1,75 @@ +package token + +import ( + "context" + "errors" + "fmt" + "net/http" + "time" + + "github.com/golang-jwt/jwt/v5" + + "github.com/e2b-dev/infra/packages/auth/pkg/auth/internal/token/jwks" +) + +// jwksClockSkew is the leeway applied to time-based claims. Service +// tokens are short-lived, so a clock a little out of step would otherwise +// reject one that is legitimately current. +const jwksClockSkew = 30 * time.Second + +// JWKSVerifier verifies JWTs against one or more configured issuers +// and returns the first successful verification. +// +// Keys come from each issuer's conventional JWKS path rather than an OIDC +// discovery document, which suits a token minted by a peer service: there is +// no discovery to perform, and no separate issuer declaration to cross-check. +type JWKSVerifier struct { + verifiers []*jwks.Verifier +} + +// NewJWKSVerifier builds a verifier from the same ProviderConfig shape +// used for AUTH_PROVIDER_CONFIG. It returns nil when the config declares no +// issuers, leaving whichever scheme uses it unconfigured. +// +// Backs AdminJWTAuth today; nothing about it is specific to that scheme. +func NewJWKSVerifier(ctx context.Context, config ProviderConfig, httpClient *http.Client) (*JWKSVerifier, error) { + normalized := config.normalize() + if !normalized.enabled() { + return nil, nil + } + + verifiers := make([]*jwks.Verifier, 0, len(normalized.JWT)) + for i, entry := range normalized.JWT { + verifier, err := jwks.NewVerifierFromIssuerJWKS(ctx, entry, httpClient, + jwks.WithParserOptions(jwt.WithLeeway(jwksClockSkew)), + ) + if err != nil { + return nil, fmt.Errorf("service token jwt[%d]: %w", i, err) + } + verifiers = append(verifiers, verifier) + } + + return &JWKSVerifier{verifiers: verifiers}, nil +} + +// Verify iterates over the configured issuers and returns the claims of the +// first successful verification. +func (v *JWKSVerifier) Verify(ctx context.Context, tokenString string) (jwt.MapClaims, error) { + if v == nil || len(v.verifiers) == 0 { + return nil, errors.New("service token verifier is not configured") + } + + errs := make([]error, 0, len(v.verifiers)) + for _, verifier := range v.verifiers { + claims, err := verifier.Verify(ctx, tokenString) + if err != nil { + errs = append(errs, err) + + continue + } + + return claims, nil + } + + return nil, fmt.Errorf("failed to verify service token: %w", errors.Join(errs...)) +} diff --git a/packages/auth/pkg/auth/internal/token/admin_test.go b/packages/auth/pkg/auth/internal/token/jwks_verifier_test.go similarity index 86% rename from packages/auth/pkg/auth/internal/token/admin_test.go rename to packages/auth/pkg/auth/internal/token/jwks_verifier_test.go index a4daf0e40e..6915baaf57 100644 --- a/packages/auth/pkg/auth/internal/token/admin_test.go +++ b/packages/auth/pkg/auth/internal/token/jwks_verifier_test.go @@ -20,7 +20,7 @@ const ( adminTestAudience = "fx1" ) -func newAdminTestVerifier(t *testing.T) (*AdminVerifier, ed25519.PrivateKey, string) { +func newAdminTestVerifier(t *testing.T) (*JWKSVerifier, ed25519.PrivateKey, string) { t.Helper() publicKey, privateKey, err := ed25519.GenerateKey(rand.Reader) @@ -28,7 +28,7 @@ func newAdminTestVerifier(t *testing.T) (*AdminVerifier, ed25519.PrivateKey, str server := jwks.NewTestServer(t, publicKey, adminTestKeyID, jose.EdDSA, "https://unexpected.example.com") - verifier, err := NewAdminVerifier(t.Context(), ProviderConfig{ + verifier, err := NewJWKSVerifier(t.Context(), ProviderConfig{ JWT: []jwks.Config{{ Issuer: jwks.Issuer{ URL: server.URL, @@ -52,7 +52,7 @@ func signAdminToken(t *testing.T, privateKey ed25519.PrivateKey, claims jwt.MapC return signed } -func TestAdminVerifier(t *testing.T) { +func TestJWKSVerifier(t *testing.T) { t.Parallel() verifier, privateKey, issuer := newAdminTestVerifier(t) @@ -91,10 +91,10 @@ func TestAdminVerifier(t *testing.T) { }) } -func TestAdminVerifierDisabled(t *testing.T) { +func TestJWKSVerifierDisabled(t *testing.T) { t.Parallel() - verifier, err := NewAdminVerifier(t.Context(), ProviderConfig{}, nil) + verifier, err := NewJWKSVerifier(t.Context(), ProviderConfig{}, nil) require.NoError(t, err) require.Nil(t, verifier) @@ -102,7 +102,7 @@ func TestAdminVerifierDisabled(t *testing.T) { require.ErrorContains(t, err, "not configured") } -func TestAdminVerifierRejectsNonEdDSA(t *testing.T) { +func TestJWKSVerifierRejectsNonEdDSA(t *testing.T) { t.Parallel() verifier, _, issuer := newAdminTestVerifier(t) @@ -120,7 +120,7 @@ func TestAdminVerifierRejectsNonEdDSA(t *testing.T) { require.Error(t, err) } -func TestAdminVerifierES256(t *testing.T) { +func TestJWKSVerifierES256(t *testing.T) { t.Parallel() privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) @@ -129,7 +129,7 @@ func TestAdminVerifierES256(t *testing.T) { server := jwks.NewTestServer(t, &privateKey.PublicKey, adminTestKeyID, jose.ES256, "https://unexpected.example.com") issuer := server.URL - verifier, err := NewAdminVerifier(t.Context(), ProviderConfig{ + verifier, err := NewJWKSVerifier(t.Context(), ProviderConfig{ JWT: []jwks.Config{{ Issuer: jwks.Issuer{ URL: issuer, @@ -152,7 +152,7 @@ func TestAdminVerifierES256(t *testing.T) { require.NoError(t, err) } -func TestAdminVerifierRejectsJWKSKeyWithoutAlgorithm(t *testing.T) { +func TestJWKSVerifierRejectsJWKSKeyWithoutAlgorithm(t *testing.T) { t.Parallel() publicKey, _, err := ed25519.GenerateKey(rand.Reader) @@ -160,7 +160,7 @@ func TestAdminVerifierRejectsJWKSKeyWithoutAlgorithm(t *testing.T) { server := jwks.NewTestServer(t, publicKey, adminTestKeyID, "", "https://unexpected.example.com") - verifier, err := NewAdminVerifier(t.Context(), ProviderConfig{ + verifier, err := NewJWKSVerifier(t.Context(), ProviderConfig{ JWT: []jwks.Config{{ Issuer: jwks.Issuer{ URL: server.URL, diff --git a/packages/auth/pkg/auth/internal/token/oidc/oidc.go b/packages/auth/pkg/auth/internal/token/oidc/oidc.go index 5d8cbaab2b..79ef50b127 100644 --- a/packages/auth/pkg/auth/internal/token/oidc/oidc.go +++ b/packages/auth/pkg/auth/internal/token/oidc/oidc.go @@ -23,8 +23,21 @@ type IdentityLookup interface { GetUserIdentity(ctx context.Context, iss, sub string) (uuid.UUID, error) } -// Verifier verifies JWTs against a single OIDC issuer and resolves the -// internal user for the token's identity. +// TokenIdentity is what a verified token asserts about itself, before +// anything has been looked up. Both claims are required: a token that names +// no issuer or no subject identifies nobody, whatever its signature says. +type TokenIdentity struct { + Issuer string + Subject string + Claims jwt.MapClaims +} + +// Verifier verifies JWTs against a single OIDC issuer. +// +// It answers at two levels. VerifyIdentity establishes what the token says it +// is; Verify goes further and resolves the internal user behind it. The +// second needs an IdentityLookup, the first does not — which matters for a +// caller acting before the user it will name exists. type Verifier struct { tokens *jwks.Verifier identities IdentityLookup @@ -38,6 +51,20 @@ func NewVerifier(ctx context.Context, entry jwks.Config, httpClient *http.Client return nil, errors.New("OIDC identity lookup is required") } + return newVerifier(ctx, entry, httpClient, identities) +} + +// NewIdentityVerifier constructs a Verifier that establishes what a token +// asserts without resolving it to a user. Only VerifyIdentity is available; +// Verify reports that no lookup is configured. +// +// Same discovery and issuer validation as NewVerifier — the difference is +// what the result is used for, not how far the token is trusted. +func NewIdentityVerifier(ctx context.Context, entry jwks.Config, httpClient *http.Client) (*Verifier, error) { + return newVerifier(ctx, entry, httpClient, nil) +} + +func newVerifier(ctx context.Context, entry jwks.Config, httpClient *http.Client, identities IdentityLookup) (*Verifier, error) { tokens, err := jwks.NewVerifier(ctx, entry, httpClient) if err != nil { return nil, err @@ -54,27 +81,42 @@ func NewVerifier(ctx context.Context, entry jwks.Config, httpClient *http.Client // IdentityLookup. When the token is valid but no matching identity exists, // the returned error wraps ErrIdentityNotFound. func (v *Verifier) Verify(ctx context.Context, tokenString string) (uuid.UUID, jwt.MapClaims, error) { + if v.identities == nil { + return uuid.Nil, nil, errors.New("auth provider verifier has no identity lookup") + } + + identity, err := v.VerifyIdentity(ctx, tokenString) + if err != nil { + return uuid.Nil, nil, err + } + + userID, err := v.identities.GetUserIdentity(ctx, identity.Issuer, identity.Subject) + if err != nil { + return uuid.Nil, nil, fmt.Errorf("resolve user identity for auth provider token: %w", err) + } + + return userID, identity.Claims, nil +} + +// VerifyIdentity validates the token and reports the issuer and subject it +// asserts, without consulting any identity store. +func (v *Verifier) VerifyIdentity(ctx context.Context, tokenString string) (TokenIdentity, error) { claims, err := v.tokens.Verify(ctx, tokenString) if err != nil { - return uuid.Nil, nil, fmt.Errorf("failed to verify auth provider token: %w", err) + return TokenIdentity{}, fmt.Errorf("failed to verify auth provider token: %w", err) } iss, ok := claimString(claims, "iss") if !ok { - return uuid.Nil, nil, errors.New("auth provider token is missing iss claim") + return TokenIdentity{}, errors.New("auth provider token is missing iss claim") } sub, ok := claimString(claims, "sub") if !ok { - return uuid.Nil, nil, errors.New("auth provider token is missing sub claim") - } - - userID, err := v.identities.GetUserIdentity(ctx, iss, sub) - if err != nil { - return uuid.Nil, nil, fmt.Errorf("resolve user identity for auth provider token: %w", err) + return TokenIdentity{}, errors.New("auth provider token is missing sub claim") } - return userID, claims, nil + return TokenIdentity{Issuer: iss, Subject: sub, Claims: claims}, nil } func claimString(claims jwt.MapClaims, name string) (string, bool) { diff --git a/packages/auth/pkg/auth/internal/token/oidc/oidc_test.go b/packages/auth/pkg/auth/internal/token/oidc/oidc_test.go index bf5b03758f..2da0516c98 100644 --- a/packages/auth/pkg/auth/internal/token/oidc/oidc_test.go +++ b/packages/auth/pkg/auth/internal/token/oidc/oidc_test.go @@ -209,3 +209,105 @@ func TestNewVerifier_DiscoveryIssuerMismatch(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "issuer") } + +// A verifier built without a lookup reports what the token asserts and never +// consults an identity store — the case a caller has when it is about to +// create the user the token names. +func TestIdentityVerifier_ReportsClaimsWithoutResolving(t *testing.T) { + t.Parallel() + + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + const keyID = "test-key" + server := jwks.NewTestServer(t, &privateKey.PublicKey, keyID, jose.RS256, testIssuerURL) + + verifier, err := NewIdentityVerifier(t.Context(), jwks.Config{ + Issuer: jwks.Issuer{ + URL: testIssuerURL, + DiscoveryURL: server.URL + "/.well-known/openid-configuration", + Audiences: []string{"dashboard-api"}, + }, + CacheDuration: time.Minute, + }, server.Client()) + require.NoError(t, err) + + const tokenSub = "external-subject-123" + token := jwt.NewWithClaims(jwt.SigningMethodRS256, jwt.MapClaims{ + "iss": testIssuerURL, + "aud": "dashboard-api", + "sub": tokenSub, + "exp": time.Now().Add(time.Hour).Unix(), + }) + token.Header["kid"] = keyID + + signedToken, err := token.SignedString(privateKey) + require.NoError(t, err) + + identity, err := verifier.VerifyIdentity(t.Context(), signedToken) + require.NoError(t, err) + require.Equal(t, testIssuerURL, identity.Issuer) + require.Equal(t, tokenSub, identity.Subject) + require.NotNil(t, identity.Claims) + + // Resolving is the one thing it cannot do, rather than something it does + // badly by returning a zero user. + _, _, err = verifier.Verify(t.Context(), signedToken) + require.ErrorContains(t, err, "no identity lookup") +} + +// The subject is only meaningful alongside the issuer that vouched for it, so +// a token missing either identifies nobody however well it is signed. +func TestIdentityVerifier_RejectsATokenWithNoSubject(t *testing.T) { + t.Parallel() + + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + const keyID = "test-key" + server := jwks.NewTestServer(t, &privateKey.PublicKey, keyID, jose.RS256, testIssuerURL) + + verifier, err := NewIdentityVerifier(t.Context(), jwks.Config{ + Issuer: jwks.Issuer{ + URL: testIssuerURL, + DiscoveryURL: server.URL + "/.well-known/openid-configuration", + Audiences: []string{"dashboard-api"}, + }, + CacheDuration: time.Minute, + }, server.Client()) + require.NoError(t, err) + + token := jwt.NewWithClaims(jwt.SigningMethodRS256, jwt.MapClaims{ + "iss": testIssuerURL, + "aud": "dashboard-api", + "exp": time.Now().Add(time.Hour).Unix(), + }) + token.Header["kid"] = keyID + + signedToken, err := token.SignedString(privateKey) + require.NoError(t, err) + + _, err = verifier.VerifyIdentity(t.Context(), signedToken) + require.ErrorContains(t, err, "missing sub claim") +} + +// The discovery document and its issuer are validated whichever level the +// caller verifies at. Choosing not to resolve must not weaken the token. +func TestNewIdentityVerifier_RejectsADiscoveryIssuerMismatch(t *testing.T) { + t.Parallel() + + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + server := jwks.NewTestServer(t, &privateKey.PublicKey, "test-key", jose.RS256, "https://someone-else.example.com") + + _, err = NewIdentityVerifier(t.Context(), jwks.Config{ + Issuer: jwks.Issuer{ + URL: testIssuerURL, + DiscoveryURL: server.URL + "/.well-known/openid-configuration", + Audiences: []string{"dashboard-api"}, + }, + CacheDuration: time.Minute, + }, server.Client()) + require.ErrorContains(t, err, "does not match configured issuer") +} diff --git a/packages/auth/pkg/auth/internal/token/provider.go b/packages/auth/pkg/auth/internal/token/provider.go index f48aafe720..99a46423f8 100644 --- a/packages/auth/pkg/auth/internal/token/provider.go +++ b/packages/auth/pkg/auth/internal/token/provider.go @@ -44,26 +44,61 @@ func (c ProviderConfig) validate() error { return nil } -// strategy is the interface satisfied by per-provider JWT verifiers used by -// ProviderVerifier. +// strategy is the interface satisfied by per-issuer JWT verifiers. type strategy interface { Verify(ctx context.Context, tokenString string) (uuid.UUID, jwt.MapClaims, error) + VerifyIdentity(ctx context.Context, tokenString string) (oidc.TokenIdentity, error) } -// ProviderVerifier aggregates one or more OIDC JWT verification strategies and -// returns the first that succeeds. -type ProviderVerifier struct { +// OIDCVerifier verifies auth-provider JWTs against every issuer in a +// ProviderConfig, via each issuer's OIDC discovery document, and reports what +// the first accepting issuer says the token asserts. +// +// It links the token to nothing. A caller that needs the internal user behind +// it wants LinkedOIDCVerifier, which is this plus a lookup. +type OIDCVerifier struct { strategies []strategy } -// NewProviderVerifier constructs a *ProviderVerifier from the given -// ProviderConfig. +// LinkedOIDCVerifier is an OIDCVerifier that also resolves the asserted +// identity to an internal user. // -// When the provided config has no JWT issuers, NewProviderVerifier returns -// (nil, nil). This is a valid configuration: the caller can pass the nil -// ProviderVerifier along, and any token verification attempt will be denied at -// runtime by ProviderVerifier.Verify. -func NewProviderVerifier(ctx context.Context, config ProviderConfig, oidcHTTPClient *http.Client, identities oidc.IdentityLookup) (*ProviderVerifier, error) { +// Separate type rather than a flag, so a verifier built without a lookup has +// no Verify to call. The capability is visible where the verifier is passed +// rather than discovered when a request reaches it. +type LinkedOIDCVerifier struct { + OIDCVerifier + + identities oidc.IdentityLookup +} + +// NewOIDCVerifier builds a verifier over every issuer in the config, +// returning (nil, nil) when it declares none. A nil verifier denies +// everything, so an unconfigured provider needs no branch at the call site. +func NewOIDCVerifier(ctx context.Context, config ProviderConfig, oidcHTTPClient *http.Client) (*OIDCVerifier, error) { + return newOIDCVerifier(ctx, config, oidcHTTPClient, nil) +} + +// NewLinkedOIDCVerifier builds a verifier that also resolves the asserted +// identity through the supplied lookup. +func NewLinkedOIDCVerifier(ctx context.Context, config ProviderConfig, oidcHTTPClient *http.Client, identities oidc.IdentityLookup) (*LinkedOIDCVerifier, error) { + // Only when there is something to resolve. A config declaring no issuers + // yields a nil verifier that denies everything, and demanding a lookup to + // reach that conclusion would make an unconfigured provider a startup + // failure rather than a supported state. + if len(config.normalize().JWT) > 0 && identities == nil { + return nil, errors.New("auth provider OIDC identity lookup is required when JWT issuers are configured") + } + + verifier, err := newOIDCVerifier(ctx, config, oidcHTTPClient, identities) + if err != nil || verifier == nil { + return nil, err + } + + return &LinkedOIDCVerifier{OIDCVerifier: *verifier, identities: identities}, nil +} + +func newOIDCVerifier(ctx context.Context, config ProviderConfig, oidcHTTPClient *http.Client, identities oidc.IdentityLookup) (*OIDCVerifier, error) { normalized := config.normalize() if err := normalized.validate(); err != nil { return nil, err @@ -73,13 +108,8 @@ func NewProviderVerifier(ctx context.Context, config ProviderConfig, oidcHTTPCli } strategies := make([]strategy, 0, len(normalized.JWT)) - - if len(normalized.JWT) > 0 && identities == nil { - return nil, errors.New("auth provider OIDC identity lookup is required when JWT issuers are configured") - } - for i, entry := range normalized.JWT { - s, err := oidc.NewVerifier(ctx, entry, oidcHTTPClient, identities) + s, err := newStrategy(ctx, entry, oidcHTTPClient, identities) if err != nil { return nil, fmt.Errorf("auth provider jwt[%d]: %w", i, err) } @@ -90,14 +120,66 @@ func NewProviderVerifier(ctx context.Context, config ProviderConfig, oidcHTTPCli return nil, errors.New("auth provider verifier has no configured signing verifier") } - return &ProviderVerifier{ - strategies: strategies, - }, nil + return &OIDCVerifier{strategies: strategies}, nil +} + +// newStrategy builds the per-issuer verifier. Both levels perform the same +// discovery and issuer validation; the lookup only decides whether the result +// can be resolved further. +func newStrategy(ctx context.Context, entry jwks.Config, httpClient *http.Client, identities oidc.IdentityLookup) (strategy, error) { + if identities == nil { + return oidc.NewIdentityVerifier(ctx, entry, httpClient) + } + + return oidc.NewVerifier(ctx, entry, httpClient, identities) +} + +// VerifyIdentity iterates over the configured issuers and returns what the +// first one to accept the token says it asserts. +// +// Unlike Verify there is no non-nil user id to insist on, because nothing has +// been resolved: the caller is asking who the token claims to be, which is +// the question worth asking before that person exists. +func (v *OIDCVerifier) VerifyIdentity(ctx context.Context, tokenString string) (oidc.TokenIdentity, error) { + if v == nil { + return oidc.TokenIdentity{}, errors.New("auth provider verifier is not configured") + } + + if len(v.strategies) == 0 { + return oidc.TokenIdentity{}, errors.New("auth provider verifier strategies are not configured") + } + + errs := make([]error, 0, len(v.strategies)) + for _, strategy := range v.strategies { + identity, err := strategy.VerifyIdentity(ctx, tokenString) + if err != nil { + errs = append(errs, err) + + continue + } + + return identity, nil + } + + return oidc.TokenIdentity{}, fmt.Errorf("failed to verify auth provider token: %w", errors.Join(errs...)) } // Verify iterates over the configured strategies and returns the first that // successfully verifies the token and resolves a non-nil internal user UUID. -func (v *ProviderVerifier) Verify(ctx context.Context, tokenString string) (uuid.UUID, jwt.MapClaims, error) { +// VerifyIdentity shadows the promoted method so a nil verifier denies rather +// than panics. Promotion computes the address of the embedded value, which +// dereferences the outer pointer before the inner nil check can run, and a +// nil verifier is a supported state here: an unconfigured provider yields one +// and callers pass it along. +func (v *LinkedOIDCVerifier) VerifyIdentity(ctx context.Context, tokenString string) (oidc.TokenIdentity, error) { + if v == nil { + return oidc.TokenIdentity{}, errors.New("auth provider verifier is not configured") + } + + return v.OIDCVerifier.VerifyIdentity(ctx, tokenString) +} + +func (v *LinkedOIDCVerifier) Verify(ctx context.Context, tokenString string) (uuid.UUID, jwt.MapClaims, error) { if v == nil { return uuid.Nil, nil, errors.New("auth provider verifier is not configured") } diff --git a/packages/auth/pkg/auth/internal/token/provider_test.go b/packages/auth/pkg/auth/internal/token/provider_test.go index 8e78a47ced..197ba66ff9 100644 --- a/packages/auth/pkg/auth/internal/token/provider_test.go +++ b/packages/auth/pkg/auth/internal/token/provider_test.go @@ -66,7 +66,7 @@ func httpClientForServers(servers ...*httptest.Server) *http.Client { func TestNewVerifier_DisabledConfigReturnsNil(t *testing.T) { t.Parallel() - verifier, err := NewProviderVerifier(t.Context(), ProviderConfig{}, nil, nil) + verifier, err := NewLinkedOIDCVerifier(t.Context(), ProviderConfig{}, nil, nil) require.NoError(t, err) require.Nil(t, verifier) } @@ -85,7 +85,7 @@ func TestVerifier_VerifyJWT(t *testing.T) { jwksUserID := uuid.New() lookup.set(testIssuerURL, jwksSub, jwksUserID) - verifier, err := NewProviderVerifier(t.Context(), ProviderConfig{ + verifier, err := NewLinkedOIDCVerifier(t.Context(), ProviderConfig{ JWT: []jwks.Config{ { Issuer: jwks.Issuer{ @@ -137,7 +137,7 @@ func TestVerifier_VerifyMultipleJWTIssuers(t *testing.T) { userID := uuid.New() lookup.set(issuer2URL, tokenSub, userID) - verifier, err := NewProviderVerifier(t.Context(), ProviderConfig{ + verifier, err := NewLinkedOIDCVerifier(t.Context(), ProviderConfig{ JWT: []jwks.Config{ { Issuer: jwks.Issuer{ @@ -174,3 +174,24 @@ func TestVerifier_VerifyMultipleJWTIssuers(t *testing.T) { require.NoError(t, err) require.Equal(t, userID, gotUserID) } + +// An unconfigured provider yields a nil verifier that callers pass along, so +// every method on it must deny rather than panic — including the one promoted +// from the embedded verifier, which would otherwise dereference the outer +// pointer before its own nil check. +func TestNilVerifiersDenyRatherThanPanic(t *testing.T) { + t.Parallel() + + var linked *LinkedOIDCVerifier + + _, _, err := linked.Verify(t.Context(), "token") + require.ErrorContains(t, err, "not configured") + + _, err = linked.VerifyIdentity(t.Context(), "token") + require.ErrorContains(t, err, "not configured") + + var unlinked *OIDCVerifier + + _, err = unlinked.VerifyIdentity(t.Context(), "token") + require.ErrorContains(t, err, "not configured") +} diff --git a/packages/auth/pkg/auth/middleware.go b/packages/auth/pkg/auth/middleware.go index 3de8a220ee..7ce3423005 100644 --- a/packages/auth/pkg/auth/middleware.go +++ b/packages/auth/pkg/auth/middleware.go @@ -18,6 +18,17 @@ var ( type Authenticator = internalauthmiddleware.Authenticator +// AuthenticatorConfig describes a header-token security scheme for +// NewAuthenticator. +type AuthenticatorConfig[T any] = internalauthmiddleware.AuthenticatorConfig[T] + +// NewAuthenticator builds an Authenticator for a scheme this package does not +// name itself. Services using one of the schemes below want that constructor +// instead — it already carries the right scheme name, header and context key. +func NewAuthenticator[T any](config AuthenticatorConfig[T]) Authenticator { + return internalauthmiddleware.NewAuthenticator(config) +} + func NewApiKeyAuthenticator(validationFunc func(context.Context, *gin.Context, string) (*types.Team, *APIError)) Authenticator { return internalauthmiddleware.NewApiKeyAuthenticator(validationFunc) } @@ -34,7 +45,7 @@ func NewAuthProviderTeamAuthenticator(validationFunc func(context.Context, *gin. return internalauthmiddleware.NewAuthProviderTeamAuthenticator(validationFunc) } -func NewAdminJWTAuthenticator(verifier *AdminVerifier) Authenticator { +func NewAdminJWTAuthenticator(verifier *JWKSVerifier) Authenticator { return internalauthmiddleware.NewAdminJWTAuthenticator(verifier) } diff --git a/packages/auth/pkg/auth/token.go b/packages/auth/pkg/auth/token.go index e18558c980..424c26bd3a 100644 --- a/packages/auth/pkg/auth/token.go +++ b/packages/auth/pkg/auth/token.go @@ -19,14 +19,52 @@ type AudienceMatchPolicy = jwks.AudienceMatchPolicy const AudienceMatchAny = jwks.AudienceMatchAny -type AdminVerifier = token.AdminVerifier +// The three verifiers below sit on one axis: how a token's keys are found, +// and how far the result is carried. +// +// JWKSVerifier keys from the issuer's JWKS path; claims +// OIDCVerifier keys via OIDC discovery; what the token asserts +// LinkedOIDCVerifier the above, resolved to an internal user +// +// Each adds to the one before it. Picking a lower one is a decision about +// what the caller needs, never a weakening of the token: discovery and issuer +// validation are identical across both OIDC levels. + +// JWKSVerifier verifies a JWT against keys published at each issuer's +// conventional JWKS path, without an OIDC discovery document. +// +// Suited to a token minted by a peer service, where there is no discovery to +// perform. Not suited to one from an identity provider: it neither fetches +// the discovery document nor cross-checks the issuer that document declares. +type JWKSVerifier = token.JWKSVerifier func ParseProviderConfig(value string) (ProviderConfig, error) { return token.ParseProviderConfig(value) } -func NewAdminVerifier(ctx context.Context, config ProviderConfig, httpClient *http.Client) (*AdminVerifier, error) { - return token.NewAdminVerifier(ctx, config, httpClient) +// NewJWKSVerifier builds a verifier for every issuer in the config, returning +// nil when it declares none. A nil verifier denies everything. +func NewJWKSVerifier(ctx context.Context, config ProviderConfig, httpClient *http.Client) (*JWKSVerifier, error) { + return token.NewJWKSVerifier(ctx, config, httpClient) +} + +// TokenIdentity is what a verified token asserts about itself: the issuer and +// subject it names, and the claims it carried. +type TokenIdentity = oidc.TokenIdentity + +// OIDCVerifier verifies auth-provider tokens across every configured issuer +// and reports what they assert, linking them to nobody. +// +// For a caller that must read a subject before that subject is a user here — +// signup, or anything else creating the identity it authenticates. The +// alternative, giving a linked verifier a lookup that answers "nobody", makes +// not-found a value and turns any missed check into an authenticated nobody. +type OIDCVerifier = token.OIDCVerifier + +// NewOIDCVerifier builds a verifier that reports what tokens assert without +// resolving them. +func NewOIDCVerifier(ctx context.Context, config ProviderConfig, httpClient *http.Client) (*OIDCVerifier, error) { + return token.NewOIDCVerifier(ctx, config, httpClient) } // IdentityLookup resolves an OIDC identity (issuer + subject) to an internal @@ -36,7 +74,7 @@ type IdentityLookup = oidc.IdentityLookup // OIDCVerifier verifies auth-provider user JWTs for a single issuer and // resolves the internal user identity through an IdentityLookup. -type OIDCVerifier = oidc.Verifier +type OIDCIssuerVerifier = oidc.Verifier // ErrIdentityNotFound is returned by OIDC verification when the token is // valid but no matching identity exists. @@ -45,17 +83,17 @@ var ErrIdentityNotFound = oidc.ErrIdentityNotFound // NewOIDCVerifier constructs an OIDCVerifier for the given issuer config. // External consumers (e.g. belt) use this to verify auth-provider tokens with // their own identity storage. -func NewOIDCVerifier(ctx context.Context, config JWTConfig, httpClient *http.Client, identities IdentityLookup) (*OIDCVerifier, error) { +func NewOIDCIssuerVerifier(ctx context.Context, config JWTConfig, httpClient *http.Client, identities IdentityLookup) (*OIDCIssuerVerifier, error) { return oidc.NewVerifier(ctx, config, httpClient, identities) } -// ProviderVerifier verifies auth-provider user JWTs across every issuer in a -// ProviderConfig. -type ProviderVerifier = token.ProviderVerifier +// LinkedOIDCVerifier verifies auth-provider user JWTs across every issuer in +// a ProviderConfig and resolves each to the internal user it names. +type LinkedOIDCVerifier = token.LinkedOIDCVerifier -// NewProviderVerifier constructs a ProviderVerifier for the given provider -// config. When the config has no JWT issuers it returns (nil, nil); a nil -// ProviderVerifier denies all verification attempts at runtime. -func NewProviderVerifier(ctx context.Context, config ProviderConfig, httpClient *http.Client, identities IdentityLookup) (*ProviderVerifier, error) { - return token.NewProviderVerifier(ctx, config, httpClient, identities) +// NewLinkedOIDCVerifier constructs a verifier for the given provider config. +// When the config has no JWT issuers it returns (nil, nil); a nil verifier +// denies all verification attempts at runtime. +func NewLinkedOIDCVerifier(ctx context.Context, config ProviderConfig, httpClient *http.Client, identities IdentityLookup) (*LinkedOIDCVerifier, error) { + return token.NewLinkedOIDCVerifier(ctx, config, httpClient, identities) } diff --git a/packages/dashboard-api/main.go b/packages/dashboard-api/main.go index f38dcb64fe..059fa726f9 100644 --- a/packages/dashboard-api/main.go +++ b/packages/dashboard-api/main.go @@ -246,7 +246,7 @@ func run() int { } swagger.Servers = nil - adminVerifier, err := sharedauth.NewAdminVerifier(ctx, config.AdminAuthProvider, authClient) + adminVerifier, err := sharedauth.NewJWKSVerifier(ctx, config.AdminAuthProvider, authClient) if err != nil { l.Error(ctx, "initializing admin JWT verifier", zap.Error(err)) diff --git a/packages/local-dev/local-dev b/packages/local-dev/local-dev new file mode 100755 index 0000000000..ff65e3a027 Binary files /dev/null and b/packages/local-dev/local-dev differ