From b52b1b02fe62b5e51a89935c1937d83724f9f45a Mon Sep 17 00:00:00 2001
From: Eugene Sokolov <eugene@eug-vs.xyz>
Date: Mon, 24 Jan 2022 01:40:18 +0000
Subject: [PATCH] Fix: consistently check allowed domains (#2985)

* fix: ensure consistency of checking allowed domain

* chore: update comment to match the logic
---
 server/commands/teamCreator.ts         | 8 ++++----
 server/routes/auth/providers/google.ts | 5 ++---
 server/routes/auth/providers/oidc.ts   | 5 ++---
 server/utils/authentication.ts         | 5 +++++
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/server/commands/teamCreator.ts b/server/commands/teamCreator.ts
index 6cbec8286..87fbf449c 100644
--- a/server/commands/teamCreator.ts
+++ b/server/commands/teamCreator.ts
@@ -1,7 +1,7 @@
 import invariant from "invariant";
 import Logger from "@server/logging/logger";
 import { Team, AuthenticationProvider } from "@server/models";
-import { getAllowedDomains } from "@server/utils/authentication";
+import { isDomainAllowed } from "@server/utils/authentication";
 import { generateAvatarUrl } from "@server/utils/avatars";
 import { MaximumTeamsError } from "../errors";
 
@@ -57,9 +57,9 @@ export default async function teamCreator({
     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)) {
+    // new team is allowed then assign the authentication provider to the
+    // existing team
+    if (teamCount === 1 && domain && isDomainAllowed(domain)) {
       const team = await Team.findOne();
       invariant(team, "Team should exist");
 
diff --git a/server/routes/auth/providers/google.ts b/server/routes/auth/providers/google.ts
index e5d34cb5d..f8ccb52a4 100644
--- a/server/routes/auth/providers/google.ts
+++ b/server/routes/auth/providers/google.ts
@@ -10,14 +10,13 @@ import {
   GoogleWorkspaceInvalidError,
 } from "@server/errors";
 import passportMiddleware from "@server/middlewares/passport";
-import { getAllowedDomains } from "@server/utils/authentication";
+import { isDomainAllowed } from "@server/utils/authentication";
 import { StateStore } from "@server/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 allowedDomains = getAllowedDomains();
 const scopes = [
   "https://www.googleapis.com/auth/userinfo.profile",
   "https://www.googleapis.com/auth/userinfo.email",
@@ -48,7 +47,7 @@ if (GOOGLE_CLIENT_ID) {
             throw GoogleWorkspaceRequiredError();
           }
 
-          if (allowedDomains.length && !allowedDomains.includes(domain)) {
+          if (!isDomainAllowed(domain)) {
             throw GoogleWorkspaceInvalidError();
           }
 
diff --git a/server/routes/auth/providers/oidc.ts b/server/routes/auth/providers/oidc.ts
index 7a6f9e3f1..b25c5a4f4 100644
--- a/server/routes/auth/providers/oidc.ts
+++ b/server/routes/auth/providers/oidc.ts
@@ -9,7 +9,7 @@ import {
   AuthenticationError,
 } from "@server/errors";
 import passportMiddleware from "@server/middlewares/passport";
-import { getAllowedDomains } from "@server/utils/authentication";
+import { isDomainAllowed } from "@server/utils/authentication";
 import { StateStore, request } from "@server/utils/passport";
 
 const router = new Router();
@@ -23,7 +23,6 @@ const OIDC_USERINFO_URI = process.env.OIDC_USERINFO_URI || "";
 const OIDC_SCOPES = process.env.OIDC_SCOPES || "";
 const OIDC_USERNAME_CLAIM =
   process.env.OIDC_USERNAME_CLAIM || "preferred_username";
-const allowedDomains = getAllowedDomains();
 
 export const config = {
   name: OIDC_DISPLAY_NAME,
@@ -84,7 +83,7 @@ if (OIDC_CLIENT_ID) {
             throw OIDCMalformedUserInfoError();
           }
 
-          if (allowedDomains.length && !allowedDomains.includes(domain)) {
+          if (!isDomainAllowed(domain)) {
             throw AuthenticationError(
               `Domain ${domain} is not on the whitelist`
             );
diff --git a/server/utils/authentication.ts b/server/utils/authentication.ts
index 0a900b71f..031aec44e 100644
--- a/server/utils/authentication.ts
+++ b/server/utils/authentication.ts
@@ -12,6 +12,11 @@ export function getAllowedDomains(): string[] {
   return env ? env.split(",") : [];
 }
 
+export function isDomainAllowed(domain: string): boolean {
+  const allowedDomains = getAllowedDomains();
+  return allowedDomains.includes(domain) || allowedDomains.length === 0;
+}
+
 export async function signIn(
   ctx: Context,
   user: User,