mirror of
https://github.com/outline/outline.git
synced 2026-06-13 11:25:03 +03:00
Add email verification check during sign-in flow (#12605)
* Add email verification check during sign-in flow * Add support for Entra External ID with OIDC standard verification claim
This commit is contained in:
@@ -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: {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<UserProvisionerResult> {
|
||||
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) {
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user