diff --git a/cla-backend-go/auth/trusted_caller.go b/cla-backend-go/auth/trusted_caller.go new file mode 100644 index 000000000..2d59c7d77 --- /dev/null +++ b/cla-backend-go/auth/trusted_caller.go @@ -0,0 +1,262 @@ +// Copyright The Linux Foundation and each contributor to CommunityBridge. +// SPDX-License-Identifier: MIT + +package auth + +import ( + "crypto/rsa" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "io" + "math/big" + "net/http" + "path" + "strings" + "sync" + "time" + + "github.com/golang-jwt/jwt/v4" + log "github.com/linuxfoundation/easycla/cla-backend-go/logging" +) + +const ( + bearerPrefix = "bearer " + defaultJWTAlgorithm = "RS256" + jwksCacheTTL = 15 * time.Minute + jwksRefreshCooldown = time.Minute + jwksStaleKeyGrace = 24 * time.Hour + jwksResponseSizeLimit = 1 << 20 + jwksRequestTimeout = 10 * time.Second +) + +// ErrNoBearerToken is returned when the request carries no parseable bearer token. An absent +// header must be denied like an invalid one: the traefik aws-lambda middleware drops duplicated +// headers, so a duplicated Authorization header reaches the lambda as an absent one. +var ErrNoBearerToken = errors.New("no bearer token in the Authorization header") + +// TrustedCaller is the OAuth2 client (azp) and subject of a signature-verified bearer token +type TrustedCaller struct { + ClientID string + Subject string + Trusted bool +} + +// TrustedCallerVerifier verifies bearer tokens against a single Auth0 tenant's JWKS and reports +// whether the token's azp claim names an allow-listed client +type TrustedCallerVerifier struct { + wellKnownURL string + algorithm string + allowedClientIDs map[string]bool + + mu sync.Mutex + keys map[string]*rsa.PublicKey + keysExpireAt time.Time + nextRefreshAt time.Time + + fetchKeys func() (map[string]*rsa.PublicKey, error) + now func() time.Time +} + +// NewTrustedCallerVerifier creates a verifier for the Auth0 tenant at the given domain that +// trusts the given client IDs. An empty allow-list yields a disabled verifier (see Enabled). +func NewTrustedCallerVerifier(domain, algorithm string, allowedClientIDs []string) (*TrustedCallerVerifier, error) { + allowed := make(map[string]bool, len(allowedClientIDs)) + for _, clientID := range allowedClientIDs { + if clientID = strings.TrimSpace(clientID); clientID != "" { + allowed[clientID] = true + } + } + if len(allowed) > 0 && domain == "" { + return nil, errors.New("missing Domain for the trusted caller verifier") + } + if algorithm == "" { + algorithm = defaultJWTAlgorithm + } + + verifier := &TrustedCallerVerifier{ + wellKnownURL: "https://" + path.Join(domain, ".well-known/jwks.json"), + algorithm: algorithm, + allowedClientIDs: allowed, + now: time.Now, + } + verifier.fetchKeys = verifier.fetchJWKS + return verifier, nil +} + +// Enabled reports whether any trusted client ID is configured +func (v *TrustedCallerVerifier) Enabled() bool { + return v != nil && len(v.allowedClientIDs) > 0 +} + +// Verify signature-verifies the bearer token in the Authorization header against the tenant JWKS +// and reports the client (azp) and subject it was issued to +func (v *TrustedCallerVerifier) Verify(authorization string) (*TrustedCaller, error) { + rawToken, err := bearerToken(authorization) + if err != nil { + return nil, err + } + + claims := jwt.MapClaims{} + parser := jwt.NewParser(jwt.WithValidMethods([]string{v.algorithm})) + token, err := parser.ParseWithClaims(rawToken, claims, v.signingKey) + if err != nil { + return nil, err + } + if !token.Valid { + return nil, errors.New("invalid bearer token") + } + // jwt.MapClaims treats exp as optional + if _, ok := claims["exp"]; !ok { + return nil, errors.New("bearer token carries no expiration") + } + clientID := stringClaim(claims, "azp") + + // Matching azp against an allow-list is sound ONLY while no user can hold a token carrying an + // allow-listed azp - only then does the azp mean "the SS backend built this request". Minting + // server-side with a client secret is NOT sufficient: the token SS currently sends here is + // minted that way and then handed to every logged-in user as v1Token by SS's + // GET /api/profile/developer, so that client ID must not be allow-listed. Allow-list only a + // client whose tokens are never surfaced to a user; otherwise any user can pass any identity. + // + // Transitional mechanism (P3/P9 of the trust-SS decision): at M6, once EasyCLA runs on the + // K8s cluster, it should call lfx.auth-service.user_identity.list itself over NATS and drop + // both this azp check and the caller-supplied identity list it authorizes. + return &TrustedCaller{ + ClientID: clientID, + Subject: stringClaim(claims, "sub"), + Trusted: clientID != "" && v.allowedClientIDs[clientID], + }, nil +} + +func stringClaim(claims jwt.MapClaims, name string) string { + value, ok := claims[name].(string) + if !ok { + return "" + } + return value +} + +func bearerToken(authorization string) (string, error) { + value := strings.TrimSpace(authorization) + if len(value) <= len(bearerPrefix) || !strings.EqualFold(value[:len(bearerPrefix)], bearerPrefix) { + return "", ErrNoBearerToken + } + rawToken := strings.TrimSpace(value[len(bearerPrefix):]) + if rawToken == "" { + return "", ErrNoBearerToken + } + return rawToken, nil +} + +func (v *TrustedCallerVerifier) signingKey(token *jwt.Token) (interface{}, error) { + kid, ok := token.Header["kid"].(string) + if !ok || kid == "" { + return nil, errors.New("bearer token carries no key ID") + } + return v.publicKey(kid) +} + +func (v *TrustedCallerVerifier) publicKey(kid string) (*rsa.PublicKey, error) { + v.mu.Lock() + defer v.mu.Unlock() + + now := v.now() + key, cached := v.keys[kid] + if cached && now.Before(v.keysExpireAt) { + return key, nil + } + usableWhileStale := cached && now.Before(v.keysExpireAt.Add(jwksStaleKeyGrace)) + + // a cache miss means a tenant key rotation or a JWKS outage, so rate limit the reload it + // triggers - without this an outage costs every request a fetch timeout once the TTL expires + if now.Before(v.nextRefreshAt) { + if usableWhileStale { + return key, nil + } + return nil, fmt.Errorf("unknown signing key ID: %s - the JWKS reload is rate limited", kid) + } + + v.nextRefreshAt = now.Add(jwksRefreshCooldown) + keys, err := v.fetchKeys() + if err != nil { + // bounded, so a revoked key cannot stay usable for the whole length of a JWKS outage + if usableWhileStale { + log.WithError(err).Warn("unable to refresh the JWKS - using the cached signing key") + return key, nil + } + return nil, err + } + v.keys = keys + v.keysExpireAt = now.Add(jwksCacheTTL) + + refreshed, ok := keys[kid] + if !ok { + return nil, fmt.Errorf("unknown signing key ID: %s", kid) + } + return refreshed, nil +} + +var jwksClient = &http.Client{Timeout: jwksRequestTimeout} + +func (v *TrustedCallerVerifier) fetchJWKS() (map[string]*rsa.PublicKey, error) { + resp, err := jwksClient.Get(v.wellKnownURL) // nolint + if err != nil { + return nil, err + } + defer func() { + if closeErr := resp.Body.Close(); closeErr != nil { + log.WithError(closeErr).Warn("problem closing the JWKS response body") + } + }() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("unable to load the JWKS from %s - status code: %d", v.wellKnownURL, resp.StatusCode) + } + + var keySet jwks + if err := json.NewDecoder(io.LimitReader(resp.Body, jwksResponseSizeLimit)).Decode(&keySet); err != nil { + return nil, err + } + + keys := make(map[string]*rsa.PublicKey, len(keySet.Keys)) + for _, webKey := range keySet.Keys { + if webKey.Kid == "" || (webKey.Kty != "" && webKey.Kty != "RSA") || (webKey.Use != "" && webKey.Use != "sig") { + continue + } + key, keyErr := parseRSAPublicKey(webKey) + if keyErr != nil { + log.WithError(keyErr).Warnf("unable to parse the JWKS signing key: %s", webKey.Kid) + continue + } + keys[webKey.Kid] = key + } + if len(keys) == 0 { + return nil, fmt.Errorf("no usable RSA signing key at %s", v.wellKnownURL) + } + return keys, nil +} + +func parseRSAPublicKey(webKey jsonWebKeys) (*rsa.PublicKey, error) { + if webKey.N != "" && webKey.E != "" { + modulus, err := base64.RawURLEncoding.DecodeString(strings.TrimRight(webKey.N, "=")) + if err != nil { + return nil, err + } + exponent, err := base64.RawURLEncoding.DecodeString(strings.TrimRight(webKey.E, "=")) + if err != nil { + return nil, err + } + if len(modulus) == 0 || len(exponent) == 0 || len(exponent) > 4 { + return nil, errors.New("key carries a malformed modulus or exponent") + } + return &rsa.PublicKey{ + N: new(big.Int).SetBytes(modulus), + E: int(new(big.Int).SetBytes(exponent).Int64()), + }, nil + } + if len(webKey.X5c) == 0 { + return nil, errors.New("key carries neither a modulus/exponent pair nor a certificate") + } + return jwt.ParseRSAPublicKeyFromPEM([]byte("-----BEGIN CERTIFICATE-----\n" + webKey.X5c[0] + "\n-----END CERTIFICATE-----")) +} diff --git a/cla-backend-go/auth/trusted_caller_test.go b/cla-backend-go/auth/trusted_caller_test.go new file mode 100644 index 000000000..2774e93b5 --- /dev/null +++ b/cla-backend-go/auth/trusted_caller_test.go @@ -0,0 +1,542 @@ +// Copyright The Linux Foundation and each contributor to CommunityBridge. +// SPDX-License-Identifier: MIT + +package auth + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/base64" + "encoding/json" + "fmt" + "math/big" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + "github.com/golang-jwt/jwt/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testTrustedClientID = "ss-confidential-client-id" + testUntrustedClientID = "some-other-client-id" + testKeyID = "kid-1" +) + +func testVerifier(t *testing.T, key *rsa.PrivateKey) (*TrustedCallerVerifier, *int) { + t.Helper() + verifier, err := NewTrustedCallerVerifier("linuxfoundation-dev.auth0.com", "RS256", []string{testTrustedClientID}) + require.NoError(t, err) + + fetches := 0 + verifier.fetchKeys = func() (map[string]*rsa.PublicKey, error) { + fetches++ + return map[string]*rsa.PublicKey{testKeyID: &key.PublicKey}, nil + } + return verifier, &fetches +} + +func testToken(t *testing.T, key *rsa.PrivateKey, kid string, claims jwt.MapClaims) string { + t.Helper() + token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + if kid != "" { + token.Header["kid"] = kid + } + signed, err := token.SignedString(key) + require.NoError(t, err) + return signed +} + +func testClaims(clientID string) jwt.MapClaims { + return jwt.MapClaims{ + "iss": "https://linuxfoundation-dev.auth0.com/", + "sub": clientID + "@clients", + "azp": clientID, + "exp": time.Now().Add(time.Hour).Unix(), + "iat": time.Now().Add(-time.Minute).Unix(), + } +} + +func testKey(t *testing.T) *rsa.PrivateKey { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + return key +} + +func TestTrustedCallerVerifierEnabled(t *testing.T) { + var nilVerifier *TrustedCallerVerifier + assert.False(t, nilVerifier.Enabled()) + + disabled, err := NewTrustedCallerVerifier("", "", nil) + require.NoError(t, err, "an unset allow-list must not fail startup, it only disables the trusted caller path") + assert.False(t, disabled.Enabled()) + + blank, err := NewTrustedCallerVerifier("", "", []string{" ", ""}) + require.NoError(t, err) + assert.False(t, blank.Enabled()) + + _, err = NewTrustedCallerVerifier("", "", []string{testTrustedClientID}) + assert.Error(t, err, "a configured allow-list without an Auth0 domain must fail startup rather than trust blindly") + + enabled, err := NewTrustedCallerVerifier("linuxfoundation-dev.auth0.com", "", []string{" " + testTrustedClientID + " "}) + require.NoError(t, err) + assert.True(t, enabled.Enabled()) + assert.Equal(t, defaultJWTAlgorithm, enabled.algorithm) + assert.Equal(t, "https://linuxfoundation-dev.auth0.com/.well-known/jwks.json", enabled.wellKnownURL) + assert.True(t, enabled.allowedClientIDs[testTrustedClientID], "a padded client ID must be trimmed, not stored verbatim") + + trailingSlash, err := NewTrustedCallerVerifier("linuxfoundation-dev.auth0.com/", "RS512", []string{testTrustedClientID}) + require.NoError(t, err) + assert.Equal(t, "https://linuxfoundation-dev.auth0.com/.well-known/jwks.json", trailingSlash.wellKnownURL) + assert.Equal(t, "RS512", trailingSlash.algorithm) +} + +// an absent header is the same as a malformed one: the traefik aws-lambda middleware drops +// duplicated headers, so a duplicated Authorization header arrives as an absent one +func TestVerifyMissingBearerToken(t *testing.T) { + verifier, fetches := testVerifier(t, testKey(t)) + + basic := "Basic " + base64.StdEncoding.EncodeToString([]byte("user:pass")) + for _, header := range []string{"", " ", basic, "Bearer", "Bearer ", "Bearer ", "bearer\t", "token abc", "Bearer\n"} { + caller, err := verifier.Verify(header) + assert.Nil(t, caller) + assert.ErrorIs(t, err, ErrNoBearerToken, "header %q must be denied", header) + } + assert.Zero(t, *fetches, "an unparseable header must not trigger a JWKS lookup") +} + +func TestVerifyTrustedAndUntrustedClients(t *testing.T) { + key := testKey(t) + verifier, _ := testVerifier(t, key) + + trusted, err := verifier.Verify("Bearer " + testToken(t, key, testKeyID, testClaims(testTrustedClientID))) + require.NoError(t, err) + assert.True(t, trusted.Trusted) + assert.Equal(t, testTrustedClientID, trusted.ClientID) + assert.Equal(t, testTrustedClientID+"@clients", trusted.Subject) + + // a valid token from any other client is verified but never trusted + untrusted, err := verifier.Verify("bearer " + testToken(t, key, testKeyID, testClaims(testUntrustedClientID))) + require.NoError(t, err) + assert.False(t, untrusted.Trusted) + assert.Equal(t, testUntrustedClientID, untrusted.ClientID) + + // a client ID that only differs in case must not match the allow-list + mixedCase, err := verifier.Verify("Bearer " + testToken(t, key, testKeyID, testClaims(strings.ToUpper(testTrustedClientID)))) + require.NoError(t, err) + assert.False(t, mixedCase.Trusted) +} + +// a verified token carrying no usable azp claim can never match the allow-list, so it is +// untrusted rather than denied and keeps the pre-existing per-identity verification +func TestVerifyNeverTrustsATokenWithoutAnAzpClaim(t *testing.T) { + key := testKey(t) + verifier, _ := testVerifier(t, key) + + absent := testClaims(testTrustedClientID) + delete(absent, "azp") + empty := testClaims(testTrustedClientID) + empty["azp"] = "" + blank := testClaims(testTrustedClientID) + blank["azp"] = " " + numeric := testClaims(testTrustedClientID) + numeric["azp"] = 42 + + for name, claims := range map[string]jwt.MapClaims{"absent": absent, "empty": empty, "blank": blank, "non-string": numeric} { + t.Run(name, func(t *testing.T) { + caller, err := verifier.Verify("Bearer " + testToken(t, key, testKeyID, claims)) + require.NoError(t, err) + assert.False(t, caller.Trusted) + }) + } +} + +func TestVerifyRejectsInvalidTokens(t *testing.T) { + key := testKey(t) + otherKey := testKey(t) + verifier, _ := testVerifier(t, key) + + expired := testClaims(testTrustedClientID) + expired["exp"] = time.Now().Add(-time.Minute).Unix() + + noExpiration := testClaims(testTrustedClientID) + delete(noExpiration, "exp") + + notYetValid := testClaims(testTrustedClientID) + notYetValid["nbf"] = time.Now().Add(time.Hour).Unix() + + issuedInTheFuture := testClaims(testTrustedClientID) + issuedInTheFuture["iat"] = time.Now().Add(time.Hour).Unix() + + tests := []struct { + name string + token string + }{ + {"expired", testToken(t, key, testKeyID, expired)}, + {"no expiration", testToken(t, key, testKeyID, noExpiration)}, + {"not yet valid", testToken(t, key, testKeyID, notYetValid)}, + {"issued in the future", testToken(t, key, testKeyID, issuedInTheFuture)}, + {"no key ID", testToken(t, key, "", testClaims(testTrustedClientID))}, + {"unknown key ID", testToken(t, key, "kid-does-not-exist", testClaims(testTrustedClientID))}, + {"signed by another key", testToken(t, otherKey, testKeyID, testClaims(testTrustedClientID))}, + {"not a JWT", "not-a-jwt"}, + {"only a header", base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"RS256","kid":"kid-1"}`))}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + caller, err := verifier.Verify("Bearer " + test.token) + assert.Nil(t, caller) + assert.Error(t, err) + }) + } +} + +// the signing algorithm must be pinned: an HMAC token would otherwise be verified with the +// JWKS RSA modulus as its shared secret, and "alg": "none" would skip verification entirely +func TestVerifyRejectsOtherSigningAlgorithms(t *testing.T) { + key := testKey(t) + verifier, _ := testVerifier(t, key) + + hmacToken := jwt.NewWithClaims(jwt.SigningMethodHS256, testClaims(testTrustedClientID)) + hmacToken.Header["kid"] = testKeyID + signed, err := hmacToken.SignedString(key.PublicKey.N.Bytes()) + require.NoError(t, err) + + caller, err := verifier.Verify("Bearer " + signed) + assert.Nil(t, caller) + assert.Error(t, err) + + noneToken := jwt.NewWithClaims(jwt.SigningMethodNone, testClaims(testTrustedClientID)) + noneToken.Header["kid"] = testKeyID + unsigned, err := noneToken.SignedString(jwt.UnsafeAllowNoneSignatureType) + require.NoError(t, err) + + caller, err = verifier.Verify("Bearer " + unsigned) + assert.Nil(t, caller) + assert.Error(t, err) +} + +func TestVerifyCachesTheJWKS(t *testing.T) { + key := testKey(t) + verifier, fetches := testVerifier(t, key) + clock := time.Now() + verifier.now = func() time.Time { return clock } + + for i := 0; i < 3; i++ { + _, err := verifier.Verify("Bearer " + testToken(t, key, testKeyID, testClaims(testTrustedClientID))) + require.NoError(t, err) + } + assert.Equal(t, 1, *fetches, "the JWKS must be fetched once and cached") + + // an unknown key ID triggers at most one refresh per cooldown window, so a flood of + // unknown-kid tokens cannot be used to hammer the Auth0 JWKS endpoint + unknown := "Bearer " + testToken(t, key, "kid-does-not-exist", testClaims(testTrustedClientID)) + for i := 0; i < 5; i++ { + _, err := verifier.Verify(unknown) + require.Error(t, err) + } + assert.Equal(t, 1, *fetches, "the cooldown started by the initial fetch is still open") + + clock = clock.Add(jwksRefreshCooldown + time.Second) + _, err := verifier.Verify(unknown) + require.Error(t, err) + assert.Equal(t, 2, *fetches, "after the cooldown an unknown key ID may refresh the key set again") + + clock = clock.Add(jwksCacheTTL) + _, err = verifier.Verify("Bearer " + testToken(t, key, testKeyID, testClaims(testTrustedClientID))) + require.NoError(t, err) + assert.Equal(t, 3, *fetches, "an expired key set is reloaded") +} + +// a JWKS endpoint outage must not invalidate an already cached key +func TestVerifyFallsBackToTheCachedKey(t *testing.T) { + key := testKey(t) + verifier, _ := testVerifier(t, key) + clock := time.Now() + verifier.now = func() time.Time { return clock } + + _, err := verifier.Verify("Bearer " + testToken(t, key, testKeyID, testClaims(testTrustedClientID))) + require.NoError(t, err) + + verifier.fetchKeys = func() (map[string]*rsa.PublicKey, error) { + return nil, assert.AnError + } + clock = clock.Add(jwksCacheTTL + time.Minute) + + caller, err := verifier.Verify("Bearer " + testToken(t, key, testKeyID, testClaims(testTrustedClientID))) + require.NoError(t, err) + assert.True(t, caller.Trusted) + + caller, err = verifier.Verify("Bearer " + testToken(t, key, "kid-does-not-exist", testClaims(testTrustedClientID))) + assert.Nil(t, caller, "an unknown key ID must still be denied when the JWKS cannot be reloaded") + assert.Error(t, err) +} + +// the fallback above is bounded: a revoked key must not stay usable for an unlimited outage +func TestVerifyRejectsAStaleCachedKey(t *testing.T) { + key := testKey(t) + verifier, _ := testVerifier(t, key) + clock := time.Now() + verifier.now = func() time.Time { return clock } + + _, err := verifier.Verify("Bearer " + testToken(t, key, testKeyID, testClaims(testTrustedClientID))) + require.NoError(t, err) + + verifier.fetchKeys = func() (map[string]*rsa.PublicKey, error) { + return nil, assert.AnError + } + + clock = clock.Add(jwksCacheTTL + jwksStaleKeyGrace - time.Minute) + caller, err := verifier.Verify("Bearer " + testToken(t, key, testKeyID, testClaims(testTrustedClientID))) + require.NoError(t, err, "within the grace period the cached key is still used") + assert.True(t, caller.Trusted) + + clock = clock.Add(2 * time.Minute) + caller, err = verifier.Verify("Bearer " + testToken(t, key, testKeyID, testClaims(testTrustedClientID))) + assert.Nil(t, caller) + assert.Error(t, err, "past the grace period a key that cannot be re-fetched must be rejected") +} + +// a misconfigured cla-auth0-algorithm cannot open a hole: the verifier only ever hands the parser +// an RSA public key, so a non-RSA algorithm fails closed instead of accepting forged signatures +func TestVerifyRejectsMisconfiguredAlgorithms(t *testing.T) { + key := testKey(t) + + hmac := jwt.NewWithClaims(jwt.SigningMethodHS256, testClaims(testTrustedClientID)) + hmac.Header["kid"] = testKeyID + hmacToken, err := hmac.SignedString(key.PublicKey.N.Bytes()) + require.NoError(t, err) + + none := jwt.NewWithClaims(jwt.SigningMethodNone, testClaims(testTrustedClientID)) + none.Header["kid"] = testKeyID + noneToken, err := none.SignedString(jwt.UnsafeAllowNoneSignatureType) + require.NoError(t, err) + + for algorithm, rawToken := range map[string]string{"HS256": hmacToken, "none": noneToken} { + verifier, verifierErr := NewTrustedCallerVerifier("linuxfoundation-dev.auth0.com", algorithm, []string{testTrustedClientID}) + require.NoError(t, verifierErr) + verifier.fetchKeys = func() (map[string]*rsa.PublicKey, error) { + return map[string]*rsa.PublicKey{testKeyID: &key.PublicKey}, nil + } + + caller, verifyErr := verifier.Verify("Bearer " + rawToken) + assert.Nil(t, caller, "a verifier configured with %s must not accept a token", algorithm) + assert.Error(t, verifyErr) + } +} + +// the grace window is exclusive: at exactly keysExpireAt+jwksStaleKeyGrace the cached key is gone +func TestVerifyRejectsTheCachedKeyAtTheGraceBoundary(t *testing.T) { + key := testKey(t) + verifier, _ := testVerifier(t, key) + clock := time.Now() + verifier.now = func() time.Time { return clock } + token := "Bearer " + testToken(t, key, testKeyID, testClaims(testTrustedClientID)) + + _, err := verifier.Verify(token) + require.NoError(t, err) + verifier.fetchKeys = func() (map[string]*rsa.PublicKey, error) { return nil, assert.AnError } + boundary := verifier.keysExpireAt.Add(jwksStaleKeyGrace) + + clock = boundary.Add(-time.Nanosecond) + _, err = verifier.Verify(token) + require.NoError(t, err, "a nanosecond before the boundary the cached key is still served") + + clock = boundary + _, err = verifier.Verify(token) + assert.Error(t, err, "at the boundary the cached key must be rejected") +} + +// the tuning has to keep these relations: a reload is rate limited for less than the cache +// lifetime, one attempt cannot outlast its own rate limit, and the outage grace outlives the cache +func TestJWKSCacheTuning(t *testing.T) { + assert.Positive(t, jwksRefreshCooldown) + assert.Less(t, jwksRefreshCooldown, jwksCacheTTL) + assert.LessOrEqual(t, jwksRequestTimeout, jwksRefreshCooldown) + assert.Greater(t, jwksStaleKeyGrace, jwksCacheTTL) +} + +// a failed reload must not be retried per request: once the TTL expires during a JWKS outage the +// cooldown has to serve the grace-bounded cached key instead of paying the fetch timeout again +func TestVerifyRateLimitsReloadsDuringAnOutage(t *testing.T) { + key := testKey(t) + verifier, _ := testVerifier(t, key) + clock := time.Now() + verifier.now = func() time.Time { return clock } + token := "Bearer " + testToken(t, key, testKeyID, testClaims(testTrustedClientID)) + + _, err := verifier.Verify(token) + require.NoError(t, err) + + attempts := 0 + verifier.fetchKeys = func() (map[string]*rsa.PublicKey, error) { + attempts++ + return nil, assert.AnError + } + clock = clock.Add(jwksCacheTTL + time.Second) + + for i := 0; i < 5; i++ { + caller, verifyErr := verifier.Verify(token) + require.NoError(t, verifyErr) + assert.True(t, caller.Trusted) + } + assert.Equal(t, 1, attempts, "a stale key set must be reloaded at most once per cooldown window") + + clock = clock.Add(jwksRefreshCooldown + time.Second) + _, err = verifier.Verify(token) + require.NoError(t, err) + assert.Equal(t, 2, attempts, "after the cooldown the reload is attempted again") + + caller, err := verifier.Verify("Bearer " + testToken(t, key, "kid-does-not-exist", testClaims(testTrustedClientID))) + assert.Nil(t, caller) + assert.ErrorContains(t, err, "rate limited", "the denial must come from the cooldown, not from a reload attempt") + assert.Equal(t, 2, attempts, "an unknown key ID must not reload the JWKS while the cooldown is open") + + // the grace bound still applies when the cooldown short-circuits the reload + clock = clock.Add(jwksStaleKeyGrace) + _, err = verifier.Verify(token) + require.Error(t, err) + require.Equal(t, 3, attempts) + caller, err = verifier.Verify(token) + assert.Nil(t, caller) + assert.Error(t, err, "past the grace period the cooldown must not serve the cached key either") + assert.Equal(t, 3, attempts) +} + +func TestVerifyIsSafeForConcurrentUse(t *testing.T) { + key := testKey(t) + verifier, fetches := testVerifier(t, key) + token := "Bearer " + testToken(t, key, testKeyID, testClaims(testTrustedClientID)) + + var wg sync.WaitGroup + results := make([]bool, 16) + for i := range results { + wg.Add(1) + go func(i int) { + defer wg.Done() + caller, err := verifier.Verify(token) + if err == nil && caller != nil { + results[i] = caller.Trusted + } + }(i) + } + wg.Wait() + + for i, trusted := range results { + assert.True(t, trusted, "concurrent verify %d must succeed", i) + } + assert.Equal(t, 1, *fetches, "concurrent verifies must share a single JWKS fetch") +} + +func TestFetchJWKS(t *testing.T) { + key := testKey(t) + body := map[string]interface{}{ + "keys": []map[string]interface{}{ + {"kty": "EC", "kid": "not-rsa", "use": "sig"}, + {"kty": "RSA", "kid": "encryption-key", "use": "enc"}, + {"kty": "RSA", "kid": "malformed", "use": "sig", "n": "!!!", "e": "AQAB"}, + {"kty": "RSA", "kid": testKeyID, "use": "sig", + "n": base64.RawURLEncoding.EncodeToString(key.PublicKey.N.Bytes()), + "e": base64.RawURLEncoding.EncodeToString(big.NewInt(int64(key.PublicKey.E)).Bytes())}, + }, + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + require.NoError(t, json.NewEncoder(w).Encode(body)) + })) + defer server.Close() + + verifier, err := NewTrustedCallerVerifier("linuxfoundation-dev.auth0.com", "RS256", []string{testTrustedClientID}) + require.NoError(t, err) + verifier.wellKnownURL = server.URL + + keys, err := verifier.fetchJWKS() + require.NoError(t, err) + require.Len(t, keys, 1, "only usable RSA signing keys are kept") + assert.Equal(t, key.PublicKey.N, keys[testKeyID].N) + assert.Equal(t, key.PublicKey.E, keys[testKeyID].E) + + caller, err := verifier.Verify("Bearer " + testToken(t, key, testKeyID, testClaims(testTrustedClientID))) + require.NoError(t, err) + assert.True(t, caller.Trusted) + + failureModes := map[string]http.HandlerFunc{ + "server error": func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusInternalServerError) }, + "not found": func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNotFound) }, + "unauthorized": func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusUnauthorized) }, + "not json": func(w http.ResponseWriter, _ *http.Request) { fmt.Fprint(w, "{not json") }, + "no keys": func(w http.ResponseWriter, _ *http.Request) { fmt.Fprint(w, `{"keys":[]}`) }, + "only unusable keys": func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"keys":[{"kty":"EC","kid":"ec","use":"sig"}]}`) + }, + } + for name, handler := range failureModes { + t.Run(name, func(t *testing.T) { + failing := httptest.NewServer(handler) + defer failing.Close() + verifier.wellKnownURL = failing.URL + keySet, fetchErr := verifier.fetchJWKS() + assert.Nil(t, keySet) + assert.Error(t, fetchErr) + }) + } + + t.Run("unreachable", func(t *testing.T) { + unreachable := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + verifier.wellKnownURL = unreachable.URL + unreachable.Close() + keySet, fetchErr := verifier.fetchJWKS() + assert.Nil(t, keySet) + assert.Error(t, fetchErr) + }) +} + +func TestParseRSAPublicKey(t *testing.T) { + key := testKey(t) + + parsed, err := parseRSAPublicKey(jsonWebKeys{ + Kid: testKeyID, + N: base64.RawURLEncoding.EncodeToString(key.PublicKey.N.Bytes()), + E: base64.RawURLEncoding.EncodeToString(big.NewInt(int64(key.PublicKey.E)).Bytes()), + }) + require.NoError(t, err) + assert.True(t, parsed.Equal(&key.PublicKey)) + + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "trusted-caller-test"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + } + der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + require.NoError(t, err) + + parsed, err = parseRSAPublicKey(jsonWebKeys{Kid: testKeyID, X5c: []string{base64.StdEncoding.EncodeToString(der)}}) + require.NoError(t, err) + assert.True(t, parsed.Equal(&key.PublicKey)) + + modulus := base64.RawURLEncoding.EncodeToString(key.PublicKey.N.Bytes()) + malformed := map[string]jsonWebKeys{ + "neither a key nor a certificate": {Kid: testKeyID}, + "malformed modulus": {Kid: testKeyID, N: "!!!", E: "AQAB"}, + "malformed exponent": {Kid: testKeyID, N: modulus, E: "!!!"}, + "empty exponent": {Kid: testKeyID, N: modulus, E: "="}, + "oversized exponent": {Kid: testKeyID, N: modulus, E: base64.RawURLEncoding.EncodeToString([]byte{1, 2, 3, 4, 5})}, + "invalid certificate": {Kid: testKeyID, X5c: []string{"not-a-certificate"}}, + } + for name, webKey := range malformed { + t.Run(name, func(t *testing.T) { + key, keyErr := parseRSAPublicKey(webKey) + assert.Nil(t, key) + assert.Error(t, keyErr) + }) + } +} diff --git a/cla-backend-go/cmd/server.go b/cla-backend-go/cmd/server.go index bdfb3e1cc..3866e21d3 100644 --- a/cla-backend-go/cmd/server.go +++ b/cla-backend-go/cmd/server.go @@ -439,6 +439,10 @@ func server(localMode bool) http.Handler { v1SignaturesService := signatures.NewService(signaturesRepo, v1CompanyService, usersService, eventsService, githubOrgValidation, v1RepositoriesService, githubOrganizationsService, v1ProjectService, gitlabApp, configFile.ClaV1ApiURL, configFile.CLALandingPage, configFile.CLALogoURL) v2SignatureService := v2Signatures.NewService(awsSession, configFile.SignatureFilesBucket, v1ProjectService, v1CompanyService, v1SignaturesService, v1ProjectClaGroupRepo, signaturesRepo, usersService, approvalsRepo) v2MyClasService := v2MyClas.NewService(v2MyClas.NewRepository(awsSession, stage), user_service.GetClient(), v1SignaturesService, v1CompanyRepo, v1ProjectClaGroupRepo, project_service.GetClient()) + trustedCallerVerifier, err := auth.NewTrustedCallerVerifier(configFile.Auth0.Domain, configFile.Auth0.Algorithm, configFile.SelfServe.TrustedClientIDs) + if err != nil { + logrus.Panic(err) + } v1ClaManagerService := cla_manager.NewService(claManagerReqRepo, v1ProjectClaGroupRepo, v1CompanyService, v1ProjectService, usersService, v1SignaturesService, eventsService, emailTemplateService, configFile.CorporateConsoleV2URL) v2ClaManagerService := v2ClaManager.NewService(emailTemplateService, v1CompanyService, v1ProjectService, v1ClaManagerService, usersService, v1RepositoriesService, v2CompanyService, eventsService, v1ProjectClaGroupRepo) v1ApprovalListService := approval_list.NewService(approvalListRepo, v1ProjectClaGroupRepo, v1ProjectService, usersRepo, v1CompanyRepo, v1CLAGroupRepo, signaturesRepo, emailTemplateService, configFile.CorporateConsoleV2URL, http.DefaultClient) @@ -513,7 +517,7 @@ func server(localMode bool) http.Handler { v2Gerrits.Configure(v2API, gerritService, v1ProjectService, eventsService, v1ProjectClaGroupRepo) v2Company.Configure(v2API, v2CompanyService, v1ProjectClaGroupRepo, configFile.LFXPortalURL) v2CurrentUser.Configure(v2API, v2CurrentUserService) - v2MyClas.Configure(v2API, v2MyClasService) + v2MyClas.Configure(v2API, v2MyClasService, trustedCallerVerifier) cla_manager.Configure(api, v1ClaManagerService, v1CompanyService, v1ProjectService, usersService, v1SignaturesService, eventsService, emailTemplateService) v2ClaManager.Configure(v2API, v2ClaManagerService, v1CompanyService, configFile.LFXPortalURL, configFile.CorporateConsoleV2URL, v1ProjectClaGroupRepo, userRepo) cla_groups.Configure(v2API, v2ClaGroupService, v1ProjectService, v1ProjectClaGroupRepo, eventsService) diff --git a/cla-backend-go/config/config.go b/cla-backend-go/config/config.go index 9d1b9eb1e..f0df045e0 100644 --- a/cla-backend-go/config/config.go +++ b/cla-backend-go/config/config.go @@ -101,6 +101,15 @@ type Config struct { // SSS holds the Sanctions Screening Service client configuration SSS SSS `json:"sss"` + + // SelfServe holds the LFX Self Serve trusted-caller configuration + SelfServe SelfServe `json:"self_serve"` +} + +// SelfServe holds the LFX Self Serve trusted-caller configuration: the Auth0 azp (client ID) +// values whose caller-supplied identity list the My CLAs endpoints accept as authorized +type SelfServe struct { + TrustedClientIDs []string `json:"trusted_client_ids"` } // Auth0 model diff --git a/cla-backend-go/config/ssm.go b/cla-backend-go/config/ssm.go index fa78dd2c1..c087bbad3 100644 --- a/cla-backend-go/config/ssm.go +++ b/cla-backend-go/config/ssm.go @@ -4,6 +4,7 @@ package config import ( + "errors" "fmt" "strconv" "strings" @@ -276,9 +277,56 @@ func loadSSMConfig(awsSession *session.Session, stage string) Config { //nolint // environments, and the caller must enforce that (see config.SSS). loadOptionalSSSConfig(ssmClient, stage, &config, f) + loadOptionalSelfServeConfig(ssmClient, stage, &config, f) + return config } +// loadOptionalSelfServeConfig fetches the comma-separated allow-list of trusted LFX Self Serve +// Auth0 client IDs without aborting startup when the key is missing, so the SSM parameter can +// be provisioned independently of this code being deployed. +func loadOptionalSelfServeConfig(ssmClient *ssm.SSM, stage string, config *Config, f logrus.Fields) { + key := fmt.Sprintf("cla-ss-trusted-client-ids-%s", stage) + out, err := ssmClient.GetParameter(&ssm.GetParameterInput{ + Name: aws.String(key), + WithDecryption: aws.Bool(false), + }) + if err != nil { + if isParameterNotFound(err) { + log.WithFields(f).Debugf("optional SSM key %s not provisioned - no Self Serve caller is trusted until it is set", key) + } else { + // no caller is trusted, which is the pre-hardening posture, so this degrades rather + // than aborting every lambda that loads config + log.WithFields(f).WithError(err).Warnf("unable to read the SSM key %s - the trusted Self Serve caller path stays disabled", key) + } + return + } + if out.Parameter == nil || out.Parameter.Value == nil { + return + } + + config.SelfServe.TrustedClientIDs = parseTrustedClientIDs(*out.Parameter.Value) + log.WithFields(f).Debugf("loaded %d trusted Self Serve client ID(s) from the SSM key %s", len(config.SelfServe.TrustedClientIDs), key) +} + +func isParameterNotFound(err error) bool { + var aerr awserr.Error + if errors.As(err, &aerr) { + return aerr.Code() == ssm.ErrCodeParameterNotFound + } + return false +} + +func parseTrustedClientIDs(value string) []string { + var clientIDs []string + for _, clientID := range strings.Split(value, ",") { + if clientID = strings.TrimSpace(clientID); clientID != "" { + clientIDs = append(clientIDs, clientID) + } + } + return clientIDs +} + // loadOptionalSSSConfig fetches the SSS keys without aborting startup when they // are missing, so the SSM parameters can be provisioned before the feature is // switched on. A stage that requires SSS must reject an empty configuration at diff --git a/cla-backend-go/config/ssm_test.go b/cla-backend-go/config/ssm_test.go new file mode 100644 index 000000000..d2413b5ec --- /dev/null +++ b/cla-backend-go/config/ssm_test.go @@ -0,0 +1,45 @@ +// Copyright The Linux Foundation and each contributor to CommunityBridge. +// SPDX-License-Identifier: MIT + +package config + +import ( + "errors" + "fmt" + "testing" + + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/service/ssm" + "github.com/stretchr/testify/assert" +) + +// a missing key is expected until the allow-list is provisioned, any other failure is not, so +// the two must stay distinguishable in the logs even when the SDK error arrives wrapped +func TestIsParameterNotFound(t *testing.T) { + assert.True(t, isParameterNotFound(awserr.New(ssm.ErrCodeParameterNotFound, "not found", nil))) + assert.True(t, isParameterNotFound(fmt.Errorf("wrapped: %w", awserr.New(ssm.ErrCodeParameterNotFound, "not found", nil)))) + assert.False(t, isParameterNotFound(awserr.New("AccessDeniedException", "denied", nil))) + assert.False(t, isParameterNotFound(awserr.New("ThrottlingException", "slow down", nil))) + assert.False(t, isParameterNotFound(errors.New("connection reset"))) +} + +func TestParseTrustedClientIDs(t *testing.T) { + tests := []struct { + name string + value string + clientIDs []string + }{ + {"unset", "", nil}, + {"blank", " ", nil}, + {"separators only", " , ,, ", nil}, + {"single", "ss-client", []string{"ss-client"}}, + {"padded", " ss-client ", []string{"ss-client"}}, + {"multiple", "ss-client, other-client ,third-client", []string{"ss-client", "other-client", "third-client"}}, + {"trailing separator", "ss-client,", []string{"ss-client"}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.clientIDs, parseTrustedClientIDs(test.value)) + }) + } +} diff --git a/cla-backend-go/swagger/cla.v2.yaml b/cla-backend-go/swagger/cla.v2.yaml index c2c35bd41..e2f99ca8a 100644 --- a/cla-backend-go/swagger/cla.v2.yaml +++ b/cla-backend-go/swagger/cla.v2.yaml @@ -2753,7 +2753,7 @@ paths: /my-clas: get: summary: Get My CLAs - description: Returns the signed ICLAs and ECLAs (employee acknowledgements) matching the provided identity - LF username, emails and GitHub/GitLab/Gerrit identities - aggregated across all matching EasyCLA user records and deduplicated, with validity evaluated against the current company CCLA approval lists. Unless the caller is an admin, each provided identity must belong to the authenticated user (per their EasyCLA user record or the identities connected to their LF account in the platform user-service) - identities that cannot be verified are not searched and are reported in skippedIdentities + description: Returns the signed ICLAs and ECLAs (employee acknowledgements) matching the provided identity - LF username, emails and GitHub/GitLab/Gerrit identities - aggregated across all matching EasyCLA user records and deduplicated, with validity evaluated against the current company CCLA approval lists. Unless the caller is an admin or a trusted LFX Self Serve client, each provided identity must belong to the authenticated user (per their EasyCLA user record or the identities connected to their LF account in the platform user-service) - identities that cannot be verified are not searched and are reported in skippedIdentities. A trusted caller is one whose Authorization bearer token is signature-verified against the Auth0 JWKS in-handler and whose azp claim is on the configured Self Serve client-ID allow-list; while that allow-list is configured every request must carry a verifiable bearer token, and a missing or unverifiable one is rejected with 401 operationId: getMyClas parameters: - $ref: "#/parameters/x-request-id" @@ -2791,7 +2791,7 @@ paths: /my-clas/{signatureID}/pdf: get: summary: Get a signed ICLA PDF download link - description: Returns a time-limited download URL for the signed ICLA PDF when the signature belongs to the provided identity - unknown, not-owned and ECLA signature IDs return 404. The same identity-ownership enforcement as GET /my-clas applies, so a non-admin caller can only download their own signed documents + description: Returns a time-limited download URL for the signed ICLA PDF when the signature belongs to the provided identity - unknown, not-owned and ECLA signature IDs return 404. The same identity-ownership enforcement and trusted-caller bearer token verification as GET /my-clas applies, so a caller that is neither an admin nor a trusted LFX Self Serve client can only download their own signed documents operationId: getMyClaPdf parameters: - $ref: "#/parameters/x-request-id" @@ -2837,7 +2837,7 @@ paths: /my-clas/identities: get: summary: Get My Identities - description: Returns the deduplicated identities connected to the authenticated user, each formatted as ":" - the union of the identities on their EasyCLA user records and the identities connected to their LF account in the platform user-service, i.e. exactly the identity set the My CLAs API authorizes a non-admin caller to search + description: Returns the deduplicated identities connected to the authenticated user, each formatted as ":" - the union of the identities on their EasyCLA user records and the identities connected to their LF account in the platform user-service, i.e. exactly the identity set the My CLAs API authorizes a non-admin, non-trusted caller to search. This endpoint always reports the authenticated principal's own identities; when the trusted Self Serve client-ID allow-list is configured the request must still carry a verifiable Authorization bearer token operationId: getMyIdentities parameters: - $ref: "#/parameters/x-request-id" @@ -4962,7 +4962,7 @@ parameters: required: true myClasLfUsername: name: lfUsername - description: The LF username (LFID) of the user - when omitted, the username of the authenticated principal is used; unless the caller is an admin, a value different from the authenticated principal is not searched and is reported in skippedIdentities + description: The LF username (LFID) of the user - when omitted, the username of the authenticated principal is used; unless the caller is an admin or a trusted LFX Self Serve client, a value different from the authenticated principal is not searched and is reported in skippedIdentities. Accepting a caller-supplied identity list is transitional - at M6, once EasyCLA runs on the K8s cluster, it should call lfx.auth-service.user_identity.list itself over NATS and drop both the caller-supplied list and the azp allow-list that authorizes it in: query type: string required: false diff --git a/cla-backend-go/v2/my_clas/handlers.go b/cla-backend-go/v2/my_clas/handlers.go index 17b65c023..6340c41df 100644 --- a/cla-backend-go/v2/my_clas/handlers.go +++ b/cla-backend-go/v2/my_clas/handlers.go @@ -5,9 +5,12 @@ package my_clas import ( "context" + "errors" + "net/http" "github.com/LF-Engineering/lfx-kit/auth" "github.com/go-openapi/runtime/middleware" + claAuth "github.com/linuxfoundation/easycla/cla-backend-go/auth" "github.com/linuxfoundation/easycla/cla-backend-go/gen/v2/restapi/operations" myClasOps "github.com/linuxfoundation/easycla/cla-backend-go/gen/v2/restapi/operations/my_clas" log "github.com/linuxfoundation/easycla/cla-backend-go/logging" @@ -17,9 +20,16 @@ import ( const missingUsernameMsg = "the authenticated principal carries no username - unable to determine whose CLAs to look up" const missingIdentityMsg = "no identity provided - provide at least one of lfUsername, email, secondaryEmail, githubId, githubUsername, gitlabId, gitlabUsername, gerritUsername" +const unverifiedCallerMsg = "unable to verify the caller's bearer token" + +// CallerVerifier re-verifies the request bearer token in-handler - see auth.TrustedCallerVerifier +type CallerVerifier interface { + Enabled() bool + Verify(authorization string) (*claAuth.TrustedCaller, error) +} // Configure sets up the My CLAs API handlers -func Configure(api *operations.EasyclaAPI, service Service) { +func Configure(api *operations.EasyclaAPI, service Service, callerVerifier CallerVerifier) { api.MyClasGetMyClasHandler = myClasOps.GetMyClasHandlerFunc( func(params myClasOps.GetMyClasParams, authUser *auth.User) middleware.Responder { reqID := utils.GetRequestID(params.XREQUESTID) @@ -32,19 +42,27 @@ func Configure(api *operations.EasyclaAPI, service Service) { "authUserEmail": utils.StringValue(params.XEMAIL), } + trustedCaller, err := verifyCaller(callerVerifier, params.HTTPRequest, f) + if err != nil { + log.WithFields(f).WithError(err).Warn(unverifiedCallerMsg) + return myClasOps.NewGetMyClasUnauthorized().WithXRequestID(reqID).WithPayload(utils.ErrorResponseUnauthorized(reqID, unverifiedCallerMsg)) + } + currentUsername, admin := principal(authUser) - if !admin && currentUsername == "" { + trusted := trustedCaller != nil && trustedCaller.Trusted + if !admin && !trusted && currentUsername == "" { log.WithFields(f).Warn(missingUsernameMsg) return myClasOps.NewGetMyClasUnauthorized().WithXRequestID(reqID).WithPayload(utils.ErrorResponseUnauthorized(reqID, missingUsernameMsg)) } requested := newIdentity(params.LfUsername, params.Email, params.SecondaryEmail, params.GithubID, params.GithubUsername, params.GitlabID, params.GitlabUsername, params.GerritUsername) - if admin && currentUsername == "" && requested.IsEmpty() { + if (admin || trusted) && currentUsername == "" && requested.IsEmpty() { log.WithFields(f).Warn(missingIdentityMsg) return myClasOps.NewGetMyClasBadRequest().WithXRequestID(reqID).WithPayload(utils.ErrorResponseBadRequest(reqID, missingIdentityMsg)) } + logCallerIdentity(f, trustedCaller, requested) - result, err := service.GetMyClas(ctx, currentUsername, admin, requested) + result, err := service.GetMyClas(ctx, &Caller{Username: currentUsername, Admin: admin, Trusted: trusted}, requested) if err != nil { msg := "unable to lookup the CLAs for the provided identity" log.WithFields(f).WithError(err).Warn(msg) @@ -67,19 +85,27 @@ func Configure(api *operations.EasyclaAPI, service Service) { "signatureID": params.SignatureID, } + trustedCaller, err := verifyCaller(callerVerifier, params.HTTPRequest, f) + if err != nil { + log.WithFields(f).WithError(err).Warn(unverifiedCallerMsg) + return myClasOps.NewGetMyClaPdfUnauthorized().WithXRequestID(reqID).WithPayload(utils.ErrorResponseUnauthorized(reqID, unverifiedCallerMsg)) + } + currentUsername, admin := principal(authUser) - if !admin && currentUsername == "" { + trusted := trustedCaller != nil && trustedCaller.Trusted + if !admin && !trusted && currentUsername == "" { log.WithFields(f).Warn(missingUsernameMsg) return myClasOps.NewGetMyClaPdfUnauthorized().WithXRequestID(reqID).WithPayload(utils.ErrorResponseUnauthorized(reqID, missingUsernameMsg)) } requested := newIdentity(params.LfUsername, params.Email, params.SecondaryEmail, params.GithubID, params.GithubUsername, params.GitlabID, params.GitlabUsername, params.GerritUsername) - if admin && currentUsername == "" && requested.IsEmpty() { + if (admin || trusted) && currentUsername == "" && requested.IsEmpty() { log.WithFields(f).Warn(missingIdentityMsg) return myClasOps.NewGetMyClaPdfBadRequest().WithXRequestID(reqID).WithPayload(utils.ErrorResponseBadRequest(reqID, missingIdentityMsg)) } + logCallerIdentity(f, trustedCaller, requested) - result, err := service.GetMyClaPdfURL(ctx, currentUsername, admin, requested, params.SignatureID) + result, err := service.GetMyClaPdfURL(ctx, &Caller{Username: currentUsername, Admin: admin, Trusted: trusted}, requested, params.SignatureID) if err != nil { msg := "unable to generate the signed document download link" log.WithFields(f).WithError(err).Warn(msg) @@ -106,6 +132,11 @@ func Configure(api *operations.EasyclaAPI, service Service) { "authUserEmail": utils.StringValue(params.XEMAIL), } + if _, err := verifyCaller(callerVerifier, params.HTTPRequest, f); err != nil { + log.WithFields(f).WithError(err).Warn(unverifiedCallerMsg) + return myClasOps.NewGetMyIdentitiesUnauthorized().WithXRequestID(reqID).WithPayload(utils.ErrorResponseUnauthorized(reqID, unverifiedCallerMsg)) + } + currentUsername, _ := principal(authUser) if currentUsername == "" { log.WithFields(f).Warn(missingUsernameMsg) @@ -123,6 +154,44 @@ func Configure(api *operations.EasyclaAPI, service Service) { }) } +// verifyCaller re-verifies the bearer token because /v4 otherwise trusts its invoke path +// unconditionally: the gateway-injected X-ACL/X-USERNAME headers are decoded but never +// signature-checked, so anything able to invoke the lambda directly could forge them. +// Returns (nil, nil) while no allow-list is configured, when no bearer token is required. +func verifyCaller(callerVerifier CallerVerifier, r *http.Request, f logrus.Fields) (*claAuth.TrustedCaller, error) { + if callerVerifier == nil || !callerVerifier.Enabled() { + return nil, nil + } + + authorization := "" + if r != nil { + authorization = r.Header.Get("Authorization") + } + trustedCaller, err := callerVerifier.Verify(authorization) + if err != nil { + return nil, err + } + if trustedCaller == nil { + return nil, errors.New("the caller verifier returned no result") + } + + f["callerClientID"] = trustedCaller.ClientID + f["callerSubject"] = trustedCaller.Subject + f["trustedCaller"] = trustedCaller.Trusted + return trustedCaller, nil +} + +func logCallerIdentity(f logrus.Fields, trustedCaller *claAuth.TrustedCaller, requested *Identity) { + if trustedCaller == nil { + return + } + if trustedCaller.Trusted { + log.WithFields(f).Infof("trusted caller requested the identities: %s", requested.Summary()) + return + } + log.WithFields(f).Debugf("untrusted caller requested the identities: %s", requested.Summary()) +} + func principal(authUser *auth.User) (string, bool) { if authUser == nil { return "", false diff --git a/cla-backend-go/v2/my_clas/handlers_test.go b/cla-backend-go/v2/my_clas/handlers_test.go new file mode 100644 index 000000000..b77de18e6 --- /dev/null +++ b/cla-backend-go/v2/my_clas/handlers_test.go @@ -0,0 +1,238 @@ +// Copyright The Linux Foundation and each contributor to CommunityBridge. +// SPDX-License-Identifier: MIT + +package my_clas + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/LF-Engineering/lfx-kit/auth" + "github.com/go-openapi/runtime" + claAuth "github.com/linuxfoundation/easycla/cla-backend-go/auth" + "github.com/linuxfoundation/easycla/cla-backend-go/gen/v2/models" + "github.com/linuxfoundation/easycla/cla-backend-go/gen/v2/restapi/operations" + myClasOps "github.com/linuxfoundation/easycla/cla-backend-go/gen/v2/restapi/operations/my_clas" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type fakeService struct { + callers []*Caller + err error + nilPdf bool +} + +func (f *fakeService) GetMyClas(_ context.Context, caller *Caller, _ *Identity) (*models.MyClaList, error) { + f.callers = append(f.callers, caller) + if f.err != nil { + return nil, f.err + } + return &models.MyClaList{}, nil +} + +func (f *fakeService) GetMyClaPdfURL(_ context.Context, caller *Caller, _ *Identity, _ string) (*models.MyClaPdf, error) { + f.callers = append(f.callers, caller) + if f.err != nil { + return nil, f.err + } + if f.nilPdf { + return nil, nil + } + return &models.MyClaPdf{}, nil +} + +func (f *fakeService) GetMyIdentities(_ context.Context, currentUsername string) (*models.MyIdentityList, error) { + f.callers = append(f.callers, &Caller{Username: currentUsername}) + if f.err != nil { + return nil, f.err + } + return &models.MyIdentityList{}, nil +} + +type fakeVerifier struct { + enabled bool + callers map[string]*claAuth.TrustedCaller + seen []string + noop string +} + +func (f *fakeVerifier) Enabled() bool { + return f.enabled +} + +func (f *fakeVerifier) Verify(authorization string) (*claAuth.TrustedCaller, error) { + f.seen = append(f.seen, authorization) + if f.noop != "" && authorization == f.noop { + return nil, nil + } + if caller, ok := f.callers[authorization]; ok { + return caller, nil + } + return nil, errors.New("unable to verify the bearer token") +} + +func configuredAPI(t *testing.T, verifier CallerVerifier) (*operations.EasyclaAPI, *fakeService) { + t.Helper() + api := operations.NewEasyclaAPI(nil) + service := &fakeService{} + Configure(api, service, verifier) + return api, service +} + +func request(t *testing.T, authorization string) *http.Request { + t.Helper() + req := httptest.NewRequest(http.MethodGet, "/v4/my-clas", nil) + if authorization != "" { + req.Header.Set("Authorization", authorization) + } + return req +} + +func statusOf(t *testing.T, responder interface { + WriteResponse(http.ResponseWriter, runtime.Producer) +}) int { + t.Helper() + recorder := httptest.NewRecorder() + responder.WriteResponse(recorder, runtime.JSONProducer()) + return recorder.Code +} + +// once the allow-list is configured every request must carry a verifiable bearer token - a +// missing one (the traefik lambda fork drops duplicated headers) is denied, never trusted +func TestHandlersDenyUnverifiedCallers(t *testing.T) { + verifier := &fakeVerifier{enabled: true, callers: map[string]*claAuth.TrustedCaller{ + "Bearer trusted": {ClientID: "ss-client", Subject: "ss-client@clients", Trusted: true}, + }} + api, service := configuredAPI(t, verifier) + authUser := &auth.User{UserName: "someone"} + + // a verifier that reports neither a caller nor an error must deny rather than panic + verifier.noop = "Bearer nothing" + + for _, authorization := range []string{"", "Bearer forged", "Bearer nothing"} { + req := request(t, authorization) + assert.Equal(t, http.StatusUnauthorized, statusOf(t, api.MyClasGetMyClasHandler.Handle(myClasOps.GetMyClasParams{HTTPRequest: req}, authUser))) + assert.Equal(t, http.StatusUnauthorized, statusOf(t, api.MyClasGetMyClaPdfHandler.Handle(myClasOps.GetMyClaPdfParams{HTTPRequest: req, SignatureID: "sig-1"}, authUser))) + assert.Equal(t, http.StatusUnauthorized, statusOf(t, api.MyClasGetMyIdentitiesHandler.Handle(myClasOps.GetMyIdentitiesParams{HTTPRequest: req}, authUser))) + } + assert.Empty(t, service.callers, "an unverified caller must never reach the service") + assert.Len(t, verifier.seen, 9, "every request must be verified") +} + +func TestHandlersTrustAllowListedCallers(t *testing.T) { + verifier := &fakeVerifier{enabled: true, callers: map[string]*claAuth.TrustedCaller{ + "Bearer trusted": {ClientID: "ss-client", Subject: "ss-client@clients", Trusted: true}, + "Bearer untrusted": {ClientID: "other-client", Subject: "someone@clients"}, + }} + api, service := configuredAPI(t, verifier) + + // a trusted caller needs no username of its own - its identity list is authoritative + githubID := int64(999) + params := myClasOps.GetMyClasParams{HTTPRequest: request(t, "Bearer trusted"), GithubID: []int64{githubID}} + assert.Equal(t, http.StatusOK, statusOf(t, api.MyClasGetMyClasHandler.Handle(params, &auth.User{}))) + require.Len(t, service.callers, 1) + assert.Equal(t, &Caller{Trusted: true}, service.callers[0]) + + pdf := myClasOps.GetMyClaPdfParams{HTTPRequest: request(t, "Bearer trusted"), SignatureID: "sig-1", GithubID: []int64{githubID}} + assert.Equal(t, http.StatusOK, statusOf(t, api.MyClasGetMyClaPdfHandler.Handle(pdf, &auth.User{}))) + require.Len(t, service.callers, 2) + assert.Equal(t, &Caller{Trusted: true}, service.callers[1]) + + // a verified token from a client that is not on the allow-list keeps the per-identity checks + params.HTTPRequest = request(t, "Bearer untrusted") + assert.Equal(t, http.StatusOK, statusOf(t, api.MyClasGetMyClasHandler.Handle(params, &auth.User{UserName: "someone"}))) + require.Len(t, service.callers, 3) + assert.Equal(t, &Caller{Username: "someone"}, service.callers[2]) + + // ... and it is still subject to the username requirement + assert.Equal(t, http.StatusUnauthorized, statusOf(t, api.MyClasGetMyClasHandler.Handle(params, &auth.User{}))) + assert.Len(t, service.callers, 3) + + // an admin whose token is verified but untrusted keeps the admin bypass + assert.Equal(t, http.StatusOK, statusOf(t, api.MyClasGetMyClasHandler.Handle(params, &auth.User{ACL: auth.ACL{Admin: true}}))) + require.Len(t, service.callers, 4) + assert.Equal(t, &Caller{Admin: true}, service.callers[3]) + + // an allow-listed admin is both, and the identity list stays authoritative + trustedAdmin := myClasOps.GetMyClasParams{HTTPRequest: request(t, "Bearer trusted"), GithubID: []int64{githubID}} + assert.Equal(t, http.StatusOK, statusOf(t, api.MyClasGetMyClasHandler.Handle(trustedAdmin, &auth.User{UserName: "admin", ACL: auth.ACL{Admin: true}}))) + require.Len(t, service.callers, 5) + assert.Equal(t, &Caller{Username: "admin", Admin: true, Trusted: true}, service.callers[4]) + + // a trusted caller with neither a username nor an identity has nothing to look up + empty := myClasOps.GetMyClasParams{HTTPRequest: request(t, "Bearer trusted")} + assert.Equal(t, http.StatusBadRequest, statusOf(t, api.MyClasGetMyClasHandler.Handle(empty, &auth.User{}))) + emptyPdf := myClasOps.GetMyClaPdfParams{HTTPRequest: request(t, "Bearer trusted"), SignatureID: "sig-1"} + assert.Equal(t, http.StatusBadRequest, statusOf(t, api.MyClasGetMyClaPdfHandler.Handle(emptyPdf, &auth.User{}))) + assert.Len(t, service.callers, 5, "a request with nothing to look up must not reach the service") + + // GetMyIdentities always reports the authenticated principal's own identities + identities := myClasOps.GetMyIdentitiesParams{HTTPRequest: request(t, "Bearer trusted")} + assert.Equal(t, http.StatusUnauthorized, statusOf(t, api.MyClasGetMyIdentitiesHandler.Handle(identities, &auth.User{}))) + assert.Equal(t, http.StatusOK, statusOf(t, api.MyClasGetMyIdentitiesHandler.Handle(identities, &auth.User{UserName: "someone"}))) + + // every request above is verified with the raw Authorization header, in order + assert.Equal(t, []string{ + "Bearer trusted", "Bearer trusted", + "Bearer untrusted", "Bearer untrusted", "Bearer untrusted", + "Bearer trusted", "Bearer trusted", "Bearer trusted", "Bearer trusted", "Bearer trusted", + }, verifier.seen) +} + +func TestHandlersMapServiceFailures(t *testing.T) { + api, service := configuredAPI(t, nil) + service.err = errors.New("boom") + authUser := &auth.User{UserName: "someone"} + req := request(t, "") + + assert.Equal(t, http.StatusInternalServerError, statusOf(t, api.MyClasGetMyClasHandler.Handle(myClasOps.GetMyClasParams{HTTPRequest: req}, authUser))) + assert.Equal(t, http.StatusInternalServerError, statusOf(t, api.MyClasGetMyClaPdfHandler.Handle(myClasOps.GetMyClaPdfParams{HTTPRequest: req, SignatureID: "sig-1"}, authUser))) + assert.Equal(t, http.StatusInternalServerError, statusOf(t, api.MyClasGetMyIdentitiesHandler.Handle(myClasOps.GetMyIdentitiesParams{HTTPRequest: req}, authUser))) + + service.err = nil + service.nilPdf = true + assert.Equal(t, http.StatusNotFound, statusOf(t, api.MyClasGetMyClaPdfHandler.Handle(myClasOps.GetMyClaPdfParams{HTTPRequest: req, SignatureID: "sig-1"}, authUser))) +} + +func TestPrincipal(t *testing.T) { + username, admin := principal(nil) + assert.Empty(t, username) + assert.False(t, admin) + + username, admin = principal(&auth.User{UserName: "someone"}) + assert.Equal(t, "someone", username) + assert.False(t, admin) + + username, admin = principal(&auth.User{UserName: "admin", ACL: auth.ACL{Admin: true}}) + assert.Equal(t, "admin", username) + assert.True(t, admin) +} + +// while no allow-list is configured nothing is trusted and no bearer token is required, so the +// endpoints keep behaving exactly as they did before +func TestHandlersWithoutAnAllowList(t *testing.T) { + verifier := &fakeVerifier{} + api, service := configuredAPI(t, verifier) + params := myClasOps.GetMyClasParams{HTTPRequest: request(t, "")} + + assert.Equal(t, http.StatusOK, statusOf(t, api.MyClasGetMyClasHandler.Handle(params, &auth.User{UserName: "someone"}))) + assert.Equal(t, http.StatusUnauthorized, statusOf(t, api.MyClasGetMyClasHandler.Handle(params, &auth.User{}))) + assert.Empty(t, verifier.seen, "a disabled verifier must not be consulted") + + admin := myClasOps.GetMyClasParams{HTTPRequest: request(t, ""), LfUsername: &[]string{"victim"}[0]} + assert.Equal(t, http.StatusOK, statusOf(t, api.MyClasGetMyClasHandler.Handle(admin, &auth.User{ACL: auth.ACL{Admin: true}}))) + require.Len(t, service.callers, 2) + assert.Equal(t, &Caller{Username: "someone"}, service.callers[0]) + assert.Equal(t, &Caller{Admin: true}, service.callers[1]) +} + +func TestHandlersWithoutAVerifier(t *testing.T) { + api, _ := configuredAPI(t, nil) + params := myClasOps.GetMyClasParams{HTTPRequest: request(t, "")} + + assert.Equal(t, http.StatusOK, statusOf(t, api.MyClasGetMyClasHandler.Handle(params, &auth.User{UserName: "someone"}))) +} diff --git a/cla-backend-go/v2/my_clas/service.go b/cla-backend-go/v2/my_clas/service.go index 2e0bdd6b6..9d05c3623 100644 --- a/cla-backend-go/v2/my_clas/service.go +++ b/cla-backend-go/v2/my_clas/service.go @@ -23,13 +23,18 @@ import ( ) const ( + identitySummaryLimit = 512 identitySourceGithub = "github" identitySourceGitlab = "gitlab" identitySourceGerrit = "gerrit" identityDataSourcePlatform = "platform" ) -// Identity holds the caller-provided identity keys used to resolve EasyCLA user records +// Identity holds the caller-provided identity keys used to resolve EasyCLA user records. +// +// Taking the identity list from the caller is a transitional mechanism (P3/P9 of the trust-SS +// decision): at M6 EasyCLA should call lfx.auth-service.user_identity.list itself over NATS and +// drop both these parameters and the azp allow-list that authorizes them. type Identity struct { LfUsername string Emails []string @@ -48,6 +53,49 @@ func (i *Identity) IsEmpty() bool { !hasValue(i.GitlabUsernames) && !hasValue(i.GerritUsernames) } +// Summary renders the identity keys as a compact, length-bounded string for the caller audit log +func (i *Identity) Summary() string { + var parts []string + addStrings := func(param string, values []string) { + if trimmed := trimAll(values); len(trimmed) > 0 { + parts = append(parts, param+":"+strings.Join(trimmed, ",")) + } + } + addIDs := func(param string, values []int64) { + ids := dedupeIDs(values) + if len(ids) == 0 { + return + } + formatted := make([]string, 0, len(ids)) + for _, id := range ids { + formatted = append(formatted, strconv.FormatInt(id, 10)) + } + parts = append(parts, param+":"+strings.Join(formatted, ",")) + } + + addStrings("lfUsername", []string{i.LfUsername}) + addStrings("email", i.Emails) + addStrings("secondaryEmail", i.SecondaryEmails) + addIDs("githubId", i.GithubIDs) + addStrings("githubUsername", i.GithubUsernames) + addIDs("gitlabId", i.GitlabIDs) + addStrings("gitlabUsername", i.GitlabUsernames) + addStrings("gerritUsername", i.GerritUsernames) + + summary := strings.Join(parts, " ") + if len(summary) > identitySummaryLimit { + summary = strings.ToValidUTF8(summary[:identitySummaryLimit], "") + "..." + } + return summary +} + +// Caller is the authenticated principal a My CLAs lookup runs as +type Caller struct { + Username string + Admin bool + Trusted bool +} + func hasValue(values []string) bool { for _, value := range values { if strings.TrimSpace(value) != "" { @@ -90,8 +138,8 @@ type ProjectService interface { // Service interface defines the My CLAs service methods type Service interface { - GetMyClas(ctx context.Context, currentUsername string, admin bool, requested *Identity) (*models.MyClaList, error) - GetMyClaPdfURL(ctx context.Context, currentUsername string, admin bool, requested *Identity, signatureID string) (*models.MyClaPdf, error) + GetMyClas(ctx context.Context, caller *Caller, requested *Identity) (*models.MyClaList, error) + GetMyClaPdfURL(ctx context.Context, caller *Caller, requested *Identity, signatureID string) (*models.MyClaPdf, error) GetMyIdentities(ctx context.Context, currentUsername string) (*models.MyIdentityList, error) } @@ -128,15 +176,16 @@ type projectInfo struct { // GetMyClas returns all signed ICLAs and ECLAs of the EasyCLA user records matching the // given identity, with validity evaluated against the current CCLA approval lists -func (s *service) GetMyClas(ctx context.Context, currentUsername string, admin bool, requested *Identity) (*models.MyClaList, error) { +func (s *service) GetMyClas(ctx context.Context, caller *Caller, requested *Identity) (*models.MyClaList, error) { f := logrus.Fields{ "functionName": "v2.my_clas.service.GetMyClas", utils.XREQUESTID: ctx.Value(utils.XREQUESTID), - "currentUsername": currentUsername, - "admin": admin, + "currentUsername": callerUsername(caller), + "admin": caller != nil && caller.Admin, + "trustedCaller": caller != nil && caller.Trusted, } - identity, skipped, err := s.effectiveIdentity(ctx, currentUsername, admin, requested) + identity, skipped, err := s.effectiveIdentity(ctx, caller, requested) if err != nil { return nil, err } @@ -234,16 +283,17 @@ func (s *service) GetMyClas(ctx context.Context, currentUsername string, admin b // GetMyClaPdfURL returns a time-limited download URL for the signed ICLA PDF when the // signature belongs to one of the EasyCLA user records matching the given identity - // a nil result means unknown, not-owned, unsigned or ECLA signature ID -func (s *service) GetMyClaPdfURL(ctx context.Context, currentUsername string, admin bool, requested *Identity, signatureID string) (*models.MyClaPdf, error) { +func (s *service) GetMyClaPdfURL(ctx context.Context, caller *Caller, requested *Identity, signatureID string) (*models.MyClaPdf, error) { f := logrus.Fields{ "functionName": "v2.my_clas.service.GetMyClaPdfURL", utils.XREQUESTID: ctx.Value(utils.XREQUESTID), - "currentUsername": currentUsername, - "admin": admin, + "currentUsername": callerUsername(caller), + "admin": caller != nil && caller.Admin, + "trustedCaller": caller != nil && caller.Trusted, "signatureID": signatureID, } - identity, _, err := s.effectiveIdentity(ctx, currentUsername, admin, requested) + identity, _, err := s.effectiveIdentity(ctx, caller, requested) if err != nil { return nil, err } @@ -356,18 +406,33 @@ func (s *service) GetMyIdentities(ctx context.Context, currentUsername string) ( }, nil } -func (s *service) effectiveIdentity(ctx context.Context, currentUsername string, admin bool, requested *Identity) (*Identity, []string, error) { - if admin { +// effectiveIdentity resolves which identity keys the lookup may search. An admin or a trusted +// LFX Self Serve caller supplies them directly; anyone else has each key verified against their +// own records first. A trusted caller's list is Auth0-derived and cannot be re-derived here: the +// historical GitHub-only signers this endpoint serves have no lf_username on their EasyCLA +// records, so verifying against those records would deny exactly the CLAs they may see. +func (s *service) effectiveIdentity(ctx context.Context, caller *Caller, requested *Identity) (*Identity, []string, error) { + if caller == nil { + return nil, nil, errors.New("no authenticated principal") + } + if caller.Admin || caller.Trusted { identity := *requested if identity.LfUsername == "" { - identity.LfUsername = currentUsername + identity.LfUsername = caller.Username } return &identity, []string{}, nil } - if currentUsername == "" { + if caller.Username == "" { return nil, nil, errors.New("no username on the authenticated principal") } - return s.authorizeIdentity(ctx, currentUsername, requested) + return s.authorizeIdentity(ctx, caller.Username, requested) +} + +func callerUsername(caller *Caller) string { + if caller == nil { + return "" + } + return caller.Username } type platformIdentitySet struct { diff --git a/cla-backend-go/v2/my_clas/service_test.go b/cla-backend-go/v2/my_clas/service_test.go index b2bc482ed..83b654b4a 100644 --- a/cla-backend-go/v2/my_clas/service_test.go +++ b/cla-backend-go/v2/my_clas/service_test.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "strings" "testing" v1Models "github.com/linuxfoundation/easycla/cla-backend-go/gen/v1/models" @@ -201,7 +202,7 @@ func TestGetMyClasUnionAndDedupe(t *testing.T) { } svc := newTestService(repo, &fakePlatform{}, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{names: map[string]string{"cla-group-1": "My CLA Group"}}) - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{ + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{ Emails: []string{"Someone@Example.org ", "someone@example.org"}, GithubIDs: []int64{12345, 12345}, }) @@ -245,7 +246,7 @@ func TestGetMyClasProjectNameAndLogo(t *testing.T) { "found-sfid": {ProjectOutput: v2ProjectServiceModels.ProjectOutput{ProjectCommon: v2ProjectServiceModels.ProjectCommon{Name: "Cloud Native Computing Foundation", ProjectLogo: "https://logos.example.org/cncf.png"}}}, }} - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{}) require.NoError(t, err) require.Len(t, result.Clas, 2) @@ -283,7 +284,7 @@ func TestGetMyClasProjectLookupDegradesGracefully(t *testing.T) { svc := newTestService(repo, &fakePlatform{}, &fakeSignatures{}, &fakeCompanies{}, claGroups) svc.projectService = &fakeProjectService{byID: map[string]*v2ProjectServiceModels.ProjectOutputDetailed{}} - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{}) require.NoError(t, err, "a project-service miss must not fail the listing") require.Len(t, result.Clas, 1) assert.Equal(t, "Kubernetes", result.Clas[0].ProjectName, "the mapping-table name is kept when the project-service has no record") @@ -316,7 +317,7 @@ func TestGetMyClasMultiProjectNonFoundation(t *testing.T) { "proj-alpha": {ProjectOutput: v2ProjectServiceModels.ProjectOutput{ProjectCommon: v2ProjectServiceModels.ProjectCommon{Name: "Alpha", ProjectLogo: "https://logos.example.org/alpha.png"}}}, }} - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{}) require.NoError(t, err) require.Len(t, result.Clas, 1) assert.Empty(t, result.Clas[0].ProjectName, "an ambiguous multi-project non-foundation group invents no project name") @@ -350,7 +351,7 @@ func TestGetMyClasProjectCacheHitPerRequest(t *testing.T) { } svc.projectService = projectSvc - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{}) require.NoError(t, err) require.Len(t, result.Clas, 2) assert.Equal(t, 1, projectSvc.calls["proj-sfid-1"], "the project-service is queried once per distinct CLA group within a request") @@ -381,7 +382,7 @@ func TestGetMyClasProjectServiceErrorAndNilClient(t *testing.T) { svc := newTestService(newRepo(), &fakePlatform{}, &fakeSignatures{}, &fakeCompanies{}, newClaGroups()) svc.projectService = &fakeProjectService{err: errors.New("project-service unavailable")} - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{}) require.NoError(t, err, "a project-service error must not fail the listing") require.Len(t, result.Clas, 1) assert.Equal(t, "Kubernetes", result.Clas[0].ProjectName, "the mapping-table name is kept on a project-service error") @@ -392,7 +393,7 @@ func TestGetMyClasProjectServiceErrorAndNilClient(t *testing.T) { svc := newTestService(newRepo(), &fakePlatform{}, &fakeSignatures{}, &fakeCompanies{}, newClaGroups()) svc.projectService = nil - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{}) require.NoError(t, err, "a nil project-service client must not fail the listing") require.Len(t, result.Clas, 1) assert.Equal(t, "Kubernetes", result.Clas[0].ProjectName, "the mapping-table name is kept with no project-service client") @@ -416,13 +417,13 @@ func TestGetMyClasMultipleRecordsSameLFID(t *testing.T) { } svc := newTestService(repo, &fakePlatform{}, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{}) - result, err := svc.GetMyClas(context.Background(), "alice", false, &Identity{GithubIDs: []int64{12345}}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "alice"}, &Identity{GithubIDs: []int64{12345}}) require.NoError(t, err) assert.Empty(t, result.SkippedIdentities, "a numeric ID stored on any of the caller's LFID records is authorized") assert.ElementsMatch(t, []string{"user-a1", "user-a2", "user-a3"}, result.UserIds, "all records per key are unioned") assert.Equal(t, int64(3), result.ResultCount) - pdf, err := svc.GetMyClaPdfURL(context.Background(), "alice", false, &Identity{}, "sig-a2") + pdf, err := svc.GetMyClaPdfURL(context.Background(), &Caller{Username: "alice"}, &Identity{}, "sig-a2") require.NoError(t, err) require.NotNil(t, pdf, "a PDF owned by the second LFID record is downloadable") } @@ -430,7 +431,7 @@ func TestGetMyClasMultipleRecordsSameLFID(t *testing.T) { func TestGetMyClasNoMatches(t *testing.T) { svc := newTestService(&fakeRepo{}, &fakePlatform{}, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{}) - result, err := svc.GetMyClas(context.Background(), "missing", false, &Identity{}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "missing"}, &Identity{}) require.NoError(t, err) assert.Empty(t, result.UserIds) assert.Empty(t, result.Clas) @@ -455,7 +456,7 @@ func TestGetMyClasOwnershipRejectsForeignIdentities(t *testing.T) { } svc := newTestService(repo, &fakePlatform{}, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{}) - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{ + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{ LfUsername: "victim", Emails: []string{"victim@example.org"}, SecondaryEmails: []string{"victim-alt@example.org"}, @@ -503,7 +504,7 @@ func TestGetMyClasOwnershipViaEasyCLARecord(t *testing.T) { platform := &fakePlatform{} svc := newTestService(repo, platform, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{}) - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{ + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{ SecondaryEmails: []string{"Alt@Example.org", "alt2@example.org", "alt@example.org"}, GitlabIDs: []int64{777}, }) @@ -538,7 +539,7 @@ func TestGetMyClasOwnershipViaPlatformIdentities(t *testing.T) { } svc := newTestService(repo, platform, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{}) - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{ + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{ GithubUsernames: []string{"octocat"}, GerritUsernames: []string{"old-ldap-id"}, }) @@ -548,14 +549,14 @@ func TestGetMyClasOwnershipViaPlatformIdentities(t *testing.T) { "the canonical spelling from user-service finds records stored with exact-match keys") assert.Equal(t, 1, platform.lookups, "platform identities are loaded once") - result, err = svc.GetMyClas(context.Background(), "someone", false, &Identity{ + result, err = svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{ GithubUsernames: []string{"not-a-code-identity"}, }) require.NoError(t, err) assert.Equal(t, []string{"githubUsername:not-a-code-identity"}, result.SkippedIdentities, "a slack username must not authorize a github search") - result, err = svc.GetMyClas(context.Background(), "someone", false, &Identity{ + result, err = svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{ GithubUsernames: []string{"lakecat"}, }) require.NoError(t, err) @@ -573,13 +574,110 @@ func TestGetMyClasAdminBypass(t *testing.T) { } svc := newTestService(repo, &fakePlatform{}, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{}) - result, err := svc.GetMyClas(context.Background(), "staff-admin", true, &Identity{LfUsername: "victim"}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "staff-admin", Admin: true}, &Identity{LfUsername: "victim"}) require.NoError(t, err) assert.Empty(t, result.SkippedIdentities) assert.Equal(t, "victim", result.LfUsername) assert.Equal(t, []string{"user-v"}, result.UserIds) } +// A trusted Self Serve caller's identity list is taken as authorized - the records it names +// typically carry no LF username at all (historical GitHub-only signers), which is exactly the +// case the per-identity verification cannot authorize +func TestGetMyClasTrustedCallerBypass(t *testing.T) { + githubOnly := &v1Models.User{UserID: "user-gh", GithubID: "999", GithubUsername: "octocat"} + repo := &fakeRepo{ + byUserID: map[string][]*signatures.ItemSignature{ + "user-gh": {icla("sig-gh", "user-gh", "cla-group-1", "2024-02-01T00:00:00Z", true)}, + }, + byGithubID: map[int64][]*v1Models.User{999: {githubOnly}}, + byGithubUsername: map[string][]*v1Models.User{"octocat": {githubOnly}}, + } + platform := &fakePlatform{} + svc := newTestService(repo, platform, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{}) + + result, err := svc.GetMyClas(context.Background(), &Caller{Trusted: true}, &Identity{ + GithubIDs: []int64{999}, + GithubUsernames: []string{"octocat"}, + }) + require.NoError(t, err) + assert.Empty(t, result.SkippedIdentities) + assert.Equal(t, []string{"user-gh"}, result.UserIds) + require.Len(t, result.Clas, 1) + assert.Equal(t, "sig-gh", result.Clas[0].SignatureID) + assert.Zero(t, platform.lookups, "a trusted caller's identity list is not verified against the platform user-service") + + pdf, err := svc.GetMyClaPdfURL(context.Background(), &Caller{Trusted: true}, &Identity{GithubIDs: []int64{999}}, "sig-gh") + require.NoError(t, err) + require.NotNil(t, pdf) + assert.Equal(t, "sig-gh", pdf.SignatureID) +} + +func TestEffectiveIdentityRequiresACaller(t *testing.T) { + svc := newTestService(&fakeRepo{}, &fakePlatform{}, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{}) + + _, err := svc.GetMyClas(context.Background(), nil, &Identity{GithubIDs: []int64{999}}) + assert.Error(t, err, "a nil caller must never be treated as authorized") + + _, err = svc.GetMyClas(context.Background(), &Caller{}, &Identity{GithubIDs: []int64{999}}) + assert.Error(t, err, "an untrusted caller without a username must never be treated as authorized") + + _, err = svc.GetMyClaPdfURL(context.Background(), &Caller{}, &Identity{GithubIDs: []int64{999}}, "sig-1") + assert.Error(t, err) +} + +func TestEffectiveIdentityForPrivilegedCallers(t *testing.T) { + platform := &fakePlatform{} + svc := newTestService(&fakeRepo{}, platform, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{}) + + tests := []struct { + name string + caller *Caller + requested *Identity + lfUsername string + }{ + {"trusted caller keeps the requested lfUsername", &Caller{Username: "ss-service", Trusted: true}, &Identity{LfUsername: "someone", GithubIDs: []int64{999}}, "someone"}, + {"trusted caller without one falls back to its own username", &Caller{Username: "someone", Trusted: true}, &Identity{GithubIDs: []int64{999}}, "someone"}, + {"trusted caller with neither stays empty", &Caller{Trusted: true}, &Identity{GithubIDs: []int64{999}}, ""}, + {"admin keeps the requested lfUsername", &Caller{Username: "staff-admin", Admin: true}, &Identity{LfUsername: "victim", GithubIDs: []int64{999}}, "victim"}, + {"admin and trusted at once", &Caller{Username: "staff-admin", Admin: true, Trusted: true}, &Identity{GithubIDs: []int64{999}}, "staff-admin"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + identity, skipped, err := svc.effectiveIdentity(context.Background(), test.caller, test.requested) + require.NoError(t, err) + assert.Equal(t, test.lfUsername, identity.LfUsername) + assert.Empty(t, skipped) + assert.Equal(t, test.requested.GithubIDs, identity.GithubIDs, "the requested keys must pass through untouched") + }) + } + + requested := &Identity{GithubIDs: []int64{999}} + _, _, err := svc.effectiveIdentity(context.Background(), &Caller{Username: "someone", Trusted: true}, requested) + require.NoError(t, err) + assert.Empty(t, requested.LfUsername, "the caller's identity list must not be mutated in place") + assert.Zero(t, platform.lookups, "a privileged caller's identity list is never verified against the platform user-service") +} + +func TestIdentitySummary(t *testing.T) { + identity := &Identity{ + LfUsername: "someone", + Emails: []string{"someone@example.org", " ", "someone@example.org"}, + GithubIDs: []int64{999, 999}, + GithubUsernames: []string{"octocat"}, + } + assert.Equal(t, "lfUsername:someone email:someone@example.org githubId:999 githubUsername:octocat", identity.Summary()) + assert.Empty(t, (&Identity{}).Summary()) + + long := make([]string, 0, 100) + for i := 0; i < 100; i++ { + long = append(long, fmt.Sprintf("user-%d@example.org", i)) + } + summary := (&Identity{Emails: long}).Summary() + assert.LessOrEqual(t, len(summary), identitySummaryLimit+3, "the audit log line must stay bounded") + assert.True(t, strings.HasSuffix(summary, "...")) +} + func TestGetMyClasIclaValidity(t *testing.T) { userA := &v1Models.User{UserID: "user-a", LfUsername: "someone"} unsigned := icla("sig-3", "user-a", "cla-group-1", "2024-03-01T00:00:00Z", true) @@ -597,7 +695,7 @@ func TestGetMyClasIclaValidity(t *testing.T) { } svc := newTestService(repo, &fakePlatform{}, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{}) - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{}) require.NoError(t, err) require.Len(t, result.Clas, 2, "unsigned records must be excluded") @@ -647,7 +745,7 @@ func TestGetMyClasEclaValidity(t *testing.T) { } svc := newTestService(repo, &fakePlatform{}, signaturesService, companies, &fakeClaGroups{}) - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{}) require.NoError(t, err) require.Len(t, result.Clas, 5) @@ -690,7 +788,7 @@ func TestGetMyClasEclaNotOnCurrentApprovalList(t *testing.T) { } svc := newTestService(repo, &fakePlatform{}, signaturesService, companies, &fakeClaGroups{}) - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{}) require.NoError(t, err) require.Len(t, result.Clas, 1) assert.True(t, result.Clas[0].Approved) @@ -716,7 +814,7 @@ func TestGetMyClasEclaGitlabGroupFallback(t *testing.T) { } svc := newTestService(repo, &fakePlatform{}, signaturesService, companies, &fakeClaGroups{}) - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{}) require.NoError(t, err) require.Len(t, result.Clas, 1) assert.True(t, result.Clas[0].Valid, "GitLab-group-approved ECLAs defer to the signature_approved flag") @@ -741,7 +839,7 @@ func TestGetMyClasEclaApprovalEvaluationError(t *testing.T) { } svc := newTestService(repo, &fakePlatform{}, signaturesService, companies, &fakeClaGroups{}) - result, err := svc.GetMyClas(context.Background(), "someone", false, &Identity{}) + result, err := svc.GetMyClas(context.Background(), &Caller{Username: "someone"}, &Identity{}) require.NoError(t, err, "approval-list evaluation problems must not fail the listing") require.Len(t, result.Clas, 1) assert.False(t, result.Clas[0].Valid, "evaluation errors leave the ECLA not covered - no GitLab fallback") @@ -764,27 +862,27 @@ func TestGetMyClaPdfURL(t *testing.T) { svc := newTestService(repo, &fakePlatform{}, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{}) identity := &Identity{} - result, err := svc.GetMyClaPdfURL(context.Background(), "someone", false, identity, "sig-icla") + result, err := svc.GetMyClaPdfURL(context.Background(), &Caller{Username: "someone"}, identity, "sig-icla") require.NoError(t, err) require.NotNil(t, result) assert.Equal(t, "sig-icla", result.SignatureID) assert.Equal(t, "https://s3.example.org/contract-group/cla-group-1/icla/user-a/sig-icla.pdf", result.URL) assert.Equal(t, int64(900), result.ExpiresInSeconds) - result, err = svc.GetMyClaPdfURL(context.Background(), "someone", false, identity, "sig-ecla") + result, err = svc.GetMyClaPdfURL(context.Background(), &Caller{Username: "someone"}, identity, "sig-ecla") require.NoError(t, err) assert.Nil(t, result, "ECLAs have no signed PDF") - result, err = svc.GetMyClaPdfURL(context.Background(), "someone", false, identity, "sig-unsigned") + result, err = svc.GetMyClaPdfURL(context.Background(), &Caller{Username: "someone"}, identity, "sig-unsigned") require.NoError(t, err) assert.Nil(t, result, "unsigned records have no signed PDF") - result, err = svc.GetMyClaPdfURL(context.Background(), "someone", false, identity, "sig-of-somebody-else") + result, err = svc.GetMyClaPdfURL(context.Background(), &Caller{Username: "someone"}, identity, "sig-of-somebody-else") require.NoError(t, err) assert.Nil(t, result, "signatures not owned by the resolved identity are not found") svc.documentExists = func(_ string) (bool, error) { return false, nil } - result, err = svc.GetMyClaPdfURL(context.Background(), "someone", false, identity, "sig-icla") + result, err = svc.GetMyClaPdfURL(context.Background(), &Caller{Username: "someone"}, identity, "sig-icla") require.NoError(t, err) assert.Nil(t, result, "missing S3 objects are reported as not found instead of returning a dead URL") } @@ -801,14 +899,14 @@ func TestGetMyClaPdfURLOwnershipEnforced(t *testing.T) { } svc := newTestService(repo, &fakePlatform{}, &fakeSignatures{}, &fakeCompanies{}, &fakeClaGroups{}) - result, err := svc.GetMyClaPdfURL(context.Background(), "someone", false, &Identity{ + result, err := svc.GetMyClaPdfURL(context.Background(), &Caller{Username: "someone"}, &Identity{ LfUsername: "victim", Emails: []string{"victim@example.org"}, }, "sig-victim") require.NoError(t, err) assert.Nil(t, result, "a non-admin cannot resolve somebody else's signature") - result, err = svc.GetMyClaPdfURL(context.Background(), "staff-admin", true, &Identity{LfUsername: "victim"}, "sig-victim") + result, err = svc.GetMyClaPdfURL(context.Background(), &Caller{Username: "staff-admin", Admin: true}, &Identity{LfUsername: "victim"}, "sig-victim") require.NoError(t, err) require.NotNil(t, result) } diff --git a/docs/MY_CLAS_API.md b/docs/MY_CLAS_API.md index ed4edfd04..028b9e23e 100644 --- a/docs/MY_CLAS_API.md +++ b/docs/MY_CLAS_API.md @@ -16,9 +16,10 @@ their current and historical ICLAs and ECLAs** — matched across their LF usern emails and GitHub/GitLab/Gerrit identities, with per-record validity evaluated against the *current* company CCLA approval lists — download their signed ICLA PDFs via time-limited links, and list the deduplicated identity set they own (the same set the -list endpoint authorizes them to search). Every provided identity key is **verified to belong to the -authenticated user** before it is searched (see "Identity ownership enforcement"), so -the endpoints cannot be used to freely enumerate other people's CLA history. +list endpoint authorizes them to search). For non-admin, untrusted callers every provided identity +key is **verified to belong to the authenticated user** before it is searched (see "Identity +ownership enforcement" for the admin and trusted-caller exceptions), so the endpoints cannot be +used to freely enumerate other people's CLA history. "Belongs to" means the identity is *currently* attached to the caller's LF account (their EasyCLA user record or their platform user-service profile/identities) — the accepted product bar for this read-only surface; the recycled-alias trade-off this @@ -78,6 +79,41 @@ by lfx-gateway), exactly like every other secured v4 endpoint: Note the ACS warden caches authorize responses for ~30 minutes; that only affects the first rollout (after `acs-cli sync`), not steady-state behavior. +### Trusted Self Serve caller (in-handler JWT verification + `azp` allow-list) + +The gateway-injected `X-ACL`/`X-USERNAME` headers are **decoded but never signature +checked**, so anything able to invoke the Lambda directly could forge them. To make the +identity-list bypass below safe, the handlers re-verify the request bearer token themselves +(`cla-backend-go/auth/trusted_caller.go`): the signing algorithm is pinned to the configured +Auth0 algorithm, the signature is verified against the tenant JWKS by `kid` +(`https://{cla-auth0-domain}/.well-known/jwks.json`, cached 15 min; a cache miss reloads it +at most once a minute; a JWKS outage keeps serving the cached key for at most 24 h), +`exp` must be present and unexpired, and the caller is **trusted** when the token's +`azp` is listed in the SSM parameter `cla-ss-trusted-client-ids-{stage}` (comma-separated +Auth0 client IDs). + +| Request, once the allow-list is configured | Result | +|---|---| +| No `Authorization` header, or an unparseable/unverifiable/expired token | **401**, the service is never reached | +| Verified token, `azp` **on** the allow-list | trusted: the caller-supplied identity list is searched as given, no per-identity verification | +| Verified token, `azp` **not** on the allow-list (or absent) | untrusted: unchanged behavior — admin bypass or per-identity ownership enforcement | + +An absent header is denied exactly like an invalid one: the traefik `aws-lambda` middleware +drops duplicated headers, so a duplicated `Authorization` header arrives as an absent one. +`iss` and `aud` are deliberately not re-checked in-handler: the JWKS already binds the token to +this one tenant, and which audience an SS token carries is still open in the +[trust-SS decision](https://github.com/linuxfoundation/lfx-self-serve/issues/1216) (session access +token vs. the P3 api-gw-audience token), so pinning one would 401 the other. A token this tenant +minted for another API therefore also verifies — accepted because `azp` stays the client signal. + +While the SSM parameter is unset the verifier is **disabled** — no bearer token is required +and nothing is trusted, so the endpoints behave exactly as before. A local `./bin/cla` run +loads the stage's SSM, so once the parameter is set local requests need a real token too +(`utils/my_clas.sh` sends one; its token-less `PRINCIPAL=...` mode does not). A non-empty +allow-list without `cla-auth0-domain` panics at startup rather than trusting blindly. Each +request is logged with `callerClientID` (`azp`), `callerSubject` (`sub`), `trustedCaller` +and the requested identity list (length-bounded) for anomaly detection. + ## `GET /v4/my-clas` ### Input parameters (all query, all optional) @@ -147,6 +183,14 @@ sampling path (e.g. the SC-001 comparison script) without weakening the contribu case. The token username is always searched for non-admin callers, so a bare `GET /v4/my-clas` works even when every extra key is skipped. +**A trusted Self Serve caller** (see [Trusted Self Serve caller](#trusted-self-serve-caller-in-handler-jwt-verification--azp-allow-list)) +also bypasses enforcement — deliberately, because here it is the *wrong* check: SS derives +the list from the caller's Auth0 identities (authoritative, and not reconstructible by +EasyCLA), while the historical GitHub-only signers this API exists to surface hold **no +`lf_username`** on their EasyCLA records, so verifying against those records would deny +exactly the CLAs the caller is entitled to see. Such a caller needs no username of its own; +supplying neither a username nor any identity key is a `400`, not an "everyone" query. + ### Identity resolution (step 1) The allowed keys are resolved to EasyCLA user records — **union of all matches, @@ -498,9 +542,9 @@ with a TODO for the missing lookup endpoint. With this API it collapses to: email as both `email` and `secondaryEmail` (they search different attributes), and provider usernames alongside numeric IDs (an ID present only on a detached pre-LFID record cannot be authorized by itself; the verified username recovers - it). The same full set goes on the PDF call (server-derived values from the - session — and EasyCLA now **re-verifies** every key against the LF account - server-side, so the enforcement is defense-in-depth rather than SS-only). Mapping to + it). The same full set goes on the PDF call (server-derived values from the session + — EasyCLA **re-verifies** every key against the LF account server-side until SS is + allow-listed, after which SS's Auth0-derived list is authoritative). Mapping to `MyClaAgreement`: `kind = claType`, `projectName = projectName` (bold top line of the Project cell) with `claGroupName` as its subtext and `projectLogo` as the logo tile (falling back to `claGroupName` / a default icon when a field is absent — the endpoint @@ -554,7 +598,23 @@ assumption is to be confirmed on dev. environment (dev → staging → prod) so the ACS warden allows the paths; until then the gateway returns 403 for them (fail-closed — nothing else can regress). 3. No lfx-gateway deploy is needed. -4. Read-only rollback: revert the ACS sync (or simply never flip the SS feature flag); +4. To switch on the trusted Self Serve caller path, provision the SSM parameter with the ID of a + Self Serve client whose tokens are **never returned to a user** — the only infrastructure change + the trust-SS hardening needs (the key matches the existing `cla-*` `ssm:GetParameter` grant). + The client SS calls with today does not qualify, so this step is on hold; see "Security notes": + + ```bash + aws --profile lfproduct-dev ssm put-parameter --name cla-ss-trusted-client-ids-dev \ + --type String --value '[,]' --overwrite + ``` + + Both partial states are safe, so deploy order does not matter, but the allow-list is read at + cold start only: after `put-parameter`, force a Lambda restart/redeploy, otherwise warm + containers keep the path disabled while fresh ones enable it. Rollback is + `ssm delete-parameter` plus the same restart. A read failure other than a missing parameter + is logged as a warning and leaves the path disabled, i.e. non-admin callers keep having every + identity verified per request — it never aborts the other lambdas that load this config. +5. Read-only rollback: revert the ACS sync (or simply never flip the SS feature flag); the endpoints write nothing. ## Verification performed @@ -576,6 +636,20 @@ assumption is to be confirmed on dev. not-found error classification; `GetMyIdentities` union/dedupe/sort of the `:` set across EasyCLA records + platform identities (deleted platform emails and non-code sources excluded, empty-username error). +- Unit tests for the trusted-caller path (`cla-backend-go/auth/trusted_caller_test.go`, + `cla-backend-go/config/ssm_test.go`, `cla-backend-go/v2/my_clas/handlers_test.go`, + `service_test.go`): allow-list parsing and on/off (incl. a configured allow-list without an + Auth0 domain failing startup); missing/blank/`Basic`/`Bearer`-only headers denied without a + JWKS lookup; trusted vs. verified-but-not-allow-listed/`azp`-less tokens; rejection of + expired, `exp`-less, `nbf`/`iat`-future, `kid`-less, unknown-`kid`, wrong-key and + `HS256`/`alg: none` tokens (algorithm pinning); JWKS caching, refresh cooldown, TTL reload, + cached-key fallback and its 24 h bound, concurrent use, fetch/decode failure modes and both + the `n`+`e` and `x5c` key forms; per-handler 401 on an unverifiable caller (service never + reached), trusted caller reaching the service with `Trusted=true` and no username, `400` + when it supplies no identity at all, service-error/404 mapping, unchanged behavior with the + verifier disabled or absent; service-level trusted bypass (a GitHub-only record with no + `lf_username` resolves, its PDF downloads, the platform user-service is never consulted and + the requested identity is not mutated) and the bounded identity-summary log line. - Read-only checks against the shared **dev** AWS environment: confirmed `reference-signature-index` on `cla-dev-signatures` and all six identity GSIs on `cla-dev-users`; sampled real ICLA/ECLA records to verify the @@ -634,3 +708,25 @@ assumption is to be confirmed on dev. (that is the platform-wide model — the gateway/ACS chain keys on the same claim), and admin-flagged principals bypass enforcement (needed for support/parity sampling; remove the `utils.IsUserAdmin` branch in `handlers.go` to revoke it). +- **The `azp` allow-list is only as sound as no user being able to hold a token that carries an + allow-listed `azp`.** Server-side minting with a client secret is not sufficient: the token SS + sends here (`req.apiGatewayToken`, a refresh-token exchange on `PCC_AUTH0_CLIENT_ID`) is minted + that way and then returned to every logged-in user as `v1Token` by SS's + `GET /api/profile/developer` ([lfx-self-serve#1045](https://github.com/linuxfoundation/lfx-self-serve/pull/1045)), + and the v2 session token shares that `azp`. Allow-listing that client would therefore let any + logged-in user pass any identity — including to the PDF endpoint, whose presigned URL exposes a + signed ICLA. So allow-list only a client whose tokens are never surfaced to a user; SS needs a + dedicated client for this hop first. The same caveat is recorded in code at the `azp` check. +- **Both the caller-supplied identity list and the `azp` allow-list are transitional (P3/P9 + of the trust-SS decision).** At M6, once EasyCLA runs on K8s, it should call + `lfx.auth-service.user_identity.list` itself over NATS for the token's subject — at which + point the `githubId`/`githubUsername`/… parameters, the allow-list and the in-handler JWT + verification all go away together. `swagger/cla.v2.yaml` carries the same note. +- **The in-handler verification covers the bearer token, not the gateway's `X-ACL` headers.** + Those are still consumed unverified (`lfx-kit/auth.SwaggerAuth`), so what is closed, once the + allow-list is configured, is the trust decision that matters here: nothing reaches the handlers + without a JWKS-verified tenant token, and nothing can claim to be Self Serve without an + allow-listed `azp`. A forged `X-ACL` can still assert a username, or the admin flag that bypasses + ownership checks — both pre-existing, both now additionally requiring a valid token. Binding the + principal to a token claim belongs to the v4 invoke-path trust work (spike 4); the M6 move removes + the header trust entirely.