Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Commit 46072ca

Browse files
Kerryrichvdh
Kerry
and
richvdh
authored
OIDC: persist refresh token (#11249)
* test persistCredentials without a pickle key * test setLoggedIn with pickle key * lint * type error * extract token persisting code into function, persist refresh token * store has_refresh_token too * pass refreshToken from oidcAuthGrant into credentials * rest restore session with pickle key * comments * prettier * Update src/Lifecycle.ts Co-authored-by: Richard van der Hoff <[email protected]> * comments --------- Co-authored-by: Richard van der Hoff <[email protected]>
1 parent 50ee43c commit 46072ca

File tree

5 files changed

+128
-49
lines changed

5 files changed

+128
-49
lines changed

‎src/Lifecycle.ts‎

Lines changed: 93 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,23 @@ import GenericToast from "./components/views/toasts/GenericToast";
7171
const HOMESERVER_URL_KEY = "mx_hs_url";
7272
const ID_SERVER_URL_KEY = "mx_is_url";
7373

74+
/*
75+
* Keys used when storing the tokens in indexeddb or localstorage
76+
*/
77+
const ACCESS_TOKEN_STORAGE_KEY = "mx_access_token";
78+
const REFRESH_TOKEN_STORAGE_KEY = "mx_refresh_token";
79+
/*
80+
* Used as initialization vector during encryption in persistTokenInStorage
81+
* And decryption in restoreFromLocalStorage
82+
*/
83+
const ACCESS_TOKEN_IV = "access_token";
84+
const REFRESH_TOKEN_IV = "refresh_token";
85+
/*
86+
* Keys for localstorage items which indicate whether we expect a token in indexeddb.
87+
*/
88+
const HAS_ACCESS_TOKEN_STORAGE_KEY = "mx_has_access_token";
89+
const HAS_REFRESH_TOKEN_STORAGE_KEY = "mx_has_refresh_token";
90+
7491
dis.register((payload) => {
7592
if (payload.action === Action.TriggerLogout) {
7693
// noinspection JSIgnoredPromiseFromCall - we don't care if it fails
@@ -261,9 +278,8 @@ export async function attemptDelegatedAuthLogin(
261278
*/
262279
async function attemptOidcNativeLogin(queryParams: QueryDict): Promise<boolean> {
263280
try {
264-
const { accessToken, homeserverUrl, identityServerUrl, clientId, issuer } = await completeOidcLogin(
265-
queryParams,
266-
);
281+
const { accessToken, refreshToken, homeserverUrl, identityServerUrl, clientId, issuer } =
282+
await completeOidcLogin(queryParams);
267283

268284
const {
269285
user_id: userId,
@@ -273,6 +289,7 @@ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise<boolean>
273289

274290
const credentials = {
275291
accessToken,
292+
refreshToken,
276293
homeserverUrl,
277294
identityServerUrl,
278295
deviceId,
@@ -503,25 +520,25 @@ export async function getStoredSessionVars(): Promise<Partial<IStoredSession>> {
503520
const isUrl = localStorage.getItem(ID_SERVER_URL_KEY) ?? undefined;
504521
let accessToken: string | undefined;
505522
try {
506-
accessToken = await StorageManager.idbLoad("account", "mx_access_token");
523+
accessToken = await StorageManager.idbLoad("account", ACCESS_TOKEN_STORAGE_KEY);
507524
} catch (e) {
508525
logger.error("StorageManager.idbLoad failed for account:mx_access_token", e);
509526
}
510527
if (!accessToken) {
511-
accessToken = localStorage.getItem("mx_access_token") ?? undefined;
528+
accessToken = localStorage.getItem(ACCESS_TOKEN_STORAGE_KEY) ?? undefined;
512529
if (accessToken) {
513530
try {
514531
// try to migrate access token to IndexedDB if we can
515-
await StorageManager.idbSave("account", "mx_access_token", accessToken);
516-
localStorage.removeItem("mx_access_token");
532+
await StorageManager.idbSave("account", ACCESS_TOKEN_STORAGE_KEY, accessToken);
533+
localStorage.removeItem(ACCESS_TOKEN_STORAGE_KEY);
517534
} catch (e) {
518535
logger.error("migration of access token to IndexedDB failed", e);
519536
}
520537
}
521538
}
522539
// if we pre-date storing "mx_has_access_token", but we retrieved an access
523540
// token, then we should say we have an access token
524-
const hasAccessToken = localStorage.getItem("mx_has_access_token") === "true" || !!accessToken;
541+
const hasAccessToken = localStorage.getItem(HAS_ACCESS_TOKEN_STORAGE_KEY) === "true" || !!accessToken;
525542
const userId = localStorage.getItem("mx_user_id") ?? undefined;
526543
const deviceId = localStorage.getItem("mx_device_id") ?? undefined;
527544

@@ -607,7 +624,7 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }):
607624
logger.log("Got pickle key");
608625
if (typeof accessToken !== "string") {
609626
const encrKey = await pickleKeyToAesKey(pickleKey);
610-
decryptedAccessToken = await decryptAES(accessToken, encrKey, "access_token");
627+
decryptedAccessToken = await decryptAES(accessToken, encrKey, ACCESS_TOKEN_IV);
611628
encrKey.fill(0);
612629
}
613630
} else {
@@ -846,28 +863,41 @@ async function showStorageEvictedDialog(): Promise<boolean> {
846863
// `instanceof`. Babel 7 supports this natively in their class handling.
847864
class AbortLoginAndRebuildStorage extends Error {}
848865

849-
async function persistCredentials(credentials: IMatrixClientCreds): Promise<void> {
850-
localStorage.setItem(HOMESERVER_URL_KEY, credentials.homeserverUrl);
851-
if (credentials.identityServerUrl) {
852-
localStorage.setItem(ID_SERVER_URL_KEY, credentials.identityServerUrl);
853-
}
854-
localStorage.setItem("mx_user_id", credentials.userId);
855-
localStorage.setItem("mx_is_guest", JSON.stringify(credentials.guest));
856-
857-
// store whether we expect to find an access token, to detect the case
866+
/**
867+
* Persist a token in storage
868+
* When pickle key is present, will attempt to encrypt the token
869+
* Stores in idb, falling back to localStorage
870+
*
871+
* @param storageKey key used to store the token
872+
* @param initializationVector Initialization vector for encrypting the token. Only used when `pickleKey` is present
873+
* @param token the token to store, when undefined any existing token at the storageKey is removed from storage
874+
* @param pickleKey optional pickle key used to encrypt token
875+
* @param hasTokenStorageKey Localstorage key for an item which stores whether we expect to have a token in indexeddb, eg "mx_has_access_token".
876+
*/
877+
async function persistTokenInStorage(
878+
storageKey: string,
879+
initializationVector: string,
880+
token: string | undefined,
881+
pickleKey: string | undefined,
882+
hasTokenStorageKey: string,
883+
): Promise<void> {
884+
// store whether we expect to find a token, to detect the case
858885
// where IndexedDB is blown away
859-
if (credentials.accessToken) {
860-
localStorage.setItem("mx_has_access_token", "true");
886+
if (token) {
887+
localStorage.setItem(hasTokenStorageKey, "true");
861888
} else {
862-
localStorage.removeItem("mx_has_access_token");
889+
localStorage.removeItem(hasTokenStorageKey);
863890
}
864891

865-
if (credentials.pickleKey) {
866-
let encryptedAccessToken: IEncryptedPayload | undefined;
892+
if (pickleKey) {
893+
let encryptedToken: IEncryptedPayload | undefined;
867894
try {
895+
if (!token) {
896+
throw new Error("No token: not attempting encryption");
897+
}
868898
// try to encrypt the access token using the pickle key
869-
const encrKey = await pickleKeyToAesKey(credentials.pickleKey);
870-
encryptedAccessToken = await encryptAES(credentials.accessToken, encrKey, "access_token");
899+
const encrKey = await pickleKeyToAesKey(pickleKey);
900+
encryptedToken = await encryptAES(token, encrKey, initializationVector);
871901
encrKey.fill(0);
872902
} catch (e) {
873903
logger.warn("Could not encrypt access token", e);
@@ -876,28 +906,56 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise<void
876906
// save either the encrypted access token, or the plain access
877907
// token if we were unable to encrypt (e.g. if the browser doesn't
878908
// have WebCrypto).
879-
await StorageManager.idbSave("account", "mx_access_token", encryptedAccessToken || credentials.accessToken);
909+
await StorageManager.idbSave("account", storageKey, encryptedToken || token);
880910
} catch (e) {
881911
// if we couldn't save to indexedDB, fall back to localStorage. We
882912
// store the access token unencrypted since localStorage only saves
883913
// strings.
884-
if (!!credentials.accessToken) {
885-
localStorage.setItem("mx_access_token", credentials.accessToken);
914+
if (!!token) {
915+
localStorage.setItem(storageKey, token);
886916
} else {
887-
localStorage.removeItem("mx_access_token");
917+
localStorage.removeItem(storageKey);
888918
}
889919
}
890-
localStorage.setItem("mx_has_pickle_key", String(true));
891920
} else {
892921
try {
893-
await StorageManager.idbSave("account", "mx_access_token", credentials.accessToken);
922+
await StorageManager.idbSave("account", storageKey, token);
894923
} catch (e) {
895-
if (!!credentials.accessToken) {
896-
localStorage.setItem("mx_access_token", credentials.accessToken);
924+
if (!!token) {
925+
localStorage.setItem(storageKey, token);
897926
} else {
898-
localStorage.removeItem("mx_access_token");
927+
localStorage.removeItem(storageKey);
899928
}
900929
}
930+
}
931+
}
932+
933+
async function persistCredentials(credentials: IMatrixClientCreds): Promise<void> {
934+
localStorage.setItem(HOMESERVER_URL_KEY, credentials.homeserverUrl);
935+
if (credentials.identityServerUrl) {
936+
localStorage.setItem(ID_SERVER_URL_KEY, credentials.identityServerUrl);
937+
}
938+
localStorage.setItem("mx_user_id", credentials.userId);
939+
localStorage.setItem("mx_is_guest", JSON.stringify(credentials.guest));
940+
941+
await persistTokenInStorage(
942+
ACCESS_TOKEN_STORAGE_KEY,
943+
ACCESS_TOKEN_IV,
944+
credentials.accessToken,
945+
credentials.pickleKey,
946+
HAS_ACCESS_TOKEN_STORAGE_KEY,
947+
);
948+
await persistTokenInStorage(
949+
REFRESH_TOKEN_STORAGE_KEY,
950+
REFRESH_TOKEN_IV,
951+
credentials.refreshToken,
952+
credentials.pickleKey,
953+
HAS_REFRESH_TOKEN_STORAGE_KEY,
954+
);
955+
956+
if (credentials.pickleKey) {
957+
localStorage.setItem("mx_has_pickle_key", String(true));
958+
} else {
901959
if (localStorage.getItem("mx_has_pickle_key") === "true") {
902960
logger.error("Expected a pickle key, but none provided. Encryption may not work.");
903961
}
@@ -1090,7 +1148,7 @@ async function clearStorage(opts?: { deleteEverything?: boolean }): Promise<void
10901148
AbstractLocalStorageSettingsHandler.clear();
10911149

10921150
try {
1093-
await StorageManager.idbDelete("account", "mx_access_token");
1151+
await StorageManager.idbDelete("account", ACCESS_TOKEN_STORAGE_KEY);
10941152
} catch (e) {
10951153
logger.error("idbDelete failed for account:mx_access_token", e);
10961154
}

‎src/MatrixClientPeg.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ export interface IMatrixClientCreds {
5656
userId: string;
5757
deviceId?: string;
5858
accessToken: string;
59+
refreshToken?: string;
5960
guest?: boolean;
6061
pickleKey?: string;
6162
freshLogin?: boolean;

‎src/utils/oidc/authorize.ts‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -68,31 +68,36 @@ const getCodeAndStateFromQueryParams = (queryParams: QueryDict): { code: string;
6868
return { code, state };
6969
};
7070

71-
/**
72-
* Attempt to complete authorization code flow to get an access token
73-
* @param queryParams the query-parameters extracted from the real query-string of the starting URI.
74-
* @returns Promise that resolves with accessToken, identityServerUrl, and homeserverUrl when login was successful
75-
* @throws When we failed to get a valid access token
76-
*/
77-
export const completeOidcLogin = async (
78-
queryParams: QueryDict,
79-
): Promise<{
71+
type CompleteOidcLoginResponse = {
72+
// url of the homeserver selected during login
8073
homeserverUrl: string;
74+
// identity server url as discovered during login
8175
identityServerUrl?: string;
76+
// accessToken gained from OIDC token issuer
8277
accessToken: string;
78+
// refreshToken gained from OIDC token issuer, when falsy token cannot be refreshed
79+
refreshToken?: string;
80+
// this client's id as registered with the OIDC issuer
8381
clientId: string;
82+
// issuer used during authentication
8483
issuer: string;
85-
}> => {
84+
};
85+
/**
86+
* Attempt to complete authorization code flow to get an access token
87+
* @param queryParams the query-parameters extracted from the real query-string of the starting URI.
88+
* @returns Promise that resolves with a CompleteOidcLoginResponse when login was successful
89+
* @throws When we failed to get a valid access token
90+
*/
91+
export const completeOidcLogin = async (queryParams: QueryDict): Promise<CompleteOidcLoginResponse> => {
8692
const { code, state } = getCodeAndStateFromQueryParams(queryParams);
8793
const { homeserverUrl, tokenResponse, identityServerUrl, oidcClientSettings } =
8894
await completeAuthorizationCodeGrant(code, state);
8995

90-
// @TODO(kerrya) do something with the refresh token https://github.com/vector-im/element-web/issues/25444
91-
9296
return {
93-
homeserverUrl: homeserverUrl,
94-
identityServerUrl: identityServerUrl,
97+
homeserverUrl,
98+
identityServerUrl,
9599
accessToken: tokenResponse.access_token,
100+
refreshToken: tokenResponse.refresh_token,
96101
clientId: oidcClientSettings.clientId,
97102
issuer: oidcClientSettings.issuer,
98103
};

‎test/Lifecycle-test.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,8 @@ describe("Lifecycle", () => {
380380
jest.spyOn(mockPlatform, "createPickleKey");
381381
});
382382

383+
const refreshToken = "test-refresh-token";
384+
383385
it("should remove fresh login flag from session storage", async () => {
384386
await setLoggedIn(credentials);
385387

@@ -410,6 +412,18 @@ describe("Lifecycle", () => {
410412
expect(localStorage.setItem).not.toHaveBeenCalledWith("mx_access_token", accessToken);
411413
});
412414

415+
it("should persist a refreshToken when present", async () => {
416+
await setLoggedIn({
417+
...credentials,
418+
refreshToken,
419+
});
420+
421+
expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken);
422+
expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_refresh_token", refreshToken);
423+
// dont put accessToken in localstorage when we have idb
424+
expect(localStorage.setItem).not.toHaveBeenCalledWith("mx_access_token", accessToken);
425+
});
426+
413427
it("should remove any access token from storage when there is none in credentials and idb save fails", async () => {
414428
jest.spyOn(StorageManager, "idbSave").mockRejectedValue("oups");
415429
await setLoggedIn({

‎test/utils/oidc/authorize-test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ describe("OIDC authorization", () => {
132132

133133
expect(result).toEqual({
134134
accessToken: tokenResponse.access_token,
135+
refreshToken: tokenResponse.refresh_token,
135136
homeserverUrl,
136137
identityServerUrl,
137138
issuer,

0 commit comments

Comments
 (0)