feat: scope login attempts to specific subdomains if available - do not switch subdomains (#3741)

* make the user lookup in user creator sensitive to team
* add team specific logic to oidc strat
* factor out slugifyDomain
* change type of req during auth to Koa.Context

app/scenes/Login/Notices.tsx

@@ -57,6 +57,15 @@ export default function Notices() {
             Please try again.
           </NoticeAlert>
         ))}
+      {notice === "invalid-authentication" &&
+        (description ? (
+          <NoticeAlert>{description}</NoticeAlert>
+        ) : (
+          <NoticeAlert>
+            Authentication failed – you do not have permission to access this
+            team.
+          </NoticeAlert>
+        ))}
       {notice === "expired-token" && (
         <NoticeAlert>
           Sorry, it looks like that sign-in link is no longer valid, please try

server/commands/accountProvisioner.ts

@@ -3,6 +3,7 @@ import { UniqueConstraintError } from "sequelize";
 import WelcomeEmail from "@server/emails/templates/WelcomeEmail";
 import {
   AuthenticationError,
+  InvalidAuthenticationError,
   EmailAuthenticationRequiredError,
   AuthenticationProviderDisabledError,
 } from "@server/errors";
@@ -20,6 +21,7 @@ type Props = {
     username?: string;
   };
   team: {
+    id?: string;
     name: string;
     domain?: string;
     subdomain: string;
@@ -56,15 +58,12 @@ async function accountProvisioner({
 
   try {
     result = await teamCreator({
-      name: teamParams.name,
+      ...teamParams,
-      domain: teamParams.domain,
-      subdomain: teamParams.subdomain,
-      avatarUrl: teamParams.avatarUrl,
       authenticationProvider: authenticationProviderParams,
       ip,
     });
   } catch (err) {
-    throw AuthenticationError(err.message);
+    throw InvalidAuthenticationError(err.message);
   }
 
   invariant(result, "Team creator result must exist");

server/commands/teamCreator.ts

@@ -1,6 +1,10 @@
 import { sequelize } from "@server/database/sequelize";
 import env from "@server/env";
-import { DomainNotAllowedError, MaximumTeamsError } from "@server/errors";
+import {
+  InvalidAuthenticationError,
+  DomainNotAllowedError,
+  MaximumTeamsError,
+} from "@server/errors";
 import Logger from "@server/logging/Logger";
 import { APM } from "@server/logging/tracing";
 import { Team, AuthenticationProvider, Event } from "@server/models";
@@ -13,6 +17,7 @@ type TeamCreatorResult = {
 };
 
 type Props = {
+  id?: string;
   name: string;
   domain?: string;
   subdomain: string;
@@ -25,6 +30,7 @@ type Props = {
 };
 
 async function teamCreator({
+  id,
   name,
   domain,
   subdomain,
@@ -33,7 +39,9 @@ async function teamCreator({
   ip,
 }: Props): Promise<TeamCreatorResult> {
   let authP = await AuthenticationProvider.findOne({
-    where: authenticationProvider,
+    where: id
+      ? { ...authenticationProvider, teamId: id }
+      : authenticationProvider,
     include: [
       {
         model: Team,
@@ -52,6 +60,11 @@ async function teamCreator({
       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");
+  }
 
   // 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

server/commands/userCreator.ts

@@ -47,6 +47,8 @@ export default async function userCreator({
       {
         model: User,
         as: "user",
+        where: { teamId },
+        required: true,
       },
     ],
   });

server/errors.ts

@@ -1,9 +1,8 @@
 import httpErrors from "http-errors";
-import env from "./env";
 
 export function AuthenticationError(
-  message = "Invalid authentication",
+  message = "Authentication required",
-  redirectUrl = env.URL
+  redirectUrl = "/"
 ) {
   return httpErrors(401, message, {
     redirectUrl,
@@ -11,6 +10,16 @@ export function AuthenticationError(
   });
 }
 
+export function InvalidAuthenticationError(
+  message = "Invalid authentication",
+  redirectUrl = "/"
+) {
+  return httpErrors(401, message, {
+    redirectUrl,
+    id: "invalid_authentication",
+  });
+}
+
 export function AuthorizationError(
   message = "You do not have permission to access this resource"
 ) {
@@ -112,7 +121,7 @@ export function MaximumTeamsError(
 
 export function EmailAuthenticationRequiredError(
   message = "User must authenticate with email",
-  redirectUrl = env.URL
+  redirectUrl = "/"
 ) {
   return httpErrors(400, message, {
     redirectUrl,
@@ -164,7 +173,7 @@ export function OIDCMalformedUserInfoError(
 
 export function AuthenticationProviderDisabledError(
   message = "Authentication method has been disabled by an admin",
-  redirectUrl = env.URL
+  redirectUrl = "/"
 ) {
   return httpErrors(400, message, {
     redirectUrl,

server/routes/auth/providers/azure.ts

@@ -1,7 +1,7 @@
 import passport from "@outlinewiki/koa-passport";
 import { Strategy as AzureStrategy } from "@outlinewiki/passport-azure-ad-oauth2";
 import jwt from "jsonwebtoken";
-import { Request } from "koa";
+import type { Context } from "koa";
 import Router from "koa-router";
 import { Profile } from "passport";
 import accountProvisioner, {
@@ -11,7 +11,11 @@ import env from "@server/env";
 import { MicrosoftGraphError } from "@server/errors";
 import passportMiddleware from "@server/middlewares/passport";
 import { User } from "@server/models";
-import { StateStore, request } from "@server/utils/passport";
+import {
+  StateStore,
+  request,
+  getTeamFromContext,
+} from "@server/utils/passport";
 
 const router = new Router();
 const providerName = "azure";
@@ -36,7 +40,7 @@ if (env.AZURE_CLIENT_ID && env.AZURE_CLIENT_SECRET) {
       scope: scopes,
     },
     async function (
-      req: Request,
+      ctx: Context,
       accessToken: string,
       refreshToken: string,
       params: { expires_in: number; id_token: string },
@@ -88,12 +92,15 @@ 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 teamName = organization.displayName;
         const result = await accountProvisioner({
-          ip: req.ip,
+          ip: ctx.ip,
           team: {
+            id: team?.id,
             name: teamName,
             domain,
             subdomain,

server/routes/auth/providers/google.ts

@@ -1,10 +1,10 @@
 import passport from "@outlinewiki/koa-passport";
-import type { Request } from "express";
+import type { Context } from "koa";
 import Router from "koa-router";
 import { capitalize } from "lodash";
 import { Profile } from "passport";
 import { Strategy as GoogleStrategy } from "passport-google-oauth2";
-import { parseDomain } from "@shared/utils/domains";
+import { slugifyDomain } from "@shared/utils/domains";
 import accountProvisioner, {
   AccountProvisionerResult,
 } from "@server/commands/accountProvisioner";
@@ -15,8 +15,8 @@ import {
   TeamDomainRequiredError,
 } from "@server/errors";
 import passportMiddleware from "@server/middlewares/passport";
-import { Team, User } from "@server/models";
+import { User } from "@server/models";
-import { StateStore, parseState } from "@server/utils/passport";
+import { StateStore, getTeamFromContext } from "@server/utils/passport";
 
 const router = new Router();
 const providerName = "google";
@@ -51,7 +51,7 @@ if (env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET) {
         scope: scopes,
       },
       async function (
-        req: Request,
+        ctx: Context,
         accessToken: string,
         refreshToken: string,
         params: { expires_in: number },
@@ -63,27 +63,32 @@ if (env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET) {
         ) => void
       ) {
         try {
-          const state = req.cookies.get("state");
-          const host = state ? parseState(state).host : req.hostname;
-          // appDomain is the domain the user originated from when attempting auth
-          const appDomain = parseDomain(host);
-
           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) {
-            const subdomain = domain.split(".")[0];
+            // 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: req.ip,
+              ip: ctx.ip,
               team: {
+                id: team?.id,
                 name: teamName,
                 domain,
                 subdomain,
@@ -107,19 +112,7 @@ if (env.GOOGLE_CLIENT_ID && env.GOOGLE_CLIENT_SECRET) {
             });
           } else {
             // No domain means it's a personal Gmail account
-            // We only allow sign-in to existing user accounts
+            // We only allow sign-in to existing user accounts with these
-
-            let team;
-            if (appDomain.custom) {
-              team = await Team.findOne({ where: { domain: appDomain.host } });
-            } else if (env.SUBDOMAINS_ENABLED && appDomain.teamSubdomain) {
-              team = await Team.findOne({
-                where: { subdomain: appDomain.teamSubdomain },
-              });
-            } else if (env.DEPLOYMENT !== "hosted") {
-              team = await Team.findOne();
-            }
-
             if (!team) {
               // No team usually means this is the apex domain
               // Throw different errors depending on whether we think the user is

server/routes/auth/providers/oidc.ts

@@ -1,8 +1,9 @@
 import passport from "@outlinewiki/koa-passport";
-import { Request } from "koa";
+import type { Context } from "koa";
 import Router from "koa-router";
 import { get } from "lodash";
 import { Strategy } from "passport-oauth2";
+import { slugifyDomain } from "@shared/utils/domains";
 import accountProvisioner, {
   AccountProvisionerResult,
 } from "@server/commands/accountProvisioner";
@@ -13,7 +14,11 @@ import {
 } from "@server/errors";
 import passportMiddleware from "@server/middlewares/passport";
 import { User } from "@server/models";
-import { StateStore, request } from "@server/utils/passport";
+import {
+  StateStore,
+  request,
+  getTeamFromContext,
+} from "@server/utils/passport";
 
 const router = new Router();
 const providerName = "oidc";
@@ -60,7 +65,7 @@ if (env.OIDC_CLIENT_ID && env.OIDC_CLIENT_SECRET) {
       // Any claim supplied in response to the userinfo request will be
       // available on the `profile` parameter
       async function (
-        req: Request,
+        ctx: Context,
         accessToken: string,
         refreshToken: string,
         params: { expires_in: number },
@@ -77,6 +82,7 @@ if (env.OIDC_CLIENT_ID && env.OIDC_CLIENT_SECRET) {
               `An email field was not returned in the profile parameter, but is required.`
             );
           }
+          const team = await getTeamFromContext(ctx);
 
           const parts = profile.email.toLowerCase().split("@");
           const domain = parts.length && parts[1];
@@ -85,10 +91,13 @@ if (env.OIDC_CLIENT_ID && env.OIDC_CLIENT_SECRET) {
             throw OIDCMalformedUserInfoError();
           }
 
-          const subdomain = domain.split(".")[0];
+          // remove the TLD and form a subdomain from the remaining
+          const subdomain = slugifyDomain(domain);
+
           const result = await accountProvisioner({
-            ip: req.ip,
+            ip: ctx.ip,
             team: {
+              id: team?.id,
               // https://github.com/outline/outline/pull/2388#discussion_r681120223
               name: "Wiki",
               domain,

server/routes/auth/providers/slack.ts

@@ -1,5 +1,5 @@
 import passport from "@outlinewiki/koa-passport";
-import { Request } from "koa";
+import type { Context } from "koa";
 import Router from "koa-router";
 import { Profile } from "passport";
 import { Strategy as SlackStrategy } from "passport-slack-oauth2";
@@ -16,7 +16,7 @@ import {
   Team,
   User,
 } from "@server/models";
-import { StateStore } from "@server/utils/passport";
+import { getTeamFromContext, StateStore } from "@server/utils/passport";
 import * as Slack from "@server/utils/slack";
 import { assertPresent, assertUuid } from "@server/validation";
 
@@ -63,7 +63,7 @@ if (env.SLACK_CLIENT_ID && env.SLACK_CLIENT_SECRET) {
       scope: scopes,
     },
     async function (
-      req: Request,
+      ctx: Context,
       accessToken: string,
       refreshToken: string,
       params: { expires_in: number },
@@ -75,9 +75,11 @@ if (env.SLACK_CLIENT_ID && env.SLACK_CLIENT_SECRET) {
       ) => void
     ) {
       try {
+        const team = await getTeamFromContext(ctx);
         const result = await accountProvisioner({
-          ip: req.ip,
+          ip: ctx.ip,
           team: {
+            id: team?.id,
             name: profile.team.name,
             subdomain: profile.team.domain,
             avatarUrl: profile.team.image_230,

server/utils/passport.ts

@@ -1,40 +1,42 @@
 import crypto from "crypto";
 import { addMinutes, subMinutes } from "date-fns";
-import type { Request } from "express";
 import fetch from "fetch-with-proxy";
+import type { Context } from "koa";
 import {
   StateStoreStoreCallback,
   StateStoreVerifyCallback,
 } from "passport-oauth2";
 import { getCookieDomain, parseDomain } from "@shared/utils/domains";
+import env from "@server/env";
+import { Team } from "@server/models";
 import { AuthRedirectError, OAuthStateMismatchError } from "../errors";
 
 export class StateStore {
   key = "state";
 
-  store = (req: Request, callback: StateStoreStoreCallback) => {
+  store = (ctx: Context, callback: StateStoreStoreCallback) => {
     // token is a short lived one-time pad to prevent replay attacks
     // appDomain is the domain the user originated from when attempting auth
     // we expect it to be a team subdomain, custom domain, or apex domain
     const token = crypto.randomBytes(8).toString("hex");
-    const appDomain = parseDomain(req.hostname);
+    const appDomain = parseDomain(ctx.hostname);
     const state = buildState(appDomain.host, token);
 
-    req.cookies.set(this.key, state, {
+    ctx.cookies.set(this.key, state, {
       httpOnly: false,
       expires: addMinutes(new Date(), 10),
-      domain: getCookieDomain(req.hostname),
+      domain: getCookieDomain(ctx.hostname),
     });
 
     callback(null, token);
   };
 
   verify = (
-    req: Request,
+    ctx: Context,
     providedToken: string,
     callback: StateStoreVerifyCallback
   ) => {
-    const state = req.cookies.get(this.key);
+    const state = ctx.cookies.get(this.key);
 
     if (!state) {
       return callback(
@@ -51,10 +53,10 @@ export class StateStore {
     // If there is an error during auth, the user will end up on the same domain
     // that they started from.
     const appDomain = parseDomain(host);
-    if (appDomain.host !== parseDomain(req.hostname).host) {
+    if (appDomain.host !== parseDomain(ctx.hostname).host) {
-      const reqProtocol = req.protocol;
+      const reqProtocol = ctx.protocol;
-      const requestHost = req.get("host");
+      const requestHost = ctx.get("host");
-      const requestPath = req.originalUrl;
+      const requestPath = ctx.originalUrl;
       const requestUrl = `${reqProtocol}://${requestHost}${requestPath}`;
       const url = new URL(requestUrl);
 
@@ -64,10 +66,10 @@ export class StateStore {
     }
 
     // Destroy the one-time pad token and ensure it matches
-    req.cookies.set(this.key, "", {
+    ctx.cookies.set(this.key, "", {
       httpOnly: false,
       expires: subMinutes(new Date(), 1),
-      domain: getCookieDomain(req.hostname),
+      domain: getCookieDomain(ctx.hostname),
     });
 
     if (!token || token !== providedToken) {
@@ -98,3 +100,24 @@ export function parseState(state: string) {
   const [host, token] = state.split("|");
   return { host, token };
 }
+
+export async function getTeamFromContext(ctx: Context) {
+  // "domain" is the domain the user came from when attempting auth
+  // we use it to infer the team they intend on signing into
+  const state = ctx.cookies.get("state");
+  const host = state ? parseState(state).host : ctx.hostname;
+  const domain = parseDomain(host);
+
+  let team;
+  if (env.DEPLOYMENT !== "hosted") {
+    team = await Team.findOne();
+  } else if (domain.custom) {
+    team = await Team.findOne({ where: { domain: domain.host } });
+  } else if (env.SUBDOMAINS_ENABLED && domain.teamSubdomain) {
+    team = await Team.findOne({
+      where: { subdomain: domain.teamSubdomain },
+    });
+  }
+
+  return team;
+}

shared/utils/domains.test.ts

@@ -1,5 +1,5 @@
 import env from "@shared/env";
-import { parseDomain, getCookieDomain } from "./domains";
+import { parseDomain, getCookieDomain, slugifyDomain } from "./domains";
 
 // test suite is based on subset of parse-domain module we want to support
 // https://github.com/peerigon/parse-domain/blob/master/test/parseDomain.test.js
@@ -158,6 +158,14 @@ describe("#parseDomain", () => {
   });
 });
 
+describe("#slugifyDomain", () => {
+  it("strips the last . delineated segment from strings", () => {
+    expect(slugifyDomain("foo.co")).toBe("foo");
+    expect(slugifyDomain("foo.co.uk")).toBe("foo-co");
+    expect(slugifyDomain("www.foo.co.uk")).toBe("www-foo-co");
+  });
+});
+
 describe("#getCookieDomain", () => {
   beforeEach(() => {
     env.URL = "https://example.com";

shared/utils/domains.ts

@@ -7,6 +7,16 @@ type Domain = {
   custom: boolean;
 };
 
+/**
+ * Removes the the top level domain from the argument and slugifies it
+ *
+ * @param domain Domain string to slugify
+ * @returns String with only non top-level domains
+ */
+export function slugifyDomain(domain: string) {
+  return domain.split(".").slice(0, -1).join("-");
+}
+
 // strips protocol and whitespace from input
 // then strips the path and query string
 function normalizeUrl(url: string) {