From e264094412a1a6797757b4b6c71073049afca5cb Mon Sep 17 00:00:00 2001 From: George Gevoian Date: Mon, 14 Feb 2022 13:26:21 -0800 Subject: [PATCH] (core) Add account page option to allow Google login Summary: Enabled by default, the new checkbox is only visible to users logged in with email/password, and controls whether it is possible to log in to the same account via a Google account (with matching email). When disabled, CognitoClient will refuse logins from Google if a Grist account with the same email exists. Test Plan: Server and browser tests for setting flag. Manual tests to verify Cognito doesn't allow signing in with Google when flag is disabled. Reviewers: paulfitz Reviewed By: paulfitz Differential Revision: https://phab.getgrist.com/D3257 --- app/client/ui/AccountPage.ts | 22 ++++++++++++++- app/common/LoginSessionAPI.ts | 1 + app/common/UserAPI.ts | 14 ++++++++++ app/gen-server/ApiServer.ts | 28 +++++++++++++++++-- app/gen-server/entity/User.ts | 5 ++++ app/gen-server/lib/HomeDBManager.ts | 10 +++++++ .../migration/1644363380225-UserOptions.ts | 17 +++++++++++ app/server/lib/requestUtils.ts | 28 +++++++++++++++---- 8 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 app/gen-server/migration/1644363380225-UserOptions.ts diff --git a/app/client/ui/AccountPage.ts b/app/client/ui/AccountPage.ts index 9e7243a2..6d1b1b2e 100644 --- a/app/client/ui/AccountPage.ts +++ b/app/client/ui/AccountPage.ts @@ -10,6 +10,7 @@ import {transientInput} from 'app/client/ui/transientInput'; import {buildNameWarningsDom, checkName} from 'app/client/ui/WelcomePage'; import {bigBasicButton, bigPrimaryButtonLink} from 'app/client/ui2018/buttons'; import {cssBreadcrumbs, cssBreadcrumbsLink, separator} from 'app/client/ui2018/breadcrumbs'; +import {labeledSquareCheckbox} from 'app/client/ui2018/checkbox'; import {icon} from 'app/client/ui2018/icons'; import {cssModalBody, cssModalButtons, cssModalTitle, modal} from 'app/client/ui2018/modals'; import {colors, vars} from 'app/client/ui2018/cssVars'; @@ -28,6 +29,8 @@ export class AccountPage extends Disposable { private _isEditingName = Observable.create(this, false); private _nameEdit = Observable.create(this, ''); private _isNameValid = Computed.create(this, this._nameEdit, (_use, val) => checkName(val)); + private _allowGoogleLogin = Computed.create(this, (use) => use(this._userObs)?.allowGoogleLogin ?? false) + .onWrite((val) => this._updateAllowGooglelogin(val)); constructor(private _appModel: AppModel) { super(); @@ -98,6 +101,14 @@ export class AccountPage extends Disposable { testId('login-method'), ), user.loginMethod !== 'Email + Password' ? null : dom.frag( + cssDataRow( + labeledSquareCheckbox( + this._allowGoogleLogin, + 'Allow signing in to this account with Google', + testId('allow-google-login-checkbox'), + ), + testId('allow-google-login'), + ), cssSubHeaderFullWidth('Two-factor authentication'), cssDescription( "Two-factor authentication is an extra layer of security for your Grist account designed " + @@ -163,10 +174,14 @@ export class AccountPage extends Disposable { private async _fetchAll() { await Promise.all([ - this._fetchUserMfaPreferences(), this._fetchApiKey(), this._fetchUserProfile(), ]); + + const user = this._userObs.get(); + if (user?.loginMethod === 'Email + Password') { + await this._fetchUserMfaPreferences(); + } } private async _updateUserName(val: string) { @@ -176,6 +191,11 @@ export class AccountPage extends Disposable { await this._appModel.api.updateUserName(val); await this._fetchAll(); } + + private async _updateAllowGooglelogin(allowGoogleLogin: boolean) { + await this._appModel.api.updateAllowGoogleLogin(allowGoogleLogin); + await this._fetchUserProfile(); + } } function confirmPwdResetModal(userEmail: string) { diff --git a/app/common/LoginSessionAPI.ts b/app/common/LoginSessionAPI.ts index 69d682e5..4fb10aa3 100644 --- a/app/common/LoginSessionAPI.ts +++ b/app/common/LoginSessionAPI.ts @@ -11,6 +11,7 @@ export interface UserProfile { // have been validated against database. export interface FullUser extends UserProfile { id: number; + allowGoogleLogin?: boolean; // when present, specifies whether logging in via Google is possible. } export interface LoginSessionAPI { diff --git a/app/common/UserAPI.ts b/app/common/UserAPI.ts index 3f23d6c7..82b2302c 100644 --- a/app/common/UserAPI.ts +++ b/app/common/UserAPI.ts @@ -131,6 +131,12 @@ export interface Document extends DocumentProperties { trunkAccess?: roles.Role|null; } +// Non-core options for a user. +export interface UserOptions { + // Whether signing in with Google is allowed. Defaults to true if unset. + allowGoogleLogin?: boolean; +} + export interface PermissionDelta { maxInheritedRole?: roles.BasicRole|null; users?: { @@ -357,6 +363,7 @@ export interface UserAPI { getUserProfile(): Promise; getUserMfaPreferences(): Promise; updateUserName(name: string): Promise; + updateAllowGoogleLogin(allowGoogleLogin: boolean): Promise; getWorker(key: string): Promise; getWorkerAPI(key: string): Promise; getBillingAPI(): BillingAPI; @@ -653,6 +660,13 @@ export class UserAPIImpl extends BaseAPI implements UserAPI { }); } + public async updateAllowGoogleLogin(allowGoogleLogin: boolean): Promise { + await this.request(`${this._url}/api/profile/allowGoogleLogin`, { + method: 'POST', + body: JSON.stringify({allowGoogleLogin}) + }); + } + public async getWorker(key: string): Promise { const json = await this.requestJson(`${this._url}/api/worker/${key}`, { method: 'GET', diff --git a/app/gen-server/ApiServer.ts b/app/gen-server/ApiServer.ts index c5d1ea64..8c358622 100644 --- a/app/gen-server/ApiServer.ts +++ b/app/gen-server/ApiServer.ts @@ -345,7 +345,7 @@ export class ApiServer { // Get user's profile this._app.get('/api/profile/user', expressWrap(async (req, res) => { const fullUser = await this._getFullUser(req); - return sendOkReply(req, res, fullUser); + return sendOkReply(req, res, fullUser, {allowedFields: new Set(['allowGoogleLogin'])}); })); // POST /api/profile/user/name @@ -361,6 +361,24 @@ export class ApiServer { res.sendStatus(200); })); + // POST /api/profile/allowGoogleLogin + // Update user's preference for allowing Google login. + this._app.post('/api/profile/allowGoogleLogin', expressWrap(async (req, res) => { + const userId = getAuthorizedUserId(req); + const fullUser = await this._getFullUser(req); + if (fullUser.loginMethod !== 'Email + Password') { + throw new ApiError('Only users signed in via email can enable/disable Google login', 401); + } + + const allowGoogleLogin: boolean | undefined = req.body.allowGoogleLogin; + if (allowGoogleLogin === undefined) { + throw new ApiError('Missing body param: allowGoogleLogin', 400); + } + + await this._dbManager.updateUserOptions(userId, {allowGoogleLogin}); + res.sendStatus(200); + })); + // GET /api/profile/apikey // Get user's apiKey this._app.get('/api/profile/apikey', expressWrap(async (req, res) => { @@ -471,11 +489,15 @@ export class ApiServer { private async _getFullUser(req: Request): Promise { const mreq = req as RequestWithLogin; const userId = getUserId(mreq); - const fullUser = await this._dbManager.getFullUser(userId); + const user = await this._dbManager.getUser(userId); + if (!user) { throw new ApiError("unable to find user", 400); } + + const fullUser = this._dbManager.makeFullUser(user); const domain = getOrgFromRequest(mreq); const sessionUser = getSessionUser(mreq.session, domain || '', fullUser.email); const loginMethod = sessionUser && sessionUser.profile ? sessionUser.profile.loginMethod : undefined; - return {...fullUser, loginMethod}; + const allowGoogleLogin = user.options?.allowGoogleLogin ?? true; + return {...fullUser, loginMethod, allowGoogleLogin}; } } diff --git a/app/gen-server/entity/User.ts b/app/gen-server/entity/User.ts index bc661cd8..b4de5247 100644 --- a/app/gen-server/entity/User.ts +++ b/app/gen-server/entity/User.ts @@ -1,3 +1,5 @@ +import {UserOptions} from 'app/common/UserAPI'; +import {nativeValues} from 'app/gen-server/lib/values'; import {BaseEntity, Column, Entity, JoinTable, ManyToMany, OneToMany, OneToOne, PrimaryGeneratedColumn} from "typeorm"; @@ -42,6 +44,9 @@ export class User extends BaseEntity { @Column({name: 'is_first_time_user', default: false}) public isFirstTimeUser: boolean; + @Column({name: 'options', type: nativeValues.jsonEntityType, nullable: true}) + public options: UserOptions | null; + /** * Get user's email. Returns undefined if logins has not been joined, or no login * is available diff --git a/app/gen-server/lib/HomeDBManager.ts b/app/gen-server/lib/HomeDBManager.ts index dece65aa..9c276e5b 100644 --- a/app/gen-server/lib/HomeDBManager.ts +++ b/app/gen-server/lib/HomeDBManager.ts @@ -11,6 +11,7 @@ import * as roles from 'app/common/roles'; import {ANONYMOUS_USER_EMAIL, DocumentProperties, EVERYONE_EMAIL, getRealAccess, ManagerDelta, NEW_DOCUMENT_CODE, OrganizationProperties, Organization as OrgInfo, PermissionData, PermissionDelta, SUPPORT_EMAIL, UserAccessData, + UserOptions, WorkspaceProperties} from "app/common/UserAPI"; import {AclRule, AclRuleDoc, AclRuleOrg, AclRuleWs} from "app/gen-server/entity/AclRule"; import {Alias} from "app/gen-server/entity/Alias"; @@ -432,6 +433,15 @@ export class HomeDBManager extends EventEmitter { await user.save(); } + public async updateUserOptions(userId: number, props: Partial) { + const user = await User.findOne(userId); + if (!user) { throw new ApiError("unable to find user", 400); } + + const newOptions = {...(user.options ?? {}), ...props}; + user.options = newOptions; + await user.save(); + } + // Fetch user from login, creating the user if previously unseen, allowing one retry // for an email key conflict failure. This is in case our transaction conflicts with a peer // doing the same thing. This is quite likely if the first page visited by a previously diff --git a/app/gen-server/migration/1644363380225-UserOptions.ts b/app/gen-server/migration/1644363380225-UserOptions.ts new file mode 100644 index 00000000..bdccbe24 --- /dev/null +++ b/app/gen-server/migration/1644363380225-UserOptions.ts @@ -0,0 +1,17 @@ +import {nativeValues} from "app/gen-server/lib/values"; +import {MigrationInterface, QueryRunner, TableColumn} from "typeorm"; + +export class UserOptions1644363380225 implements MigrationInterface { + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.addColumn("users", new TableColumn({ + name: "options", + type: nativeValues.jsonType, + isNullable: true, + })); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.dropColumn("users", "options"); + } +} diff --git a/app/server/lib/requestUtils.ts b/app/server/lib/requestUtils.ts index 3353784a..b604f0b2 100644 --- a/app/server/lib/requestUtils.ts +++ b/app/server/lib/requestUtils.ts @@ -20,7 +20,7 @@ export const TEST_HTTPS_OFFSET = process.env.GRIST_TEST_HTTPS_OFFSET ? // Database fields that we permit in entities but don't want to cross the api. const INTERNAL_FIELDS = new Set(['apiKey', 'billingAccountId', 'firstLoginAt', 'filteredOut', 'ownerId', 'stripeCustomerId', 'stripeSubscriptionId', 'stripePlanId', - 'stripeProductId', 'userId', 'isFirstTimeUser']); + 'stripeProductId', 'userId', 'isFirstTimeUser', 'allowGoogleLogin']); /** * Adapt a home-server or doc-worker URL to match the hostname in the request URL. For custom @@ -159,11 +159,20 @@ export function addPermit(scope: Scope, userId: number, specialPermit: Permit): return {...scope, ...(scope.userId === userId ? {specialPermit} : {})}; } +export interface SendReplyOptions { + allowedFields?: Set; +} + // Return a JSON response reflecting the output of a query. // Filter out keys we don't want crossing the api. // Set req to null to not log any information about request. -export async function sendReply(req: Request|null, res: Response, result: QueryResult) { - const data = pruneAPIResult(result.data || null); +export async function sendReply( + req: Request|null, + res: Response, + result: QueryResult, + options: SendReplyOptions = {}, +) { + const data = pruneAPIResult(result.data || null, options.allowedFields); if (shouldLogApiDetails && req) { const mreq = req as RequestWithLogin; log.rawDebug('api call', { @@ -183,11 +192,16 @@ export async function sendReply(req: Request|null, res: Response, result: Que } } -export async function sendOkReply(req: Request|null, res: Response, result?: T) { - return sendReply(req, res, {status: 200, data: result}); +export async function sendOkReply( + req: Request|null, + res: Response, + result?: T, + options: SendReplyOptions = {} +) { + return sendReply(req, res, {status: 200, data: result}, options); } -export function pruneAPIResult(data: T): T { +export function pruneAPIResult(data: T, allowedFields?: Set): T { // TODO: This can be optimized by pruning data recursively without serializing in between. But // it's fairly fast even with serializing (on the order of 15usec/kb). const output = JSON.stringify(data, @@ -197,6 +211,8 @@ export function pruneAPIResult(data: T): T { if (key === 'removedAt' && value === null) { return undefined; } // Don't bother sending option fields if there are no options set. if (key === 'options' && value === null) { return undefined; } + // Don't prune anything that is explicitly allowed. + if (allowedFields?.has(key)) { return value; } return INTERNAL_FIELDS.has(key) ? undefined : value; }); return JSON.parse(output);