fix(controlplane): surface token expiry instead of federated error for expired tokens (#3227)

Author: migmartri

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
 						}

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")
+}