diff --git a/.env.sample b/.env.sample index 1b41ae5fe..d83d1768b 100644 --- a/.env.sample +++ b/.env.sample @@ -71,7 +71,7 @@ DEBUG=cache,presenters,events,emails,mailer,utils,multiplayer,server,services # Comma separated list of domains to be allowed to signin to the wiki. If not # set, all domains are allowed by default when using Google OAuth to signin -GOOGLE_ALLOWED_DOMAINS= +ALLOWED_DOMAINS= # For a complete Slack integration with search and posting to channels the # following configs are also needed, some more details diff --git a/app.json b/app.json index 4af94bb08..76d8e094d 100644 --- a/app.json +++ b/app.json @@ -55,7 +55,7 @@ "description": "", "required": false }, - "GOOGLE_ALLOWED_DOMAINS": { + "ALLOWED_DOMAINS": { "description": "Comma separated list of domains to be allowed (optional). If not set, all Google apps domains are allowed by default", "required": false }, @@ -148,4 +148,4 @@ "required": false } } -} +} \ No newline at end of file diff --git a/app/scenes/Login/Notices.js b/app/scenes/Login/Notices.js index b45a2e8a9..d7be46d39 100644 --- a/app/scenes/Login/Notices.js +++ b/app/scenes/Login/Notices.js @@ -15,6 +15,12 @@ export default function Notices({ notice }: Props) { signing in with your Google Workspace account. )} + {notice === "maximum-teams" && ( + + The team you authenticated with is not authorized on this + installation. Try another? + + )} {notice === "hd-not-allowed" && ( Sorry, your Google apps domain is not allowed. Please try again with diff --git a/server/auth/google.js b/server/auth/google.js index 9f85c2270..8a100cf50 100644 --- a/server/auth/google.js +++ b/server/auth/google.js @@ -11,14 +11,14 @@ import { } from "../errors"; import auth from "../middlewares/authentication"; import passportMiddleware from "../middlewares/passport"; +import { getAllowedDomains } from "../utils/authentication"; import { StateStore } from "../utils/passport"; const router = new Router(); const providerName = "google"; const GOOGLE_CLIENT_ID = process.env.GOOGLE_CLIENT_ID; const GOOGLE_CLIENT_SECRET = process.env.GOOGLE_CLIENT_SECRET; -const allowedDomainsEnv = process.env.GOOGLE_ALLOWED_DOMAINS; -const allowedDomains = allowedDomainsEnv ? allowedDomainsEnv.split(",") : []; +const allowedDomains = getAllowedDomains(); const scopes = [ "https://www.googleapis.com/auth/userinfo.profile", diff --git a/server/commands/teamCreator.js b/server/commands/teamCreator.js index a7e180373..3bd113d38 100644 --- a/server/commands/teamCreator.js +++ b/server/commands/teamCreator.js @@ -1,11 +1,12 @@ // @flow import debug from "debug"; +import { MaximumTeamsError } from "../errors"; import { Team, AuthenticationProvider } from "../models"; import { sequelize } from "../sequelize"; +import { getAllowedDomains } from "../utils/authentication"; import { generateAvatarUrl } from "../utils/avatars"; const log = debug("server"); - type TeamCreatorResult = {| team: Team, authenticationProvider: AuthenticationProvider, @@ -28,7 +29,7 @@ export default async function teamCreator({ providerId: string, |}, |}): Promise { - const authP = await AuthenticationProvider.findOne({ + let authP = await AuthenticationProvider.findOne({ where: authenticationProvider, include: [ { @@ -49,6 +50,31 @@ export default async function teamCreator({ }; } + // This team has never been seen before, if self hosted the logic is different + // to the multi-tenant version, we want to restrict to a single team that MAY + // have multiple authentication providers + if (process.env.DEPLOYMENT !== "hosted") { + const teamCount = await Team.count(); + + // If the self-hosted installation has a single team and the domain for the + // new team matches one in the allowed domains env variable then assign the + // authentication provider to the existing team + if (teamCount === 1 && domain && getAllowedDomains().includes(domain)) { + const team = await Team.findOne(); + authP = await team.createAuthenticationProvider(authenticationProvider); + + return { + authenticationProvider: authP, + team, + isNewTeam: false, + }; + } + + if (teamCount >= 1) { + throw new MaximumTeamsError(); + } + } + // If the service did not provide a logo/avatar then we attempt to generate // one via ClearBit, or fallback to colored initials in worst case scenario if (!avatarUrl) { @@ -59,9 +85,9 @@ export default async function teamCreator({ }); } - // This team has never been seen before, time to create all the new stuff let transaction = await sequelize.transaction(); let team; + try { team = await Team.create( { diff --git a/server/commands/teamCreator.test.js b/server/commands/teamCreator.test.js index 336aecfdf..1c2dd578e 100644 --- a/server/commands/teamCreator.test.js +++ b/server/commands/teamCreator.test.js @@ -34,6 +34,52 @@ describe("teamCreator", () => { expect(isNewTeam).toEqual(true); }); + it("should now allow creating multiple teams in installation", async () => { + await buildTeam(); + let error; + + try { + await teamCreator({ + name: "Test team", + subdomain: "example", + avatarUrl: "http://example.com/logo.png", + authenticationProvider: { + name: "google", + providerId: "example.com", + }, + }); + } catch (err) { + error = err; + } + + expect(error).toBeTruthy(); + }); + + it("should return existing team when within allowed domains", async () => { + const existing = await buildTeam(); + + const result = await teamCreator({ + name: "Updated name", + subdomain: "example", + domain: "allowed-domain.com", + authenticationProvider: { + name: "google", + providerId: "allowed-domain.com", + }, + }); + + const { team, authenticationProvider, isNewTeam } = result; + + expect(team.id).toEqual(existing.id); + expect(team.name).toEqual(existing.name); + expect(authenticationProvider.name).toEqual("google"); + expect(authenticationProvider.providerId).toEqual("allowed-domain.com"); + expect(isNewTeam).toEqual(false); + + const providers = await team.getAuthenticationProviders(); + expect(providers.length).toEqual(2); + }); + it("should return exising team", async () => { const authenticationProvider = { name: "google", diff --git a/server/errors.js b/server/errors.js index 412195d7c..7aab2f8d4 100644 --- a/server/errors.js +++ b/server/errors.js @@ -69,6 +69,12 @@ export function OAuthStateMismatchError( return httpErrors(400, message, { id: "state_mismatch" }); } +export function MaximumTeamsError( + message: string = "The maximum number of teams has been reached" +) { + return httpErrors(400, message, { id: "maximum_teams" }); +} + export function EmailAuthenticationRequiredError( message: string = "User must authenticate with email", redirectUrl: string = env.URL diff --git a/server/test/helper.js b/server/test/helper.js index 7572493f5..dd87fa978 100644 --- a/server/test/helper.js +++ b/server/test/helper.js @@ -6,6 +6,8 @@ process.env.DATABASE_URL = process.env.DATABASE_URL_TEST; process.env.NODE_ENV = "test"; process.env.GOOGLE_CLIENT_ID = "123"; process.env.SLACK_KEY = "123"; +process.env.DEPLOYMENT = ""; +process.env.ALLOWED_DOMAINS = "allowed-domain.com"; // This is needed for the relative manual mock to be picked up jest.mock("../events"); diff --git a/server/utils/authentication.js b/server/utils/authentication.js new file mode 100644 index 000000000..baf3cdc80 --- /dev/null +++ b/server/utils/authentication.js @@ -0,0 +1,7 @@ +// @flow + +export function getAllowedDomains(): string[] { + // GOOGLE_ALLOWED_DOMAINS included here for backwards compatability + const env = process.env.ALLOWED_DOMAINS || process.env.GOOGLE_ALLOWED_DOMAINS; + return env ? env.split(",") : []; +}