From dfd1b6f391c8b117ceb522371bbe099d95db5171 Mon Sep 17 00:00:00 2001 From: aymericcousaert Date: Wed, 25 Mar 2026 17:00:33 +0100 Subject: [PATCH 1/6] fix: oidc multi-tenant cache collision and site applications fallback - 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 --- api/src/auth/router.ts | 4 +-- api/src/oauth/oidc.ts | 11 ++++--- test-it/external-apps-authorization.ts | 44 ++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/api/src/auth/router.ts b/api/src/auth/router.ts index f638a4ba..cdad5393 100644 --- a/api/src/auth/router.ts +++ b/api/src/auth/router.ts @@ -868,7 +868,7 @@ router.get('/apps/authorize', async (req, res) => { const site = await reqSite(req) let client = (site?.applications || []).find(c => c.id === clientId) - if (!client && !site) { + if (!client && (!site || !site.applications?.length)) { client = (config.applications || []).find(c => c.id === clientId) } if (!client) return res.status(400).send('Unknown client_id') @@ -896,7 +896,7 @@ router.post('/apps/authorize', async (req, res) => { const site = await reqSite(req) let client = (site?.applications || []).find(c => c.id === clientId) - if (!client && !site) { + if (!client && (!site || !site.applications?.length)) { client = (config.applications || []).find(c => c.id === clientId) } if (!client) return res.status(400).send('Unknown client_id') diff --git a/api/src/oauth/oidc.ts b/api/src/oauth/oidc.ts index b85db20a..2ee53faa 100644 --- a/api/src/oauth/oidc.ts +++ b/api/src/oauth/oidc.ts @@ -11,13 +11,16 @@ const slug = _slug.default const debug = Debug('oauth') export const getOidcProviderId = (url: string) => { - let host = url + let base = url try { - host = new URL(url).host + const u = new URL(url) + let path = u.pathname.replace(/\/\.well-known\/openid-configuration\/?$/, '') + path = path.replace(/\/$/, '') + base = u.host + path } catch (err) { - console.warn('invalide oauth provider url', url) + console.warn('invalid oauth provider url', url) } - return slug(host, { lower: true, strict: true }) + return slug(base, { lower: true, strict: true }) } export async function completeOidcProvider (p: OpenIDConnect): Promise { diff --git a/test-it/external-apps-authorization.ts b/test-it/external-apps-authorization.ts index 5db4afa0..3668bb7b 100644 --- a/test-it/external-apps-authorization.ts +++ b/test-it/external-apps-authorization.ts @@ -147,6 +147,50 @@ describe('External Apps Authorization Flow', () => { assert.match(authorizeRes.data, /Not authenticated/) }) + it('should fall back to global applications when site has no applications', async () => { + const config = (await import('../api/src/config.ts')).default + const { ax: adminAx } = await createUser('admin@test.com', true) + const org = (await adminAx.post('/api/organizations', { name: 'Site Org Fallback' })).data + const port = new URL(adminAx.defaults.baseURL || '').port + const siteHost = `127.0.0.1:${port}` + + const originalApps = config.applications + config.applications = [{ + id: 'global-app', + name: 'Global App', + redirectUris: ['native-app://global-callback'] + }] + + try { + const anonymousAx = await axios() + await anonymousAx.post('/api/sites', + { + _id: 'test-site-fallback', + owner: { type: 'organization', id: org.id, name: org.name }, + host: siteHost, + theme: { primaryColor: '#000000' } + }, + { params: { key: config.secretKeys.sites } } + ) + + await adminAx.patch('/api/sites/test-site-fallback', { authMode: 'onlyLocal' }); + (await import('../api/src/sites/service.ts')).getSiteByHost.clear() + + const siteAx = await axios({ baseURL: `http://${siteHost}/simple-directory` }) + + // Should find global-app via fallback + const authorizeRes = await siteAx.get( + '/api/auth/apps/authorize?client_id=global-app&redirect_uri=native-app://global-callback', + { maxRedirects: 0, validateStatus: (status) => status === 302 } + ) + const loginRedirectUrl = new URL(authorizeRes.headers.location) + assert.equal(loginRedirectUrl.searchParams.get('client_id'), 'global-app') + assert.equal(loginRedirectUrl.searchParams.get('client_name'), 'Global App') + } finally { + config.applications = originalApps + } + }) + it('should reject invalid client_id', async () => { const config = (await import('../api/src/config.ts')).default const { ax: adminAx } = await createUser('admin@test.com', true) From fe0d877c6ee216ea08ea9eb23775457c11f79877 Mon Sep 17 00:00:00 2001 From: aymericcousaert Date: Wed, 25 Mar 2026 17:12:35 +0100 Subject: [PATCH 2/6] fix: allow editing application client id in site form --- api/types/site/schema.js | 1 - 1 file changed, 1 deletion(-) diff --git a/api/types/site/schema.js b/api/types/site/schema.js index b7d08ff5..32ac169e 100644 --- a/api/types/site/schema.js +++ b/api/types/site/schema.js @@ -330,7 +330,6 @@ export default { id: { type: 'string', title: 'Client ID', - readOnly: true, layout: { cols: 12 } }, name: { From 0ed77de322574775bbd3f032e4154022fb5b07fb Mon Sep 17 00:00:00 2001 From: aymericcousaert Date: Wed, 25 Mar 2026 17:17:28 +0100 Subject: [PATCH 3/6] fix: merge global and site applications instead of fallback Site applications take priority on duplicate IDs, global applications are always available as a base. --- api/src/auth/router.ts | 4 +- test-it/external-apps-authorization.ts | 66 ++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/api/src/auth/router.ts b/api/src/auth/router.ts index cdad5393..fbba85a1 100644 --- a/api/src/auth/router.ts +++ b/api/src/auth/router.ts @@ -868,7 +868,7 @@ router.get('/apps/authorize', async (req, res) => { const site = await reqSite(req) let client = (site?.applications || []).find(c => c.id === clientId) - if (!client && (!site || !site.applications?.length)) { + if (!client) { client = (config.applications || []).find(c => c.id === clientId) } if (!client) return res.status(400).send('Unknown client_id') @@ -896,7 +896,7 @@ router.post('/apps/authorize', async (req, res) => { const site = await reqSite(req) let client = (site?.applications || []).find(c => c.id === clientId) - if (!client && (!site || !site.applications?.length)) { + if (!client) { client = (config.applications || []).find(c => c.id === clientId) } if (!client) return res.status(400).send('Unknown client_id') diff --git a/test-it/external-apps-authorization.ts b/test-it/external-apps-authorization.ts index 3668bb7b..e95a3292 100644 --- a/test-it/external-apps-authorization.ts +++ b/test-it/external-apps-authorization.ts @@ -191,6 +191,72 @@ describe('External Apps Authorization Flow', () => { } }) + it('should merge global and site applications, site takes priority', async () => { + const config = (await import('../api/src/config.ts')).default + const { ax: adminAx } = await createUser('admin@test.com', true) + const org = (await adminAx.post('/api/organizations', { name: 'Site Org Merge' })).data + const port = new URL(adminAx.defaults.baseURL || '').port + const siteHost = `127.0.0.1:${port}` + + const originalApps = config.applications + config.applications = [ + { id: 'global-only', name: 'Global Only', redirectUris: ['native-app://global-only'] }, + { id: 'shared-id', name: 'Global Shared', redirectUris: ['native-app://global-shared'] } + ] + + try { + const anonymousAx = await axios() + await anonymousAx.post('/api/sites', + { + _id: 'test-site-merge', + owner: { type: 'organization', id: org.id, name: org.name }, + host: siteHost, + theme: { primaryColor: '#000000' }, + applications: [ + { id: 'site-only', name: 'Site Only', redirectUris: ['native-app://site-only'] }, + { id: 'shared-id', name: 'Site Shared', redirectUris: ['native-app://site-shared'] } + ] + }, + { params: { key: config.secretKeys.sites } } + ) + + await adminAx.patch('/api/sites/test-site-merge', { authMode: 'onlyLocal' }); + (await import('../api/src/sites/service.ts')).getSiteByHost.clear() + + const siteAx = await axios({ baseURL: `http://${siteHost}/simple-directory` }) + + // Site-only app should work + const res1 = await siteAx.get( + '/api/auth/apps/authorize?client_id=site-only&redirect_uri=native-app://site-only', + { maxRedirects: 0, validateStatus: (status) => status === 302 } + ) + assert.equal(new URL(res1.headers.location).searchParams.get('client_name'), 'Site Only') + + // Global-only app should work via merge + const res2 = await siteAx.get( + '/api/auth/apps/authorize?client_id=global-only&redirect_uri=native-app://global-only', + { maxRedirects: 0, validateStatus: (status) => status === 302 } + ) + assert.equal(new URL(res2.headers.location).searchParams.get('client_name'), 'Global Only') + + // Shared ID should use site version (site overrides global) + const res3 = await siteAx.get( + '/api/auth/apps/authorize?client_id=shared-id&redirect_uri=native-app://site-shared', + { maxRedirects: 0, validateStatus: (status) => status === 302 } + ) + assert.equal(new URL(res3.headers.location).searchParams.get('client_name'), 'Site Shared') + + // Global redirect URI for shared-id should be rejected (site version takes priority) + const res4 = await siteAx.get( + '/api/auth/apps/authorize?client_id=shared-id&redirect_uri=native-app://global-shared', + { maxRedirects: 0, validateStatus: (status) => status === 400 } + ) + assert.match(res4.data, /Invalid redirect_uri/) + } finally { + config.applications = originalApps + } + }) + it('should reject invalid client_id', async () => { const config = (await import('../api/src/config.ts')).default const { ax: adminAx } = await createUser('admin@test.com', true) From ea6825042740871200f7bea5368d557607aaf836 Mon Sep 17 00:00:00 2001 From: aymericcousaert Date: Wed, 25 Mar 2026 18:01:10 +0100 Subject: [PATCH 4/6] fix: add backward compat for coreIdProvider with old OIDC provider ID 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. --- api/src/auth/router.ts | 6 +++--- api/src/oauth/oidc.ts | 13 +++++++++++++ api/src/oauth/service.ts | 3 ++- api/src/services.ts | 2 +- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/api/src/auth/router.ts b/api/src/auth/router.ts index fbba85a1..6b7d30db 100644 --- a/api/src/auth/router.ts +++ b/api/src/auth/router.ts @@ -5,7 +5,7 @@ import { reqUser, reqIp, reqSiteUrl, reqUserAuthenticated, session, httpError, r import bodyParser from 'body-parser' import Cookies from 'cookies' import Debug from 'debug' -import { sendMailI18n, postUserIdentityWebhook, getOidcProviderId, oauthGlobalProviders, initOidcProvider, getOAuthProviderById, getOAuthProviderByState, reqSite, getSiteByUrl, check2FASession, is2FAValid, cookie2FAName, getTokenPayload, prepareCallbackUrl, signToken, decodeToken, setSessionCookies, getDefaultUserOrg, logout, keepalive, logoutOAuthToken, readOAuthToken, writeOAuthToken, authProviderMemberInfo, patchCoreAuthUser, saml2ServiceProvider, initServerSession, getSamlProviderById, authProviderLoginCallback, getDefaultLoginRedirect } from '#services' +import { sendMailI18n, postUserIdentityWebhook, getOidcProviderId, getOidcProviderIdCompat, oauthGlobalProviders, initOidcProvider, getOAuthProviderById, getOAuthProviderByState, reqSite, getSiteByUrl, check2FASession, is2FAValid, cookie2FAName, getTokenPayload, prepareCallbackUrl, signToken, decodeToken, setSessionCookies, getDefaultUserOrg, logout, keepalive, logoutOAuthToken, readOAuthToken, writeOAuthToken, authProviderMemberInfo, patchCoreAuthUser, saml2ServiceProvider, initServerSession, getSamlProviderById, authProviderLoginCallback, getDefaultLoginRedirect } from '#services' import type { SdStorage } from '../storages/interface.ts' import type { ActionPayload, ServerSession, User } from '#types' import eventsLog, { type EventLogContext } from '@data-fair/lib-express/events-log.js' @@ -468,13 +468,13 @@ router.post('/keepalive', async (req, res, next) => { let provider const site = await reqSite(req) if (site?.authMode === 'onlyBackOffice' || !site?.authMode) { - provider = oauthGlobalProviders().find(p => p.id === coreIdProvider.id) + provider = oauthGlobalProviders().find(p => p.id === coreIdProvider.id || p.compatId === coreIdProvider.id) } else { let authSite = site if (site.authMode === 'onlyOtherSite' && site.authOnlyOtherSite) { authSite = await getSiteByUrl('https://' + site.authOnlyOtherSite) ?? site } - const providerInfo = authSite.authProviders?.find(p => p.type === 'oidc' && getOidcProviderId(p.discovery) === coreIdProvider.id) as OpenIDConnect | undefined + const providerInfo = authSite.authProviders?.find(p => p.type === 'oidc' && (getOidcProviderId(p.discovery) === coreIdProvider.id || getOidcProviderIdCompat(p.discovery) === coreIdProvider.id)) as OpenIDConnect | undefined provider = providerInfo && await initOidcProvider(providerInfo, `https://${authSite.host}${authSite.path ?? ''}/simple-directory`) } if (!provider) { diff --git a/api/src/oauth/oidc.ts b/api/src/oauth/oidc.ts index 2ee53faa..9ee5a171 100644 --- a/api/src/oauth/oidc.ts +++ b/api/src/oauth/oidc.ts @@ -23,6 +23,17 @@ export const getOidcProviderId = (url: string) => { return slug(base, { lower: true, strict: true }) } +// old ID format (hostname only) for backward compatibility with stored coreIdProvider references +export const getOidcProviderIdCompat = (url: string) => { + let host = url + try { + host = new URL(url).host + } catch (err) { + // ignore + } + return slug(host, { lower: true, strict: true }) +} + export async function completeOidcProvider (p: OpenIDConnect): Promise { const id = getOidcProviderId(p.discovery) let discoveryContent = (await mongo.oidcDiscovery.findOne({ _id: id }))?.content @@ -96,9 +107,11 @@ export async function completeOidcProvider (p: OpenIDConnect): Promise & { id: string, + compatId?: string, type: 'oauth' | 'oidc', oidc?: boolean, title?: string, diff --git a/api/src/services.ts b/api/src/services.ts index 3e46f9ab..b32dd39d 100644 --- a/api/src/services.ts +++ b/api/src/services.ts @@ -4,7 +4,7 @@ export * from './avatars/service.ts' export * from './invitations/service.ts' export * from './limits/service.ts' export * from './mails/service.ts' -export { initOidcProvider, oauthGlobalProviders, getOidcProviderId, getOAuthProviderById, getOAuthProviderByState } from './oauth/service.ts' +export { initOidcProvider, oauthGlobalProviders, getOidcProviderId, getOidcProviderIdCompat, getOAuthProviderById, getOAuthProviderByState } from './oauth/service.ts' export * from './oauth-tokens/service.ts' export { saml2ServiceProvider, saml2GlobalProviders, getSamlProviderId, getSamlConfigId, getSamlProviderById } from './saml2/service.ts' export { reqSite, getSiteByUrl, getRedirectSite, getSiteBaseUrl, getSiteByHost } from './sites/service.ts' From e04e58ae34e2663f0a6e72dab7ff716d0fb43faf Mon Sep 17 00:00:00 2001 From: aymericcousaert Date: Mon, 30 Mar 2026 10:41:05 +0200 Subject: [PATCH 5/6] fix: replace provider ID change with discovery cache key fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- api/i18n/messages.ts | 2 +- api/src/auth/router.ts | 6 +++--- api/src/oauth/oidc.ts | 25 ++++++------------------- api/src/oauth/service.ts | 3 +-- api/src/services.ts | 2 +- ui/src/pages/index.vue | 2 +- ui/src/pages/invitation.vue | 4 ++-- 7 files changed, 15 insertions(+), 29 deletions(-) diff --git a/api/i18n/messages.ts b/api/i18n/messages.ts index 5de04626..12d53708 100644 --- a/api/i18n/messages.ts +++ b/api/i18n/messages.ts @@ -6,7 +6,7 @@ const flatOpts = { delimiter: '_' } // {fr: {msg1: 'libellé 1'}, en: {msg1: 'label 1'}} const _messages: any = {} for (const l of config.i18n.locales) { - _messages[l] = (await import ('./' + l + '.js')).default + _messages[l] = (await import('./' + l + '.js')).default } export const flatMessages = flatten(_messages, flatOpts) as Record diff --git a/api/src/auth/router.ts b/api/src/auth/router.ts index 6b7d30db..fbba85a1 100644 --- a/api/src/auth/router.ts +++ b/api/src/auth/router.ts @@ -5,7 +5,7 @@ import { reqUser, reqIp, reqSiteUrl, reqUserAuthenticated, session, httpError, r import bodyParser from 'body-parser' import Cookies from 'cookies' import Debug from 'debug' -import { sendMailI18n, postUserIdentityWebhook, getOidcProviderId, getOidcProviderIdCompat, oauthGlobalProviders, initOidcProvider, getOAuthProviderById, getOAuthProviderByState, reqSite, getSiteByUrl, check2FASession, is2FAValid, cookie2FAName, getTokenPayload, prepareCallbackUrl, signToken, decodeToken, setSessionCookies, getDefaultUserOrg, logout, keepalive, logoutOAuthToken, readOAuthToken, writeOAuthToken, authProviderMemberInfo, patchCoreAuthUser, saml2ServiceProvider, initServerSession, getSamlProviderById, authProviderLoginCallback, getDefaultLoginRedirect } from '#services' +import { sendMailI18n, postUserIdentityWebhook, getOidcProviderId, oauthGlobalProviders, initOidcProvider, getOAuthProviderById, getOAuthProviderByState, reqSite, getSiteByUrl, check2FASession, is2FAValid, cookie2FAName, getTokenPayload, prepareCallbackUrl, signToken, decodeToken, setSessionCookies, getDefaultUserOrg, logout, keepalive, logoutOAuthToken, readOAuthToken, writeOAuthToken, authProviderMemberInfo, patchCoreAuthUser, saml2ServiceProvider, initServerSession, getSamlProviderById, authProviderLoginCallback, getDefaultLoginRedirect } from '#services' import type { SdStorage } from '../storages/interface.ts' import type { ActionPayload, ServerSession, User } from '#types' import eventsLog, { type EventLogContext } from '@data-fair/lib-express/events-log.js' @@ -468,13 +468,13 @@ router.post('/keepalive', async (req, res, next) => { let provider const site = await reqSite(req) if (site?.authMode === 'onlyBackOffice' || !site?.authMode) { - provider = oauthGlobalProviders().find(p => p.id === coreIdProvider.id || p.compatId === coreIdProvider.id) + provider = oauthGlobalProviders().find(p => p.id === coreIdProvider.id) } else { let authSite = site if (site.authMode === 'onlyOtherSite' && site.authOnlyOtherSite) { authSite = await getSiteByUrl('https://' + site.authOnlyOtherSite) ?? site } - const providerInfo = authSite.authProviders?.find(p => p.type === 'oidc' && (getOidcProviderId(p.discovery) === coreIdProvider.id || getOidcProviderIdCompat(p.discovery) === coreIdProvider.id)) as OpenIDConnect | undefined + const providerInfo = authSite.authProviders?.find(p => p.type === 'oidc' && getOidcProviderId(p.discovery) === coreIdProvider.id) as OpenIDConnect | undefined provider = providerInfo && await initOidcProvider(providerInfo, `https://${authSite.host}${authSite.path ?? ''}/simple-directory`) } if (!provider) { diff --git a/api/src/oauth/oidc.ts b/api/src/oauth/oidc.ts index 9ee5a171..948144be 100644 --- a/api/src/oauth/oidc.ts +++ b/api/src/oauth/oidc.ts @@ -11,38 +11,27 @@ const slug = _slug.default const debug = Debug('oauth') export const getOidcProviderId = (url: string) => { - let base = url - try { - const u = new URL(url) - let path = u.pathname.replace(/\/\.well-known\/openid-configuration\/?$/, '') - path = path.replace(/\/$/, '') - base = u.host + path - } catch (err) { - console.warn('invalid oauth provider url', url) - } - return slug(base, { lower: true, strict: true }) -} - -// old ID format (hostname only) for backward compatibility with stored coreIdProvider references -export const getOidcProviderIdCompat = (url: string) => { let host = url try { host = new URL(url).host } catch (err) { - // ignore + console.warn('invalide oauth provider url', url) } return slug(host, { lower: true, strict: true }) } export async function completeOidcProvider (p: OpenIDConnect): Promise { const id = getOidcProviderId(p.discovery) - let discoveryContent = (await mongo.oidcDiscovery.findOne({ _id: id }))?.content + // use full discovery URL as cache key to avoid collision between providers + // on the same host but different paths (e.g. Azure AD multi-tenant) + const cacheKey = slug(p.discovery, { lower: true, strict: true }) + let discoveryContent = (await mongo.oidcDiscovery.findOne({ _id: cacheKey }))?.content if (discoveryContent) { debug(`Read pre-fetched OIDC discovery info from db for provider ${id}`, discoveryContent) } else { discoveryContent = (await axios.get(p.discovery)).data debug(`Fetched OIDC discovery info from ${p.discovery}`, discoveryContent) - await mongo.oidcDiscovery.insertOne({ _id: id, content: discoveryContent }) + await mongo.oidcDiscovery.insertOne({ _id: cacheKey, content: discoveryContent }) } const tokenURL = new URL(discoveryContent.token_endpoint) const authURL = new URL(discoveryContent.authorization_endpoint) @@ -107,11 +96,9 @@ export async function completeOidcProvider (p: OpenIDConnect): Promise & { id: string, - compatId?: string, type: 'oauth' | 'oidc', oidc?: boolean, title?: string, diff --git a/api/src/services.ts b/api/src/services.ts index b32dd39d..3e46f9ab 100644 --- a/api/src/services.ts +++ b/api/src/services.ts @@ -4,7 +4,7 @@ export * from './avatars/service.ts' export * from './invitations/service.ts' export * from './limits/service.ts' export * from './mails/service.ts' -export { initOidcProvider, oauthGlobalProviders, getOidcProviderId, getOidcProviderIdCompat, getOAuthProviderById, getOAuthProviderByState } from './oauth/service.ts' +export { initOidcProvider, oauthGlobalProviders, getOidcProviderId, getOAuthProviderById, getOAuthProviderByState } from './oauth/service.ts' export * from './oauth-tokens/service.ts' export { saml2ServiceProvider, saml2GlobalProviders, getSamlProviderId, getSamlConfigId, getSamlProviderById } from './saml2/service.ts' export { reqSite, getSiteByUrl, getRedirectSite, getSiteBaseUrl, getSiteByHost } from './sites/service.ts' diff --git a/ui/src/pages/index.vue b/ui/src/pages/index.vue index d28be40e..8a3e07f6 100644 --- a/ui/src/pages/index.vue +++ b/ui/src/pages/index.vue @@ -22,7 +22,7 @@ - {{ $t('root.description') }} + {{ $t('root.description') }} diff --git a/ui/src/pages/invitation.vue b/ui/src/pages/invitation.vue index e82d096d..36dfc387 100644 --- a/ui/src/pages/invitation.vue +++ b/ui/src/pages/invitation.vue @@ -14,12 +14,12 @@ From 7847e4078ffb617ce18cfa1b908a7d44e31cd1e7 Mon Sep 17 00:00:00 2001 From: aymericcousaert Date: Mon, 30 Mar 2026 11:20:16 +0200 Subject: [PATCH 6/6] fix: scope oauth tokens by site to prevent 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, 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 --- api/src/auth/router.ts | 7 ++++--- api/src/mongo.ts | 2 +- api/src/oauth-tokens/service.ts | 30 ++++++++++++++++++++++++------ api/src/users/worker.ts | 4 ++-- api/types/index.ts | 1 + 5 files changed, 32 insertions(+), 12 deletions(-) diff --git a/api/src/auth/router.ts b/api/src/auth/router.ts index fbba85a1..77a25496 100644 --- a/api/src/auth/router.ts +++ b/api/src/auth/router.ts @@ -481,7 +481,7 @@ router.post('/keepalive', async (req, res, next) => { await logout(req, res) return res.status(401).send('Fournisseur d\'identité principal inconnu') } - const oauthToken = (await readOAuthToken(user, provider)) + const oauthToken = (await readOAuthToken(user, provider, site?._id)) if (!oauthToken) { await logout(req, res) @@ -501,7 +501,7 @@ router.post('/keepalive', async (req, res, next) => { const userInfo = await provider.userInfo(newToken.access_token, newToken.id_token) const memberInfos = await authProviderMemberInfo(await reqSite(req), provider, userInfo) user = await patchCoreAuthUser(provider, user, userInfo, memberInfos) - await writeOAuthToken(user, provider, newToken, offlineRefreshToken) + await writeOAuthToken(user, provider, newToken, offlineRefreshToken, undefined, site?._id) eventsLog.info('sd.auth.keepalive.oauth-refresh-ok', `a user refreshed their info from their core identity provider ${provider.id}`, { req }) } } catch (err: any) { @@ -719,7 +719,8 @@ const oauthCallback: RequestHandler = async (req, res, next) => { try { const [callbackUrl, user] = await authProviderLoginCallback(req, invitToken, authInfo, logContext, provider, redirect, org, dep, adminMode) if (provider.coreIdProvider) { - await writeOAuthToken(user, provider, token, offlineRefreshToken) + const callbackSite = await reqSite(req) + await writeOAuthToken(user, provider, token, offlineRefreshToken, undefined, callbackSite?._id) } res.redirect(callbackUrl) } catch (err : any) { diff --git a/api/src/mongo.ts b/api/src/mongo.ts index 9c6c5787..751e7d55 100644 --- a/api/src/mongo.ts +++ b/api/src/mongo.ts @@ -127,7 +127,7 @@ export class SdMongo { 'sites-owner': { 'owner.type': 1, 'owner.id': 1, 'owner.department': 1 } }, 'oauth-tokens': { - 'oauth-tokens-key': [{ 'user.id': 1, 'provider.id': 1 }, { unique: true }], + 'oauth-tokens-key': [{ 'user.id': 1, 'provider.id': 1, site: 1 }, { unique: true }], 'oauth-tokens-provider': { 'provider.id': 1 }, 'oauth-tokens-offline': { offlineRefreshToken: 1 }, 'oauth-tokens-sid': { 'token.session_state': 1 } diff --git a/api/src/oauth-tokens/service.ts b/api/src/oauth-tokens/service.ts index 3575decf..025d87c0 100644 --- a/api/src/oauth-tokens/service.ts +++ b/api/src/oauth-tokens/service.ts @@ -1,24 +1,41 @@ import type { User, OAuthToken } from '#types' import mongo from '#mongo' -export async function writeOAuthToken (user: User, provider: any, token: any, offlineRefreshToken: boolean, loggedOut?: Date) { +export async function writeOAuthToken (user: User, provider: any, token: any, offlineRefreshToken: boolean, loggedOut?: Date, site?: string | null) { + const siteValue = site ?? null const tokenInfo: OAuthToken = { user: { id: user.id, email: user.email, name: user.name }, provider: { id: provider.id, type: provider.type, title: provider.title }, + site: siteValue, token } if (offlineRefreshToken) tokenInfo.offlineRefreshToken = true if (loggedOut) tokenInfo.loggedOut = loggedOut await mongo.oauthTokens - .replaceOne({ 'user.id': user.id, 'provider.id': provider.id }, tokenInfo, { upsert: true }) + .replaceOne({ 'user.id': user.id, 'provider.id': provider.id, site: siteValue }, tokenInfo, { upsert: true }) + // lazy migration: clean up legacy token without site + if (siteValue !== null) { + await mongo.oauthTokens.deleteOne({ 'user.id': user.id, 'provider.id': provider.id, site: null }) + } } -export async function readOAuthToken (user: User, provider: any) { - return mongo.oauthTokens.findOne({ 'user.id': user.id, 'provider.id': provider.id }) +export async function readOAuthToken (user: User, provider: any, site?: string | null) { + const siteValue = site ?? null + const token = await mongo.oauthTokens.findOne({ 'user.id': user.id, 'provider.id': provider.id, site: siteValue }) + // backward compat: fall back to legacy token (no site) if site-specific token not found + if (!token && siteValue !== null) { + return mongo.oauthTokens.findOne({ 'user.id': user.id, 'provider.id': provider.id, site: null }) + } + return token } -export async function deleteOAuthToken (user: User, provider: any) { - await mongo.oauthTokens.deleteOne({ 'user.id': user.id, 'provider.id': provider.id }) +export async function deleteOAuthToken (user: User, provider: any, site?: string | null) { + const siteValue = site ?? null + await mongo.oauthTokens.deleteOne({ 'user.id': user.id, 'provider.id': provider.id, site: siteValue }) + // also clean up legacy token without site + if (siteValue !== null) { + await mongo.oauthTokens.deleteOne({ 'user.id': user.id, 'provider.id': provider.id, site: null }) + } } export async function readOAuthTokens () { @@ -28,6 +45,7 @@ export async function readOAuthTokens () { 'token.session_state': 1, offlineRefreshToken: 1, provider: 1, + site: 1, loggedOut: 1 }).toArray() return { diff --git a/api/src/users/worker.ts b/api/src/users/worker.ts index c8641f37..21e31df0 100644 --- a/api/src/users/worker.ts +++ b/api/src/users/worker.ts @@ -66,11 +66,11 @@ const task = async () => { const userInfo = await provider.userInfo(newToken.access_token, newToken.id_token) const memberInfos = await authProviderMemberInfo(undefined, provider, userInfo) await patchCoreAuthUser(provider, user, userInfo, memberInfos) - await writeOAuthToken(user, provider, newToken, offlineRefreshToken, token.loggedOut) + await writeOAuthToken(user, provider, newToken, offlineRefreshToken, token.loggedOut, token.site) eventsLog.info('sd.cleanup-cron.offline-token.refresh-ok', `a user refreshed their info from their core identity provider ${provider.id}`, { user }) } catch (err: any) { if (err?.data?.payload?.error === 'invalid_grant') { - await deleteOAuthToken(user, provider) + await deleteOAuthToken(user, provider, token.site) eventsLog.warn('sd.cleanup-cron.offline-token.delete', `deleted invalid offline token for user ${user.id} and provider ${provider.id}`, { user }) await planDeletion(user) } else { diff --git a/api/types/index.ts b/api/types/index.ts index 2f6debd9..778a8090 100644 --- a/api/types/index.ts +++ b/api/types/index.ts @@ -26,6 +26,7 @@ export type OAuthToken = { token: any, provider: { type: string, id: string, title: string }, user: { id: string, name: string, email: string }, + site?: string | null, offlineRefreshToken?: boolean, loggedOut?: Date }