diff --git a/src/packages/database/postgres-server-queries.coffee b/src/packages/database/postgres-server-queries.coffee index e995ae09e7..97d2517014 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; {updateUnreadMessageCount} = require('./postgres/messages'); centralLog = require('./postgres/central-log').default; @@ -2524,6 +2525,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 27873436d7..3c390ee24a 100644 --- a/src/packages/database/postgres/account-queries.ts +++ b/src/packages/database/postgres/account-queries.ts @@ -12,7 +12,7 @@ import { len, } from "@cocalc/util/misc"; import { is_a_site_license_manager } from "./site-license/search"; -import { PostgreSQL } from "./types"; +import { PostgreSQL, SetAccountFields } from "./types"; //import getLogger from "@cocalc/backend/logger"; //const L = getLogger("db:pg:account-queries"); @@ -44,18 +44,10 @@ export async function is_paying_customer( return await is_a_site_license_manager(db, account_id); } -interface SetAccountFields { - db: PostgreSQL; - account_id: string; - email_address?: string | undefined; - first_name?: string | undefined; - last_name?: string | undefined; -} - // 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); } @@ -63,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" @@ -105,6 +97,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 3ae7b957c0..7d571a3961 100644 --- a/src/packages/database/postgres/passport.ts +++ b/src/packages/database/postgres/passport.ts @@ -8,25 +8,27 @@ import { PassportStrategyDB } from "@cocalc/database/settings/auth-sso-types"; import { getPassportsCached, - setPassportsCached + setPassportsCached, } from "@cocalc/database/settings/server-settings"; +import { callback2 as cb2 } from "@cocalc/util/async-utils"; import { to_json } from "@cocalc/util/misc"; import { CB } from "@cocalc/util/types/database"; import { set_account_info_if_different, set_account_info_if_not_set, - set_email_address_verified + set_email_address_verified, } from "./account-queries"; import { CreatePassportOpts, PassportExistsOpts, PostgreSQL, - UpdateAccountInfoAndPassportOpts + SetAccountFields, + UpdateAccountInfoAndPassportOpts, } from "./types"; export async function set_passport_settings( db: PostgreSQL, - opts: PassportStrategyDB & { cb?: CB } + opts: PassportStrategyDB & { cb?: CB }, ): Promise { const { strategy, conf, info } = opts; let err = null; @@ -50,7 +52,7 @@ export async function set_passport_settings( export async function get_passport_settings( db: PostgreSQL, - opts: { strategy: string; cb?: (data: object) => void } + opts: { strategy: string; cb?: (data: object) => void }, ): Promise { const { rows } = await db.async_query({ query: "SELECT conf, info FROM passport_settings", @@ -63,7 +65,7 @@ export async function get_passport_settings( } export async function get_all_passport_settings( - db: PostgreSQL + db: PostgreSQL, ): Promise { return ( await db.async_query({ @@ -73,7 +75,7 @@ export async function get_all_passport_settings( } export async function get_all_passport_settings_cached( - db: PostgreSQL + db: PostgreSQL, ): Promise { const passports = getPassportsCached(); if (passports != null) { @@ -103,7 +105,7 @@ export function _passport_key(opts) { export async function create_passport( db: PostgreSQL, - opts: CreatePassportOpts + opts: CreatePassportOpts, ): Promise { const dbg = db._dbg("create_passport"); dbg({ id: opts.id, strategy: opts.strategy, profile: to_json(opts.profile) }); @@ -121,10 +123,10 @@ export async function create_passport( }); dbg( - `setting other account info ${opts.account_id}: ${opts.email_address}, ${opts.first_name}, ${opts.last_name}` + `setting other account info ${opts.account_id}: ${opts.email_address}, ${opts.first_name}, ${opts.last_name}`, ); await set_account_info_if_not_set({ - db: db, + db, account_id: opts.account_id, email_address: opts.email_address, first_name: opts.first_name, @@ -150,7 +152,7 @@ export async function create_passport( export async function passport_exists( db: PostgreSQL, - opts: PassportExistsOpts + opts: PassportExistsOpts, ): Promise { try { const result = await db.async_query({ @@ -176,27 +178,45 @@ export async function passport_exists( } } +// this is only used in passport-login/maybeUpdateAccountAndPassport! export async function update_account_and_passport( db: PostgreSQL, - opts: UpdateAccountInfoAndPassportOpts + opts: UpdateAccountInfoAndPassportOpts, ) { - // we deliberately do not update the email address, because if the SSO - // strategy sends a different one, this would break the "link". - // rather, if the email (and hence most likely the email address) changes on the - // SSO side, this would equal to creating a new account. + // This also updates the email address, if it is set in opts and does not exist with another account yet. + // NOTE: this changed in July 2024. Prior to that, changing the email address of the same account (by ID) in SSO, + // would not change the email address. const dbg = db._dbg("update_account_and_passport"); dbg( `updating account info ${to_json({ first_name: opts.first_name, last_name: opts.last_name, - })}` + email_addres: opts.email_address, + })}`, ); - await set_account_info_if_different({ + + const upd: SetAccountFields = { db: db, account_id: opts.account_id, first_name: opts.first_name, last_name: opts.last_name, + }; + + // Most likely, this just returns the very same account (since the account already exists). + 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; + dbg( + `No existing account with email address ${opts.email_address}. Therefore, we change the email address of account ${opts.account_id} as well.`, + ); + } + + // this set_account_info_if_different checks again if the email exists on another account, but it would throw an error. + 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({ @@ -208,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 9b290f3798..742e13362b 100644 --- a/src/packages/database/postgres/types.ts +++ b/src/packages/database/postgres/types.ts @@ -15,6 +15,7 @@ import { QueryRows, UntypedQueryResult, } from "@cocalc/util/types/database"; +import type { Strategy } from "@cocalc/util/types/sso"; import { Changes } from "./changefeed"; export type { QueryResult }; @@ -104,6 +105,7 @@ export interface UpdateAccountInfoAndPassportOpts { id: string; profile: any; passport_profile: any; + email_address?: string; } export interface PostgreSQL extends EventEmitter { @@ -323,6 +325,8 @@ export interface PostgreSQL extends EventEmitter { }>; }): Promise; + getStrategiesSSO(): Promise; + user_query_cancel_changefeed(opts: { id: any; cb?: CB }): void; save_blob(opts: { @@ -384,5 +388,13 @@ export interface PostgreSQL extends EventEmitter { }); } +export interface SetAccountFields { + db: PostgreSQL; + account_id: string; + email_address?: string | undefined; + first_name?: string | undefined; + last_name?: string | undefined; +} + // This is an extension of BaseProject in projects/control/base.ts type Project = EventEmitter & {}; diff --git a/src/packages/database/settings/auth-sso-types.ts b/src/packages/database/settings/auth-sso-types.ts index 0dbb8ad6af..43093595cb 100644 --- a/src/packages/database/settings/auth-sso-types.ts +++ b/src/packages/database/settings/auth-sso-types.ts @@ -119,7 +119,7 @@ export interface PassportStrategyDBConfig { * - description: A longer description of the strategy, could be markdown, shown on the dedicated /sso/... pages. * - icon: A URL to an icon * - disabled: if true, this is ignored during the initialization - * - update_on_login: if true, the user's profile is updated on login (first and last name, not email) + * - update_on_login: if true, the user's profile is updated on login (first and last name, not email) and NOT by the user. * - cookie_ttl_s: how long the remember_me cookied lasts (default is a month or so). * This could be set to a much shorter period to force users more frequently to re-login. */ 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/account/config/account/name.tsx b/src/packages/next/components/account/config/account/name.tsx index 19c6fd4fb2..1fd0d9e1e1 100644 --- a/src/packages/next/components/account/config/account/name.tsx +++ b/src/packages/next/components/account/config/account/name.tsx @@ -85,7 +85,7 @@ function ConfigureName() { type="error" showIcon /> - )}{" "} + )} {get.loading ? ( ) : ( @@ -131,7 +131,6 @@ function ConfigureName() { for content you share publicly. {original.name && ( <> - {" "} Setting a username provides optional nicer URL's for shared public documents. Your username can be between 1 and 39 characters, contain upper and lower case letters, numbers, and 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/next/components/misc/save-button.tsx b/src/packages/next/components/misc/save-button.tsx index 69d9f15b1d..10e3f782b3 100644 --- a/src/packages/next/components/misc/save-button.tsx +++ b/src/packages/next/components/misc/save-button.tsx @@ -1,12 +1,14 @@ -import { CSSProperties, useEffect, useMemo, useRef, useState } from "react"; -import { cloneDeep, debounce, isEqual } from "lodash"; import { Alert, Button, Space } from "antd"; -import useIsMounted from "lib/hooks/mounted"; +import { cloneDeep, debounce, isEqual } from "lodash"; +import { CSSProperties, useEffect, useMemo, useRef, useState } from "react"; + import Loading from "components/share/loading"; import api from "lib/api/post"; +import useIsMounted from "lib/hooks/mounted"; + import { Icon } from "@cocalc/frontend/components/icon"; -import { SCHEMA } from "@cocalc/util/schema"; import { keys } from "@cocalc/util/misc"; +import { SCHEMA } from "@cocalc/util/schema"; interface Props { edited: any; @@ -119,7 +121,7 @@ export default function SaveButton({ const doSaveDebounced = useMemo( () => debounce(doSave, debounce_ms), - [onSave] + [onSave], ); useEffect(() => { diff --git a/src/packages/next/pages/api/v2/auth/sign-up.ts b/src/packages/next/pages/api/v2/auth/sign-up.ts index 908eaaf0ce..efc76d4601 100644 --- a/src/packages/next/pages/api/v2/auth/sign-up.ts +++ b/src/packages/next/pages/api/v2/auth/sign-up.ts @@ -35,9 +35,9 @@ import { v4 } from "uuid"; import { getServerSettings } from "@cocalc/database/settings/server-settings"; import createAccount from "@cocalc/server/accounts/create-account"; import isAccountAvailable from "@cocalc/server/auth/is-account-available"; -import isDomainExclusiveSSO from "@cocalc/server/auth/is-domain-exclusive-sso"; import passwordStrength from "@cocalc/server/auth/password-strength"; import reCaptcha from "@cocalc/server/auth/recaptcha"; +import { isExclusiveSSOEmail } from "@cocalc/server/auth/throttle"; import redeemRegistrationToken from "@cocalc/server/auth/tokens/redeem"; import sendWelcomeEmail from "@cocalc/server/email/welcome-email"; import getSiteLicenseId from "@cocalc/server/public-paths/site-license-id"; @@ -45,7 +45,6 @@ import { is_valid_email_address as isValidEmailAddress, len, } from "@cocalc/util/misc"; - import getAccountId from "lib/account/get-account"; import { apiRoute, apiRouteOperation } from "lib/api"; import assertTrusted from "lib/api/assert-trusted"; @@ -171,11 +170,12 @@ export async function signUp(req, res) { }); return; } - const exclusive = await isDomainExclusiveSSO(email); + const exclusive = await isExclusiveSSOEmail(email); if (exclusive) { + const name = exclusive.display ?? exclusive.name; res.json({ issues: { - email: `To sign up with "@${exclusive}", you have to use the corresponding single sign on mechanism. Delete your email address above, then click the SSO icon.`, + email: `To sign up with "@${name}", you have to use the corresponding single sign on mechanism. Delete your email address above, then click the SSO icon.`, }, }); return; diff --git a/src/packages/next/pages/sso/[id].tsx b/src/packages/next/pages/sso/[id].tsx index 835fe8ec9a..64889848f4 100644 --- a/src/packages/next/pages/sso/[id].tsx +++ b/src/packages/next/pages/sso/[id].tsx @@ -57,9 +57,9 @@ export default function Signup(props: Props) { {r_human_list( (domains ?? []).map((d) => ( - {d} + {d === "*" ? "all domains" : d} - )) + )), )} ); diff --git a/src/packages/next/pages/sso/index.tsx b/src/packages/next/pages/sso/index.tsx index 28c7ca577c..dfe057156b 100644 --- a/src/packages/next/pages/sso/index.tsx +++ b/src/packages/next/pages/sso/index.tsx @@ -32,7 +32,8 @@ export default function SignupIndex(props: Props) { function renderDomains(domains) { if (domains == null || domains.length === 0) return; - return {to_human_list(domains ?? [])}; + const names = (domains ?? []).map((d) => (d === "*" ? "all domains" : d)); + return {to_human_list(names)}; } function extra(sso) { diff --git a/src/packages/server/accounts/set-email-address.ts b/src/packages/server/accounts/set-email-address.ts index 4ad701aa0d..7c465b0758 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/is-domain-exclusive-sso.ts b/src/packages/server/auth/is-domain-exclusive-sso.ts deleted file mode 100644 index c1537e4153..0000000000 --- a/src/packages/server/auth/is-domain-exclusive-sso.ts +++ /dev/null @@ -1,48 +0,0 @@ -import getPool from "@cocalc/database/pool"; -import { parseDomain, ParseResultType } from "parse-domain"; - -export default async function isDomainExclusiveSSO( - email_address: string -): Promise { - if (!email_address) { - return; - } - - const raw_domain = email_address.split("@")[1]?.trim().toLowerCase(); - if (!raw_domain) { - return; - } - - const exclusiveDomains = await getExclusiveDomains(); - if (exclusiveDomains.length == 0) { - // For most servers, this is the case. - return; - } - - const parsed = parseDomain(raw_domain); - if (parsed.type != ParseResultType.Listed) { - // Domain not in the public suffix list - return; - } - - const { domain, topLevelDomains } = parsed; - const canonical = [domain ?? "", ...topLevelDomains].join("."); - if (exclusiveDomains.includes(canonical)) { - return canonical; - } -} - -async function getExclusiveDomains(): Promise { - const pool = getPool("minutes"); // exclusive sso is meant for a on prem settings where config RARELY changes. - const { rows } = await pool.query( - "SELECT conf#>'{exclusive_domains}' as exclusive_domains FROM passport_settings" - ); - const v: string[] = []; - for (const row of rows) { - const { exclusive_domains } = row; - if (exclusive_domains) { - v.push(...exclusive_domains); - } - } - return v; -} diff --git a/src/packages/server/auth/sso/passport-login.ts b/src/packages/server/auth/sso/passport-login.ts index f923b999d0..8862952212 100644 --- a/src/packages/server/auth/sso/passport-login.ts +++ b/src/packages/server/auth/sso/passport-login.ts @@ -13,36 +13,44 @@ * 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. */ import Cookies from "cookies"; import * as _ from "lodash"; -import { isEmpty } from "lodash"; + +import { REMEMBER_ME_COOKIE_NAME } from "@cocalc/backend/auth/cookie-names"; import base_path from "@cocalc/backend/base-path"; import getLogger from "@cocalc/backend/logger"; import { set_email_address_verified } from "@cocalc/database/postgres/account-queries"; -import type { PostgreSQL } from "@cocalc/database/postgres/types"; -import generateHash from "@cocalc/server/auth/hash"; -import { REMEMBER_ME_COOKIE_NAME } from "@cocalc/backend/auth/cookie-names"; -import { sanitizeID } from "@cocalc/server/auth/sso/sanitize-id"; -import { sanitizeProfile } from "@cocalc/server/auth/sso/sanitize-profile"; +import type { + PostgreSQL, + UpdateAccountInfoAndPassportOpts, +} from "@cocalc/database/postgres/types"; import { PassportLoginLocals, PassportLoginOpts, PassportStrategyDB, } from "@cocalc/database/settings/auth-sso-types"; -import { callback2 as cb2 } from "@cocalc/util/async-utils"; -import { HELP_EMAIL } from "@cocalc/util/theme"; -import getEmailAddress from "../../accounts/get-email-address"; -import { emailBelongsToDomain, getEmailDomain } from "./check-required-sso"; -import { SSO_API_KEY_COOKIE_NAME } from "./consts"; -import isBanned from "@cocalc/server/accounts/is-banned"; import accountCreationActions from "@cocalc/server/accounts/account-creation-actions"; +import getEmailAddress from "@cocalc/server/accounts/get-email-address"; +import isBanned from "@cocalc/server/accounts/is-banned"; import clientSideRedirect from "@cocalc/server/auth/client-side-redirect"; +import generateHash from "@cocalc/server/auth/hash"; import setSignInCookies from "@cocalc/server/auth/set-sign-in-cookies"; +import { sanitizeID } from "@cocalc/server/auth/sso/sanitize-id"; +import { sanitizeProfile } from "@cocalc/server/auth/sso/sanitize-profile"; +import { callback2 as cb2 } from "@cocalc/util/async-utils"; +import { + emailBelongsToDomain, + getEmailDomain, +} from "@cocalc/util/auth-check-required-sso"; +import { is_valid_email_address } from "@cocalc/util/misc"; +import { HELP_EMAIL } from "@cocalc/util/theme"; +import { SSO_API_KEY_COOKIE_NAME } from "./consts"; const logger = getLogger("server:auth:sso:passport-login"); @@ -231,32 +239,38 @@ export class PassportLogin { }); } + private emailBelongsToStrategy( + email_address: string, + strategy: PassportStrategyDB, + ): boolean { + const exclusiveDomains = strategy.info?.exclusive_domains ?? []; + const emailDomain = getEmailDomain(email_address.toLowerCase()); + for (const ssoDomain of exclusiveDomains) { + if (ssoDomain === "*" || emailBelongsToDomain(emailDomain, ssoDomain)) { + return true; + } + } + return false; + } + // this checks if the login info contains an email address, which belongs to an exclusive SSO strategy + // this only checks for a single (given) strategy, in the checkPassportExists method. private checkExclusiveSSO(opts: PassportLoginOpts): boolean { const strategy = opts.passports[opts.strategyName]; - const exclusiveDomains = strategy.info?.exclusive_domains ?? []; - if (!isEmpty(exclusiveDomains)) { - for (const email of opts.emails ?? []) { - const emailDomain = getEmailDomain(email.toLocaleLowerCase()); - for (const ssoDomain of exclusiveDomains) { - if (emailBelongsToDomain(emailDomain, ssoDomain)) { - return true; - } - } + for (const email_address of opts.emails ?? []) { + if (this.emailBelongsToStrategy(email_address, strategy)) { + return true; } } return false; } - // similar to the above, for a specific email address - private checkEmailExclusiveSSO(email_address): boolean { - const emailDomain = getEmailDomain(email_address.toLocaleLowerCase()); + // similar to the above, for a specific email address. The difference is, this covers all known strategies. + private checkEmailExclusiveSSO(email_address: string): boolean { for (const strategyName in this.opts.passports) { const strategy = this.opts.passports[strategyName]; - for (const ssoDomain of strategy.info?.exclusive_domains ?? []) { - if (emailBelongsToDomain(emailDomain, ssoDomain)) { - return true; - } + if (this.emailBelongsToStrategy(email_address, strategy)) { + return true; } } return false; @@ -274,6 +288,7 @@ export class PassportLogin { L( "check to see if the passport already exists indexed by the given id -- in that case we will log user in", ); + // L({ locals }); const passport_account_id = await this.database.passport_exists({ strategy: opts.strategyName, @@ -312,11 +327,12 @@ export class PassportLogin { // user authenticated, passport not known, adding to the user's account await this.createPassport(opts, locals); } else { + L(`passport_account_id=${passport_account_id}`); if ( locals.has_valid_remember_me && locals.account_id !== passport_account_id ) { - L("passport exists but is associated with another account already"); + L("passport exists, but is associated with another account already"); throw Error( `Your ${opts.strategyName} account is already attached to another CoCalc account. First sign into that account and unlink ${opts.strategyName} in account settings, if you want to instead associate it with this account.`, ); @@ -345,6 +361,7 @@ export class PassportLogin { locals: PassportLoginLocals, ): Promise { const L = logger.extend("check_existing_emails").debug; + // L({ locals }); // handle case where passport doesn't exist, but we know one or more email addresses → check for matching email if (locals.account_id != null || opts.emails == null) return; @@ -411,6 +428,7 @@ export class PassportLogin { ): Promise { if (locals.account_id) return; const L = logger.extend("maybe_create_account").debug; + // L({ locals }); L( "no existing account to link, so create new account that can be accessed using this passport", @@ -468,22 +486,28 @@ export class PassportLogin { if (locals.new_account_created || locals.account_id == null) return; const L = logger.extend("maybe_update_account_profile").debug; - // if (opts.emails != null) { - // locals.email_address = opts.emails[0]; - // } - - L(`account exists and we update name of user based on SSO`); - await this.database.update_account_and_passport({ + const upd: UpdateAccountInfoAndPassportOpts = { account_id: locals.account_id, first_name: opts.first_name, last_name: opts.last_name, strategy: opts.strategyName, id: opts.id, profile: opts.profile, - // but not the email address, at least for now - // email_address: locals.email_address, passport_profile: opts.profile, - }); + }; + + if (Array.isArray(opts.emails) && opts.emails.length >= 1) { + locals.email_address = opts.emails[0]; + } + + // 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; + } + + L(`account exists and we update name of user based on SSO`); + await this.database.update_account_and_passport(upd); } // ebfore recording the sign-in below, we check if a user is banned diff --git a/src/packages/server/auth/sso/unlink-strategy.ts b/src/packages/server/auth/sso/unlink-strategy.ts index 923b07233e..af843d8909 100644 --- a/src/packages/server/auth/sso/unlink-strategy.ts +++ b/src/packages/server/auth/sso/unlink-strategy.ts @@ -10,9 +10,9 @@ upstream SSO provider. */ import getPool from "@cocalc/database/pool"; -import { is_valid_uuid_string } from "@cocalc/util/misc"; -import { checkRequiredSSO } from "./check-required-sso"; import getStrategies from "@cocalc/database/settings/get-sso-strategies"; +import { is_valid_uuid_string } from "@cocalc/util/misc"; +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. @@ -42,7 +42,7 @@ export default async function unlinkStrategy(opts: Options): Promise { // if we can't find the strategy, we still let users unlink it – maybe no longer available? await pool.query( "UPDATE accounts SET passports = passports - $2 WHERE account_id=$1", - [account_id, name] + [account_id, name], ); } @@ -62,7 +62,7 @@ export async function isBlockedUnlinkStrategy(opts: Opts): Promise { const emailQuery = await pool.query( "SELECT email_address FROM accounts WHERE account_id=$1", - [account_id] + [account_id], ); const email = emailQuery.rows[0].email_address; if (email) { diff --git a/src/packages/server/auth/throttle.ts b/src/packages/server/auth/throttle.ts index 7c73830967..0c67a02900 100644 --- a/src/packages/server/auth/throttle.ts +++ b/src/packages/server/auth/throttle.ts @@ -4,9 +4,10 @@ attacks. This is in memory per-backend server, and doesn't touch the database. */ -import getStrategies from "@cocalc/database/settings/get-sso-strategies"; import LRU from "lru-cache"; -import { checkRequiredSSO } from "./sso/check-required-sso"; + +import getStrategies from "@cocalc/database/settings/get-sso-strategies"; +import { checkRequiredSSO } from "@cocalc/util/auth-check-required-sso"; const emailShortCache = new LRU({ max: 10000, // avoid memory issues @@ -22,7 +23,7 @@ const ipLongCache = new LRU({ ttl: 1000 * 60 * 60, }); -async function isExclusiveEmail(email: string) { +export async function isExclusiveSSOEmail(email: string) { const strategies = await getStrategies(); return checkRequiredSSO({ email, strategies }); } @@ -30,7 +31,7 @@ async function isExclusiveEmail(email: string) { export async function signInCheck( email: string, ip?: string, - auth_token: boolean = false + auth_token: boolean = false, ): Promise { if ((emailShortCache.get(email) ?? 0) > 5) { // A given email address is allowed at most 5 failed login attempts per minute @@ -51,7 +52,7 @@ export async function signInCheck( } // unless user has an auth token, we check if the email address is part of an exclusive SSO mechanism (and block password sign ins) if (!auth_token) { - const exclusiveSSO = await isExclusiveEmail(email); + const exclusiveSSO = await isExclusiveSSOEmail(email); if (exclusiveSSO != null) { const name = exclusiveSSO.display ?? exclusiveSSO.name; return `You have to sign in using the Single-Sign-On mechanism "${name}" of your institution.`; diff --git a/src/packages/util/auth-check-required-sso.test.ts b/src/packages/util/auth-check-required-sso.test.ts new file mode 100644 index 0000000000..ed18506c10 --- /dev/null +++ b/src/packages/util/auth-check-required-sso.test.ts @@ -0,0 +1,83 @@ +import { + emailBelongsToDomain, + getEmailDomain, + checkRequiredSSO, +} from "./auth-check-required-sso"; +import { Strategy } from "./types/sso"; + +const SSO: Readonly> = { + display: "", + backgroundColor: "", + public: false, + doNotHide: true, + updateOnLogin: true, +} as const; + +describe("Check Required SSO", () => { + test("getEmailDomain", () => { + expect(getEmailDomain("foo@bar.com")).toBe("bar.com"); + expect(getEmailDomain("foo@bar.co.uk")).toBe("bar.co.uk"); + }); + + test("emailBelongsToDomain", () => { + expect(emailBelongsToDomain("foo.com", "foo.com")).toBe(true); + expect(emailBelongsToDomain("bar.foo.com", "foo.com")).toBe(true); + expect(emailBelongsToDomain("foo.com", "bar.com")).toBe(false); + expect(emailBelongsToDomain("foo.com", "foo.co.uk")).toBe(false); + expect(emailBelongsToDomain("foo.com", "foo.com.uk")).toBe(false); + expect(emailBelongsToDomain("foobar.com", "bar.com")).toBe(false); + expect(emailBelongsToDomain("foobar.com", "bazfoobar.com")).toBe(false); + expect(emailBelongsToDomain("foobar.com", "*")).toBe(false); + }); + + const foo = { name: "foo", exclusiveDomains: ["foo.co.uk"], ...SSO }; + const bar = { name: "bar", exclusiveDomains: ["*"], ...SSO }; + const baz = { + name: "baz", + exclusiveDomains: ["baz.com", "abc.com"], + ...SSO, + }; + + test("checkRequiredSSO", () => { + const strategies: Strategy[] = [foo, baz] as const; + + expect(checkRequiredSSO({ email: "x@baz.com", strategies })?.name).toEqual( + "baz", + ); + expect( + checkRequiredSSO({ email: "x@foo.abc.com", strategies })?.name, + ).toEqual("baz"); + expect( + checkRequiredSSO({ email: "instructor+123@foo.co.uk", strategies })?.name, + ).toEqual("foo"); + expect( + checkRequiredSSO({ email: "x@students.foo.co.uk", strategies })?.name, + ).toEqual("foo"); + // no match on naive substring from the right + expect( + checkRequiredSSO({ email: "abc@foobaz.com", strategies }), + ).toBeUndefined(); + // no catch-all for an unrelated domain, returns no strategy + expect( + checkRequiredSSO({ email: "x@gmail.com", strategies }), + ).toBeUndefined(); + }); + + test("checkRequiredSSO/catchall", () => { + const strategies: Strategy[] = [foo, bar, baz] as const; + + expect(checkRequiredSSO({ email: "x@baz.com", strategies })?.name).toEqual( + "baz", + ); + expect( + checkRequiredSSO({ email: "x@foo.abc.com", strategies })?.name, + ).toEqual("baz"); + expect( + checkRequiredSSO({ email: "x@students.foo.co.uk", strategies })?.name, + ).toEqual("foo"); + // this is the essential difference to above + expect( + checkRequiredSSO({ email: "x@gmail.com", strategies })?.name, + ).toEqual("bar"); + }); +}); diff --git a/src/packages/server/auth/sso/check-required-sso.ts b/src/packages/util/auth-check-required-sso.ts similarity index 61% rename from src/packages/server/auth/sso/check-required-sso.ts rename to src/packages/util/auth-check-required-sso.ts index 1554a39961..b966a856df 100644 --- a/src/packages/server/auth/sso/check-required-sso.ts +++ b/src/packages/util/auth-check-required-sso.ts @@ -3,45 +3,64 @@ * License: MS-RSL – see LICENSE.md for details */ +import { is_valid_email_address } from "@cocalc/util/misc"; 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", + * while "foo@barbaz.edu" is NOT goverend by "baz.edu". + * + * Special case: an sso domain "*" covers all domains, not covered by any other + * exclusive SSO strategy. If there is just one such "*"-SSO strategy, it will deal with all + * accounts. + * + * Optionally, if @specificStrategy is set, only that strategy or "*" 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 - if (email == null) return; + if (!email) return; if (strategies == null || strategies.length === 0) return; if (email.indexOf("@") === -1) return; + if (!is_valid_email_address(email)) return; const emailDomain = getEmailDomain(email); if (!emailDomain) return; for (const strategy of strategies) { if (specificStrategy && specificStrategy !== strategy.name) continue; for (const ssoDomain of strategy.exclusiveDomains) { + if (ssoDomain === "*") continue; // dealt with below if (emailBelongsToDomain(emailDomain, ssoDomain)) { return strategy; } } } + // At this point, we either matched an existing strategy (above) or there is a "*" strategy + for (const strategy of strategies) { + if (strategy.exclusiveDomains.includes("*")) { + return strategy; + } + } } export function getEmailDomain(email: string): string { return email.trim().toLowerCase().split("@")[1]; } +/** + * This checks if the email's domain is either exactly the ssoDomain or a subdomain. + * E.g. for "foo.edu", an email "name@mail.foo.edu" is covered as well. + */ export function emailBelongsToDomain( emailDomain: string, - ssoDomain: string + ssoDomain: string, ): boolean { return emailDomain === ssoDomain || emailDomain.endsWith(`.${ssoDomain}`); } diff --git a/src/packages/util/db-schema/accounts.ts b/src/packages/util/db-schema/accounts.ts index 0d742989ff..bb4bd4ea11 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"; export const USER_SEARCH_LIMIT = 250; export const ADMIN_SEARCH_LIMIT = 2500; @@ -567,6 +569,7 @@ Table({ auto_balance: true, }, 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 { @@ -583,6 +586,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"]) { @@ -595,7 +599,7 @@ Table({ } } } - + // Make sure auto_balance is valid. if (obj["auto_balance"] != null) { try { @@ -605,6 +609,40 @@ Table({ return; } } + + // 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/misc.test.ts b/src/packages/util/misc.test.ts index f162a5ece5..e3ae929d70 100644 --- a/src/packages/util/misc.test.ts +++ b/src/packages/util/misc.test.ts @@ -343,3 +343,38 @@ describe("suggest_duplicate_filename", () => { expect(dup("asdf-")).toBe("asdf--1"); }); }); + +describe("is_valid email_address", () => { + const ivea = misc.is_valid_email_address; + test("valid", () => { + expect(ivea("foo@bar.com")).toBe(true); + expect(ivea("foo+bar@bar.com")).toBe(true); + expect(ivea("foo.bar@bar.com")).toBe(true); + expect(ivea("foo-bar@bar.com")).toBe(true); + expect(ivea("foo_bar@bar.com")).toBe(true); + expect(ivea("123@bar.com")).toBe(true); + expect(ivea("foo@123.com")).toBe(true); + expect(ivea("foo@bar.co.uk")).toBe(true); + expect(ivea("foobar@bar.com")).toBe(true); + expect(ivea("foo.bar@bar.com")).toBe(true); + expect(ivea("foo+bar@bar.com")).toBe(true); + expect(ivea("FOO@BAR.BAZ")).toBe(true); + }); + test("invalid", () => { + expect(ivea(123)).toBe(false); + expect(ivea({})).toBe(false); + expect(ivea([])).toBe(false); + expect(ivea(null)).toBe(false); + expect(ivea(undefined)).toBe(false); + expect(ivea("abc")).toBe(false); + expect(ivea("abc@foo@bar.com")).toBe(false); + expect(ivea("foo@bar.")).toBe(false); + expect(ivea("foo@.bar.com")).toBe(false); + expect(ivea("foo@bar..com")).toBe(false); + expect(ivea("@bar.com")).toBe(false); + expect(ivea("foo@")).toBe(false); + expect(ivea("foo")).toBe(false); + expect(ivea("foo bar@bar.com")).toBe(false); + expect(ivea("foo@bar@bar.com")).toBe(false); + }); +}); diff --git a/src/packages/util/misc.ts b/src/packages/util/misc.ts index 0f02f9d2ee..abf3a8432a 100644 --- a/src/packages/util/misc.ts +++ b/src/packages/util/misc.ts @@ -417,10 +417,10 @@ const reValidEmail = (function () { return new RegExp(sValidEmail); })(); -export function is_valid_email_address(email: string): boolean { +export function is_valid_email_address(email?: unknown): boolean { // From http://stackoverflow.com/questions/46155/validate-email-address-in-javascript // but converted to Javascript; it's near the middle but claims to be exactly RFC822. - if (reValidEmail.test(email)) { + if (typeof email === "string" && reValidEmail.test(email)) { return true; } else { return false; diff --git a/src/packages/util/types/sso.ts b/src/packages/util/types/sso.ts index 0cd7a6d016..d927ba43f2 100644 --- a/src/packages/util/types/sso.ts +++ b/src/packages/util/types/sso.ts @@ -10,6 +10,7 @@ export interface Strategy { icon?: string; // name of or URL to icon to display for SSO backgroundColor: string; // background color for icon, if not a link 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) + exclusiveDomains: string[]; // list of domains, e.g. ["foo.com"], which must go through that SSO mechanism (and block regular email signup). The domain "*" implies all domains are "taken over" by that startegy – only use that once for one strategy. 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 } diff --git a/src/scripts/auth/gen-sso.py b/src/scripts/auth/gen-sso.py index c20b93d297..81dd40285f 100755 --- a/src/scripts/auth/gen-sso.py +++ b/src/scripts/auth/gen-sso.py @@ -159,7 +159,7 @@ "last_name": "lastName", "full_name": "displayName", "emails": "email", - "id": "email", + "id": "id", }, "issuer": "https://cocalc.com", "cert": saml20cert @@ -170,7 +170,7 @@ "public": False, "display": "Saml20", "description": "Testing my SAML 2.0 IdP", - "exclusive_domains": ["example.com"], + "exclusive_domains": ["*"], "update_on_login": True, "cookie_ttl_s": 24 * 60 * 60, # 24 hours }