1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-19 17:53:39 +00:00

feat(tokens): Allow Inactive user authenticated API calls

This commit is contained in:
Justin Baur
2025-09-03 10:09:02 -04:00
committed by GitHub
parent bcd7eb746a
commit 73e8532ecc
15 changed files with 406 additions and 362 deletions

View File

@@ -1,13 +1,19 @@
import { mock, MockProxy } from "jest-mock-extended";
import { of } from "rxjs";
import { ObservedValueOf, of } from "rxjs";
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
// eslint-disable-next-line no-restricted-imports
import { LogoutReason } from "@bitwarden/auth/common";
import { UserId } from "@bitwarden/user-core";
import { AccountService } from "../auth/abstractions/account.service";
import { TokenService } from "../auth/abstractions/token.service";
import { DeviceType } from "../enums";
import { VaultTimeoutSettingsService } from "../key-management/vault-timeout";
import {
VaultTimeoutAction,
VaultTimeoutSettingsService,
VaultTimeoutStringType,
} from "../key-management/vault-timeout";
import { ErrorResponse } from "../models/response/error.response";
import { AppIdService } from "../platform/abstractions/app-id.service";
import { Environment, EnvironmentService } from "../platform/abstractions/environment.service";
@@ -25,10 +31,14 @@ describe("ApiService", () => {
let logService: MockProxy<LogService>;
let logoutCallback: jest.Mock<Promise<void>, [reason: LogoutReason]>;
let vaultTimeoutSettingsService: MockProxy<VaultTimeoutSettingsService>;
let accountService: MockProxy<AccountService>;
let httpOperations: MockProxy<HttpOperations>;
let sut: ApiService;
const testActiveUser = "activeUser" as UserId;
const testInactiveUser = "inactiveUser" as UserId;
beforeEach(() => {
tokenService = mock();
platformUtilsService = mock();
@@ -40,6 +50,15 @@ describe("ApiService", () => {
logService = mock();
logoutCallback = jest.fn();
vaultTimeoutSettingsService = mock();
accountService = mock();
accountService.activeAccount$ = of({
id: testActiveUser,
email: "user1@example.com",
emailVerified: true,
name: "Test Name",
} satisfies ObservedValueOf<AccountService["activeAccount$"]>);
httpOperations = mock();
sut = new ApiService(
@@ -51,6 +70,7 @@ describe("ApiService", () => {
logService,
logoutCallback,
vaultTimeoutSettingsService,
accountService,
httpOperations,
"custom-user-agent",
);
@@ -62,6 +82,12 @@ describe("ApiService", () => {
getApiUrl: () => "https://example.com",
} satisfies Partial<Environment> as Environment);
environmentService.getEnvironment$.mockReturnValue(
of({
getApiUrl: () => "https://authed.example.com",
} satisfies Partial<Environment> as Environment),
);
httpOperations.createRequest.mockImplementation((url, request) => {
return {
url: url,
@@ -96,6 +122,7 @@ describe("ApiService", () => {
expect(nativeFetch).toHaveBeenCalledTimes(1);
const request = nativeFetch.mock.calls[0][0];
expect(request.url).toBe("https://authed.example.com/something");
// This should get set for users of send
expect(request.cache).toBe("no-store");
// TODO: Could expect on the credentials parameter
@@ -109,6 +136,185 @@ describe("ApiService", () => {
// The response body
expect(response).toEqual({ hello: "world" });
});
it("authenticates with non-active user when user is passed in", async () => {
environmentService.environment$ = of({
getApiUrl: () => "https://example.com",
} satisfies Partial<Environment> as Environment);
environmentService.getEnvironment$.calledWith(testInactiveUser).mockReturnValueOnce(
of({
getApiUrl: () => "https://inactive.example.com",
} satisfies Partial<Environment> as Environment),
);
httpOperations.createRequest.mockImplementation((url, request) => {
return {
url: url,
cache: request.cache,
credentials: request.credentials,
method: request.method,
mode: request.mode,
signal: request.signal,
headers: new Headers(request.headers),
} satisfies Partial<Request> as unknown as Request;
});
tokenService.getAccessToken
.calledWith(testInactiveUser)
.mockResolvedValue("inactive_access_token");
tokenService.tokenNeedsRefresh.calledWith(testInactiveUser).mockResolvedValue(false);
const nativeFetch = jest.fn<Promise<Response>, [request: Request]>();
nativeFetch.mockImplementation((request) => {
return Promise.resolve({
ok: true,
status: 200,
json: () => Promise.resolve({ hello: "world" }),
headers: new Headers({
"content-type": "application/json",
}),
} satisfies Partial<Response> as Response);
});
sut.nativeFetch = nativeFetch;
const response = await sut.send(
"GET",
"/something",
null,
testInactiveUser,
true,
null,
null,
);
expect(nativeFetch).toHaveBeenCalledTimes(1);
const request = nativeFetch.mock.calls[0][0];
expect(request.url).toBe("https://inactive.example.com/something");
// This should get set for users of send
expect(request.cache).toBe("no-store");
// TODO: Could expect on the credentials parameter
expect(request.headers.get("Device-Type")).toBe("2"); // Chrome Extension
// Custom user agent should get set
expect(request.headers.get("User-Agent")).toBe("custom-user-agent");
// This should be set when the caller has indicated there is a response
expect(request.headers.get("Accept")).toBe("application/json");
// If they have indicated that it's authed, then the authorization header should get set.
expect(request.headers.get("Authorization")).toBe("Bearer inactive_access_token");
// The response body
expect(response).toEqual({ hello: "world" });
});
const cases: {
name: string;
authedOrUserId: boolean | UserId;
expectedEffectiveUser: UserId;
}[] = [
{
name: "refreshes active user when true passed in for auth",
authedOrUserId: true,
expectedEffectiveUser: testActiveUser,
},
{
name: "refreshes acess token when the user passed in happens to be the active one",
authedOrUserId: testActiveUser,
expectedEffectiveUser: testActiveUser,
},
{
name: "refreshes access token when the user passed in happens to be inactive",
authedOrUserId: testInactiveUser,
expectedEffectiveUser: testInactiveUser,
},
];
it.each(cases)("$name does", async ({ authedOrUserId, expectedEffectiveUser }) => {
environmentService.getEnvironment$.calledWith(expectedEffectiveUser).mockReturnValue(
of({
getApiUrl: () => `https://${expectedEffectiveUser}.example.com`,
getIdentityUrl: () => `https://${expectedEffectiveUser}.identity.example.com`,
} satisfies Partial<Environment> as Environment),
);
tokenService.getAccessToken
.calledWith(expectedEffectiveUser)
.mockResolvedValue(`${expectedEffectiveUser}_access_token`);
tokenService.tokenNeedsRefresh.calledWith(expectedEffectiveUser).mockResolvedValue(true);
tokenService.getRefreshToken
.calledWith(expectedEffectiveUser)
.mockResolvedValue(`${expectedEffectiveUser}_refresh_token`);
tokenService.decodeAccessToken
.calledWith(expectedEffectiveUser)
.mockResolvedValue({ client_id: "web" });
tokenService.decodeAccessToken
.calledWith(`${expectedEffectiveUser}_new_access_token`)
.mockResolvedValue({ sub: expectedEffectiveUser });
vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$
.calledWith(expectedEffectiveUser)
.mockReturnValue(of(VaultTimeoutAction.Lock));
vaultTimeoutSettingsService.getVaultTimeoutByUserId$
.calledWith(expectedEffectiveUser)
.mockReturnValue(of(VaultTimeoutStringType.Never));
tokenService.setTokens
.calledWith(
`${expectedEffectiveUser}_new_access_token`,
VaultTimeoutAction.Lock,
VaultTimeoutStringType.Never,
`${expectedEffectiveUser}_new_refresh_token`,
)
.mockResolvedValue({ accessToken: `${expectedEffectiveUser}_refreshed_access_token` });
httpOperations.createRequest.mockImplementation((url, request) => {
return {
url: url,
cache: request.cache,
credentials: request.credentials,
method: request.method,
mode: request.mode,
signal: request.signal,
headers: new Headers(request.headers),
} satisfies Partial<Request> as unknown as Request;
});
const nativeFetch = jest.fn<Promise<Response>, [request: Request]>();
nativeFetch.mockImplementation((request) => {
if (request.url.includes("identity")) {
return Promise.resolve({
ok: true,
status: 200,
json: () =>
Promise.resolve({
access_token: `${expectedEffectiveUser}_new_access_token`,
refresh_token: `${expectedEffectiveUser}_new_refresh_token`,
}),
} satisfies Partial<Response> as Response);
}
return Promise.resolve({
ok: true,
status: 200,
json: () => Promise.resolve({ hello: "world" }),
headers: new Headers({
"content-type": "application/json",
}),
} satisfies Partial<Response> as Response);
});
sut.nativeFetch = nativeFetch;
await sut.send("GET", "/something", null, authedOrUserId, true, null, null);
expect(nativeFetch).toHaveBeenCalledTimes(2);
});
});
const errorData: {
@@ -169,9 +375,11 @@ describe("ApiService", () => {
it.each(errorData)(
"throws error-like response when not ok response with $name",
async ({ input, error }) => {
environmentService.environment$ = of({
getApiUrl: () => "https://example.com",
} satisfies Partial<Environment> as Environment);
environmentService.getEnvironment$.calledWith(testActiveUser).mockReturnValue(
of({
getApiUrl: () => "https://example.com",
} satisfies Partial<Environment> as Environment),
);
httpOperations.createRequest.mockImplementation((url, request) => {
return {

View File

@@ -1,6 +1,6 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { firstValueFrom } from "rxjs";
import { firstValueFrom, map } from "rxjs";
// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
// eslint-disable-next-line no-restricted-imports
@@ -47,6 +47,7 @@ import {
ProviderUserUserDetailsResponse,
} from "../admin-console/models/response/provider/provider-user.response";
import { SelectionReadOnlyResponse } from "../admin-console/models/response/selection-read-only.response";
import { AccountService } from "../auth/abstractions/account.service";
import { TokenService } from "../auth/abstractions/token.service";
import { DeviceVerificationRequest } from "../auth/models/request/device-verification.request";
import { DisableTwoFactorAuthenticatorRequest } from "../auth/models/request/disable-two-factor-authenticator.request";
@@ -121,7 +122,7 @@ import { ListResponse } from "../models/response/list.response";
import { ProfileResponse } from "../models/response/profile.response";
import { UserKeyResponse } from "../models/response/user-key.response";
import { AppIdService } from "../platform/abstractions/app-id.service";
import { EnvironmentService } from "../platform/abstractions/environment.service";
import { Environment, EnvironmentService } from "../platform/abstractions/environment.service";
import { LogService } from "../platform/abstractions/log.service";
import { PlatformUtilsService } from "../platform/abstractions/platform-utils.service";
import { flagEnabled } from "../platform/misc/flags";
@@ -155,7 +156,7 @@ export type HttpOperations = {
export class ApiService implements ApiServiceAbstraction {
private device: DeviceType;
private deviceType: string;
private refreshTokenPromise: Promise<string> | undefined;
private refreshTokenPromise: Record<UserId, Promise<string>> = {};
/**
* The message (responseJson.ErrorModel.Message) that comes back from the server when a new device verification is required.
@@ -172,6 +173,7 @@ export class ApiService implements ApiServiceAbstraction {
private logService: LogService,
private logoutCallback: (logoutReason: LogoutReason) => Promise<void>,
private vaultTimeoutSettingsService: VaultTimeoutSettingsService,
private readonly accountService: AccountService,
private readonly httpOperations: HttpOperations,
private customUserAgent: string = null,
) {
@@ -209,7 +211,7 @@ export class ApiService implements ApiServiceAbstraction {
const response = await this.fetch(
this.httpOperations.createRequest(env.getIdentityUrl() + "/connect/token", {
body: this.qsStringify(identityToken),
credentials: await this.getCredentials(),
credentials: await this.getCredentials(env),
cache: "no-store",
headers: headers,
method: "POST",
@@ -241,9 +243,13 @@ export class ApiService implements ApiServiceAbstraction {
return Promise.reject(new ErrorResponse(responseJson, response.status, true));
}
async refreshIdentityToken(): Promise<any> {
async refreshIdentityToken(userId: UserId | null = null): Promise<any> {
const normalizedUser = (userId ??= await this.getActiveUser());
if (normalizedUser == null) {
throw new Error("No user provided and no active user, cannot refresh the identity token.");
}
try {
await this.refreshToken();
await this.refreshToken(normalizedUser);
} catch (e) {
this.logService.error("Error refreshing access token: ", e);
throw e;
@@ -1398,11 +1404,16 @@ export class ApiService implements ApiServiceAbstraction {
if (this.customUserAgent != null) {
headers.set("User-Agent", this.customUserAgent);
}
const env = await firstValueFrom(this.environmentService.environment$);
const env = await firstValueFrom(
userId == null
? this.environmentService.environment$
: this.environmentService.getEnvironment$(userId),
);
const response = await this.fetch(
this.httpOperations.createRequest(env.getEventsUrl() + "/collect", {
cache: "no-store",
credentials: await this.getCredentials(),
credentials: await this.getCredentials(env),
method: "POST",
body: JSON.stringify(request),
headers: headers,
@@ -1444,7 +1455,11 @@ export class ApiService implements ApiServiceAbstraction {
async getMasterKeyFromKeyConnector(
keyConnectorUrl: string,
): Promise<KeyConnectorUserKeyResponse> {
const authHeader = await this.getActiveBearerToken();
const activeUser = await this.getActiveUser();
if (activeUser == null) {
throw new Error("No active user, cannot get master key from key connector.");
}
const authHeader = await this.getActiveBearerToken(activeUser);
const response = await this.fetch(
this.httpOperations.createRequest(keyConnectorUrl + "/user-keys", {
@@ -1469,7 +1484,11 @@ export class ApiService implements ApiServiceAbstraction {
keyConnectorUrl: string,
request: KeyConnectorUserKeyRequest,
): Promise<void> {
const authHeader = await this.getActiveBearerToken();
const activeUser = await this.getActiveUser();
if (activeUser == null) {
throw new Error("No active user, cannot post key to key connector.");
}
const authHeader = await this.getActiveBearerToken(activeUser);
const response = await this.fetch(
this.httpOperations.createRequest(keyConnectorUrl + "/user-keys", {
@@ -1521,10 +1540,10 @@ export class ApiService implements ApiServiceAbstraction {
// Helpers
async getActiveBearerToken(): Promise<string> {
let accessToken = await this.tokenService.getAccessToken();
if (await this.tokenService.tokenNeedsRefresh()) {
accessToken = await this.refreshToken();
async getActiveBearerToken(userId: UserId): Promise<string> {
let accessToken = await this.tokenService.getAccessToken(userId);
if (await this.tokenService.tokenNeedsRefresh(userId)) {
accessToken = await this.refreshToken(userId);
}
return accessToken;
}
@@ -1563,7 +1582,7 @@ export class ApiService implements ApiServiceAbstraction {
const response = await this.fetch(
this.httpOperations.createRequest(env.getIdentityUrl() + path, {
cache: "no-store",
credentials: await this.getCredentials(),
credentials: await this.getCredentials(env),
headers: headers,
method: "GET",
}),
@@ -1646,26 +1665,27 @@ export class ApiService implements ApiServiceAbstraction {
}
// Keep the running refreshTokenPromise to prevent parallel calls.
protected refreshToken(): Promise<string> {
if (this.refreshTokenPromise === undefined) {
this.refreshTokenPromise = this.internalRefreshToken();
void this.refreshTokenPromise.finally(() => {
this.refreshTokenPromise = undefined;
protected refreshToken(userId: UserId): Promise<string> {
if (this.refreshTokenPromise[userId] === undefined) {
// TODO: Have different promise for each user
this.refreshTokenPromise[userId] = this.internalRefreshToken(userId);
void this.refreshTokenPromise[userId].finally(() => {
delete this.refreshTokenPromise[userId];
});
}
return this.refreshTokenPromise;
return this.refreshTokenPromise[userId];
}
private async internalRefreshToken(): Promise<string> {
const refreshToken = await this.tokenService.getRefreshToken();
private async internalRefreshToken(userId: UserId): Promise<string> {
const refreshToken = await this.tokenService.getRefreshToken(userId);
if (refreshToken != null && refreshToken !== "") {
return this.refreshAccessToken();
return await this.refreshAccessToken(userId);
}
const clientId = await this.tokenService.getClientId();
const clientSecret = await this.tokenService.getClientSecret();
const clientId = await this.tokenService.getClientId(userId);
const clientSecret = await this.tokenService.getClientSecret(userId);
if (!Utils.isNullOrWhitespace(clientId) && !Utils.isNullOrWhitespace(clientSecret)) {
return this.refreshApiToken();
return await this.refreshApiToken(userId);
}
this.refreshAccessTokenErrorCallback();
@@ -1673,8 +1693,8 @@ export class ApiService implements ApiServiceAbstraction {
throw new Error("Cannot refresh access token, no refresh token or api keys are stored.");
}
protected async refreshAccessToken(): Promise<string> {
const refreshToken = await this.tokenService.getRefreshToken();
private async refreshAccessToken(userId: UserId): Promise<string> {
const refreshToken = await this.tokenService.getRefreshToken(userId);
if (refreshToken == null || refreshToken === "") {
throw new Error();
}
@@ -1687,8 +1707,8 @@ export class ApiService implements ApiServiceAbstraction {
headers.set("User-Agent", this.customUserAgent);
}
const env = await firstValueFrom(this.environmentService.environment$);
const decodedToken = await this.tokenService.decodeAccessToken();
const env = await firstValueFrom(this.environmentService.getEnvironment$(userId));
const decodedToken = await this.tokenService.decodeAccessToken(userId);
const response = await this.fetch(
this.httpOperations.createRequest(env.getIdentityUrl() + "/connect/token", {
body: this.qsStringify({
@@ -1697,7 +1717,7 @@ export class ApiService implements ApiServiceAbstraction {
refresh_token: refreshToken,
}),
cache: "no-store",
credentials: await this.getCredentials(),
credentials: await this.getCredentials(env),
headers: headers,
method: "POST",
}),
@@ -1732,9 +1752,9 @@ export class ApiService implements ApiServiceAbstraction {
}
}
protected async refreshApiToken(): Promise<string> {
const clientId = await this.tokenService.getClientId();
const clientSecret = await this.tokenService.getClientSecret();
protected async refreshApiToken(userId: UserId): Promise<string> {
const clientId = await this.tokenService.getClientId(userId);
const clientSecret = await this.tokenService.getClientSecret(userId);
const appId = await this.appIdService.getAppId();
const deviceRequest = new DeviceRequest(appId, this.platformUtilsService);
@@ -1751,7 +1771,12 @@ export class ApiService implements ApiServiceAbstraction {
}
const newDecodedAccessToken = await this.tokenService.decodeAccessToken(response.accessToken);
const userId = newDecodedAccessToken.sub;
if (newDecodedAccessToken.sub !== userId) {
throw new Error(
`Token was supposed to be refreshed for ${userId} but the token we got back was for ${newDecodedAccessToken.sub}`,
);
}
const vaultTimeoutAction = await firstValueFrom(
this.vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$(userId),
@@ -1772,12 +1797,28 @@ export class ApiService implements ApiServiceAbstraction {
method: "GET" | "POST" | "PUT" | "DELETE" | "PATCH",
path: string,
body: any,
authed: boolean,
authedOrUserId: UserId | boolean,
hasResponse: boolean,
apiUrl?: string | null,
alterHeaders?: (headers: Headers) => void,
): Promise<any> {
const env = await firstValueFrom(this.environmentService.environment$);
if (authedOrUserId == null) {
throw new Error("A user id was given but it was null, cannot complete API request.");
}
let userId: UserId | null = null;
if (typeof authedOrUserId === "boolean" && authedOrUserId) {
// Backwards compatible for authenticating the active user when `true` is passed in
userId = await this.getActiveUser();
} else if (typeof authedOrUserId === "string") {
userId = authedOrUserId;
}
const env = await firstValueFrom(
userId == null
? this.environmentService.environment$
: this.environmentService.getEnvironment$(userId),
);
apiUrl = Utils.isNullOrWhitespace(apiUrl) ? env.getApiUrl() : apiUrl;
// Prevent directory traversal from malicious paths
@@ -1786,7 +1827,7 @@ export class ApiService implements ApiServiceAbstraction {
apiUrl + Utils.normalizePath(pathParts[0]) + (pathParts.length > 1 ? `?${pathParts[1]}` : "");
const [requestHeaders, requestBody] = await this.buildHeadersAndBody(
authed,
userId,
hasResponse,
body,
alterHeaders,
@@ -1794,7 +1835,7 @@ export class ApiService implements ApiServiceAbstraction {
const requestInit: RequestInit = {
cache: "no-store",
credentials: await this.getCredentials(),
credentials: await this.getCredentials(env),
method: method,
};
requestInit.headers = requestHeaders;
@@ -1810,13 +1851,13 @@ export class ApiService implements ApiServiceAbstraction {
} else if (hasResponse && response.status === 200 && responseIsCsv) {
return await response.text();
} else if (response.status !== 200 && response.status !== 204) {
const error = await this.handleError(response, false, authed);
const error = await this.handleError(response, false, userId != null);
return Promise.reject(error);
}
}
private async buildHeadersAndBody(
authed: boolean,
userToAuthenticate: UserId | null,
hasResponse: boolean,
body: any,
alterHeaders: (headers: Headers) => void,
@@ -1838,8 +1879,8 @@ export class ApiService implements ApiServiceAbstraction {
if (alterHeaders != null) {
alterHeaders(headers);
}
if (authed) {
const authHeader = await this.getActiveBearerToken();
if (userToAuthenticate != null) {
const authHeader = await this.getActiveBearerToken(userToAuthenticate);
headers.set("Authorization", "Bearer " + authHeader);
} else {
// For unauthenticated requests, we need to tell the server what the device is for flag targeting,
@@ -1901,8 +1942,11 @@ export class ApiService implements ApiServiceAbstraction {
.join("&");
}
private async getCredentials(): Promise<RequestCredentials> {
const env = await firstValueFrom(this.environmentService.environment$);
private async getActiveUser(): Promise<UserId | null> {
return await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id)));
}
private async getCredentials(env: Environment): Promise<RequestCredentials> {
if (this.platformUtilsService.getClientType() !== ClientType.Web || env.hasBaseUrl()) {
return "include";
}