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
This commit is contained in:
Nan Yu
2022-06-20 01:33:16 -07:00
committed by GitHub
parent 188c1e409b
commit e0d2b6cace
6 changed files with 158 additions and 56 deletions

View File

@@ -9,10 +9,13 @@ export default function Notices() {
return (
<>
{notice === "google-hd" && (
{notice === "domain-required" && (
<NoticeAlert>
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.
<hr />
If you were invited to a team, you will find a link to it in the
invite email.
</NoticeAlert>
)}
{notice === "maximum-teams" && (
@@ -21,7 +24,7 @@ export default function Notices() {
installation. Try another?
</NoticeAlert>
)}
{notice === "malformed_user_info" && (
{notice === "malformed-user-info" && (
<NoticeAlert>
We could not read the user info supplied by your identity provider.
</NoticeAlert>
@@ -38,7 +41,7 @@ export default function Notices() {
try again in a few minutes.
</NoticeAlert>
)}
{notice === "auth-error" &&
{(notice === "auth-error" || notice === "state-mismatch") &&
(description ? (
<NoticeAlert>{description}</NoticeAlert>
) : (

View File

@@ -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,
});
}

View File

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

View File

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

View File

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

View File

@@ -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})$/;