From 2bbfbc7ddd5feee06d0e5404841775f662ee1ddb Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 6 Aug 2020 09:20:46 -0600 Subject: [PATCH] [CORL-1242] Ensure SSO Access Tokens are not persisted (#3080) * fix: ensure access tokens from sso are not persisted * fix: increment storage key field to bust existing usage Co-authored-by: Vinh --- src/core/client/framework/lib/auth/auth.ts | 13 ++++++++++--- .../framework/lib/bootstrap/CoralContext.tsx | 5 ++++- .../framework/lib/bootstrap/createManaged.tsx | 10 ++++++++-- .../framework/mutations/SetAccessTokenMutation.ts | 14 ++++++++++++-- .../stream/App/listeners/OnPymLogin.spec.tsx | 2 +- src/core/client/stream/App/listeners/OnPymLogin.ts | 2 +- src/core/client/stream/local/initLocalState.ts | 5 +++-- 7 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/core/client/framework/lib/auth/auth.ts b/src/core/client/framework/lib/auth/auth.ts index 4cb6f3b0d..e48dfd695 100644 --- a/src/core/client/framework/lib/auth/auth.ts +++ b/src/core/client/framework/lib/auth/auth.ts @@ -3,7 +3,7 @@ import { Claims, computeExpiresIn, parseAccessTokenClaims } from "./helpers"; /** * ACCESS_TOKEN_KEY is the key in storage where the accessToken is stored. */ -const ACCESS_TOKEN_KEY = "coral:v1:accessToken"; +const ACCESS_TOKEN_KEY = "coral:v2:accessToken"; export interface AuthState { /** @@ -19,7 +19,7 @@ export interface AuthState { export type AccessTokenProvider = () => string | undefined; -function parseAccessToken(accessToken: string) { +export function parseAccessToken(accessToken: string) { // Try to parse the access token claims. const claims = parseAccessTokenClaims(accessToken); if (!claims) { @@ -59,6 +59,13 @@ export function retrieveAccessToken() { } export function storeAccessToken(accessToken: string) { + // Parse the access token that's trying to be stored now. If it can't be + // parsed, it can't be stored. + const parsed = parseAccessToken(accessToken); + if (!parsed) { + return; + } + try { // Update the access token in storage. localStorage.setItem(ACCESS_TOKEN_KEY, accessToken); @@ -69,7 +76,7 @@ export function storeAccessToken(accessToken: string) { } // Return the parsed access token. - return parseAccessToken(accessToken); + return parsed; } export function deleteAccessToken() { diff --git a/src/core/client/framework/lib/bootstrap/CoralContext.tsx b/src/core/client/framework/lib/bootstrap/CoralContext.tsx index 93cca2971..8bc71bc2e 100644 --- a/src/core/client/framework/lib/bootstrap/CoralContext.tsx +++ b/src/core/client/framework/lib/bootstrap/CoralContext.tsx @@ -63,7 +63,10 @@ export interface CoralContext { eventEmitter: EventEmitter2; /** Clear session data. */ - clearSession: (nextAccessToken?: string | null) => Promise; + clearSession: ( + nextAccessToken?: string | null, + ephemeral?: boolean + ) => Promise; /** Change locale and rerender */ changeLocale: (locale: LanguageCode) => Promise; diff --git a/src/core/client/framework/lib/bootstrap/createManaged.tsx b/src/core/client/framework/lib/bootstrap/createManaged.tsx index 84edf1de7..3eedf4f9f 100644 --- a/src/core/client/framework/lib/bootstrap/createManaged.tsx +++ b/src/core/client/framework/lib/bootstrap/createManaged.tsx @@ -23,6 +23,7 @@ import { AccessTokenProvider, AuthState, deleteAccessToken, + parseAccessToken, retrieveAccessToken, storeAccessToken, } from "../auth"; @@ -160,7 +161,10 @@ function createManagedCoralContextProvider( } // This is called every time a user session starts or ends. - private clearSession = async (nextAccessToken?: string) => { + private clearSession = async ( + nextAccessToken?: string, + ephemeral?: boolean + ) => { // Clear session storage. void this.state.context.sessionStorage.clear(); @@ -169,7 +173,9 @@ function createManagedCoralContextProvider( // Parse the claims/token and update storage. const auth = nextAccessToken - ? storeAccessToken(nextAccessToken) + ? ephemeral + ? parseAccessToken(nextAccessToken) + : storeAccessToken(nextAccessToken) : deleteAccessToken(); // Create the new environment. diff --git a/src/core/client/framework/mutations/SetAccessTokenMutation.ts b/src/core/client/framework/mutations/SetAccessTokenMutation.ts index 73af5958f..e901ede7e 100644 --- a/src/core/client/framework/mutations/SetAccessTokenMutation.ts +++ b/src/core/client/framework/mutations/SetAccessTokenMutation.ts @@ -4,18 +4,28 @@ import { CoralContext } from "coral-framework/lib/bootstrap"; import { createMutation } from "coral-framework/lib/relay"; interface SetAccessTokenInput { + /** + * accessToken is the new access token to use for this session or persisted if + * `ephemeral` is falsy. + */ accessToken: string | null; + + /** + * ephemeral when set to true will ensure that the passed token is not + * persisted to storage. + */ + ephemeral?: boolean; } const SetAccessTokenMutation = createMutation( "setAccessToken", async ( environment: Environment, - input: SetAccessTokenInput, + { accessToken, ephemeral }: SetAccessTokenInput, { clearSession }: CoralContext ) => { // Clear current session, as we are starting a new one. - await clearSession(input.accessToken); + await clearSession(accessToken, ephemeral); } ); diff --git a/src/core/client/stream/App/listeners/OnPymLogin.spec.tsx b/src/core/client/stream/App/listeners/OnPymLogin.spec.tsx index 2002bcb38..9327b4573 100644 --- a/src/core/client/stream/App/listeners/OnPymLogin.spec.tsx +++ b/src/core/client/stream/App/listeners/OnPymLogin.spec.tsx @@ -16,7 +16,7 @@ it("Listens to event and calls setAccessToken", () => { const setAccessToken = createSinonStub( (s) => s.throws(), - (s) => s.withArgs({ accessToken }).returns(null) + (s) => s.withArgs({ accessToken, ephemeral: true }).returns(null) ); createRenderer().render( diff --git a/src/core/client/stream/App/listeners/OnPymLogin.ts b/src/core/client/stream/App/listeners/OnPymLogin.ts index f57429721..92e18632e 100644 --- a/src/core/client/stream/App/listeners/OnPymLogin.ts +++ b/src/core/client/stream/App/listeners/OnPymLogin.ts @@ -16,7 +16,7 @@ export class OnPymLogin extends Component { // Sets comment id through pym. props.pym.onMessage("login", (accessToken) => { - void this.props.setAccessToken({ accessToken }); + void this.props.setAccessToken({ accessToken, ephemeral: true }); }); } diff --git a/src/core/client/stream/local/initLocalState.ts b/src/core/client/stream/local/initLocalState.ts index dae8465d0..12cca6929 100644 --- a/src/core/client/stream/local/initLocalState.ts +++ b/src/core/client/stream/local/initLocalState.ts @@ -1,7 +1,7 @@ import { commitLocalUpdate, Environment } from "relay-runtime"; import { parseQuery } from "coral-common/utils"; -import { AuthState, storeAccessToken } from "coral-framework/lib/auth"; +import { AuthState, parseAccessToken } from "coral-framework/lib/auth"; import { CoralContext } from "coral-framework/lib/bootstrap"; import { getExternalConfig } from "coral-framework/lib/externalConfig"; import { createAndRetain, initLocalBaseState } from "coral-framework/lib/relay"; @@ -20,7 +20,8 @@ export default async function initLocalState( const config = await getExternalConfig(context.pym); if (config) { if (config.accessToken) { - auth = storeAccessToken(config.accessToken); + // Access tokens passed via the config should not be persisted. + auth = parseAccessToken(config.accessToken); } // append body class name if set in config. if (config.bodyClassName) {