Skip to content

Commit

Permalink
Auth/pm 7672/Update token service to return new token from state (#9706)
Browse files Browse the repository at this point in the history 
* Changed return structure

* Object changes

* Added missing assert.

* Updated tests to use SetTokensResult

* Fixed constructor

* PM-7672 - Fix tests + add new setTokens test around refresh token

* Removed change to refreshIdentityToken.

* Updated return definition.

---------

Co-authored-by: Jared Snider <[email protected]>
(cherry picked from commit 88cc37e)
  • Loading branch information
@trmartin4
trmartin4 committed Jun 19, 2024
1 parent 2ca778c commit a3eb262
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 62 deletions.
17 changes: 9 additions & 8 deletions libs/common/src/auth/abstractions/token.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Observable } from "rxjs";
import { VaultTimeoutAction } from "../../enums/vault-timeout-action.enum";
import { UserId } from "../../types/guid";
import { VaultTimeout } from "../../types/vault-timeout.type";
import { SetTokensResult } from "../models/domain/set-tokens-result";
import { DecodedAccessToken } from "../services/token.service";

export abstract class TokenService {
Expand All @@ -23,15 +24,15 @@ export abstract class TokenService {
* @param refreshToken The optional refresh token to set. Note: this is undefined when using the CLI Login Via API Key flow
* @param clientIdClientSecret The API Key Client ID and Client Secret to set.
*
* @returns A promise that resolves when the tokens have been set.
* @returns A promise that resolves with the SetTokensResult containing the tokens that were set.
*/
setTokens: (
accessToken: string,
vaultTimeoutAction: VaultTimeoutAction,
vaultTimeout: VaultTimeout,
refreshToken?: string,
clientIdClientSecret?: [string, string],
) => Promise<void>;
) => Promise<SetTokensResult>;

/**
* Clears the access token, refresh token, API Key Client ID, and API Key Client Secret out of memory, disk, and secure storage if supported.
Expand All @@ -47,13 +48,13 @@ export abstract class TokenService {
* @param accessToken The access token to set.
* @param vaultTimeoutAction The action to take when the vault times out.
* @param vaultTimeout The timeout for the vault.
* @returns A promise that resolves when the access token has been set.
* @returns A promise that resolves with the access token that has been set.
*/
setAccessToken: (
accessToken: string,
vaultTimeoutAction: VaultTimeoutAction,
vaultTimeout: VaultTimeout,
) => Promise<void>;
) => Promise<string>;

// TODO: revisit having this public clear method approach once the state service is fully deprecated.
/**
Expand Down Expand Up @@ -86,14 +87,14 @@ export abstract class TokenService {
* @param clientId The API Key Client ID to set.
* @param vaultTimeoutAction The action to take when the vault times out.
* @param vaultTimeout The timeout for the vault.
* @returns A promise that resolves when the API Key Client ID has been set.
* @returns A promise that resolves with the API Key Client ID that has been set.
*/
setClientId: (
clientId: string,
vaultTimeoutAction: VaultTimeoutAction,
vaultTimeout: VaultTimeout,
userId?: UserId,
) => Promise<void>;
) => Promise<string>;

/**
* Gets the API Key Client ID for the active user.
Expand All @@ -106,14 +107,14 @@ export abstract class TokenService {
* @param clientSecret The API Key Client Secret to set.
* @param vaultTimeoutAction The action to take when the vault times out.
* @param vaultTimeout The timeout for the vault.
* @returns A promise that resolves when the API Key Client Secret has been set.
* @returns A promise that resolves with the client secret that has been set.
*/
setClientSecret: (
clientSecret: string,
vaultTimeoutAction: VaultTimeoutAction,
vaultTimeout: VaultTimeout,
userId?: UserId,
) => Promise<void>;
) => Promise<string>;

