fix: oidc multi-tenant discovery cache collision and site applications fallback#112
Open
aymericcousaert wants to merge 6 commits intomasterfrom
Open
fix: oidc multi-tenant discovery cache collision and site applications fallback#112aymericcousaert wants to merge 6 commits intomasterfrom
aymericcousaert wants to merge 6 commits intomasterfrom
Conversation
- use full discovery URL path (not just hostname) for OIDC provider ID, fixing cache collisions when multiple Azure AD tenants share login.microsoftonline.com - fall back to global APPLICATIONS config when a site has no applications defined, instead of only when there is no site at all
Site applications take priority on duplicate IDs, global applications are always available as a base.
… format Providers now expose a compatId (old hostname-only format) so that existing user sessions with stored coreIdProvider references still match after the ID format change.
Revert the getOidcProviderId format change (host+path) and its incomplete compatId backward-compat layer. Instead, fix the actual bug by decoupling the oidc-discovery cache key from the provider ID. The cache now uses the full discovery URL slug as key, so Azure AD multi-tenant providers on the same host get separate cache entries without changing any stored provider IDs — zero migration risk. Also fix pre-existing lint errors (func-call-spacing, deprecated Vuetify classes).
aymericcousaert
changed the title
When the same user logs in on two sites that share the same OIDC
provider hostname (e.g. same Azure AD tenant, different apps), their
oauth-tokens collided on the unique {user.id, provider.id} key.
Add site to the token storage key. Backward compatible:
- read falls back to site:null for legacy tokens without the field
- write with a site lazily migrates by deleting the legacy token
- index auto-migrated on startup (ensureIndex handles key conflict)
- all queries use explicit null (not undefined) due to ignoreUndefined
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
1. OIDC discovery cache collision (multi-tenant Azure AD)
The
oidc-discoverycache in MongoDB used the provider ID (hostname-only slug) as key. Multiple Azure AD tenants sharelogin.microsoftonline.com, so they got the same cache key — the first tenant's discovery content (authorization/token endpoints) was cached and reused for all others, redirecting users to the wrong tenant.Fix: use the full discovery URL slug as cache key, without changing the provider ID — zero migration risk.
2. OAuth token cross-site collision
When the same user logs in on two sites that share the same OIDC provider hostname (e.g. same Azure AD tenant with different client apps), their oauth-tokens collided on the unique
{user.id, provider.id}key — one overwrote the other, causing keepalive failures.Fix: add
siteto the token storage key ({user.id, provider.id, site}). Backward compatible:readOAuthTokenfalls back tosite: nullfor legacy tokens without the fieldwriteOAuthTokenwith a site lazily migrates by deleting the legacy tokenensureIndexhandles key conflict)null(notundefined) due to MongoDBignoreUndefined: true3. Applications merge
Global applications (from
APPLICATIONSenv var) are now always available as fallback. Site applications are checked first, then global. Previously, sites with any config (even without applications) could not access global ones, causing runner-gui OAuth to fail with "Unknown client_id" on secondary sites.4. Application client ID editable
Removed
readOnlyconstraint on the client ID field in site applications form.Test plan
drop then recreate index oauth-tokens/oauth-tokens-key