Skip to content

Commit

Permalink
db/account: refactor checkRequiredSSO and use it to check, if modifyi…
Browse files Browse the repository at this point in the history
…ng the email_address is allowed or if update_on_login is set, additionally prohibit changing first or last name
  • Loading branch information
haraldschilly committed Aug 28, 2024
1 parent c85f74a commit 9e5bf97
Show file tree
Hide file tree
Showing 14 changed files with 95 additions and 25 deletions.
14 changes: 14 additions & 0 deletions src/packages/database/postgres-server-queries.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
6 changes: 4 additions & 2 deletions src/packages/database/postgres/account-queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,15 @@ 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<void> {
): Promise<{ email_changed: boolean }> {
return await set_account_info_if_different(opts, false);
}

// This sets the given fields of an account, if it is different from the current value – except for the email address, which we only set but not change
export async function set_account_info_if_different(
opts: SetAccountFields,
overwrite = true,
): Promise<void> {
): Promise<{ email_changed: boolean }> {
const columns = ["email_address", "first_name", "last_name"];

// this could throw an error for "no such account"
Expand Down Expand Up @@ -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(
Expand Down
14 changes: 13 additions & 1 deletion src/packages/database/postgres/passport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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({
Expand All @@ -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,
});
}
}
3 changes: 3 additions & 0 deletions src/packages/database/postgres/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down Expand Up @@ -317,6 +318,8 @@ export interface PostgreSQL extends EventEmitter {
email_address: string;
}>;
}): Promise<void>;

getStrategiesSSO(): Promise<Strategy[]>;
}

export interface SetAccountFields {
Expand Down
4 changes: 3 additions & 1 deletion src/packages/database/settings/get-sso-strategies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ export default async function getStrategies(): Promise<Strategy[]> {
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'
Expand All @@ -39,6 +40,7 @@ export default async function getStrategies(): Promise<Strategy[]> {
public: row.public ?? true,
exclusiveDomains: row.exclusive_domains ?? [],
doNotHide: row.do_not_hide ?? false,
updateOnLogin: row.update_on_login ?? false,
};
});
}
Expand Down
3 changes: 2 additions & 1 deletion src/packages/next/components/auth/sso.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -67,6 +67,7 @@ export default function SSO(props: SSOProps) {
public: true,
exclusiveDomains: [],
doNotHide: true,
updateOnLogin: false,
};

return (
Expand Down
2 changes: 1 addition & 1 deletion src/packages/server/accounts/set-email-address.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/packages/server/auth/check-email-exclusive-sso.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 8 additions & 6 deletions src/packages/server/auth/sso/passport-login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/

Expand Down Expand Up @@ -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");

Expand Down Expand Up @@ -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;
Expand All @@ -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 ?? []) {
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/packages/server/auth/sso/unlink-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/packages/server/auth/throttle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, number>({
max: 10000, // avoid memory issues
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. "[email protected]" 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. "[email protected]" 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
Expand Down
40 changes: 36 additions & 4 deletions src/packages/util/db-schema/accounts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand All @@ -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"]) {
Expand All @@ -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();
},
Expand Down
1 change: 1 addition & 0 deletions src/packages/util/types/sso.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 9e5bf97

Please sign in to comment.