diff --git a/plugins/azure/server/auth/azure.ts b/plugins/azure/server/auth/azure.ts index 6b0deee13d..33d39543db 100644 --- a/plugins/azure/server/auth/azure.ts +++ b/plugins/azure/server/auth/azure.ts @@ -102,6 +102,18 @@ if (env.AZURE_CLIENT_ID && env.AZURE_CLIENT_SECRET) { const user = context.state?.auth?.user ?? (await getUserFromOAuthState(context)); + // Microsoft's email claim is mutable, only trust it when a verification + // claim confirms it — xms_edov for workforce tenants, or the standard + // email_verified claim in External ID / OIDC scenarios. + // https://learn.microsoft.com/en-us/entra/identity-platform/reference-claims-customization + const verificationClaims = [profile.xms_edov, profile.email_verified]; + const presentClaims = verificationClaims.filter( + (claim) => claim !== undefined + ); + const emailVerified = presentClaims.length + ? presentClaims.some((claim) => claim === true || claim === "true") + : undefined; + const domain = parseEmail(email).domain; const subdomain = slugifyDomain(domain); @@ -121,6 +133,7 @@ if (env.AZURE_CLIENT_ID && env.AZURE_CLIENT_SECRET) { user: { name: profile.name, email, + emailVerified, avatarUrl: profile.picture, }, authenticationProvider: { diff --git a/plugins/discord/server/auth/discord.ts b/plugins/discord/server/auth/discord.ts index 00d940243e..c2a9e73f87 100644 --- a/plugins/discord/server/auth/discord.ts +++ b/plugins/discord/server/auth/discord.ts @@ -201,6 +201,7 @@ if (env.DISCORD_CLIENT_ID && env.DISCORD_CLIENT_SECRET) { }, user: { email, + emailVerified: profile.verified, name: userName, language, avatarUrl: userAvatarUrl, diff --git a/plugins/google/server/auth/google.ts b/plugins/google/server/auth/google.ts index dbf241a8fa..26c197e3e6 100644 --- a/plugins/google/server/auth/google.ts +++ b/plugins/google/server/auth/google.ts @@ -127,6 +127,8 @@ if (env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET) { }, user: { email: profile.email, + // Google only returns confirmed workspace email addresses. + emailVerified: true, name: profile.displayName, language, avatarUrl, diff --git a/plugins/oidc/server/auth/oidcRouter.ts b/plugins/oidc/server/auth/oidcRouter.ts index 4d8a98ca11..131b5300eb 100644 --- a/plugins/oidc/server/auth/oidcRouter.ts +++ b/plugins/oidc/server/auth/oidcRouter.ts @@ -105,6 +105,7 @@ export function createOIDCRouter( return decoded as { email?: string; + email_verified?: boolean | string; preferred_username?: string; sub?: string; }; @@ -122,6 +123,15 @@ export function createOIDCRouter( ); } + // The email_verified claim is part of the OIDC standard claims. + // https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims + const emailVerifiedClaim = + profile.email_verified ?? token.email_verified; + const emailVerified = + emailVerifiedClaim === undefined + ? undefined + : emailVerifiedClaim === true || emailVerifiedClaim === "true"; + const team = await getTeamFromContext(context); const client = getClientFromOAuthState(context); const user = @@ -206,6 +216,7 @@ export function createOIDCRouter( user: { name, email, + emailVerified, avatarUrl, }, authenticationProvider: { diff --git a/plugins/slack/server/auth/slack.ts b/plugins/slack/server/auth/slack.ts index 6906702fe2..b33c6cf53f 100644 --- a/plugins/slack/server/auth/slack.ts +++ b/plugins/slack/server/auth/slack.ts @@ -110,6 +110,8 @@ if (env.SLACK_CLIENT_ID && env.SLACK_CLIENT_SECRET) { user: { name: profile.user.name, email: profile.user.email, + // Slack only returns confirmed workspace email addresses. + emailVerified: true, avatarUrl: profile.user.image_192, }, authenticationProvider: { diff --git a/server/commands/accountProvisioner.test.ts b/server/commands/accountProvisioner.test.ts index 4d1e33baed..9be87981d2 100644 --- a/server/commands/accountProvisioner.test.ts +++ b/server/commands/accountProvisioner.test.ts @@ -115,6 +115,7 @@ describe("accountProvisioner", () => { user: { name: userWithoutAuth.name, email, + emailVerified: true, avatarUrl: userWithoutAuth.avatarUrl, }, team: { @@ -138,6 +139,54 @@ describe("accountProvisioner", () => { expect(isNewUser).toEqual(false); }); + it("should not allow authentication by email matching when email is unverified", async () => { + const subdomain = faker.internet.domainWord(); + const existingTeam = await buildTeam({ + subdomain, + }); + + const providers = await existingTeam.$get("authenticationProviders"); + const authenticationProvider = providers[0]; + const email = faker.internet.email(); + const userWithoutAuth = await buildUser({ + email, + teamId: existingTeam.id, + authentications: [], + }); + + let error; + try { + await accountProvisioner(ctx, { + user: { + name: userWithoutAuth.name, + email, + emailVerified: false, + avatarUrl: userWithoutAuth.avatarUrl, + }, + team: { + teamId: existingTeam.id, + name: existingTeam.name, + avatarUrl: existingTeam.avatarUrl, + subdomain, + }, + authenticationProvider: { + name: authenticationProvider.name, + providerId: authenticationProvider.providerId, + }, + authentication: { + providerId: randomUUID(), + accessToken: "123", + scopes: ["read"], + }, + }); + } catch (err) { + error = err; + } + + expect(error).toBeTruthy(); + expect(error.id).toEqual("invalid_authentication"); + }); + it("should throw an error when authentication provider is disabled", async () => { const existingTeam = await buildTeam(); const providers = await existingTeam.$get("authenticationProviders"); @@ -250,6 +299,7 @@ describe("accountProvisioner", () => { user: { name: "Jenny Tester", email, + emailVerified: true, avatarUrl: faker.image.avatar(), }, team: { @@ -291,6 +341,7 @@ describe("accountProvisioner", () => { user: { name: "Jenny Tester", email, + emailVerified: true, avatarUrl: faker.image.avatar(), }, team: { diff --git a/server/commands/accountProvisioner.ts b/server/commands/accountProvisioner.ts index 5992b0cc33..7a81d1231d 100644 --- a/server/commands/accountProvisioner.ts +++ b/server/commands/accountProvisioner.ts @@ -17,6 +17,7 @@ import { Event, Team, } from "@server/models"; +import AuthenticationHelper from "@server/models/helpers/AuthenticationHelper"; import { DocumentHelper } from "@server/models/helpers/DocumentHelper"; import { sequelize } from "@server/storage/database"; import { PluginManager } from "@server/utils/PluginManager"; @@ -34,6 +35,8 @@ type Props = { name: string; /** The email address of the user */ email: string; + /** Whether the provider has verified the user owns the email address */ + emailVerified?: boolean; /** The public url of an image representing the user */ avatarUrl?: string | null; /** The language of the user, if known */ @@ -178,6 +181,10 @@ async function accountProvisioner( result = await userProvisioner(ctx, { name: userParams.name, email: userParams.email, + emailVerified: userParams.emailVerified, + authenticationProviderName: AuthenticationHelper.getProviderName( + authenticationProviderParams.name + ), language: userParams.language, role: isNewTeam ? UserRole.Admin : undefined, avatarUrl: userParams.avatarUrl, diff --git a/server/commands/userProvisioner.test.ts b/server/commands/userProvisioner.test.ts index eac4c3bb9d..a41d3df189 100644 --- a/server/commands/userProvisioner.test.ts +++ b/server/commands/userProvisioner.test.ts @@ -57,6 +57,7 @@ describe("userProvisioner", () => { const result = await userProvisioner(ctx, { name: existing.name, email, + emailVerified: true, avatarUrl: existing.avatarUrl, teamId: existing.teamId, authentication: { @@ -77,6 +78,34 @@ describe("userProvisioner", () => { expect(isNewUser).toEqual(false); }); + it("should not match an existing user by email when email is unverified", async () => { + const team = await buildTeam(); + const teamAuthProviders = await team.$get("authenticationProviders"); + const authenticationProvider = teamAuthProviders[0]; + + const email = "mynam@email.com"; + await buildUser({ + email, + teamId: team.id, + authentications: [], + }); + + await expect( + userProvisioner(ctx, { + name: "Imposter", + email, + emailVerified: false, + teamId: team.id, + authentication: { + authenticationProviderId: authenticationProvider.id, + providerId: randomUUID(), + accessToken: "123", + scopes: ["read"], + }, + }) + ).rejects.toThrow("has not been verified by"); + }); + it("should add authentication provider to invited users", async () => { const team = await buildTeam({ inviteRequired: true }); const teamAuthProviders = await team.$get("authenticationProviders"); @@ -91,6 +120,7 @@ describe("userProvisioner", () => { const result = await userProvisioner(ctx, { name: existing.name, email, + emailVerified: true, avatarUrl: existing.avatarUrl, teamId: existing.teamId, authentication: { @@ -264,6 +294,7 @@ describe("userProvisioner", () => { const result = await userProvisioner(ctx, { name: invite.name, email: "invite@ExamPle.com", + emailVerified: true, teamId: invite.teamId, authentication: { authenticationProviderId: authenticationProvider.id, @@ -295,6 +326,7 @@ describe("userProvisioner", () => { const result = await userProvisioner(ctx, { name: invite.name, email: "external@ExamPle.com", // ensure that email is case insensistive + emailVerified: true, teamId: invite.teamId, }); const { user, authentication, isNewUser } = result; @@ -340,6 +372,7 @@ describe("userProvisioner", () => { const result = await userProvisioner(ctx, { name: faker.person.fullName(), email, + emailVerified: true, teamId: team.id, authentication: { authenticationProviderId: authenticationProvider.id, @@ -357,6 +390,36 @@ describe("userProvisioner", () => { expect(isNewUser).toEqual(true); }); + it("should reject an unverified email when the team has allowed domains", async () => { + const team = await buildTeam(); + const admin = await buildAdmin({ teamId: team.id }); + const domain = faker.internet.domainName(); + await TeamDomain.create({ + teamId: team.id, + name: domain, + createdById: admin.id, + }); + + const authenticationProviders = await team.$get("authenticationProviders"); + const authenticationProvider = authenticationProviders[0]; + const email = faker.internet.email({ provider: domain }); + + await expect( + userProvisioner(ctx, { + name: faker.person.fullName(), + email, + emailVerified: false, + teamId: team.id, + authentication: { + authenticationProviderId: authenticationProvider.id, + providerId: "fake-service-id", + accessToken: "123", + scopes: ["read"], + }, + }) + ).rejects.toThrow("has not been verified by"); + }); + it("should create a user from allowed domain with emailMatchOnly", async () => { const team = await buildTeam(); const admin = await buildAdmin({ teamId: team.id }); @@ -372,6 +435,7 @@ describe("userProvisioner", () => { const result = await userProvisioner(ctx, { name: "Test Name", email, + emailVerified: true, teamId: team.id, }); const { user, authentication, isNewUser } = result; @@ -408,6 +472,7 @@ describe("userProvisioner", () => { userProvisioner(ctx, { name: "Bad Domain User", email: faker.internet.email(), + emailVerified: true, teamId: team.id, authentication: { authenticationProviderId: authenticationProvider.id, diff --git a/server/commands/userProvisioner.ts b/server/commands/userProvisioner.ts index d2d15cece9..1977faec10 100644 --- a/server/commands/userProvisioner.ts +++ b/server/commands/userProvisioner.ts @@ -25,6 +25,13 @@ type Props = { name: string; /** The email address of the user */ email: string; + /** + * Whether the provider has verified the user owns the email address. + * Matching an existing account by email only happens when explicitly true. + */ + emailVerified?: boolean; + /** The display name of the authentication provider, eg "Google". */ + authenticationProviderName?: string; /** The language of the user, if known */ language?: string; /** The role for new user, Member if none is provided */ @@ -54,7 +61,17 @@ type Props = { export default async function userProvisioner( ctx: APIContext, - { name, email, role, language, avatarUrl, teamId, authentication }: Props + { + name, + email, + emailVerified, + authenticationProviderName, + role, + language, + avatarUrl, + teamId, + authentication, + }: Props ): Promise { const auth = authentication ? await UserAuthentication.findOne({ @@ -135,6 +152,14 @@ export default async function userProvisioner( attributes: ["defaultUserRole", "inviteRequired", "id"], }); + // Unverified emails cannot match an existing account or pass allow listed domains + if (emailVerified !== true && (existingUser || team?.allowedDomains.length)) { + const providerName = authenticationProviderName ?? "your identity provider"; + throw InvalidAuthenticationError( + `Your email address has not been verified by ${providerName}. Please verify your email and try signing in again.` + ); + } + // We have an existing user, so we need to update it with our // new details and count this as a new user creation. if (existingUser) { diff --git a/server/models/helpers/AuthenticationHelper.ts b/server/models/helpers/AuthenticationHelper.ts index cdb32fdb1d..9550e4e3a2 100644 --- a/server/models/helpers/AuthenticationHelper.ts +++ b/server/models/helpers/AuthenticationHelper.ts @@ -17,6 +17,19 @@ export default class AuthenticationHelper { return PluginManager.getHooks(Hook.AuthProvider); } + /** + * Returns the human-readable display name for an authentication provider. + * + * @param id The authentication provider id, eg "google". + * @returns The display name if known, otherwise the provided id. + */ + public static getProviderName(id: string): string { + const provider = AuthenticationHelper.providers.find( + (hook) => hook.value.id === id + ); + return provider?.name ?? id; + } + /** * Returns the enabled authentication provider configurations for a team, * if given otherwise all enabled providers are returned.