diff --git a/package.json b/package.json index 9a422e25f..c574c93a1 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "db:create-migration": "sequelize migration:create", "db:migrate": "sequelize db:migrate", "db:rollback": "sequelize db:migrate:undo", + "db:reset": "sequelize db:drop && sequelize db:create && sequelize db:migrate", "upgrade": "git fetch && git pull && yarn install && yarn heroku-postbuild", "test": "yarn test:app && yarn test:server", "test:app": "jest --config=app/.jestconfig.json --runInBand --forceExit", diff --git a/server/commands/accountProvisioner.test.ts b/server/commands/accountProvisioner.test.ts index 7a65280de..33b96048b 100644 --- a/server/commands/accountProvisioner.test.ts +++ b/server/commands/accountProvisioner.test.ts @@ -108,6 +108,44 @@ describe("accountProvisioner", () => { spy.mockRestore(); }); + it("should allow authentication by email matching", async () => { + const existingTeam = await buildTeam(); + const providers = await existingTeam.$get("authenticationProviders"); + const authenticationProvider = providers[0]; + const userWithoutAuth = await buildUser({ + email: "email@example.com", + teamId: existingTeam.id, + authentications: [], + }); + + const { user, isNewUser, isNewTeam } = await accountProvisioner({ + ip, + user: { + name: userWithoutAuth.name, + email: "email@example.com", + avatarUrl: userWithoutAuth.avatarUrl, + }, + team: { + name: existingTeam.name, + avatarUrl: existingTeam.avatarUrl, + subdomain: "example", + }, + authenticationProvider: { + name: authenticationProvider.name, + providerId: authenticationProvider.providerId, + }, + authentication: { + providerId: "anything", + accessToken: "123", + scopes: ["read"], + }, + }); + expect(user.id).toEqual(userWithoutAuth.id); + expect(isNewTeam).toEqual(false); + expect(isNewUser).toEqual(false); + expect(user.authentications.length).toEqual(0); + }); + it("should throw an error when authentication provider is disabled", async () => { const existingTeam = await buildTeam(); const providers = await existingTeam.$get("authenticationProviders"); diff --git a/server/commands/accountProvisioner.ts b/server/commands/accountProvisioner.ts index 27dabc5a6..f576f8393 100644 --- a/server/commands/accountProvisioner.ts +++ b/server/commands/accountProvisioner.ts @@ -8,9 +8,9 @@ import { AuthenticationProviderDisabledError, } from "@server/errors"; import { APM } from "@server/logging/tracing"; -import { Collection, Team, User } from "@server/models"; -import teamCreator from "./teamCreator"; -import userCreator from "./userCreator"; +import { AuthenticationProvider, Collection, Team, User } from "@server/models"; +import teamProvisioner from "./teamProvisioner"; +import userProvisioner from "./userProvisioner"; type Props = { ip: string; @@ -21,7 +21,7 @@ type Props = { username?: string; }; team: { - id?: string; + teamId?: string; name: string; domain?: string; subdomain: string; @@ -55,15 +55,45 @@ async function accountProvisioner({ authentication: authenticationParams, }: Props): Promise { let result; + let emailMatchOnly; try { - result = await teamCreator({ + result = await teamProvisioner({ ...teamParams, authenticationProvider: authenticationProviderParams, ip, }); } catch (err) { - throw InvalidAuthenticationError(err.message); + // The account could not be provisioned for the provided teamId + // check to see if we can try authentication using email matching only + if (err.id === "invalid_authentication") { + const authenticationProvider = await AuthenticationProvider.findOne({ + where: { + name: authenticationProviderParams.name, // example: "google" + teamId: teamParams.teamId, + }, + include: [ + { + model: Team, + as: "team", + required: true, + }, + ], + }); + + if (authenticationProvider) { + emailMatchOnly = true; + result = { + authenticationProvider, + team: authenticationProvider.team, + isNewTeam: false, + }; + } + } + + if (!result) { + throw InvalidAuthenticationError(err.message); + } } invariant(result, "Team creator result must exist"); @@ -74,20 +104,21 @@ async function accountProvisioner({ } try { - const result = await userCreator({ + const result = await userProvisioner({ name: userParams.name, email: userParams.email, username: userParams.username, isAdmin: isNewTeam || undefined, avatarUrl: userParams.avatarUrl, teamId: team.id, + emailMatchOnly, ip, authentication: { + authenticationProviderId: authenticationProvider.id, ...authenticationParams, expiresAt: authenticationParams.expiresIn ? new Date(Date.now() + authenticationParams.expiresIn * 1000) : undefined, - authenticationProviderId: authenticationProvider.id, }, }); const { isNewUser, user } = result; diff --git a/server/commands/teamCreator.test.ts b/server/commands/teamProvisioner.test.ts similarity index 73% rename from server/commands/teamCreator.test.ts rename to server/commands/teamProvisioner.test.ts index a6aba15f2..c6e5f3b43 100644 --- a/server/commands/teamCreator.test.ts +++ b/server/commands/teamProvisioner.test.ts @@ -2,16 +2,16 @@ import env from "@server/env"; import TeamDomain from "@server/models/TeamDomain"; import { buildTeam, buildUser } from "@server/test/factories"; import { flushdb } from "@server/test/support"; -import teamCreator from "./teamCreator"; +import teamProvisioner from "./teamProvisioner"; beforeEach(() => flushdb()); -describe("teamCreator", () => { +describe("teamProvisioner", () => { const ip = "127.0.0.1"; it("should create team and authentication provider", async () => { env.DEPLOYMENT = "hosted"; - const result = await teamCreator({ + const result = await teamProvisioner({ name: "Test team", subdomain: "example", avatarUrl: "http://example.com/logo.png", @@ -35,7 +35,7 @@ describe("teamCreator", () => { await buildTeam({ subdomain: "myteam", }); - const result = await teamCreator({ + const result = await teamProvisioner({ name: "Test team", subdomain: "myteam", avatarUrl: "http://example.com/logo.png", @@ -58,7 +58,7 @@ describe("teamCreator", () => { await buildTeam({ subdomain: "myteam1", }); - const result = await teamCreator({ + const result = await teamProvisioner({ name: "Test team", subdomain: "myteam", avatarUrl: "http://example.com/logo.png", @@ -72,10 +72,63 @@ describe("teamCreator", () => { expect(result.team.subdomain).toEqual("myteam2"); }); + it("should return existing team", async () => { + env.DEPLOYMENT = "hosted"; + const authenticationProvider = { + name: "google", + providerId: "example.com", + }; + const existing = await buildTeam({ + subdomain: "example", + authenticationProviders: [authenticationProvider], + }); + const result = await teamProvisioner({ + name: "Updated name", + subdomain: "example", + authenticationProvider, + ip, + }); + const { team, isNewTeam } = result; + expect(team.id).toEqual(existing.id); + expect(team.name).toEqual(existing.name); + expect(team.subdomain).toEqual("example"); + expect(isNewTeam).toEqual(false); + }); + + it("should error on mismatched team and authentication provider", async () => { + env.DEPLOYMENT = "hosted"; + const exampleTeam = await buildTeam({ + subdomain: "example", + authenticationProviders: [ + { + name: "google", + providerId: "example.com", + }, + ], + }); + + let error; + try { + await teamProvisioner({ + teamId: exampleTeam.id, + name: "name", + subdomain: "other", + authenticationProvider: { + name: "google", + providerId: "other.com", + }, + ip, + }); + } catch (e) { + error = e; + } + expect(error.id).toEqual("invalid_authentication"); + }); + describe("self hosted", () => { it("should allow creating first team", async () => { env.DEPLOYMENT = undefined; - const { team, isNewTeam } = await teamCreator({ + const { team, isNewTeam } = await teamProvisioner({ name: "Test team", subdomain: "example", avatarUrl: "http://example.com/logo.png", @@ -96,7 +149,7 @@ describe("teamCreator", () => { let error; try { - await teamCreator({ + await teamProvisioner({ name: "Test team", subdomain: "example", avatarUrl: "http://example.com/logo.png", @@ -124,7 +177,7 @@ describe("teamCreator", () => { name: "allowed-domain.com", createdById: user.id, }); - const result = await teamCreator({ + const result = await teamProvisioner({ name: "Updated name", subdomain: "example", domain: "allowed-domain.com", @@ -158,7 +211,7 @@ describe("teamCreator", () => { let error; try { - await teamCreator({ + await teamProvisioner({ name: "Updated name", subdomain: "example", domain: "other-domain.com", @@ -175,7 +228,7 @@ describe("teamCreator", () => { expect(error).toBeTruthy(); }); - it("should return exising team", async () => { + it("should return existing team", async () => { env.DEPLOYMENT = undefined; const authenticationProvider = { name: "google", @@ -185,7 +238,7 @@ describe("teamCreator", () => { subdomain: "example", authenticationProviders: [authenticationProvider], }); - const result = await teamCreator({ + const result = await teamProvisioner({ name: "Updated name", subdomain: "example", authenticationProvider, diff --git a/server/commands/teamCreator.ts b/server/commands/teamProvisioner.ts similarity index 88% rename from server/commands/teamCreator.ts rename to server/commands/teamProvisioner.ts index 210a09a26..621b46c31 100644 --- a/server/commands/teamCreator.ts +++ b/server/commands/teamProvisioner.ts @@ -1,8 +1,8 @@ import { sequelize } from "@server/database/sequelize"; import env from "@server/env"; import { - InvalidAuthenticationError, DomainNotAllowedError, + InvalidAuthenticationError, MaximumTeamsError, } from "@server/errors"; import Logger from "@server/logging/Logger"; @@ -10,14 +10,14 @@ import { APM } from "@server/logging/tracing"; import { Team, AuthenticationProvider, Event } from "@server/models"; import { generateAvatarUrl } from "@server/utils/avatars"; -type TeamCreatorResult = { +type TeamProvisionerResult = { team: Team; authenticationProvider: AuthenticationProvider; isNewTeam: boolean; }; type Props = { - id?: string; + teamId?: string; name: string; domain?: string; subdomain: string; @@ -29,18 +29,18 @@ type Props = { ip: string; }; -async function teamCreator({ - id, +async function teamProvisioner({ + teamId, name, domain, subdomain, avatarUrl, authenticationProvider, ip, -}: Props): Promise { +}: Props): Promise { let authP = await AuthenticationProvider.findOne({ - where: id - ? { ...authenticationProvider, teamId: id } + where: teamId + ? { ...authenticationProvider, teamId } : authenticationProvider, include: [ { @@ -59,11 +59,9 @@ async function teamCreator({ team: authP.team, isNewTeam: false, }; - } - // A team id was provided but no auth provider was found matching those credentials - // The user is attempting to log into a team with an incorrect SSO - fail the login - else if (id) { - throw InvalidAuthenticationError("incorrect authentication credentials"); + } else if (teamId) { + // The user is attempting to log into a team with an unfamiliar SSO provider + throw InvalidAuthenticationError(); } // This team has never been seen before, if self hosted the logic is different @@ -176,5 +174,5 @@ async function provisionSubdomain(team: Team, requestedSubdomain: string) { export default APM.traceFunction({ serviceName: "command", - spanName: "teamCreator", -})(teamCreator); + spanName: "teamProvisioner", +})(teamProvisioner); diff --git a/server/commands/userCreator.test.ts b/server/commands/userProvisioner.test.ts similarity index 76% rename from server/commands/userCreator.test.ts rename to server/commands/userProvisioner.test.ts index 6969ba7f6..31aa47375 100644 --- a/server/commands/userCreator.test.ts +++ b/server/commands/userProvisioner.test.ts @@ -2,20 +2,20 @@ import { v4 as uuidv4 } from "uuid"; import { TeamDomain } from "@server/models"; import { buildUser, buildTeam, buildInvite } from "@server/test/factories"; import { flushdb, seed } from "@server/test/support"; -import userCreator from "./userCreator"; +import userProvisioner from "./userProvisioner"; beforeEach(() => flushdb()); -describe("userCreator", () => { +describe("userProvisioner", () => { const ip = "127.0.0.1"; - it("should update exising user and authentication", async () => { + it("should update existing user and authentication", async () => { const existing = await buildUser(); const authentications = await existing.$get("authentications"); const existingAuth = authentications[0]; const newEmail = "test@example.com"; const newUsername = "tname"; - const result = await userCreator({ + const result = await userProvisioner({ name: existing.name, email: newEmail, username: newUsername, @@ -30,9 +30,10 @@ describe("userCreator", () => { }, }); const { user, authentication, isNewUser } = result; - expect(authentication.accessToken).toEqual("123"); - expect(authentication.scopes.length).toEqual(1); - expect(authentication.scopes[0]).toEqual("read"); + expect(authentication).toBeDefined(); + expect(authentication?.accessToken).toEqual("123"); + expect(authentication?.scopes.length).toEqual(1); + expect(authentication?.scopes[0]).toEqual("read"); expect(user.email).toEqual(newEmail); expect(user.username).toEqual(newUsername); expect(isNewUser).toEqual(false); @@ -50,7 +51,7 @@ describe("userCreator", () => { authentications: [], }); - const result = await userCreator({ + const result = await userProvisioner({ name: existing.name, email, username: "new-username", @@ -65,9 +66,10 @@ describe("userCreator", () => { }, }); const { user, authentication, isNewUser } = result; - expect(authentication.accessToken).toEqual("123"); - expect(authentication.scopes.length).toEqual(1); - expect(authentication.scopes[0]).toEqual("read"); + expect(authentication).toBeDefined(); + expect(authentication?.accessToken).toEqual("123"); + expect(authentication?.scopes.length).toEqual(1); + expect(authentication?.scopes[0]).toEqual("read"); const authentications = await user.$get("authentications"); expect(authentications.length).toEqual(1); @@ -85,7 +87,7 @@ describe("userCreator", () => { teamId: team.id, }); - const result = await userCreator({ + const result = await userProvisioner({ name: existing.name, email, username: "new-username", @@ -100,9 +102,10 @@ describe("userCreator", () => { }, }); const { user, authentication, isNewUser } = result; - expect(authentication.accessToken).toEqual("123"); - expect(authentication.scopes.length).toEqual(1); - expect(authentication.scopes[0]).toEqual("read"); + expect(authentication).toBeDefined(); + expect(authentication?.accessToken).toEqual("123"); + expect(authentication?.scopes.length).toEqual(1); + expect(authentication?.scopes[0]).toEqual("read"); const authentications = await user.$get("authentications"); expect(authentications.length).toEqual(1); @@ -115,7 +118,7 @@ describe("userCreator", () => { const existingAuth = authentications[0]; const newEmail = "test@example.com"; await existing.destroy(); - const result = await userCreator({ + const result = await userProvisioner({ name: "Test Name", email: "test@example.com", teamId: existing.teamId, @@ -128,9 +131,10 @@ describe("userCreator", () => { }, }); const { user, authentication, isNewUser } = result; - expect(authentication.accessToken).toEqual("123"); - expect(authentication.scopes.length).toEqual(1); - expect(authentication.scopes[0]).toEqual("read"); + expect(authentication).toBeDefined(); + expect(authentication?.accessToken).toEqual("123"); + expect(authentication?.scopes.length).toEqual(1); + expect(authentication?.scopes[0]).toEqual("read"); expect(user.email).toEqual(newEmail); expect(isNewUser).toEqual(true); }); @@ -142,13 +146,13 @@ describe("userCreator", () => { let error; try { - await userCreator({ + await userProvisioner({ name: "Test Name", email: "test@example.com", teamId: existing.teamId, ip, authentication: { - authenticationProviderId: "example.org", + authenticationProviderId: uuidv4(), providerId: existingAuth.providerId, accessToken: "123", scopes: ["read"], @@ -165,7 +169,7 @@ describe("userCreator", () => { const team = await buildTeam(); const authenticationProviders = await team.$get("authenticationProviders"); const authenticationProvider = authenticationProviders[0]; - const result = await userCreator({ + const result = await userProvisioner({ name: "Test Name", email: "test@example.com", username: "tname", @@ -179,9 +183,10 @@ describe("userCreator", () => { }, }); const { user, authentication, isNewUser } = result; - expect(authentication.accessToken).toEqual("123"); - expect(authentication.scopes.length).toEqual(1); - expect(authentication.scopes[0]).toEqual("read"); + expect(authentication).toBeDefined(); + expect(authentication?.accessToken).toEqual("123"); + expect(authentication?.scopes.length).toEqual(1); + expect(authentication?.scopes[0]).toEqual("read"); expect(user.email).toEqual("test@example.com"); expect(user.username).toEqual("tname"); expect(user.isAdmin).toEqual(false); @@ -195,7 +200,7 @@ describe("userCreator", () => { }); const authenticationProviders = await team.$get("authenticationProviders"); const authenticationProvider = authenticationProviders[0]; - const result = await userCreator({ + const result = await userProvisioner({ name: "Test Name", email: "test@example.com", username: "tname", @@ -219,7 +224,7 @@ describe("userCreator", () => { }); const authenticationProviders = await team.$get("authenticationProviders"); const authenticationProvider = authenticationProviders[0]; - const result = await userCreator({ + const result = await userProvisioner({ name: "Test Name", email: "test@example.com", username: "tname", @@ -236,7 +241,7 @@ describe("userCreator", () => { expect(tname.username).toEqual("tname"); expect(tname.isAdmin).toEqual(false); expect(tname.isViewer).toEqual(true); - const tname2Result = await userCreator({ + const tname2Result = await userProvisioner({ name: "Test2 Name", email: "tes2@example.com", username: "tname2", @@ -264,7 +269,7 @@ describe("userCreator", () => { }); const authenticationProviders = await team.$get("authenticationProviders"); const authenticationProvider = authenticationProviders[0]; - const result = await userCreator({ + const result = await userProvisioner({ name: invite.name, email: "invite@ExamPle.com", teamId: invite.teamId, @@ -277,13 +282,46 @@ describe("userCreator", () => { }, }); const { user, authentication, isNewUser } = result; - expect(authentication.accessToken).toEqual("123"); - expect(authentication.scopes.length).toEqual(1); - expect(authentication.scopes[0]).toEqual("read"); + expect(authentication).toBeDefined(); + expect(authentication?.accessToken).toEqual("123"); + expect(authentication?.scopes.length).toEqual(1); + expect(authentication?.scopes[0]).toEqual("read"); expect(user.email).toEqual(invite.email); expect(isNewUser).toEqual(true); }); + it("should create a user from an invited user using email match", async () => { + const externalUser = await buildUser({ + email: "external@example.com", + }); + + const team = await buildTeam({ inviteRequired: true }); + const invite = await buildInvite({ + teamId: team.id, + email: externalUser.email, + }); + + const authenticationProviders = await team.$get("authenticationProviders"); + const authenticationProvider = authenticationProviders[0]; + const result = await userProvisioner({ + name: invite.name, + email: "external@ExamPle.com", // ensure that email is case insensistive + teamId: invite.teamId, + emailMatchOnly: true, + ip, + authentication: { + authenticationProviderId: authenticationProvider.id, + providerId: "whatever", + accessToken: "123", + scopes: ["read"], + }, + }); + const { user, authentication, isNewUser } = result; + expect(authentication).toEqual(null); + expect(user.id).toEqual(invite.id); + expect(isNewUser).toEqual(true); + }); + it("should reject an uninvited user when invites are required", async () => { const team = await buildTeam({ inviteRequired: true }); @@ -292,7 +330,7 @@ describe("userCreator", () => { let error; try { - await userCreator({ + await userProvisioner({ name: "Uninvited User", email: "invite@ExamPle.com", teamId: team.id, @@ -323,7 +361,7 @@ describe("userCreator", () => { const authenticationProviders = await team.$get("authenticationProviders"); const authenticationProvider = authenticationProviders[0]; - const result = await userCreator({ + const result = await userProvisioner({ name: "Test Name", email: "user@example-company.com", teamId: team.id, @@ -336,9 +374,10 @@ describe("userCreator", () => { }, }); const { user, authentication, isNewUser } = result; - expect(authentication.accessToken).toEqual("123"); - expect(authentication.scopes.length).toEqual(1); - expect(authentication.scopes[0]).toEqual("read"); + expect(authentication).toBeDefined(); + expect(authentication?.accessToken).toEqual("123"); + expect(authentication?.scopes.length).toEqual(1); + expect(authentication?.scopes[0]).toEqual("read"); expect(user.email).toEqual("user@example-company.com"); expect(isNewUser).toEqual(true); }); @@ -356,7 +395,7 @@ describe("userCreator", () => { let error; try { - await userCreator({ + await userProvisioner({ name: "Bad Domain User", email: "user@example.com", teamId: team.id, diff --git a/server/commands/userCreator.ts b/server/commands/userProvisioner.ts similarity index 83% rename from server/commands/userCreator.ts rename to server/commands/userProvisioner.ts index f9c6a9bfb..9e51ddb05 100644 --- a/server/commands/userCreator.ts +++ b/server/commands/userProvisioner.ts @@ -1,12 +1,16 @@ import { sequelize } from "@server/database/sequelize"; import InviteAcceptedEmail from "@server/emails/templates/InviteAcceptedEmail"; -import { DomainNotAllowedError, InviteRequiredError } from "@server/errors"; +import { + DomainNotAllowedError, + InvalidAuthenticationError, + InviteRequiredError, +} from "@server/errors"; import { Event, Team, User, UserAuthentication } from "@server/models"; -type UserCreatorResult = { +type UserProvisionerResult = { user: User; isNewUser: boolean; - authentication: UserAuthentication; + authentication: UserAuthentication | null; }; type Props = { @@ -16,6 +20,7 @@ type Props = { isAdmin?: boolean; avatarUrl?: string | null; teamId: string; + emailMatchOnly?: boolean; ip: string; authentication: { authenticationProviderId: string; @@ -27,17 +32,18 @@ type Props = { }; }; -export default async function userCreator({ +export default async function userProvisioner({ name, email, username, isAdmin, + emailMatchOnly, avatarUrl, teamId, authentication, ip, -}: Props): Promise { - const { authenticationProviderId, providerId, ...rest } = authentication; +}: Props): Promise { + const { providerId, authenticationProviderId, ...rest } = authentication; const auth = await UserAuthentication.findOne({ where: { @@ -87,9 +93,8 @@ export default async function userCreator({ await auth.destroy(); } - // A `user` record might exist in the form of an invite even if there is no - // existing authentication record that matches. In Outline an invite is a - // shell user record. + // A `user` record may exist even if there is no existing authentication record. + // This is either an invite or a user that's external to the team const existingUser = await User.scope([ "withAuthentications", "withTeam", @@ -102,12 +107,11 @@ export default async function userCreator({ }, }); - // We have an existing invite for his user, so we need to update it with our - // new details, link up the authentication method, and count this as a new - // user creation. + // 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) { // A `user` record might exist in the form of an invite. - // In Outline an invite is a shell user record with no authentication method + // An invite is a shell user record with no authentication method // that's never been active before. const isInvite = existingUser.isInvited; @@ -145,6 +149,12 @@ export default async function userCreator({ } ); + // We don't want to associate a user auth with the auth provider + // if we're doing a simple email match, so early return here + if (emailMatchOnly) { + return null; + } + return await existingUser.$create( "authentication", authentication, @@ -171,9 +181,16 @@ export default async function userCreator({ authentication: auth, isNewUser: isInvite, }; + } else if (emailMatchOnly) { + // There's no existing invite or user that matches the external auth email + // This is simply unauthorized + throw InvalidAuthenticationError(); } + // // No auth, no user – this is an entirely new sign in. + // + const transaction = await User.sequelize!.transaction(); try { diff --git a/server/routes/auth/providers/azure.ts b/server/routes/auth/providers/azure.ts index 0e99e4bf0..18649906c 100644 --- a/server/routes/auth/providers/azure.ts +++ b/server/routes/auth/providers/azure.ts @@ -4,6 +4,7 @@ import jwt from "jsonwebtoken"; import type { Context } from "koa"; import Router from "koa-router"; import { Profile } from "passport"; +import { slugifyDomain } from "@shared/utils/domains"; import accountProvisioner, { AccountProvisionerResult, } from "@server/commands/accountProvisioner"; @@ -95,12 +96,13 @@ if (env.AZURE_CLIENT_ID && env.AZURE_CLIENT_SECRET) { const team = await getTeamFromContext(ctx); const domain = email.split("@")[1]; - const subdomain = domain.split(".")[0]; + const subdomain = slugifyDomain(domain); + const teamName = organization.displayName; const result = await accountProvisioner({ ip: ctx.ip, team: { - id: team?.id, + teamId: team?.id, name: teamName, domain, subdomain, diff --git a/server/routes/auth/providers/google.ts b/server/routes/auth/providers/google.ts index 16af9d912..4f9cd23de 100644 --- a/server/routes/auth/providers/google.ts +++ b/server/routes/auth/providers/google.ts @@ -11,7 +11,6 @@ import accountProvisioner, { import env from "@server/env"; import { GmailAccountCreationError, - InviteRequiredError, TeamDomainRequiredError, } from "@server/errors"; import passportMiddleware from "@server/middlewares/passport"; @@ -19,7 +18,7 @@ import { User } from "@server/models"; import { StateStore, getTeamFromContext } from "@server/utils/passport"; const router = new Router(); -const providerName = "google"; +const GOOGLE = "google"; const scopes = [ "https://www.googleapis.com/auth/userinfo.profile", "https://www.googleapis.com/auth/userinfo.email", @@ -34,7 +33,7 @@ type GoogleProfile = Profile & { email: string; picture: string; _json: { - hd: string; + hd?: string; }; }; @@ -63,87 +62,66 @@ if (env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET) { ) => void ) { try { - let result; - // "domain" is the Google Workspaces domain const domain = profile._json.hd; const team = await getTeamFromContext(ctx); - // Existence of domain means this is a Google Workspaces account - // so we'll attempt to provision an account (team and user) - if (domain) { - // remove the TLD and form a subdomain from the remaining - // subdomains of the form "foo.bar.com" are allowed as primary Google Workspaces domains - // see https://support.google.com/nonprofits/thread/19685140/using-a-subdomain-as-a-primary-domain - const subdomain = slugifyDomain(domain); - const teamName = capitalize(subdomain); - - // Request a larger size profile picture than the default by tweaking - // the query parameter. - const avatarUrl = profile.picture.replace("=s96-c", "=s128-c"); - - // if a team can be inferred, we assume the user is only interested in signing into - // that team in particular; otherwise, we will do a best effort at finding their account - // or provisioning a new one (within AccountProvisioner) - result = await accountProvisioner({ - ip: ctx.ip, - team: { - id: team?.id, - name: teamName, - domain, - subdomain, - }, - user: { - email: profile.email, - name: profile.displayName, - avatarUrl, - }, - authenticationProvider: { - name: providerName, - providerId: domain, - }, - authentication: { - providerId: profile.id, - accessToken, - refreshToken, - expiresIn: params.expires_in, - scopes, - }, - }); - } else { - // No domain means it's a personal Gmail account - // We only allow sign-in to existing user accounts with these - if (!team) { - // No team usually means this is the apex domain - // Throw different errors depending on whether we think the user is - // trying to create a new account, or log-in to an existing one - const userExists = await User.count({ - where: { email: profile.email.toLowerCase() }, - }); - - if (!userExists) { - throw GmailAccountCreationError(); - } - - throw TeamDomainRequiredError(); - } - - const user = await User.findOne({ - where: { teamId: team.id, email: profile.email.toLowerCase() }, + // No profile domain means personal gmail account + // No team implies the request came from the apex domain + // This combination is always an error + if (!domain && !team) { + const userExists = await User.count({ + where: { email: profile.email.toLowerCase() }, }); - if (!user) { - throw InviteRequiredError(); + // Users cannot create a team with personal gmail accounts + if (!userExists) { + throw GmailAccountCreationError(); } - result = { - user, - team, - isNewUser: false, - isNewTeam: false, - }; + // To log-in with a personal account, users must specify a team subdomain + throw TeamDomainRequiredError(); } + // remove the TLD and form a subdomain from the remaining + // subdomains of the form "foo.bar.com" are allowed as primary Google Workspaces domains + // see https://support.google.com/nonprofits/thread/19685140/using-a-subdomain-as-a-primary-domain + const subdomain = domain ? slugifyDomain(domain) : ""; + const teamName = capitalize(subdomain); + + // Request a larger size profile picture than the default by tweaking + // the query parameter. + const avatarUrl = profile.picture.replace("=s96-c", "=s128-c"); + + // if a team can be inferred, we assume the user is only interested in signing into + // that team in particular; otherwise, we will do a best effort at finding their account + // or provisioning a new one (within AccountProvisioner) + const result = await accountProvisioner({ + ip: ctx.ip, + team: { + teamId: team?.id, + name: teamName, + domain, + subdomain, + }, + user: { + email: profile.email, + name: profile.displayName, + avatarUrl, + }, + authenticationProvider: { + name: GOOGLE, + providerId: domain ?? "", + }, + authentication: { + providerId: profile.id, + accessToken, + refreshToken, + expiresIn: params.expires_in, + scopes, + }, + }); + return done(null, result.user, result); } catch (err) { return done(err, null); @@ -154,13 +132,13 @@ if (env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET) { router.get( "google", - passport.authenticate(providerName, { + passport.authenticate(GOOGLE, { accessType: "offline", prompt: "select_account consent", }) ); - router.get("google.callback", passportMiddleware(providerName)); + router.get("google.callback", passportMiddleware(GOOGLE)); } export default router; diff --git a/server/routes/auth/providers/oidc.ts b/server/routes/auth/providers/oidc.ts index 625a36d10..8b8e15924 100644 --- a/server/routes/auth/providers/oidc.ts +++ b/server/routes/auth/providers/oidc.ts @@ -97,7 +97,7 @@ if (env.OIDC_CLIENT_ID && env.OIDC_CLIENT_SECRET) { const result = await accountProvisioner({ ip: ctx.ip, team: { - id: team?.id, + teamId: team?.id, // https://github.com/outline/outline/pull/2388#discussion_r681120223 name: "Wiki", domain, diff --git a/server/routes/auth/providers/slack.ts b/server/routes/auth/providers/slack.ts index ba2126f36..4d98a60ca 100644 --- a/server/routes/auth/providers/slack.ts +++ b/server/routes/auth/providers/slack.ts @@ -79,7 +79,7 @@ if (env.SLACK_CLIENT_ID && env.SLACK_CLIENT_SECRET) { const result = await accountProvisioner({ ip: ctx.ip, team: { - id: team?.id, + teamId: team?.id, name: profile.team.name, subdomain: profile.team.domain, avatarUrl: profile.team.image_230,