diff --git a/OAUTH2_IDENTITY_PROVIDERS.md b/OAUTH2_IDENTITY_PROVIDERS.md index e29d873f1c..ab0ae8fc5b 100644 --- a/OAUTH2_IDENTITY_PROVIDERS.md +++ b/OAUTH2_IDENTITY_PROVIDERS.md @@ -59,6 +59,121 @@ called from Boot): `oauth2.oidc_provider`: flagged as a likely leftover — its tokens will be rejected with OBP-20218. +## Google client ID policy: how many OAuth clients, and per what? + +`oauth2.google.allowed_audiences` (and the Yahoo/Microsoft equivalents) is a +flat, instance-wide, comma-separated list. That flexibility raises an +operational question: should the operator register **one** Google OAuth client +and share it, or one **per app**, **per environment**, **per tenant**? The +allowlist mechanism supports all of these; the security and operational +properties differ substantially. + +One implementation fact shapes the whole comparison: for public-IdP logins, +OBP resolves the token to a Consumer keyed by the **`(azp, iss)` pair** +(`MappedConsumersProvider.getOrCreateConsumer`, `OAuth.scala`) — **one +Consumer per OAuth client per issuer**, auto-created on first login. (The +original implementation keyed Consumers by `` — one per *user* per +client; that is no longer the case, the `sub` claim is stored on the Consumer +but is not part of the lookup key.) A **pre-registered** Consumer whose `key` +equals the OAuth2 client ID takes priority over auto-creation (its +`azp`/`iss` are populated on first use), so the operator can register one +Consumer per Google client up front. Everything OBP hangs off a Consumer — +rate limits, metrics, the enable/disable switch (enforced after auth by +`AfterApiAuth.checkConsumerIsDisabled`) — therefore has per-client +granularity: the client ID policy decides whether "per client" means per app, +per instance, or per environment. One cosmetic caveat: auto-created Consumers +take their name from the token's `name` claim — the display name of whichever +user logged in first (falling back to "OpenID Connect") — so pre-register +Consumers when you want meaningful app names in metrics. All of this is +pinned down by `OAuth2ConsumerResolutionTest`. + +### The four policies + +| | 1. Per instance (shared) | 2. Per app | 3. Per app × environment | 4. Per tenant | +|---|---|---|---|---| +| Google clients needed | 1 | 1 per app (Explorer, Manager, Portal, …) | apps × environments | apps × environments × tenants | +| `allowed_audiences` entries | 1 | one per app | one per app (each instance lists only its own environment's IDs) | per-tenant scoping **not expressible** in a flat props list | +| Revoke a single app | ✗ — removing the entry kills all apps | ✓ — remove its entry | ✓ | ✓ | +| Distinguish apps in Consumers / metrics | ✗ — same `azp` everywhere; all apps share one Consumer | ✓ — one Consumer per app | ✓ | ✓ | +| Cross-app token replay (token minted for app A used as app B) | possible by construction — all apps share one audience | prevented at the app boundary | prevented | prevented | +| Cross-**environment** replay (test token → prod) | **open** if the same client ID is reused across environments | **open** if IDs are reused across environments | **closed** — prod allowlist never contains a test client ID | closed | +| Blast radius of a leaked client secret | every app on the instance | one app | one app in one environment | one app in one environment for one tenant | +| Google-side consent screen / redirect URIs | one shared consent screen; redirect URI list grows unbounded | per-app branding and URIs | per-app, per-env URIs (no `localhost` on the prod client) | as per app × env | +| Operational overhead | minimal | moderate | moderate (naming convention keeps it sane) | high; flat props list is the limiting factor | + +### Implications per policy + +**1. Per instance (one shared client).** Simplest to set up, and acceptable +only for throwaway sandboxes. Because every app presents the same `aud`/`azp`, +the audience check degenerates to "is it ours at all": an id_token obtained by +logging into the Portal is equally valid when replayed as if it came from API +Manager, and all apps resolve to the same `(azp, iss)` Consumer — so +per-Consumer rate limits, metrics and the disable switch cannot tell apps +apart, and there is no per-app kill switch. A leaked client secret (for +confidential flows) burns every app at once. + +**2. Per app.** The intended reading of the example in the audience-allowlist +section above. Each ecosystem app (Explorer, Manager, Portal) gets its own +Google OAuth client; all are listed comma-separated. Each app then maps to its +own Consumer, so per-app rate limits and metrics work, and revoking one app +can be done two ways: remove its allowlist entry (props change, needs a +restart) or disable its Consumer (immediate, runtime). Each app controls its +own consent-screen branding and redirect URIs. Remaining weakness: the +allowlist is still flat — every listed client is equally trusted for the +*entire* API surface (there is no "tokens from the Portal client may log in +but not reach admin endpoints" distinction; that would require Consumer-level +authorisation policy, see "Binding client IDs to registered Consumers" +below). + +**3. Per app × environment (recommended).** Policy 2 plus the rule: **a +Google client ID is never reused across environments.** This is the axis the +replay warning at the top of this document is about — signature, issuer, +expiry and even audience validation all pass when a token minted on a test +instance is replayed against production *if both list the same client ID*. +Separate clients per environment (naming convention helps: +`obp--`, e.g. `obp-prod-explorer`, `obp-test-explorer`) close this +cheaply, and also keep `localhost` redirect URIs off the production client. +Cost is bookkeeping only; the allowlist per instance stays exactly as small +as in policy 2. + +**4. Per tenant.** Only relevant if a single OBP instance serves multiple +tenants/banks. The current mechanism cannot express it: the props list is +instance-global, so a client ID listed for tenant A also authenticates +against tenant B. Multi-tenant deployments should either run one instance per +tenant (reducing this to policy 3) or wait for a Consumer-bound allowlist +(below). + +### Rotation + +Because the allowlist holds client **IDs** (not secrets) and is +comma-separated, rotation needs no downtime and no code: add the new client +ID alongside the old one, migrate the app to the new client, then remove the +old entry. The overlap window is as long as you need. (A props change +requires an instance restart to take effect — plan the two edits around +normal restart cycles.) + +### Binding client IDs to registered Consumers (half implemented) + +The **attachment** half already exists: `getOrCreateConsumer` gives priority +to a pre-registered Consumer whose `key` equals the token's client ID +(`azp`), before falling back to the auto-created `(azp, iss)` Consumer — and +it displaces a stale auto-created Consumer holding the same `(azp, iss)`. So +an operator can register one Consumer per Google client today and get +meaningful Consumer names in metrics, per-app rate limits, and a runtime +disable switch — instead of relying on auto-creation and its +user-display-name naming. + +The **rejection** half is not implemented: an id_token whose client ID +matches no registered Consumer is still accepted (a Consumer is auto-created +on the fly), so `allowed_audiences` remains the only control that actually +rejects foreign client IDs — and it is instance-global (no per-tenant +scoping) and needs a restart to change. The natural end-state — validation +asking "does an *enabled, registered* Consumer with this client ID exist?" — +would make the allowlist manageable at runtime via API Manager and express +per-tenant scoping (the Hydra introspection path already resolves consumers +by client ID this way). Until then, the recommended combination is policy 3 +above for rejection plus pre-registered Consumers for per-app control. + ## Operator-controlled IdPs (Keycloak, OBP-OIDC, Hydra) These run under the API operator's control: only applications the operator @@ -90,3 +205,6 @@ JWKS URL must be present in `oauth2.jwk_set.url` for signature validation. and `OAuth2Util.validateProviderEnabled` - `obp-api/src/test/scala/code/api/OAuth2AudienceValidationTest.scala` — executable specification of both checks +- `obp-api/src/test/scala/code/api/OAuth2ConsumerResolutionTest.scala` — + executable specification of token-to-Consumer resolution: `(azp, iss)` + granularity, pre-registered-Consumer priority, auto-created metadata diff --git a/obp-api/src/main/scala/code/api/OAuth2.scala b/obp-api/src/main/scala/code/api/OAuth2.scala index 5a1dfdef7f..7eda828616 100644 --- a/obp-api/src/main/scala/code/api/OAuth2.scala +++ b/obp-api/src/main/scala/code/api/OAuth2.scala @@ -585,11 +585,17 @@ object OAuth2Login extends MdcLoggable { } /** - * This function creates a consumer based on "azp", "sub", "iss", "name" and "email" fields - * Please note that a user must be created before consumer. - * Unique criteria to decide do we create or get a consumer is pair o values: < sub : azp > i.e. - * We cannot find consumer by sub and azp => Create - * We can find consumer by sub and azp => Get + * This function resolves the token to a consumer based on the "azp", "aud", "iss", "sub", + * "name" and "email" claims. Please note that a user must be created before consumer. + * Lookup order (see MappedConsumersProvider.getOrCreateConsumer): + * 1. by consumerId (not used from here — None is passed) + * 2. by Consumer.key == azp — a pre-registered consumer whose key is the OAuth2 client ID + * takes priority over auto-created ones + * 3. by the (azp, iss) pair — the auto-created consumer for this client+issuer + * No match => a new consumer is auto-created for the (azp, iss) pair. Granularity is one + * consumer per OAuth client per issuer, NOT per user: the sub claim is stored on the + * consumer but is not part of the lookup key. The consumer name comes from the token's + * "name" claim (the first user who logged in with this client) falling back to description. * @param jwtToken Google's response example: * { * "access_token": "ya29.GluUBg5DflrJciFikW5hqeKEp9r1whWnU5x2JXCm9rKkRMs2WseXX8O5UugFMDsIKuKCZlE7tTm1fMII_YYpvcMX6quyR5DXNHH8Lbx5TrZN__fA92kszHJEVqPc", diff --git a/obp-api/src/test/scala/code/api/OAuth2ConsumerResolutionTest.scala b/obp-api/src/test/scala/code/api/OAuth2ConsumerResolutionTest.scala new file mode 100644 index 0000000000..24631ca260 --- /dev/null +++ b/obp-api/src/test/scala/code/api/OAuth2ConsumerResolutionTest.scala @@ -0,0 +1,141 @@ +package code.api + +import code.api.util.APIUtil +import code.consumer.Consumers +import code.model.Consumer +import code.setup.ServerSetup +import com.nimbusds.jose.crypto.MACSigner +import com.nimbusds.jose.{JWSAlgorithm, JWSHeader} +import com.nimbusds.jwt.{JWTClaimsSet, SignedJWT} +import net.liftweb.common.Empty +import net.liftweb.mapper.By + +import java.net.URI + +/** + * Executable specification of how OAuth2/OIDC id_tokens are resolved to Consumers + * (OAuth2Login.OAuth2Util.getOrCreateConsumer -> MappedConsumersProvider.getOrCreateConsumer). + * + * The contract under test (see OAUTH2_IDENTITY_PROVIDERS.md, "Google client ID policy"): + * - granularity is one Consumer per (azp, iss) — per OAuth client per issuer, NOT per user; + * the sub claim is stored on the Consumer but is not part of the lookup key + * - a pre-registered Consumer whose key equals the OAuth2 client ID takes priority over + * auto-creation, and displaces a stale auto-created Consumer holding the same (azp, iss) + * - auto-created Consumers are named from the token's name claim (the first user who logged + * in with that client), falling back to the description; their consumerId derives from azp + */ +class OAuth2ConsumerResolutionTest extends ServerSetup { + + private val googleIssuer = "https://accounts.google.com" + + private object oidcProvider extends OAuth2Login.OAuth2Util { + override def wellKnownOpenidConfiguration: URI = new URI("https://accounts.google.com/.well-known/openid-configuration") + } + + // getOrCreateConsumer only parses claims (no signature verification), so an + // HMAC-signed token is enough to exercise it — same trick as OAuth2AudienceValidationTest. + private def idToken(azp: String, iss: String, sub: String, name: Option[String] = None): String = { + val builder = new JWTClaimsSet.Builder().issuer(iss).subject(sub).audience(azp).claim("azp", azp) + name.foreach(builder.claim("name", _)) + val jwt = new SignedJWT(new JWSHeader(JWSAlgorithm.HS256), builder.build()) + jwt.sign(new MACSigner("0123456789abcdef0123456789abcdef")) + jwt.serialize() + } + + private def freshClientId() = s"${APIUtil.generateUUID().takeWhile(_ != '-')}-test.apps.googleusercontent.com" + + private def resolve(token: String, description: String = "OpenID Connect"): Consumer = + oidcProvider.getOrCreateConsumer(token, Empty, Some(description)) + .openOrThrowException("getOrCreateConsumer must return a consumer") + + feature("consumer resolution is per (azp, iss) — one Consumer per OAuth client per issuer") { + + scenario("two different users of the same client resolve to the same consumer") { + val clientId = freshClientId() + When("two users with different sub claims log in with the same client and issuer") + val first = resolve(idToken(clientId, googleIssuer, sub = "user-one", name = Some(s"Alice ${APIUtil.generateUUID()}"))) + val second = resolve(idToken(clientId, googleIssuer, sub = "user-two", name = Some(s"Bob ${APIUtil.generateUUID()}"))) + Then("both resolve to the same consumer and no duplicate is created") + second.consumerId.get should equal(first.consumerId.get) + Consumer.findAll(By(Consumer.azp, clientId)).size should equal(1) + And("the sub claim is stored from the first login but does not key the lookup") + second.sub.get should equal("user-one") + } + + scenario("the same client ID under a different issuer resolves to a different consumer") { + val clientId = freshClientId() + When("the same client ID is presented by two different issuers") + val googleConsumer = resolve(idToken(clientId, googleIssuer, sub = "user-one")) + val otherConsumer = resolve(idToken(clientId, "https://keycloak.example.com/realms/obp", sub = "user-two")) + Then("each issuer gets its own consumer for that client ID") + otherConsumer.consumerId.get should not equal googleConsumer.consumerId.get + Consumer.findAll(By(Consumer.azp, clientId)).size should equal(2) + } + } + + feature("auto-created consumer metadata") { + + scenario("the consumer is named from the token's name claim, falling back to the description") { + val namedUser = s"Alice ${APIUtil.generateUUID()}" + When("the token carries a name claim") + val named = resolve(idToken(freshClientId(), googleIssuer, sub = "user-one", name = Some(namedUser))) + Then("the consumer is named after the first user who logged in with that client") + named.name.get should equal(namedUser) + When("the token carries no name claim") + val unnamed = resolve(idToken(freshClientId(), googleIssuer, sub = "user-one")) + Then("the consumer name falls back to the description") + unnamed.name.get should startWith("OpenID Connect") + } + + scenario("the consumerId is derived from the client ID") { + Given("a google-style (non-UUID) client ID") + val clientId = freshClientId() + resolve(idToken(clientId, googleIssuer, sub = "user-one")).consumerId.get should startWith(s"${clientId}_") + Given("a UUID client ID") + val uuidClientId = APIUtil.generateUUID() + resolve(idToken(uuidClientId, googleIssuer, sub = "user-one")).consumerId.get should equal(uuidClientId) + } + } + + feature("a pre-registered consumer whose key is the OAuth2 client ID takes priority") { + + scenario("the token resolves to the pre-registered consumer instead of auto-creating one") { + val clientId = freshClientId() + Given("an operator pre-registered a consumer with key = the Google client ID") + val registered = Consumers.consumers.vend.createConsumer( + key = Some(clientId), secret = Some(APIUtil.generateUUID()), isActive = Some(true), + name = Some(s"API Explorer ${APIUtil.generateUUID()}"), appType = None, + description = Some("pre-registered"), developerEmail = Some("operator@example.com"), redirectURL = None, + createdByUserId = None, clientCertificate = None, company = None, logoURL = None + ).openOrThrowException("test consumer must be created") + When("a token minted for that client ID arrives") + val resolved = resolve(idToken(clientId, googleIssuer, sub = "user-one")) + Then("the pre-registered consumer is used and its azp/iss are populated") + resolved.consumerId.get should equal(registered.consumerId.get) + resolved.azp.get should equal(clientId) + resolved.iss.get should equal(googleIssuer) + Consumer.findAll(By(Consumer.azp, clientId)).size should equal(1) + } + + scenario("a stale auto-created consumer is displaced by the pre-registered one") { + val clientId = freshClientId() + Given("a consumer was auto-created before the operator registered the client ID") + val stale = resolve(idToken(clientId, googleIssuer, sub = "user-one")) + val registered = Consumers.consumers.vend.createConsumer( + key = Some(clientId), secret = Some(APIUtil.generateUUID()), isActive = Some(true), + name = Some(s"API Manager ${APIUtil.generateUUID()}"), appType = None, + description = Some("pre-registered"), developerEmail = Some("operator@example.com"), redirectURL = None, + createdByUserId = None, clientCertificate = None, company = None, logoURL = None + ).openOrThrowException("test consumer must be created") + When("the next token for that client ID arrives") + val resolved = resolve(idToken(clientId, googleIssuer, sub = "user-two")) + Then("it resolves to the pre-registered consumer, not the stale auto-created one") + resolved.consumerId.get should equal(registered.consumerId.get) + And("the stale consumer no longer holds the (azp, iss) pair") + val staleReloaded = Consumer.find(By(Consumer.consumerId, stale.consumerId.get)) + .openOrThrowException("stale consumer must still exist") + staleReloaded.azp.get should not equal clientId + Consumer.findAll(By(Consumer.azp, clientId)).size should equal(1) + } + } +}