sso: tweak the checkRequiredSSO logic (match "*" at the end, if no others match) – and write tests to nail this down

Author: haraldschilly

src/packages/util/auth-check-required-sso.test.ts

@@ -0,0 +1,80 @@
+import {
+  emailBelongsToDomain,
+  getEmailDomain,
+  checkRequiredSSO,
+} from "./auth-check-required-sso";
+import { Strategy } from "./types/sso";
+
+const SSO = {
+  display: "",
+  backgroundColor: "",
+  public: false,
+  doNotHide: true,
+  updateOnLogin: true,
+} as const;
+
+describe("Check Required SSO", () => {
+  test("getEmailDomain", () => {
+    expect(getEmailDomain("[email protected]")).toBe("bar.com");
+    expect(getEmailDomain("[email protected]")).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", "bar.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: "[email protected]", strategies })?.name).toEqual(
+      "baz",
+    );
+    expect(
+      checkRequiredSSO({ email: "[email protected]", strategies })?.name,
+    ).toEqual("baz");
+    expect(
+      checkRequiredSSO({ email: "[email protected]", strategies })?.name,
+    ).toEqual("foo");
+    // no match on naive substring from the right
+    expect(
+      checkRequiredSSO({ email: "[email protected]", strategies }),
+    ).toBeUndefined();
+    // no catch-all for an unrelated domain, returns no strategy
+    expect(
+      checkRequiredSSO({ email: "[email protected]", strategies }),
+    ).toBeUndefined();
+  });
+
+  test("checkRequiredSSO/catchall", () => {
+    const strategies: Strategy[] = [foo, bar, baz] as const;
+
+    expect(checkRequiredSSO({ email: "[email protected]", strategies })?.name).toEqual(
+      "baz",
+    );
+    expect(
+      checkRequiredSSO({ email: "[email protected]", strategies })?.name,
+    ).toEqual("baz");
+    expect(
+      checkRequiredSSO({ email: "[email protected]", strategies })?.name,
+    ).toEqual("foo");
+    // this is the essential difference to above
+    expect(
+      checkRequiredSSO({ email: "[email protected]", strategies })?.name,
+    ).toEqual("bar");
+  });
+});

src/packages/util/auth-check-required-sso.ts

@@ -16,7 +16,11 @@ interface Opts {
  * 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!
+ * 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;
@@ -29,11 +33,18 @@ export function checkRequiredSSO(opts: Opts): Strategy | undefined {
   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 {
@@ -43,15 +54,10 @@ export function getEmailDomain(email: string): string {
 /**
  * This checks if the email's domain is either exactly the ssoDomain or a subdomain.
  * E.g. for "foo.edu", an email "[email protected]" is covered as well.
- *
- * Special case: an sso domain "*" covers all domains. This is kind of a complete "take over",
- * because all accounts on that instance of CoCalc have to go through that SSO mechanism.
- * Note: In that case, it makes no sense to have more than one SSO mechanism configured.
  */
 export function emailBelongsToDomain(
   emailDomain: string,
   ssoDomain: string,
 ): boolean {
-  if (ssoDomain === "*") return true;
   return emailDomain === ssoDomain || emailDomain.endsWith(`.${ssoDomain}`);
 }

src/packages/util/types/sso.ts

@@ -10,7 +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
 }