From e0d2b6cace743248369a69f05e772f08a20a741e Mon Sep 17 00:00:00 2001 From: Nan Yu Date: Mon, 20 Jun 2022 01:33:16 -0700 Subject: [PATCH] feat: allow personal gmail accounts to be used to sign into teams with an existing invite (#3652) * feat: allow personal gmail accounts to be used to sign into teams with an existing invite * address comments * add comment for appDomain * address comments --- app/scenes/Login/Notices.tsx | 13 +-- server/errors.ts | 16 +++- server/routes/auth/providers/google.ts | 109 +++++++++++++++++-------- server/utils/passport.ts | 67 +++++++++++---- shared/utils/domains.test.ts | 7 ++ shared/utils/urlHelpers.ts | 2 +- 6 files changed, 158 insertions(+), 56 deletions(-) diff --git a/app/scenes/Login/Notices.tsx b/app/scenes/Login/Notices.tsx index 6d7de2b90..480222986 100644 --- a/app/scenes/Login/Notices.tsx +++ b/app/scenes/Login/Notices.tsx @@ -9,10 +9,13 @@ export default function Notices() { return ( <> - {notice === "google-hd" && ( + {notice === "domain-required" && ( - Sorry, Google sign in cannot be used with a personal email. Please try - signing in with your Google Workspace account. + Unable to sign-in. Please navigate to your team's custom URL, then try + to sign-in again. +
+ If you were invited to a team, you will find a link to it in the + invite email. )} {notice === "maximum-teams" && ( @@ -21,7 +24,7 @@ export default function Notices() { installation. Try another? )} - {notice === "malformed_user_info" && ( + {notice === "malformed-user-info" && ( We could not read the user info supplied by your identity provider. @@ -38,7 +41,7 @@ export default function Notices() { try again in a few minutes. )} - {notice === "auth-error" && + {(notice === "auth-error" || notice === "state-mismatch") && (description ? ( {description} ) : ( diff --git a/server/errors.ts b/server/errors.ts index 1730e2449..42358c6d3 100644 --- a/server/errors.ts +++ b/server/errors.ts @@ -128,11 +128,21 @@ export function MicrosoftGraphError( }); } -export function GoogleWorkspaceRequiredError( - message = "Google Workspace is required to authenticate" +export function TeamDomainRequiredError( + message = "Unable to determine team from current domain or subdomain" ) { return httpErrors(400, message, { - id: "google_hd", + id: "domain_required", + }); +} + +export function AuthRedirectError( + message = "Redirect to the correct domain after authentication", + redirectUrl: string +) { + return httpErrors(400, message, { + id: "auth_redirect", + redirectUrl, }); } diff --git a/server/routes/auth/providers/google.ts b/server/routes/auth/providers/google.ts index 899d74edd..ffb5641f8 100644 --- a/server/routes/auth/providers/google.ts +++ b/server/routes/auth/providers/google.ts @@ -1,17 +1,18 @@ import passport from "@outlinewiki/koa-passport"; -import { Request } from "koa"; +import type { Request } from "express"; 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 accountProvisioner, { AccountProvisionerResult, } from "@server/commands/accountProvisioner"; import env from "@server/env"; -import { GoogleWorkspaceRequiredError } from "@server/errors"; +import { InviteRequiredError, TeamDomainRequiredError } from "@server/errors"; import passportMiddleware from "@server/middlewares/passport"; -import { User } from "@server/models"; -import { StateStore } from "@server/utils/passport"; +import { Team, User } from "@server/models"; +import { StateStore, parseState } from "@server/utils/passport"; const router = new Router(); const providerName = "google"; @@ -58,38 +59,82 @@ 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; const domain = profile._json.hd; - if (!domain) { - throw GoogleWorkspaceRequiredError(); + // 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]; + const teamName = capitalize(subdomain); + + result = await accountProvisioner({ + ip: req.ip, + team: { + name: teamName, + domain, + subdomain, + }, + user: { + email: profile.email, + name: profile.displayName, + avatarUrl: profile.picture, + }, + authenticationProvider: { + name: providerName, + providerId: domain, + }, + authentication: { + providerId: profile.id, + accessToken, + refreshToken, + expiresIn: params.expires_in, + scopes, + }, + }); + } else { + // No domain means it's a personal Gmail account + // We only allow sign-in to existing invites here + 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) { + throw TeamDomainRequiredError(); + } + + const user = await User.findOne({ + where: { teamId: team.id, email: profile.email.toLowerCase() }, + }); + + if (!user) { + throw InviteRequiredError(); + } + + await user.update({ + lastActiveAt: new Date(), + }); + + result = { + user, + team, + isNewUser: false, + isNewTeam: false, + }; } - const subdomain = domain.split(".")[0]; - const teamName = capitalize(subdomain); - const result = await accountProvisioner({ - ip: req.ip, - team: { - name: teamName, - domain, - subdomain, - }, - user: { - email: profile.email, - name: profile.displayName, - avatarUrl: profile.picture, - }, - authenticationProvider: { - name: providerName, - providerId: domain, - }, - authentication: { - providerId: profile.id, - accessToken, - refreshToken, - expiresIn: params.expires_in, - scopes, - }, - }); return done(null, result.user, result); } catch (err) { return done(err, null); diff --git a/server/utils/passport.ts b/server/utils/passport.ts index 0eeefc73d..dd11fdbd1 100644 --- a/server/utils/passport.ts +++ b/server/utils/passport.ts @@ -6,31 +6,35 @@ import { StateStoreStoreCallback, StateStoreVerifyCallback, } from "passport-oauth2"; -import { getCookieDomain } from "@shared/utils/domains"; -import { OAuthStateMismatchError } from "../errors"; +import { getCookieDomain, parseDomain } from "@shared/utils/domains"; +import { AuthRedirectError, OAuthStateMismatchError } from "../errors"; export class StateStore { key = "state"; - store = (ctx: Request, callback: StateStoreStoreCallback) => { - // Produce a random string as state - const state = crypto.randomBytes(8).toString("hex"); + store = (req: Request, 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 state = buildState(appDomain.host, token); - ctx.cookies.set(this.key, state, { + req.cookies.set(this.key, state, { httpOnly: false, expires: addMinutes(new Date(), 10), - domain: getCookieDomain(ctx.hostname), + domain: getCookieDomain(req.hostname), }); - callback(null, state); + callback(null, token); }; verify = ( - ctx: Request, - providedState: string, + req: Request, + providedToken: string, callback: StateStoreVerifyCallback ) => { - const state = ctx.cookies.get(this.key); + const state = req.cookies.get(this.key); if (!state) { return callback( @@ -40,14 +44,38 @@ export class StateStore { ); } - ctx.cookies.set(this.key, "", { + const { host, token } = parseState(state); + + // Oauth callbacks are hard-coded to come to the apex domain, so we + // redirect to the original app domain before attempting authentication. + // 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; + const requestUrl = `${reqProtocol}://${requestHost}${requestPath}`; + const url = new URL(requestUrl); + + url.host = appDomain.host; + + return callback( + AuthRedirectError(`redirect to: ${url.toString()}`, url.toString()), + false, + token + ); + } + + // Destroy the one-time pad token and ensure it matches + req.cookies.set(this.key, "", { httpOnly: false, expires: subMinutes(new Date(), 1), - domain: getCookieDomain(ctx.hostname), + domain: getCookieDomain(req.hostname), }); - if (state !== providedState) { - return callback(OAuthStateMismatchError(), false, state); + if (!token || token !== providedToken) { + return callback(OAuthStateMismatchError(), false, token); } // @ts-expect-error Type in library is wrong @@ -66,3 +94,12 @@ export async function request(endpoint: string, accessToken: string) { }); return response.json(); } + +function buildState(host: string, token: string) { + return [host, token].join("|"); +} + +export function parseState(state: string) { + const [host, token] = state.split("|"); + return { host, token }; +} diff --git a/shared/utils/domains.test.ts b/shared/utils/domains.test.ts index ebec54c91..d18f2bc30 100644 --- a/shared/utils/domains.test.ts +++ b/shared/utils/domains.test.ts @@ -42,6 +42,13 @@ describe("#parseDomain", () => { }); }); + it("should return the same result when parsing the returned host", () => { + const customDomain = parseDomain("www.example.com"); + const subDomain = parseDomain("myteam.example.com"); + expect(parseDomain(customDomain.host)).toMatchObject(customDomain); + expect(parseDomain(subDomain.host)).toMatchObject(subDomain); + }); + it("should remove the path", () => { expect(parseDomain("example.com/some/path?and&query")).toMatchObject({ teamSubdomain: "", diff --git a/shared/utils/urlHelpers.ts b/shared/utils/urlHelpers.ts index 76215feb4..6e8c63c14 100644 --- a/shared/utils/urlHelpers.ts +++ b/shared/utils/urlHelpers.ts @@ -49,7 +49,7 @@ export function changelogUrl(): string { } export function signin(service = "slack"): string { - return `${env.URL}/auth/${service}`; + return `/auth/${service}`; } export const SLUG_URL_REGEX = /^(?:[0-9a-zA-Z-_~]*-)?([a-zA-Z0-9]{10,15})$/;