From 9e5bf9703970317b616d23fe61c05d2bb6465381 Mon Sep 17 00:00:00 2001 From: Harald Schilly Date: Wed, 28 Aug 2024 15:52:54 +0200 Subject: [PATCH] db/account: refactor checkRequiredSSO and use it to check, if modifying the email_address is allowed or if update_on_login is set, additionally prohibit changing first or last name --- .../database/postgres-server-queries.coffee | 14 +++++++ .../database/postgres/account-queries.ts | 6 ++- src/packages/database/postgres/passport.ts | 14 ++++++- src/packages/database/postgres/types.ts | 3 ++ .../database/settings/get-sso-strategies.ts | 4 +- src/packages/next/components/auth/sso.tsx | 3 +- .../server/accounts/set-email-address.ts | 2 +- .../server/auth/check-email-exclusive-sso.ts | 2 +- .../server/auth/sso/passport-login.ts | 14 ++++--- .../server/auth/sso/unlink-strategy.ts | 2 +- src/packages/server/auth/throttle.ts | 2 +- .../auth-check-required-sso.ts} | 13 +++--- src/packages/util/db-schema/accounts.ts | 40 +++++++++++++++++-- src/packages/util/types/sso.ts | 1 + 14 files changed, 95 insertions(+), 25 deletions(-) rename src/packages/{server/auth/sso/check-required-sso.ts => util/auth-check-required-sso.ts} (96%) diff --git a/src/packages/database/postgres-server-queries.coffee b/src/packages/database/postgres-server-queries.coffee index de60cbdf46..b9886cb8da 100644 --- a/src/packages/database/postgres-server-queries.coffee +++ b/src/packages/database/postgres-server-queries.coffee @@ -61,6 +61,7 @@ read = require('read') {pii_expire} = require("./postgres/pii") passwordHash = require("@cocalc/backend/auth/password-hash").default; registrationTokens = require('./postgres/registration-tokens').default; +getStrategiesSSO = require("@cocalc/database/settings/get-sso-strategies").default; stripe_name = require('@cocalc/util/stripe/name').default; @@ -2647,6 +2648,19 @@ exports.extend_PostgreSQL = (ext) -> class PostgreSQL extends ext return result.rows[0].organization_id return undefined + getStrategiesSSO: () => + return await getStrategiesSSO() + + get_email_address_for_account_id: (account_id) => + result = await @async_query + query : 'SELECT email_address FROM accounts' + where : [ 'account_id :: UUID = $1' ] + params : [ account_id ] + if result.rows.length > 0 + result.rows[0].email_address + else + return undefined + # async registrationTokens: (options, query) => return await registrationTokens(@, options, query) diff --git a/src/packages/database/postgres/account-queries.ts b/src/packages/database/postgres/account-queries.ts index 2a26615110..e4ccd1e49b 100644 --- a/src/packages/database/postgres/account-queries.ts +++ b/src/packages/database/postgres/account-queries.ts @@ -47,7 +47,7 @@ export async function is_paying_customer( // this is like set_account_info_if_different, but only sets the fields if they're not set export async function set_account_info_if_not_set( opts: SetAccountFields, -): Promise { +): Promise<{ email_changed: boolean }> { return await set_account_info_if_different(opts, false); } @@ -55,7 +55,7 @@ export async function set_account_info_if_not_set( export async function set_account_info_if_different( opts: SetAccountFields, overwrite = true, -): Promise { +): Promise<{ email_changed: boolean }> { const columns = ["email_address", "first_name", "last_name"]; // this could throw an error for "no such account" @@ -93,6 +93,8 @@ export async function set_account_info_if_different( account_id: opts.account_id, }); } + + return { email_changed: !!do_email }; } export async function set_account( diff --git a/src/packages/database/postgres/passport.ts b/src/packages/database/postgres/passport.ts index b80588d267..8bc2718068 100644 --- a/src/packages/database/postgres/passport.ts +++ b/src/packages/database/postgres/passport.ts @@ -178,6 +178,7 @@ export async function passport_exists( } } +// this is only used in passport-login/maybeUpdateAccountAndPassport! export async function update_account_and_passport( db: PostgreSQL, opts: UpdateAccountInfoAndPassportOpts, @@ -205,6 +206,7 @@ export async function update_account_and_passport( const existing_account_id = await cb2(db.account_exists, { email_address: opts.email_address, }); + if (!existing_account_id) { // There is no account with the new email address, hence we can update the email address as well upd.email_address = opts.email_address; @@ -214,7 +216,7 @@ export async function update_account_and_passport( } // this set_account_info_if_different checks again if the email exists on another account, but it would throw an error. - await set_account_info_if_different(upd); + const { email_changed } = await set_account_info_if_different(upd); const key = _passport_key(opts); dbg(`updating passport ${to_json({ key, profile: opts.profile })}`); await db.async_query({ @@ -226,4 +228,14 @@ export async function update_account_and_passport( "account_id = $::UUID": opts.account_id, }, }); + + // since we update the email address of an account based on a change from the SSO mechanism + // we can assume the new email address is also "verified" + if (email_changed && typeof upd.email_address === "string") { + await set_email_address_verified({ + db, + account_id: opts.account_id, + email_address: upd.email_address, + }); + } } diff --git a/src/packages/database/postgres/types.ts b/src/packages/database/postgres/types.ts index d8ba457476..faae50ac7b 100644 --- a/src/packages/database/postgres/types.ts +++ b/src/packages/database/postgres/types.ts @@ -14,6 +14,7 @@ import { QueryRows, UntypedQueryResult, } from "@cocalc/util/types/database"; +import type { Strategy } from "@cocalc/util/types/sso"; import { Changes } from "./changefeed"; export type { QueryResult }; @@ -317,6 +318,8 @@ export interface PostgreSQL extends EventEmitter { email_address: string; }>; }): Promise; + + getStrategiesSSO(): Promise; } export interface SetAccountFields { diff --git a/src/packages/database/settings/get-sso-strategies.ts b/src/packages/database/settings/get-sso-strategies.ts index 045c533198..5447b3fa7d 100644 --- a/src/packages/database/settings/get-sso-strategies.ts +++ b/src/packages/database/settings/get-sso-strategies.ts @@ -19,7 +19,8 @@ export default async function getStrategies(): Promise { COALESCE(info -> 'display', conf -> 'display') as display, COALESCE(info -> 'public', conf -> 'public') as public, COALESCE(info -> 'exclusive_domains', conf -> 'exclusive_domains') as exclusive_domains, - COALESCE(info -> 'do_not_hide', 'false'::JSONB) as do_not_hide + COALESCE(info -> 'do_not_hide', 'false'::JSONB) as do_not_hide, + COALESCE(info -> 'update_on_login', 'false'::JSONB) as update_on_login FROM passport_settings WHERE strategy != 'site_conf' @@ -39,6 +40,7 @@ export default async function getStrategies(): Promise { public: row.public ?? true, exclusiveDomains: row.exclusive_domains ?? [], doNotHide: row.do_not_hide ?? false, + updateOnLogin: row.update_on_login ?? false, }; }); } diff --git a/src/packages/next/components/auth/sso.tsx b/src/packages/next/components/auth/sso.tsx index c83b6e5829..7ba333f4e2 100644 --- a/src/packages/next/components/auth/sso.tsx +++ b/src/packages/next/components/auth/sso.tsx @@ -9,7 +9,7 @@ import { join } from "path"; import { CSSProperties, ReactNode, useMemo } from "react"; import { Icon } from "@cocalc/frontend/components/icon"; -import { checkRequiredSSO } from "@cocalc/server/auth/sso/check-required-sso"; +import { checkRequiredSSO } from "@cocalc/util/auth-check-required-sso"; import { PRIMARY_SSO } from "@cocalc/util/types/passport-types"; import { Strategy } from "@cocalc/util/types/sso"; import Loading from "components/share/loading"; @@ -67,6 +67,7 @@ export default function SSO(props: SSOProps) { public: true, exclusiveDomains: [], doNotHide: true, + updateOnLogin: false, }; return ( diff --git a/src/packages/server/accounts/set-email-address.ts b/src/packages/server/accounts/set-email-address.ts index 04ae58bfbc..a2f09a4430 100644 --- a/src/packages/server/accounts/set-email-address.ts +++ b/src/packages/server/accounts/set-email-address.ts @@ -21,7 +21,7 @@ import passwordHash, { verifyPassword, } from "@cocalc/backend/auth/password-hash"; import getPool from "@cocalc/database/pool"; -import { checkRequiredSSO } from "@cocalc/server/auth/sso/check-required-sso"; +import { checkRequiredSSO } from "@cocalc/util/auth-check-required-sso"; import getStrategies from "@cocalc/database/settings/get-sso-strategies"; import { isValidUUID, diff --git a/src/packages/server/auth/check-email-exclusive-sso.ts b/src/packages/server/auth/check-email-exclusive-sso.ts index c8f5654e5c..9bd2fac393 100644 --- a/src/packages/server/auth/check-email-exclusive-sso.ts +++ b/src/packages/server/auth/check-email-exclusive-sso.ts @@ -5,7 +5,7 @@ import { PostgreSQL } from "@cocalc/database/postgres/types"; import getStrategies from "@cocalc/database/settings/get-sso-strategies"; -import { checkRequiredSSO } from "@cocalc/server/auth/sso/check-required-sso"; +import { checkRequiredSSO } from "@cocalc/util/auth-check-required-sso"; export async function checkEmailExclusiveSSO( db: PostgreSQL, diff --git a/src/packages/server/auth/sso/passport-login.ts b/src/packages/server/auth/sso/passport-login.ts index 6074feaacc..214256cdbe 100644 --- a/src/packages/server/auth/sso/passport-login.ts +++ b/src/packages/server/auth/sso/passport-login.ts @@ -13,8 +13,9 @@ * via an SSO strategy, we link this passport to your exsiting account. There is just one exception, * which are SSO strategies which "exclusively" manage a domain. * 2. If you're not signed in and try to sign in, this checks if there is already an account – and creates it if not. - * 3. If you sign in and the SSO strategy is set to "update_on_login", it will reset the name of the user to the - * data from the SSO provider. However, the user can still modify the name. + * 3. If you sign in and the SSO strategy is set to "update_on_login", + * it will reset the name of the user to the data from the SSO provider. + * Users can only modify their first and last name, if that SSO mechanism isn't exclusive! * 4. If you already have an email address belonging to a newly introduced exclusive domain, it will start to be controlled by it. */ @@ -45,8 +46,9 @@ import { sanitizeProfile } from "@cocalc/server/auth/sso/sanitize-profile"; import { callback2 as cb2 } from "@cocalc/util/async-utils"; import { is_valid_email_address } from "@cocalc/util/misc"; import { HELP_EMAIL } from "@cocalc/util/theme"; -import { emailBelongsToDomain, getEmailDomain } from "./check-required-sso"; +import { emailBelongsToDomain } from "@cocalc/util/auth-check-required-sso"; import { SSO_API_KEY_COOKIE_NAME } from "./consts"; +import { getEmailDomain } from "@cocalc/util/auth-check-required-sso"; const logger = getLogger("server:auth:sso:passport-login"); @@ -240,7 +242,7 @@ export class PassportLogin { const exclusiveDomains = strategy.info?.exclusive_domains ?? []; if (!isEmpty(exclusiveDomains)) { for (const email of opts.emails ?? []) { - const emailDomain = getEmailDomain(email.toLocaleLowerCase()); + const emailDomain = getEmailDomain(email.toLowerCase()); for (const ssoDomain of exclusiveDomains) { if (emailBelongsToDomain(emailDomain, ssoDomain)) { return true; @@ -253,7 +255,7 @@ export class PassportLogin { // similar to the above, for a specific email address private checkEmailExclusiveSSO(email_address: string): boolean { - const emailDomain = getEmailDomain(email_address.toLocaleLowerCase()); + const emailDomain = getEmailDomain(email_address.toLowerCase()); for (const strategyName in this.opts.passports) { const strategy = this.opts.passports[strategyName]; for (const ssoDomain of strategy.info?.exclusive_domains ?? []) { @@ -510,7 +512,7 @@ export class PassportLogin { } // We update the email address, if it does not belong to another account. - + if (is_valid_email_address(locals.email_address)) { upd.email_address = locals.email_address; } diff --git a/src/packages/server/auth/sso/unlink-strategy.ts b/src/packages/server/auth/sso/unlink-strategy.ts index 1e75e791eb..af843d8909 100644 --- a/src/packages/server/auth/sso/unlink-strategy.ts +++ b/src/packages/server/auth/sso/unlink-strategy.ts @@ -12,7 +12,7 @@ upstream SSO provider. import getPool from "@cocalc/database/pool"; import getStrategies from "@cocalc/database/settings/get-sso-strategies"; import { is_valid_uuid_string } from "@cocalc/util/misc"; -import { checkRequiredSSO } from "./check-required-sso"; +import { checkRequiredSSO } from "@cocalc/util/auth-check-required-sso"; // The name should be something like "google-9999601658192", i.e., a key // of the passports field. diff --git a/src/packages/server/auth/throttle.ts b/src/packages/server/auth/throttle.ts index e8fab593ae..02bf63b6b7 100644 --- a/src/packages/server/auth/throttle.ts +++ b/src/packages/server/auth/throttle.ts @@ -7,7 +7,7 @@ the database. import LRU from "lru-cache"; import getStrategies from "@cocalc/database/settings/get-sso-strategies"; -import { checkRequiredSSO } from "./sso/check-required-sso"; +import { checkRequiredSSO } from "@cocalc/util/auth-check-required-sso"; const emailShortCache = new LRU({ max: 10000, // avoid memory issues diff --git a/src/packages/server/auth/sso/check-required-sso.ts b/src/packages/util/auth-check-required-sso.ts similarity index 96% rename from src/packages/server/auth/sso/check-required-sso.ts rename to src/packages/util/auth-check-required-sso.ts index 6a92c5cac9..62ed0a76f7 100644 --- a/src/packages/server/auth/sso/check-required-sso.ts +++ b/src/packages/util/auth-check-required-sso.ts @@ -5,18 +5,19 @@ import { Strategy } from "@cocalc/util/types/sso"; -/** - * If the domain of a given email address belongs to an SSO strategy, - * which is configured to be an "exclusive" domain, then return the Strategy. - * This also matches subdomains, i.e. "foo@bar.baz.edu" is goverend by "baz.edu". - */ - interface Opts { email: string | undefined; strategies: Strategy[] | undefined; specificStrategy?: string; } +/** + * If the domain of a given email address belongs to an SSO strategy, + * which is configured to be an "exclusive" domain, then return the Strategy. + * This also matches subdomains, i.e. "foo@bar.baz.edu" is goverend by "baz.edu". + * + * Optionally, if @specificStrategy is set, only that strategy is checked! + */ export function checkRequiredSSO(opts: Opts): Strategy | undefined { const { email, strategies, specificStrategy } = opts; // if the domain of email is contained in any of the strategie's exclusiveDomain array, return that strategy's name diff --git a/src/packages/util/db-schema/accounts.ts b/src/packages/util/db-schema/accounts.ts index eae815df93..31bcb733b7 100644 --- a/src/packages/util/db-schema/accounts.ts +++ b/src/packages/util/db-schema/accounts.ts @@ -15,7 +15,9 @@ import { OTHER_SETTINGS_USERDEFINED_LLM, } from "./defaults"; +import { checkRequiredSSO } from "@cocalc/util/auth-check-required-sso"; import { DEFAULT_LOCALE } from "@cocalc/util/consts/locale"; +import { Strategy } from "@cocalc/util/types/sso"; Table({ name: "accounts", @@ -399,6 +401,7 @@ Table({ // obviously min_balance can't be set! }, async check_hook(db, obj, account_id, _project_id, cb) { + // db is of type PostgreSQL defined in @cocalc/database/postgres/types if (obj["name"] != null) { // NOTE: there is no way to unset/remove a username after one is set... try { @@ -415,6 +418,7 @@ Table({ return; } } + // Hook to truncate some text fields to at most 254 characters, to avoid // further trouble down the line. for (const field of ["first_name", "last_name", "email_address"]) { @@ -427,10 +431,38 @@ Table({ } } } - // check, if account is exclusively controlled by SSO and its update_on_login config is true - const {email_address} = obj - if (email_address != null) { - // TODO + + // if account is exclusively controlled by SSO, you're maybe prohibited from changing account details + const current_email_address = + await db.get_email_address_for_account_id(account_id); + console.log({ current_email_address }); + if (typeof current_email_address === "string") { + const strategies: Strategy[] = await db.getStrategiesSSO(); + const strategy = checkRequiredSSO({ + strategies, + email: current_email_address, + }); + console.log({ strategy }); + console.log(obj); + // we got a required exclusive SSO for the given account_id + if (strategy != null) { + // if user tries to change email_address + if (typeof obj.email_address === "string") { + cb(`You are not allowed to change your email address.`); + return; + } + // ... or tries to change first or last name, but strategy has update_on_login set + if ( + strategy.updateOnLogin && + (typeof obj.first_name === "string" || + obj.last_name === "string") + ) { + cb( + `You are not allowed to change your first or last name. You have to change it at your single-sign-on provider: ${strategy.display}.`, + ); + return; + } + } } cb(); }, diff --git a/src/packages/util/types/sso.ts b/src/packages/util/types/sso.ts index 0cd7a6d016..a3d47bc916 100644 --- a/src/packages/util/types/sso.ts +++ b/src/packages/util/types/sso.ts @@ -12,4 +12,5 @@ export interface Strategy { public: boolean; // true for general broad audiences, like google, default true exclusiveDomains: string[]; // list of domains, e.g. ["foo.com"], which must go through that SSO mechanism (and block regular email signup) doNotHide: boolean; // if true and a public=false, show it directly on the login/signup page + updateOnLogin: boolean; // if true and account is goverend by an exclusiveDomain, user's are not allowed to change their first and last name }