feat: Enforce single team when self-hosted (#1954)

* fix: Enforce single team when self hosting

* test: positive case

* refactor

* fix: Visible error message on login screen for max teams scenario

* Update Notices.js

* lint
This commit is contained in:
Tom Moor 2021-03-18 21:56:24 -07:00 committed by GitHub
parent 138336639d
commit 1b972070d7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 101 additions and 8 deletions

View File

@ -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

View File

@ -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
}
}
}
}

View File

@ -15,6 +15,12 @@ export default function Notices({ notice }: Props) {
signing in with your Google Workspace account.
</NoticeAlert>
)}
{notice === "maximum-teams" && (
<NoticeAlert>
The team you authenticated with is not authorized on this
installation. Try another?
</NoticeAlert>
)}
{notice === "hd-not-allowed" && (
<NoticeAlert>
Sorry, your Google apps domain is not allowed. Please try again with

View File

@ -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",

View File

@ -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<TeamCreatorResult> {
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(
{

View File

@ -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",

View File

@ -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

View File

@ -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");

View File

@ -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(",") : [];
}