/**
* Gets the API Key Client Secret for the active user.
Expand Down
10 changes: 10 additions & 0 deletions libs/common/src/auth/models/domain/set-tokens-result.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export class SetTokensResult {
constructor(accessToken: string, refreshToken?: string, clientIdSecretPair?: [string, string]) {
this.accessToken = accessToken;
this.refreshToken = refreshToken;
this.clientIdSecretPair = clientIdSecretPair;
}
accessToken: string;
refreshToken?: string;
clientIdSecretPair?: [string, string];
}
100 changes: 83 additions & 17 deletions libs/common/src/auth/services/token.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypt
import { CsprngArray } from "../../types/csprng";
import { UserId } from "../../types/guid";
import { VaultTimeout, VaultTimeoutStringType } from "../../types/vault-timeout.type";
import { SetTokensResult } from "../models/domain/set-tokens-result";

import { ACCOUNT_ACTIVE_ACCOUNT_ID } from "./account.service";
import {
Expand Down Expand Up @@ -232,7 +233,7 @@ describe("TokenService", () => {
describe("Memory storage tests", () => {
it("set the access token in memory", async () => {
// Act
await tokenService.setAccessToken(
const result = await tokenService.setAccessToken(
accessTokenJwt,
memoryVaultTimeoutAction,
memoryVaultTimeout,
Expand All @@ -241,13 +242,14 @@ describe("TokenService", () => {
expect(
singleUserStateProvider.getFake(userIdFromAccessToken, ACCESS_TOKEN_MEMORY).nextMock,
).toHaveBeenCalledWith(accessTokenJwt);
expect(result).toEqual(accessTokenJwt);
});
});

describe("Disk storage tests (secure storage not supported on platform)", () => {
it("should set the access token in disk", async () => {
// Act
await tokenService.setAccessToken(
const result = await tokenService.setAccessToken(
accessTokenJwt,
diskVaultTimeoutAction,
diskVaultTimeout,
Expand All @@ -256,6 +258,7 @@ describe("TokenService", () => {
expect(
singleUserStateProvider.getFake(userIdFromAccessToken, ACCESS_TOKEN_DISK).nextMock,
).toHaveBeenCalledWith(accessTokenJwt);
expect(result).toEqual(accessTokenJwt);
});
});

Expand Down Expand Up @@ -295,7 +298,7 @@ describe("TokenService", () => {
secureStorageService.get.mockResolvedValueOnce(null).mockResolvedValue(accessTokenKeyB64);

// Act
await tokenService.setAccessToken(
const result = await tokenService.setAccessToken(
accessTokenJwt,
diskVaultTimeoutAction,
diskVaultTimeout,
Expand All @@ -318,6 +321,9 @@ describe("TokenService", () => {
expect(
singleUserStateProvider.getFake(userIdFromAccessToken, ACCESS_TOKEN_MEMORY).nextMock,
).toHaveBeenCalledWith(null);

// assert that the decrypted access token was returned
expect(result).toEqual(accessTokenJwt);
});

it("should fallback to disk storage for the access token if the access token cannot be set in secure storage", async () => {
Expand All @@ -331,7 +337,7 @@ describe("TokenService", () => {
secureStorageService.get.mockResolvedValueOnce(null).mockResolvedValue(null);

// Act
await tokenService.setAccessToken(
const result = await tokenService.setAccessToken(
accessTokenJwt,
diskVaultTimeoutAction,
diskVaultTimeout,
Expand All @@ -355,6 +361,9 @@ describe("TokenService", () => {
expect(
singleUserStateProvider.getFake(userIdFromAccessToken, ACCESS_TOKEN_DISK).nextMock,
).toHaveBeenCalledWith(accessTokenJwt);

// assert that the decrypted access token was returned
expect(result).toEqual(accessTokenJwt);
});

it("should fallback to disk storage for the access token if secure storage errors on trying to get an existing access token key", async () => {
Expand All @@ -368,7 +377,7 @@ describe("TokenService", () => {
secureStorageService.get.mockRejectedValue(new Error(secureStorageError));

// Act
await tokenService.setAccessToken(
const result = await tokenService.setAccessToken(
accessTokenJwt,
diskVaultTimeoutAction,
diskVaultTimeout,
Expand All @@ -385,6 +394,9 @@ describe("TokenService", () => {
expect(
singleUserStateProvider.getFake(userIdFromAccessToken, ACCESS_TOKEN_DISK).nextMock,
).toHaveBeenCalledWith(accessTokenJwt);

// assert that the decrypted access token was returned
expect(result).toEqual(accessTokenJwt);
});
});
});
Expand Down Expand Up @@ -2376,18 +2388,21 @@ describe("TokenService", () => {
const clientId = "clientId";
const clientSecret = "clientSecret";

(tokenService as any)._setAccessToken = jest.fn();
// any hack allows for mocking private method.
(tokenService as any).setRefreshToken = jest.fn();
tokenService.setClientId = jest.fn();
tokenService.setClientSecret = jest.fn();
(tokenService as any)._setAccessToken = jest.fn().mockReturnValue(accessTokenJwt);
(tokenService as any).setRefreshToken = jest.fn().mockReturnValue(refreshToken);
tokenService.setClientId = jest.fn().mockReturnValue(clientId);
tokenService.setClientSecret = jest.fn().mockReturnValue(clientSecret);

// Act
// Note: passing a valid access token so that a valid user id can be determined from the access token
await tokenService.setTokens(accessTokenJwt, vaultTimeoutAction, vaultTimeout, refreshToken, [
clientId,
clientSecret,
]);
const result = await tokenService.setTokens(
accessTokenJwt,
vaultTimeoutAction,
vaultTimeout,
refreshToken,
[clientId, clientSecret],
);

// Assert
expect((tokenService as any)._setAccessToken).toHaveBeenCalledWith(
Expand Down Expand Up @@ -2417,6 +2432,44 @@ describe("TokenService", () => {
vaultTimeout,
userIdFromAccessToken,
);

expect(result).toStrictEqual(
new SetTokensResult(accessTokenJwt, refreshToken, [clientId, clientSecret]),
);
});

it("does not try to set the refresh token when it is not passed in", async () => {
// Arrange
const vaultTimeoutAction = VaultTimeoutAction.Lock;
const vaultTimeout = 30;

(tokenService as any)._setAccessToken = jest.fn().mockReturnValue(accessTokenJwt);
(tokenService as any).setRefreshToken = jest.fn();
tokenService.setClientId = jest.fn();
tokenService.setClientSecret = jest.fn();

// Act
const result = await tokenService.setTokens(
accessTokenJwt,
vaultTimeoutAction,
vaultTimeout,
null,
);

// Assert
expect((tokenService as any)._setAccessToken).toHaveBeenCalledWith(
accessTokenJwt,
vaultTimeoutAction,
vaultTimeout,
userIdFromAccessToken,
);

// any hack allows for testing private methods
expect((tokenService as any).setRefreshToken).not.toHaveBeenCalled();
expect(tokenService.setClientId).not.toHaveBeenCalled();
expect(tokenService.setClientSecret).not.toHaveBeenCalled();

expect(result).toStrictEqual(new SetTokensResult(accessTokenJwt));
});

it("does not try to set client id and client secret when they are not passed in", async () => {
Expand All @@ -2425,13 +2478,18 @@ describe("TokenService", () => {
const vaultTimeoutAction = VaultTimeoutAction.Lock;
const vaultTimeout = 30;

(tokenService as any)._setAccessToken = jest.fn();
(tokenService as any).setRefreshToken = jest.fn();
(tokenService as any)._setAccessToken = jest.fn().mockReturnValue(accessTokenJwt);
(tokenService as any).setRefreshToken = jest.fn().mockReturnValue(refreshToken);
tokenService.setClientId = jest.fn();
tokenService.setClientSecret = jest.fn();

// Act
await tokenService.setTokens(accessTokenJwt, vaultTimeoutAction, vaultTimeout, refreshToken);
const result = await tokenService.setTokens(
accessTokenJwt,
vaultTimeoutAction,
vaultTimeout,
refreshToken,
);

// Assert
expect((tokenService as any)._setAccessToken).toHaveBeenCalledWith(
Expand All @@ -2451,6 +2509,8 @@ describe("TokenService", () => {

expect(tokenService.setClientId).not.toHaveBeenCalled();
expect(tokenService.setClientSecret).not.toHaveBeenCalled();

expect(result).toStrictEqual(new SetTokensResult(accessTokenJwt, refreshToken));
});

it("throws an error when the access token is invalid", async () => {
Expand Down Expand Up @@ -2535,10 +2595,16 @@ describe("TokenService", () => {
(tokenService as any).setRefreshToken = jest.fn();

// Act
await tokenService.setTokens(accessTokenJwt, vaultTimeoutAction, vaultTimeout, refreshToken);
const result = await tokenService.setTokens(
accessTokenJwt,
vaultTimeoutAction,
vaultTimeout,
refreshToken,
);

// Assert
expect((tokenService as any).setRefreshToken).not.toHaveBeenCalled();
expect(result).toStrictEqual(new SetTokensResult(accessTokenJwt));
});
});

Expand Down
Loading

0 comments on commit a3eb262

Please sign in to comment.