From 8d8f30f8dc027166a5aaa68f9e1f73896a8f858d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 21:01:49 +0000 Subject: [PATCH] feat(jwtauth): WithTenantClaim names the claim the tenant is read from A stock Socrate has no tenant model: a tenant reaches tokens only through a client's claim mapping, under Socrate's claims namespace, as https://socrate/tenant_id. The middleware read only a plain tenant_id, so httpware.RequireTenant refused every Socrate token. WithTenantClaim reads the named claim from the verified token instead: it replaces SocrateClaims.TenantID (so the revocation check sees it), a plain tenant_id is then ignored, a value that is not a UUID string (null included) is a 401, and an empty name rejects every token. The default is unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GKRxaeYxyDhmt42cehLsGA --- CHANGELOG.md | 9 +++ README.md | 9 ++- docs/CLIENT-INTEGRATION.md | 31 +++++++- jwtauth/middleware.go | 85 +++++++++++++++++++- jwtauth/tenant_claim_test.go | 146 +++++++++++++++++++++++++++++++++++ 5 files changed, 271 insertions(+), 9 deletions(-) create mode 100644 jwtauth/tenant_claim_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ec91290..a68f0e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ All notable changes to backendkit are documented here. Format: ## [Unreleased] +### Added +- `jwtauth.WithTenantClaim(name)`: read the tenant from the named claim instead of `tenant_id`. + A stock Socrate has no tenant model and issues claim mappings under its claims namespace, so a + tenant reaches tokens as `https://socrate/tenant_id`, which the middleware ignored: + `httpware.RequireTenant` then refused every token. With the option, the named claim replaces + `SocrateClaims.TenantID` (the revocation check sees it too) and a plain `tenant_id` is ignored; + a value that is not a UUID string, `null` included, is a 401; an empty name rejects every token. + The default is unchanged. Reported by Lakebridge (go-oauth2 #308). + ## [1.20.0] - 2026-10-04 Minor release on the **v1** line: additive API, and a new minimum Go. `bff.WithPostgresManagedSchema` diff --git a/README.md b/README.md index f141639..52bd2c2 100644 --- a/README.md +++ b/README.md @@ -643,6 +643,13 @@ auth := jwtauth.New(jwksURL, issuer, logger, - **Revocation.** Local signature validation alone keeps a token valid until its `exp`, even after logout or a password change. The check typically compares `token_version` with the user's current value; with none configured, behaviour is unchanged. +- **Tenant claim.** The tenant is read from `tenant_id` by default. A stock Socrate has no tenant + model: a tenant reaches tokens only through a client's claim mapping + (`"tenant_id": "user.attributes.tenant_id"`), under Socrate's claims namespace, as + `https://socrate/tenant_id`. `WithTenantClaim("https://socrate/tenant_id")` reads that claim + instead; a plain `tenant_id` is then ignored. The value must be a UUID string (anything else + is a 401); without the claim no tenant is set and `httpware.RequireTenant` rejects the request. + An empty name rejects every token (fail closed). - **Authentication facts.** `auth_time` and `amr` (when and how the user authenticated) are exposed as `ctxutil.GetAuthTime` / `ctxutil.GetAMR`. They are what a step-up or MFA check needs; `pep` uses them to honour policy obligations. @@ -1050,7 +1057,7 @@ runnable `Example*` functions (visible on | Symptom | Likely cause & fix | |---------|--------------------| | **Every request returns 401** | No `Authorization: Bearer ` header, an `iss` that doesn't match `SOCRATE_ISSUER`, an `aud` that doesn't contain the `WithAudience` value (or any `WithAudiences` value), or the JWKS URL is unreachable. Stale keys are reused on a *transient* fetch failure, but a wrong/empty JWKS URL fails closed. | -| **`GetTenantID` is `uuid.Nil` / `GetUserPlan` is always `"freemium"`** | `tenant_id` and `plan` are **custom** claims. A stock Socrate server does not emit them — configure Socrate to include them, or these helpers return their zero/default values by design. | +| **`GetTenantID` is `uuid.Nil` / `GetUserPlan` is always `"freemium"`** | `tenant_id` and `plan` are **custom** claims. A stock Socrate server does not emit them — configure Socrate to include them, or these helpers return their zero/default values by design. Socrate's claim mappings issue them under its namespace (`https://socrate/tenant_id`): pass `jwtauth.WithTenantClaim("https://socrate/tenant_id")`. | | **`GetUserEmail` / `GetUserName` are empty** | Email and name live in the **ID token**, not the access token. For access-token requests, fetch them via `socrate.Client.GetCurrentUserProfile`. | | **Compile error passing a logger to `httpware.Logger`** | `Logger` takes the base `*logrus.Logger`; `Recover`, `NewRBAC`, `jwtauth.New`, and `tiering.NewGate` take a `*logrus.Entry`. See the [httpware](#httpware) note. | | **Service-account call errors with "AppID must be set"** | Set `AppID` in `ClientConfig` (`SOCRATE_APP_ID`). The `/api/admin/apps` lookup needs a human-admin JWT, so a service token cannot resolve the app ID at runtime. | diff --git a/docs/CLIENT-INTEGRATION.md b/docs/CLIENT-INTEGRATION.md index 1e775da..22134dc 100644 --- a/docs/CLIENT-INTEGRATION.md +++ b/docs/CLIENT-INTEGRATION.md @@ -240,8 +240,8 @@ auth := jwtauth.New(jwksURL, issuer, log, ) // On tenant-scoped route groups, guarantee a tenant is present so no nil-tenant -// request reaches your handlers. Precondition: Socrate issues the `tenant_id` -// claim (the default server does not — see §5). +// request reaches your handlers. Precondition: Socrate issues a tenant claim +// (the default server does not — see §5), read with jwtauth.WithTenantClaim. r.Group(func(r chi.Router) { r.Use(auth.Handler) r.Use(httpware.RequireTenant) // 401 when ctxutil.GetTenantID == uuid.Nil @@ -284,7 +284,7 @@ This trips people up, so it's worth stating plainly: |-------|------------------|-------| | `sub`, `role`, `app_roles`, `token_version` | ✅ always | the dependable identity set | | `email`, `name` | ❌ **not** in access tokens | present in ID tokens / `userinfo` only | -| `tenant_id` | ⚠️ only if the server is configured to issue it | else `ctxutil.GetTenantID` → `uuid.Nil` | +| `tenant_id` | ⚠️ only if the server is configured to issue it | else `ctxutil.GetTenantID` → `uuid.Nil`; Socrate issues it as `https://socrate/tenant_id` (see "Tenant isolation", §9) | | `plan` | ⚠️ only if the server is configured to issue it | else `ctxutil.GetUserPlan` → `"freemium"` | **`role` is the user's role in the application the token was issued for**, not in yours. @@ -1074,9 +1074,26 @@ upgrade URL. For multi-tenant apps, mount `httpware.RequireTenant` after `auth.Handler` on tenant-scoped route groups. It returns **401** when no tenant is in context (`ctxutil.GetTenantID == uuid.Nil`), so a handler can never run against the nil -tenant. Requires Socrate to issue the `tenant_id` claim (see §5). +tenant. Requires Socrate to issue a tenant claim (see §5). + +Socrate has no tenant model of its own. The tenant comes from a user attribute, +projected into your client's tokens by a claim mapping, and every mapped claim +carries Socrate's claims namespace (`CLAIMS_NAMESPACE`, default +`https://socrate/`): + +1. A global admin sets each user's tenant: + `PUT /api/admin/users/{id}/attributes` with + `{"attributes": {"tenant_id": ""}}`. +2. Your client declares the mapping: + `"claim_mappings": {"tenant_id": "user.attributes.tenant_id"}`. + Its access tokens then carry `"https://socrate/tenant_id": ""`. +3. The middleware reads that claim: ```go +auth := jwtauth.New(jwksURL, issuer, logger, + jwtauth.WithAudience(clientID), + jwtauth.WithTenantClaim("https://socrate/tenant_id")) + r.Group(func(r chi.Router) { r.Use(auth.Handler) r.Use(httpware.RequireTenant) @@ -1084,6 +1101,12 @@ r.Group(func(r chi.Router) { }) ``` +Without `WithTenantClaim` the middleware reads a plain `tenant_id`, which a +stock Socrate never issues, so `RequireTenant` refuses every request. With it, a +plain `tenant_id` in the token is ignored; a value that is not a UUID string is +rejected with 401; an empty name rejects every token. Use the namespace your +Socrate is configured with, if it is not the default. + --- ## 10. Enforcing central policy decisions (pep) diff --git a/jwtauth/middleware.go b/jwtauth/middleware.go index 94d521a..d268491 100644 --- a/jwtauth/middleware.go +++ b/jwtauth/middleware.go @@ -40,7 +40,8 @@ import ( // - TokenVersion — monotonic counter; incremented on password change / token revocation // // Claims NOT issued by the default Socrate server (require custom server configuration): -// - TenantID — multi-tenancy identifier; will be empty unless the server is extended +// - TenantID — multi-tenancy identifier, read from the tenant_id claim, or from the claim +// named by WithTenantClaim; empty unless the server is configured to issue it // - Plan — commercial tier; will be empty, causing GetUserPlan to default to "freemium" // // Claims only present in ID tokens (OIDC flow), NOT in access tokens: @@ -58,6 +59,8 @@ type SocrateClaims struct { Amr []string `json:"amr,omitempty"` // Custom claims — require server-side configuration to be populated. + // With WithTenantClaim, the middleware replaces TenantID with the value of + // the named claim (empty when the token does not carry it). TenantID string `json:"tenant_id,omitempty"` Plan string `json:"plan,omitempty"` @@ -94,9 +97,14 @@ type Middleware struct { audiences []string audienceSet bool revocationCheck RevocationChecker - logger *logrus.Entry - httpClient *http.Client - cacheTTL time.Duration + // tenantClaim is the claim the tenant is read from when tenantClaimSet + // (WithTenantClaim); otherwise it is tenant_id, decoded into SocrateClaims. + // An empty tenantClaim with tenantClaimSet rejects every token. + tenantClaim string + tenantClaimSet bool + logger *logrus.Entry + httpClient *http.Client + cacheTTL time.Duration // leeway is the clock-skew tolerance applied to time-based claim validation // (exp/nbf/iat). See WithLeeway. @@ -203,6 +211,32 @@ func WithRevocationCheck(fn RevocationChecker) Option { return func(m *Middleware) { m.revocationCheck = fn } } +// WithTenantClaim names the claim the tenant is read from, instead of +// tenant_id. Use it when the issuer does not emit a plain tenant_id: a stock +// Socrate projects custom claims through a client's claim mappings under its +// claims namespace, so a mapping named tenant_id reaches the token as +// "https://socrate/tenant_id": +// +// auth := jwtauth.New(jwksURL, issuer, logger, +// jwtauth.WithAudience(clientID), +// jwtauth.WithTenantClaim("https://socrate/tenant_id")) +// +// The tenant still comes only from the signed token. With this option, the +// named claim replaces SocrateClaims.TenantID (so a RevocationChecker sees the +// same tenant as the request context) and a plain tenant_id claim is ignored. +// The claim must be a string holding a UUID: any other value rejects the token +// with 401. When the token does not carry the claim, no tenant is set and +// httpware.RequireTenant rejects the request. +// +// Fail closed: an empty or blank name rejects every token (and New logs an +// error) rather than falling back to tenant_id. The last WithTenantClaim +// passed to New wins. +func WithTenantClaim(name string) Option { + return func(m *Middleware) { + m.tenantClaim, m.tenantClaimSet = strings.TrimSpace(name), true + } +} + // WithLeeway sets the clock-skew tolerance applied to time-based claim checks // (exp/nbf/iat). The default is 60s. A negative value is ignored. func WithLeeway(d time.Duration) Option { @@ -273,6 +307,9 @@ func New(jwksURL, issuer string, logger *logrus.Entry, opts ...Option) *Middlewa if m.audienceSet && len(m.audiences) == 0 && logger != nil { logger.Error("jwtauth: WithAudiences was given no audience — every token is rejected") } + if m.tenantClaimSet && m.tenantClaim == "" && logger != nil { + logger.Error("jwtauth: WithTenantClaim was given an empty claim name — every token is rejected") + } return m } @@ -414,9 +451,49 @@ func (m *Middleware) validateToken(tokenString string) (*SocrateClaims, error) { if err != nil || !tok.Valid { return nil, fmt.Errorf("token invalid") } + if m.tenantClaimSet { + tenant, err := stringClaim(tokenString, m.tenantClaim) + if err != nil { + return nil, err + } + claims.TenantID = tenant + } return claims, nil } +// stringClaim returns the named claim of an already-verified token: "" when +// absent, an error when it is not a JSON string. SocrateClaims cannot carry a +// claim whose name is only known at run time, hence the second decode of the +// payload, which runs only after the signature has been checked. +func stringClaim(tokenString, name string) (string, error) { + if name == "" { + return "", fmt.Errorf("tenant claim not configured") + } + parts := strings.Split(tokenString, ".") + if len(parts) != 3 { + return "", fmt.Errorf("token invalid") + } + payload, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + return "", fmt.Errorf("token payload: %w", err) + } + var all map[string]json.RawMessage + if err := json.Unmarshal(payload, &all); err != nil { + return "", fmt.Errorf("token payload: %w", err) + } + raw, ok := all[name] + if !ok { + return "", nil + } + // json.Unmarshal accepts null into a string without error; a present claim + // that is not a string is rejected, null included. + var v string + if strings.TrimSpace(string(raw)) == "null" || json.Unmarshal(raw, &v) != nil { + return "", fmt.Errorf("claim %q is not a string", name) + } + return v, nil +} + func (m *Middleware) getKey(kid string) (*rsa.PublicKey, error) { m.mu.RLock() key, ok := m.keys[kid] diff --git a/jwtauth/tenant_claim_test.go b/jwtauth/tenant_claim_test.go new file mode 100644 index 0000000..a8bc4cf --- /dev/null +++ b/jwtauth/tenant_claim_test.go @@ -0,0 +1,146 @@ +package jwtauth_test + +import ( + "bytes" + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/google/uuid" + "github.com/sirupsen/logrus" + + "github.com/ovander/backendkit/ctxutil" + "github.com/ovander/backendkit/jwtauth" +) + +const ( + nsTenantClaim = "https://socrate/tenant_id" + tenantA = "00000000-0000-0000-0000-00000000000a" + tenantB = "00000000-0000-0000-0000-00000000000b" +) + +// serveTenant signs claims (plus sub and exp) and returns the response code and +// the tenant the handler saw. +func serveTenant(t *testing.T, opts []jwtauth.Option, extra jwt.MapClaims) (int, uuid.UUID) { + t.Helper() + key := generateTestKey(t) + srv := jwksServer(t, "k1", key) + defer srv.Close() + + claims := jwt.MapClaims{"sub": "42", "exp": time.Now().Add(time.Hour).Unix()} + for k, v := range extra { + claims[k] = v + } + m := jwtauth.New(srv.URL, "", testLogger(), opts...) + var seen uuid.UUID + h := m.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seen = ctxutil.GetTenantID(r.Context()) + w.WriteHeader(http.StatusOK) + })) + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.Header.Set("Authorization", "Bearer "+signToken(t, key, "k1", claims)) + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + return w.Code, seen +} + +func TestHandler_TenantClaim(t *testing.T) { + ns := []jwtauth.Option{jwtauth.WithTenantClaim(nsTenantClaim)} + tests := []struct { + name string + opts []jwtauth.Option + claims jwt.MapClaims + code int + tenant string // "" = uuid.Nil + }{ + {"default reads tenant_id", nil, jwt.MapClaims{"tenant_id": tenantA}, http.StatusOK, tenantA}, + {"default ignores a namespaced tenant claim", nil, jwt.MapClaims{nsTenantClaim: tenantA}, http.StatusOK, ""}, + {"named claim is read", ns, jwt.MapClaims{nsTenantClaim: tenantA}, http.StatusOK, tenantA}, + {"named claim wins over tenant_id", ns, jwt.MapClaims{nsTenantClaim: tenantA, "tenant_id": tenantB}, http.StatusOK, tenantA}, + {"plain tenant_id is ignored when another claim is named", ns, jwt.MapClaims{"tenant_id": tenantB}, http.StatusOK, ""}, + {"absent named claim sets no tenant", ns, nil, http.StatusOK, ""}, + {"surrounding spaces in the name are ignored", []jwtauth.Option{jwtauth.WithTenantClaim(" " + nsTenantClaim + " ")}, jwt.MapClaims{nsTenantClaim: tenantA}, http.StatusOK, tenantA}, + {"naming tenant_id equals the default", []jwtauth.Option{jwtauth.WithTenantClaim("tenant_id")}, jwt.MapClaims{"tenant_id": tenantA}, http.StatusOK, tenantA}, + {"non-UUID value is rejected", ns, jwt.MapClaims{nsTenantClaim: "acme"}, http.StatusUnauthorized, ""}, + {"number is rejected", ns, jwt.MapClaims{nsTenantClaim: 7}, http.StatusUnauthorized, ""}, + {"object is rejected", ns, jwt.MapClaims{nsTenantClaim: map[string]any{"id": tenantA}}, http.StatusUnauthorized, ""}, + {"null is rejected", ns, jwt.MapClaims{nsTenantClaim: nil}, http.StatusUnauthorized, ""}, + {"empty name fails closed", []jwtauth.Option{jwtauth.WithTenantClaim("")}, jwt.MapClaims{"tenant_id": tenantA}, http.StatusUnauthorized, ""}, + {"blank name fails closed", []jwtauth.Option{jwtauth.WithTenantClaim(" ")}, jwt.MapClaims{"tenant_id": tenantA}, http.StatusUnauthorized, ""}, + {"last option wins", []jwtauth.Option{jwtauth.WithTenantClaim(""), jwtauth.WithTenantClaim(nsTenantClaim)}, jwt.MapClaims{nsTenantClaim: tenantA}, http.StatusOK, tenantA}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + code, seen := serveTenant(t, tt.opts, tt.claims) + if code != tt.code { + t.Fatalf("code = %d, want %d", code, tt.code) + } + want := uuid.Nil + if tt.tenant != "" { + want = uuid.MustParse(tt.tenant) + } + if code == http.StatusOK && seen != want { + t.Errorf("tenant = %s, want %s", seen, want) + } + }) + } +} + +// The revocation check sees the tenant the request context gets. +func TestHandler_TenantClaim_RevocationCheckSeesNamedTenant(t *testing.T) { + var got string + check := jwtauth.WithRevocationCheck(func(_ context.Context, c *jwtauth.SocrateClaims) error { + got = c.TenantID + return nil + }) + code, _ := serveTenant(t, []jwtauth.Option{jwtauth.WithTenantClaim(nsTenantClaim), check}, + jwt.MapClaims{nsTenantClaim: tenantA, "tenant_id": tenantB}) + if code != http.StatusOK || got != tenantA { + t.Errorf("code=%d, RevocationChecker saw TenantID %q, want 200 and %q", code, got, tenantA) + } +} + +// A forged token carrying the named claim is still refused: the claim is read +// only after the signature is verified. +func TestHandler_TenantClaim_UnsignedTokenRejected(t *testing.T) { + key := generateTestKey(t) + srv := jwksServer(t, "k1", key) + defer srv.Close() + m := jwtauth.New(srv.URL, "", testLogger(), jwtauth.WithTenantClaim(nsTenantClaim)) + + other := generateTestKey(t) + token := signToken(t, other, "k1", jwt.MapClaims{"sub": "42", "exp": time.Now().Add(time.Hour).Unix(), nsTenantClaim: tenantA}) + called := false + h := m.Handler(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { called = true })) + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.Header.Set("Authorization", "Bearer "+token) + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + if w.Code != http.StatusUnauthorized || called { + t.Errorf("code=%d called=%v, want 401 and the handler not called", w.Code, called) + } +} + +func TestNew_WithTenantClaimLogging(t *testing.T) { + var buf bytes.Buffer + l := logrus.New() + l.SetOutput(&buf) + entry := logrus.NewEntry(l) + + _ = jwtauth.New("http://example/jwks.json", "https://issuer.example", entry, + jwtauth.WithAudience("app"), jwtauth.WithTenantClaim(nsTenantClaim)) + if buf.Len() != 0 { + t.Errorf("expected no log with a named tenant claim, got: %s", buf.String()) + } + + buf.Reset() + _ = jwtauth.New("http://example/jwks.json", "https://issuer.example", entry, + jwtauth.WithAudience("app"), jwtauth.WithTenantClaim(" ")) + if !strings.Contains(buf.String(), "every token is rejected") { + t.Errorf("expected the fail-closed error, got: %s", buf.String()) + } +}