diff --git a/app/scenes/Login/Notices.tsx b/app/scenes/Login/Notices.tsx index 6b3a890c5..c43816dee 100644 --- a/app/scenes/Login/Notices.tsx +++ b/app/scenes/Login/Notices.tsx @@ -57,6 +57,15 @@ export default function Notices() { Please try again. ))} + {notice === "invalid-authentication" && + (description ? ( + {description} + ) : ( + + Authentication failed – you do not have permission to access this + team. + + ))} {notice === "expired-token" && ( Sorry, it looks like that sign-in link is no longer valid, please try diff --git a/server/commands/accountProvisioner.ts b/server/commands/accountProvisioner.ts index ef60088fc..27dabc5a6 100644 --- a/server/commands/accountProvisioner.ts +++ b/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, - domain: teamParams.domain, - subdomain: teamParams.subdomain, - avatarUrl: teamParams.avatarUrl, + ...teamParams, authenticationProvider: authenticationProviderParams, ip, }); } catch (err) { - throw AuthenticationError(err.message); + throw InvalidAuthenticationError(err.message); } invariant(result, "Team creator result must exist"); diff --git a/server/commands/teamCreator.ts b/server/commands/teamCreator.ts index 2c175ef1e..7d43f146a 100644 --- a/server/commands/teamCreator.ts +++ b/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 { 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 diff --git a/server/commands/userCreator.ts b/server/commands/userCreator.ts index 59448f67b..f9c6a9bfb 100644 --- a/server/commands/userCreator.ts +++ b/server/commands/userCreator.ts @@ -47,6 +47,8 @@ export default async function userCreator({ { model: User, as: "user", + where: { teamId }, + required: true, }, ], }); diff --git a/server/errors.ts b/server/errors.ts index 40233102a..d1735581e 100644 --- a/server/errors.ts +++ b/server/errors.ts @@ -1,9 +1,8 @@ import httpErrors from "http-errors"; -import env from "./env"; export function AuthenticationError( - message = "Invalid authentication", - redirectUrl = env.URL + message = "Authentication required", + 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, diff --git a/server/routes/auth/providers/azure.ts b/server/routes/auth/providers/azure.ts index 9c2ad25ef..0e99e4bf0 100644 --- a/server/routes/auth/providers/azure.ts +++ b/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, diff --git a/server/routes/auth/providers/google.ts b/server/routes/auth/providers/google.ts index a1251e11d..16af9d912 100644 --- a/server/routes/auth/providers/google.ts +++ b/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 { StateStore, parseState } from "@server/utils/passport"; +import { User } from "@server/models"; +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 - - 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(); - } - + // 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 diff --git a/server/routes/auth/providers/oidc.ts b/server/routes/auth/providers/oidc.ts index 276c204ba..625a36d10 100644 --- a/server/routes/auth/providers/oidc.ts +++ b/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, diff --git a/server/routes/auth/providers/slack.ts b/server/routes/auth/providers/slack.ts index f020edda7..ba2126f36 100644 --- a/server/routes/auth/providers/slack.ts +++ b/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, diff --git a/server/utils/passport.ts b/server/utils/passport.ts index 34f43e6be..95915911e 100644 --- a/server/utils/passport.ts +++ b/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) { - const reqProtocol = req.protocol; - const requestHost = req.get("host"); - const requestPath = req.originalUrl; + if (appDomain.host !== parseDomain(ctx.hostname).host) { + const reqProtocol = ctx.protocol; + const requestHost = ctx.get("host"); + 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; +} diff --git a/shared/utils/domains.test.ts b/shared/utils/domains.test.ts index d18f2bc30..3746e7b0c 100644 --- a/shared/utils/domains.test.ts +++ b/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"; diff --git a/shared/utils/domains.ts b/shared/utils/domains.ts index a27b8633a..0dc96c5fd 100644 --- a/shared/utils/domains.ts +++ b/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) {