diff --git a/api/strategies/openidStrategy.js b/api/strategies/openidStrategy.js index 079bed9e10..26143b226a 100644 --- a/api/strategies/openidStrategy.js +++ b/api/strategies/openidStrategy.js @@ -357,16 +357,18 @@ async function setupOpenId() { }; const appConfig = await getAppConfig(); - if (!isEmailDomainAllowed(userinfo.email, appConfig?.registration?.allowedDomains)) { + /** Azure AD sometimes doesn't return email, use preferred_username as fallback */ + const email = userinfo.email || userinfo.preferred_username || userinfo.upn; + if (!isEmailDomainAllowed(email, appConfig?.registration?.allowedDomains)) { logger.error( - `[OpenID Strategy] Authentication blocked - email domain not allowed [Email: ${userinfo.email}]`, + `[OpenID Strategy] Authentication blocked - email domain not allowed [Email: ${email}]`, ); return done(null, false, { message: 'Email domain not allowed' }); } const result = await findOpenIDUser({ findUser, - email: claims.email, + email: email, openidId: claims.sub, idOnTheSource: claims.oid, strategyName: 'openidStrategy', @@ -433,7 +435,7 @@ async function setupOpenId() { provider: 'openid', openidId: userinfo.sub, username, - email: userinfo.email || '', + email: email || '', emailVerified: userinfo.email_verified || false, name: fullName, idOnTheSource: userinfo.oid, @@ -447,8 +449,8 @@ async function setupOpenId() { user.username = username; user.name = fullName; user.idOnTheSource = userinfo.oid; - if (userinfo.email && userinfo.email !== user.email) { - user.email = userinfo.email; + if (email && email !== user.email) { + user.email = email; user.emailVerified = userinfo.email_verified || false; } } diff --git a/packages/api/src/auth/domain.spec.ts b/packages/api/src/auth/domain.spec.ts index c84d32cbcd..4f6c25ec51 100644 --- a/packages/api/src/auth/domain.spec.ts +++ b/packages/api/src/auth/domain.spec.ts @@ -6,15 +6,27 @@ describe('isEmailDomainAllowed', () => { jest.clearAllMocks(); }); - it('should return false if email is falsy', async () => { + it('should return true if email is falsy and no domain restrictions exist', async () => { const email = ''; const result = isEmailDomainAllowed(email); + expect(result).toBe(true); + }); + + it('should return true if domain is not present in the email and no domain restrictions exist', async () => { + const email = 'test'; + const result = isEmailDomainAllowed(email); + expect(result).toBe(true); + }); + + it('should return false if email is falsy and domain restrictions exist', async () => { + const email = ''; + const result = isEmailDomainAllowed(email, ['domain1.com']); expect(result).toBe(false); }); - it('should return false if domain is not present in the email', async () => { + it('should return false if domain is not present in the email and domain restrictions exist', async () => { const email = 'test'; - const result = isEmailDomainAllowed(email); + const result = isEmailDomainAllowed(email, ['domain1.com']); expect(result).toBe(false); }); diff --git a/packages/api/src/auth/domain.ts b/packages/api/src/auth/domain.ts index 06710ac6d7..00bcf91787 100644 --- a/packages/api/src/auth/domain.ts +++ b/packages/api/src/auth/domain.ts @@ -3,6 +3,12 @@ * @param allowedDomains */ export function isEmailDomainAllowed(email: string, allowedDomains?: string[] | null): boolean { + /** If no domain restrictions are configured, allow all */ + if (!allowedDomains || !Array.isArray(allowedDomains) || !allowedDomains.length) { + return true; + } + + /** If restrictions exist, validate email format */ if (!email) { return false; } @@ -13,12 +19,6 @@ export function isEmailDomainAllowed(email: string, allowedDomains?: string[] | return false; } - if (!allowedDomains) { - return true; - } else if (!Array.isArray(allowedDomains) || !allowedDomains.length) { - return true; - } - return allowedDomains.some((allowedDomain) => allowedDomain?.toLowerCase() === domain); }