mirror of
https://github.com/bitwarden/browser
synced 2025-12-15 07:43:35 +00:00
feat(SSO): (Auth/[PM-22110] Remove Alternate Login Options when SSO Required (#16340)
If a user is part of an org that has the `RequireSso` policy, when that user successfully logs in we add their email to a local `ssoRequiredCache` on their device. The next time this user goes to the `/login` screen on this device, we will use that cache to determine that for this email we should only show the "Use single sign-on" button and disable the alternate login buttons. These changes are behind the flag: `PM22110_DisableAlternateLoginMethods`
This commit is contained in:
@@ -1,3 +1,5 @@
|
||||
import { Observable } from "rxjs";
|
||||
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
|
||||
export abstract class SsoLoginServiceAbstraction {
|
||||
@@ -70,6 +72,10 @@ export abstract class SsoLoginServiceAbstraction {
|
||||
*
|
||||
*/
|
||||
abstract setSsoEmail: (email: string) => Promise<void>;
|
||||
/**
|
||||
* Clear the SSO email
|
||||
*/
|
||||
abstract clearSsoEmail: () => Promise<void>;
|
||||
/**
|
||||
* Gets the value of the active user's organization sso identifier.
|
||||
*
|
||||
@@ -86,4 +92,24 @@ export abstract class SsoLoginServiceAbstraction {
|
||||
organizationIdentifier: string,
|
||||
userId: UserId | undefined,
|
||||
) => Promise<void>;
|
||||
|
||||
/**
|
||||
* A cache list of user emails for whom the `PolicyType.RequireSso` policy is applied (that is, a list
|
||||
* of users who are required to authenticate via SSO only). The cache lives on the current device only.
|
||||
*/
|
||||
abstract ssoRequiredCache$: Observable<Set<string> | null>;
|
||||
|
||||
/**
|
||||
* Remove an email from the cached list of emails that must authenticate via SSO.
|
||||
*/
|
||||
abstract removeFromSsoRequiredCacheIfPresent: (email: string) => Promise<void>;
|
||||
|
||||
/**
|
||||
* Check if the user is required to authenticate via SSO. If so, add their email to a cache list.
|
||||
* We'll use this cache list to display ONLY the "Use single sign-on" button to the
|
||||
* user the next time they are on the /login page.
|
||||
*
|
||||
* If the user is not required to authenticate via SSO, remove their email from the cache list if it is present.
|
||||
*/
|
||||
abstract updateSsoRequiredCache: (ssoLoginEmail: string, userId: UserId) => Promise<void>;
|
||||
}
|
||||
|
||||
@@ -1,9 +1,13 @@
|
||||
import { mock, MockProxy } from "jest-mock-extended";
|
||||
import { of } from "rxjs";
|
||||
|
||||
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
|
||||
import { PolicyType } from "@bitwarden/common/admin-console/enums";
|
||||
import {
|
||||
CODE_VERIFIER,
|
||||
GLOBAL_ORGANIZATION_SSO_IDENTIFIER,
|
||||
SSO_EMAIL,
|
||||
SSO_REQUIRED_CACHE,
|
||||
SSO_STATE,
|
||||
SsoLoginService,
|
||||
USER_ORGANIZATION_SSO_IDENTIFIER,
|
||||
@@ -18,8 +22,9 @@ describe("SSOLoginService ", () => {
|
||||
let sut: SsoLoginService;
|
||||
|
||||
let accountService: FakeAccountService;
|
||||
let mockSingleUserStateProvider: FakeStateProvider;
|
||||
let mockStateProvider: FakeStateProvider;
|
||||
let mockLogService: MockProxy<LogService>;
|
||||
let mockPolicyService: MockProxy<PolicyService>;
|
||||
let userId: UserId;
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -27,10 +32,11 @@ describe("SSOLoginService ", () => {
|
||||
|
||||
userId = Utils.newGuid() as UserId;
|
||||
accountService = mockAccountServiceWith(userId);
|
||||
mockSingleUserStateProvider = new FakeStateProvider(accountService);
|
||||
mockStateProvider = new FakeStateProvider(accountService);
|
||||
mockLogService = mock<LogService>();
|
||||
mockPolicyService = mock<PolicyService>();
|
||||
|
||||
sut = new SsoLoginService(mockSingleUserStateProvider, mockLogService);
|
||||
sut = new SsoLoginService(mockStateProvider, mockLogService, mockPolicyService);
|
||||
});
|
||||
|
||||
it("instantiates", () => {
|
||||
@@ -40,7 +46,7 @@ describe("SSOLoginService ", () => {
|
||||
it("gets and sets code verifier", async () => {
|
||||
const codeVerifier = "test-code-verifier";
|
||||
await sut.setCodeVerifier(codeVerifier);
|
||||
mockSingleUserStateProvider.getGlobal(CODE_VERIFIER);
|
||||
mockStateProvider.getGlobal(CODE_VERIFIER);
|
||||
|
||||
const result = await sut.getCodeVerifier();
|
||||
expect(result).toBe(codeVerifier);
|
||||
@@ -49,7 +55,7 @@ describe("SSOLoginService ", () => {
|
||||
it("gets and sets SSO state", async () => {
|
||||
const ssoState = "test-sso-state";
|
||||
await sut.setSsoState(ssoState);
|
||||
mockSingleUserStateProvider.getGlobal(SSO_STATE);
|
||||
mockStateProvider.getGlobal(SSO_STATE);
|
||||
|
||||
const result = await sut.getSsoState();
|
||||
expect(result).toBe(ssoState);
|
||||
@@ -58,7 +64,7 @@ describe("SSOLoginService ", () => {
|
||||
it("gets and sets organization SSO identifier", async () => {
|
||||
const orgIdentifier = "test-org-identifier";
|
||||
await sut.setOrganizationSsoIdentifier(orgIdentifier);
|
||||
mockSingleUserStateProvider.getGlobal(GLOBAL_ORGANIZATION_SSO_IDENTIFIER);
|
||||
mockStateProvider.getGlobal(GLOBAL_ORGANIZATION_SSO_IDENTIFIER);
|
||||
|
||||
const result = await sut.getOrganizationSsoIdentifier();
|
||||
expect(result).toBe(orgIdentifier);
|
||||
@@ -67,7 +73,7 @@ describe("SSOLoginService ", () => {
|
||||
it("gets and sets SSO email", async () => {
|
||||
const email = "test@example.com";
|
||||
await sut.setSsoEmail(email);
|
||||
mockSingleUserStateProvider.getGlobal(SSO_EMAIL);
|
||||
mockStateProvider.getGlobal(SSO_EMAIL);
|
||||
|
||||
const result = await sut.getSsoEmail();
|
||||
expect(result).toBe(email);
|
||||
@@ -77,7 +83,7 @@ describe("SSOLoginService ", () => {
|
||||
const userId = Utils.newGuid() as UserId;
|
||||
const orgIdentifier = "test-active-org-identifier";
|
||||
await sut.setActiveUserOrganizationSsoIdentifier(orgIdentifier, userId);
|
||||
mockSingleUserStateProvider.getUser(userId, USER_ORGANIZATION_SSO_IDENTIFIER);
|
||||
mockStateProvider.getUser(userId, USER_ORGANIZATION_SSO_IDENTIFIER);
|
||||
|
||||
const result = await sut.getActiveUserOrganizationSsoIdentifier(userId);
|
||||
expect(result).toBe(orgIdentifier);
|
||||
@@ -91,4 +97,153 @@ describe("SSOLoginService ", () => {
|
||||
"Tried to set a user organization sso identifier with an undefined user id.",
|
||||
);
|
||||
});
|
||||
|
||||
describe("updateSsoRequiredCache()", () => {
|
||||
it("should add email to cache when SSO is required", async () => {
|
||||
const email = "test@example.com";
|
||||
|
||||
mockStateProvider.global.getFake(SSO_REQUIRED_CACHE).stateSubject.next([]);
|
||||
mockStateProvider.global.getFake(SSO_EMAIL).stateSubject.next(email);
|
||||
mockPolicyService.policyAppliesToUser$.mockReturnValue(of(true));
|
||||
|
||||
await sut.updateSsoRequiredCache(email, userId);
|
||||
|
||||
const cacheState = mockStateProvider.global.getFake(SSO_REQUIRED_CACHE);
|
||||
expect(cacheState.nextMock).toHaveBeenCalledWith([email.toLowerCase()]);
|
||||
});
|
||||
|
||||
it("should add email to existing cache when SSO is required and email is not already present", async () => {
|
||||
const existingEmail = "existing@example.com";
|
||||
const newEmail = "new@example.com";
|
||||
|
||||
mockStateProvider.global.getFake(SSO_REQUIRED_CACHE).stateSubject.next([existingEmail]);
|
||||
mockStateProvider.global.getFake(SSO_EMAIL).stateSubject.next(newEmail);
|
||||
mockPolicyService.policyAppliesToUser$.mockReturnValue(of(true));
|
||||
|
||||
await sut.updateSsoRequiredCache(newEmail, userId);
|
||||
|
||||
const cacheState = mockStateProvider.global.getFake(SSO_REQUIRED_CACHE);
|
||||
expect(cacheState.nextMock).toHaveBeenCalledWith([existingEmail, newEmail.toLowerCase()]);
|
||||
});
|
||||
|
||||
it("should not add duplicate email to cache when SSO is required", async () => {
|
||||
const duplicateEmail = "duplicate@example.com";
|
||||
|
||||
mockStateProvider.global.getFake(SSO_REQUIRED_CACHE).stateSubject.next([duplicateEmail]);
|
||||
mockStateProvider.global.getFake(SSO_EMAIL).stateSubject.next(duplicateEmail);
|
||||
mockPolicyService.policyAppliesToUser$.mockReturnValue(of(true));
|
||||
|
||||
await sut.updateSsoRequiredCache(duplicateEmail, userId);
|
||||
|
||||
const cacheState = mockStateProvider.global.getFake(SSO_REQUIRED_CACHE);
|
||||
expect(cacheState.nextMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should initialize new cache with email when SSO is required and no cache exists", async () => {
|
||||
const email = "test@example.com";
|
||||
|
||||
mockStateProvider.global.getFake(SSO_REQUIRED_CACHE).stateSubject.next(null);
|
||||
mockStateProvider.global.getFake(SSO_EMAIL).stateSubject.next(email);
|
||||
mockPolicyService.policyAppliesToUser$.mockReturnValue(of(true));
|
||||
|
||||
await sut.updateSsoRequiredCache(email, userId);
|
||||
|
||||
const cacheState = mockStateProvider.global.getFake(SSO_REQUIRED_CACHE);
|
||||
expect(cacheState.nextMock).toHaveBeenCalledWith([email.toLowerCase()]);
|
||||
});
|
||||
|
||||
it("should remove email from cache when SSO is not required", async () => {
|
||||
const emailToRemove = "remove@example.com";
|
||||
const remainingEmail = "keep@example.com";
|
||||
|
||||
mockStateProvider.global
|
||||
.getFake(SSO_REQUIRED_CACHE)
|
||||
.stateSubject.next([emailToRemove, remainingEmail]);
|
||||
mockStateProvider.global.getFake(SSO_EMAIL).stateSubject.next(emailToRemove);
|
||||
mockPolicyService.policyAppliesToUser$.mockReturnValue(of(false));
|
||||
|
||||
await sut.updateSsoRequiredCache(emailToRemove, userId);
|
||||
|
||||
const cacheState = mockStateProvider.global.getFake(SSO_REQUIRED_CACHE);
|
||||
expect(cacheState.nextMock).toHaveBeenCalledWith([remainingEmail]);
|
||||
});
|
||||
|
||||
it("should not update cache when SSO is not required and email is not present", async () => {
|
||||
const existingEmail = "existing@example.com";
|
||||
const nonExistentEmail = "nonexistent@example.com";
|
||||
|
||||
mockStateProvider.global.getFake(SSO_REQUIRED_CACHE).stateSubject.next([existingEmail]);
|
||||
mockStateProvider.global.getFake(SSO_EMAIL).stateSubject.next(nonExistentEmail);
|
||||
mockPolicyService.policyAppliesToUser$.mockReturnValue(of(false));
|
||||
|
||||
await sut.updateSsoRequiredCache(nonExistentEmail, userId);
|
||||
|
||||
const cacheState = mockStateProvider.global.getFake(SSO_REQUIRED_CACHE);
|
||||
expect(cacheState.nextMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should check policy for correct PolicyType and userId", async () => {
|
||||
const email = "test@example.com";
|
||||
|
||||
mockStateProvider.global.getFake(SSO_REQUIRED_CACHE).stateSubject.next([]);
|
||||
mockPolicyService.policyAppliesToUser$.mockReturnValue(of(true));
|
||||
|
||||
await sut.updateSsoRequiredCache(email, userId);
|
||||
|
||||
expect(mockPolicyService.policyAppliesToUser$).toHaveBeenCalledWith(
|
||||
PolicyType.RequireSso,
|
||||
userId,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("removeFromSsoRequiredCacheIfPresent()", () => {
|
||||
it("should remove email from cache when present", async () => {
|
||||
const emailToRemove = "remove@example.com";
|
||||
const remainingEmail = "keep@example.com";
|
||||
|
||||
mockStateProvider.global
|
||||
.getFake(SSO_REQUIRED_CACHE)
|
||||
.stateSubject.next([emailToRemove, remainingEmail]);
|
||||
|
||||
await sut.removeFromSsoRequiredCacheIfPresent(emailToRemove);
|
||||
|
||||
const cacheState = mockStateProvider.global.getFake(SSO_REQUIRED_CACHE);
|
||||
expect(cacheState.nextMock).toHaveBeenCalledWith([remainingEmail]);
|
||||
});
|
||||
|
||||
it("should not update cache when email is not present", async () => {
|
||||
const existingEmail = "existing@example.com";
|
||||
const nonExistentEmail = "nonexistent@example.com";
|
||||
|
||||
mockStateProvider.global.getFake(SSO_REQUIRED_CACHE).stateSubject.next([existingEmail]);
|
||||
|
||||
await sut.removeFromSsoRequiredCacheIfPresent(nonExistentEmail);
|
||||
|
||||
const cacheState = mockStateProvider.global.getFake(SSO_REQUIRED_CACHE);
|
||||
expect(cacheState.nextMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should not update cache when cache is already null", async () => {
|
||||
const email = "test@example.com";
|
||||
|
||||
mockStateProvider.global.getFake(SSO_REQUIRED_CACHE).stateSubject.next(null);
|
||||
|
||||
await sut.removeFromSsoRequiredCacheIfPresent(email);
|
||||
|
||||
const cacheState = mockStateProvider.global.getFake(SSO_REQUIRED_CACHE);
|
||||
expect(cacheState.nextMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should result in an empty array when removing last email", async () => {
|
||||
const email = "test@example.com";
|
||||
|
||||
mockStateProvider.global.getFake(SSO_REQUIRED_CACHE).stateSubject.next([email]);
|
||||
|
||||
await sut.removeFromSsoRequiredCacheIfPresent(email);
|
||||
|
||||
const cacheState = mockStateProvider.global.getFake(SSO_REQUIRED_CACHE);
|
||||
expect(cacheState.nextMock).toHaveBeenCalledWith([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import { firstValueFrom } from "rxjs";
|
||||
import { firstValueFrom, map, Observable } from "rxjs";
|
||||
|
||||
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
|
||||
import { PolicyType } from "@bitwarden/common/admin-console/enums";
|
||||
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
|
||||
@@ -8,6 +10,7 @@ import {
|
||||
KeyDefinition,
|
||||
SingleUserState,
|
||||
SSO_DISK,
|
||||
SSO_DISK_LOCAL,
|
||||
StateProvider,
|
||||
UserKeyDefinition,
|
||||
} from "../../platform/state";
|
||||
@@ -57,20 +60,35 @@ export const SSO_EMAIL = new KeyDefinition<string>(SSO_DISK, "ssoEmail", {
|
||||
deserializer: (state) => state,
|
||||
});
|
||||
|
||||
/**
|
||||
* A cache list of user emails for whom the `PolicyType.RequireSso` policy is applied (that is, a list
|
||||
* of users who are required to authenticate via SSO only). The cache lives on the current device only.
|
||||
*/
|
||||
export const SSO_REQUIRED_CACHE = new KeyDefinition<string[]>(SSO_DISK_LOCAL, "ssoRequiredCache", {
|
||||
deserializer: (ssoRequiredCache) => ssoRequiredCache,
|
||||
});
|
||||
|
||||
export class SsoLoginService implements SsoLoginServiceAbstraction {
|
||||
private codeVerifierState: GlobalState<string>;
|
||||
private ssoState: GlobalState<string>;
|
||||
private orgSsoIdentifierState: GlobalState<string>;
|
||||
private ssoEmailState: GlobalState<string>;
|
||||
private ssoRequiredCacheState: GlobalState<string[]>;
|
||||
|
||||
ssoRequiredCache$: Observable<Set<string> | null>;
|
||||
|
||||
constructor(
|
||||
private stateProvider: StateProvider,
|
||||
private logService: LogService,
|
||||
private policyService: PolicyService,
|
||||
) {
|
||||
this.codeVerifierState = this.stateProvider.getGlobal(CODE_VERIFIER);
|
||||
this.ssoState = this.stateProvider.getGlobal(SSO_STATE);
|
||||
this.orgSsoIdentifierState = this.stateProvider.getGlobal(GLOBAL_ORGANIZATION_SSO_IDENTIFIER);
|
||||
this.ssoEmailState = this.stateProvider.getGlobal(SSO_EMAIL);
|
||||
this.ssoRequiredCacheState = this.stateProvider.getGlobal(SSO_REQUIRED_CACHE);
|
||||
|
||||
this.ssoRequiredCache$ = this.ssoRequiredCacheState.state$.pipe(map((cache) => new Set(cache)));
|
||||
}
|
||||
|
||||
getCodeVerifier(): Promise<string | null> {
|
||||
@@ -105,6 +123,10 @@ export class SsoLoginService implements SsoLoginServiceAbstraction {
|
||||
await this.ssoEmailState.update((_) => email);
|
||||
}
|
||||
|
||||
async clearSsoEmail(): Promise<void> {
|
||||
await this.ssoEmailState.update((_) => null);
|
||||
}
|
||||
|
||||
getActiveUserOrganizationSsoIdentifier(userId: UserId): Promise<string | null> {
|
||||
return firstValueFrom(this.userOrgSsoIdentifierState(userId).state$);
|
||||
}
|
||||
@@ -125,4 +147,53 @@ export class SsoLoginService implements SsoLoginServiceAbstraction {
|
||||
private userOrgSsoIdentifierState(userId: UserId): SingleUserState<string> {
|
||||
return this.stateProvider.getUser(userId, USER_ORGANIZATION_SSO_IDENTIFIER);
|
||||
}
|
||||
|
||||
/**
|
||||
* Add an email to the cached list of emails that must authenticate via SSO.
|
||||
*/
|
||||
private async addToSsoRequiredCache(email: string): Promise<void> {
|
||||
await this.ssoRequiredCacheState.update(
|
||||
(cache) => (cache == null ? [email] : [...cache, email]),
|
||||
{
|
||||
shouldUpdate: (cache) => {
|
||||
if (cache == null) {
|
||||
return true;
|
||||
}
|
||||
return !cache.includes(email);
|
||||
},
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
async removeFromSsoRequiredCacheIfPresent(email: string): Promise<void> {
|
||||
await this.ssoRequiredCacheState.update(
|
||||
(cache) => cache?.filter((cachedEmail) => cachedEmail !== email) ?? cache,
|
||||
{
|
||||
shouldUpdate: (cache) => {
|
||||
if (cache == null) {
|
||||
return false;
|
||||
}
|
||||
return cache.includes(email);
|
||||
},
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
async updateSsoRequiredCache(ssoLoginEmail: string, userId: UserId): Promise<void> {
|
||||
const ssoRequired = await firstValueFrom(
|
||||
this.policyService.policyAppliesToUser$(PolicyType.RequireSso, userId),
|
||||
);
|
||||
|
||||
if (ssoRequired) {
|
||||
await this.addToSsoRequiredCache(ssoLoginEmail.toLowerCase());
|
||||
} else {
|
||||
/**
|
||||
* If user is not required to authenticate via SSO, remove email from the cache
|
||||
* list (if it was on the list). This is necessary because the user may have been
|
||||
* required to authenticate via SSO at some point in the past, but now their org
|
||||
* no longer requires SSO authenticaiton.
|
||||
*/
|
||||
await this.removeFromSsoRequiredCacheIfPresent(ssoLoginEmail.toLowerCase());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ export enum FeatureFlag {
|
||||
|
||||
/* Auth */
|
||||
PM14938_BrowserExtensionLoginApproval = "pm-14938-browser-extension-login-approvals",
|
||||
PM22110_DisableAlternateLoginMethods = "pm-22110-disable-alternate-login-methods",
|
||||
|
||||
/* Autofill */
|
||||
MacOsNativeCredentialSync = "macos-native-credential-sync",
|
||||
@@ -98,6 +99,7 @@ export const DefaultFeatureFlagValue = {
|
||||
|
||||
/* Auth */
|
||||
[FeatureFlag.PM14938_BrowserExtensionLoginApproval]: FALSE,
|
||||
[FeatureFlag.PM22110_DisableAlternateLoginMethods]: FALSE,
|
||||
|
||||
/* Billing */
|
||||
[FeatureFlag.TrialPaymentOptional]: FALSE,
|
||||
|
||||
Reference in New Issue
Block a user