diff --git a/app/controlplane/internal/usercontext/attjwtmiddleware/attmiddleware.go b/app/controlplane/internal/usercontext/attjwtmiddleware/attmiddleware.go index ef109ae44..b7dffddca 100644 --- a/app/controlplane/internal/usercontext/attjwtmiddleware/attmiddleware.go +++ b/app/controlplane/internal/usercontext/attjwtmiddleware/attmiddleware.go @@ -275,6 +275,20 @@ func WithJWTMulti(l log.Logger, opts ...JWTOption) middleware.Middleware { // Check if it's the last provider and still failed if err != nil { + // A well-formed, correctly-signed Chainloop token that has simply + // expired must surface as an expiry error. Trying the remaining + // providers or falling back to the federated path would only mask + // the real reason (e.g. "no issuers configured"), leaving the user + // with a misleading message instead of "token has expired". + // + // runProviderValidator returns the ErrTokenExpired sentinel directly, + // so we compare by identity. We intentionally avoid errors.Is here: + // kratos errors compare only Code and Reason, so every 401/UNAUTHORIZED + // error (e.g. an invalid signature) would wrongly match ErrTokenExpired. + if err == ErrTokenExpired { //nolint:errorlint // sentinel identity, see comment above + return nil, ErrTokenExpired + } + if idx < tokenProviderLen-1 { continue } diff --git a/app/controlplane/internal/usercontext/attjwtmiddleware/attmiddleware_test.go b/app/controlplane/internal/usercontext/attjwtmiddleware/attmiddleware_test.go index 10a66eb3d..01078e0c8 100644 --- a/app/controlplane/internal/usercontext/attjwtmiddleware/attmiddleware_test.go +++ b/app/controlplane/internal/usercontext/attjwtmiddleware/attmiddleware_test.go @@ -19,12 +19,19 @@ import ( "context" "io" "net/http" + "net/http/httptest" "testing" + "time" + conf "github.com/chainloop-dev/chainloop/app/controlplane/internal/conf/controlplane/config/v1" "github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext/attjwtmiddleware" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/jwt/user" + "github.com/chainloop-dev/chainloop/pkg/cache" "github.com/go-kratos/kratos/v2/log" "github.com/go-kratos/kratos/v2/transport" + "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var emptyHandler = func(_ context.Context, _ interface{}) (interface{}, error) { return nil, nil } @@ -137,3 +144,52 @@ func TestAttestationAPITokenProvider(t *testing.T) { }) } } + +// newExpiredUserToken builds a Chainloop user token whose expiration is in the +// past, signed with the given key. +func newExpiredUserToken(t *testing.T, key string) string { + t.Helper() + token := jwt.NewWithClaims(user.SigningMethod, jwt.RegisteredClaims{ + Audience: jwt.ClaimStrings{user.Audience}, + ExpiresAt: jwt.NewNumericDate(time.Now().Add(-1 * time.Hour)), + IssuedAt: jwt.NewNumericDate(time.Now().Add(-2 * time.Hour)), + }) + signed, err := token.SignedString([]byte(key)) + require.NoError(t, err) + return signed +} + +// TestExpiredTokenWithFederatedProvider ensures that an expired Chainloop token +// surfaces the proper "token has expired" error even when the federated +// provider is enabled. Otherwise the expiry is masked by the federated path +// (e.g. "no issuers configured"), leaving the user with a misleading message. +func TestExpiredTokenWithFederatedProvider(t *testing.T) { + // Federated endpoint that mimics the masking behaviour: it rejects the + // expired token with an unrelated authorization error. + federatedSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"code":1,"message":"no issuers configured"}`)) + })) + defer federatedSrv.Close() + + claimsCache, err := cache.New[*jwt.MapClaims](cache.WithTTL(time.Minute)) + require.NoError(t, err) + + logger := log.NewStdLogger(io.Discard) + header := newTokenHeader("Authorization", "Bearer "+newExpiredUserToken(t, signingKey)) + ctx := transport.NewServerContext(context.Background(), &mockTransport{reqHeader: header}) + + m := attjwtmiddleware.WithJWTMulti(logger, + attjwtmiddleware.NewRobotAccountProvider(signingKey), + attjwtmiddleware.NewAPITokenProvider(signingKey), + attjwtmiddleware.NewUserTokenProvider(signingKey), + attjwtmiddleware.WithFederatedProvider(&conf.FederatedAuthentication{Enabled: true, Url: federatedSrv.URL}), + attjwtmiddleware.WithClaimsCache(claimsCache), + ) + + _, err = m(emptyHandler)(ctx, nil) + require.Error(t, err) + // The expiry must be reported, not the masked federated error. + assert.ErrorContains(t, err, "JWT token has expired") + assert.NotContains(t, err.Error(), "no issuers configured") +}