From 518ddc00bc38b2eeb820acb6c6e4d83c14a17811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 20 Dec 2021 12:11:14 +0100 Subject: [PATCH] Validate persisted session info on both save and load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/wise-melons-hope.md | 5 ++ packages/core-app-api/package.json | 3 +- .../implementations/auth/github/GithubAuth.ts | 15 +++--- .../apis/implementations/auth/github/index.ts | 2 +- .../apis/implementations/auth/github/types.ts | 23 ++++++++ .../implementations/auth/saml/SamlAuth.ts | 15 +++--- .../apis/implementations/auth/saml/types.ts | 22 +++++++- .../AuthSessionStore.test.ts | 48 +++++++++++++++++ .../AuthSessionManager/AuthSessionStore.ts | 52 ++++++++++++++----- 9 files changed, 155 insertions(+), 30 deletions(-) create mode 100644 .changeset/wise-melons-hope.md diff --git a/.changeset/wise-melons-hope.md b/.changeset/wise-melons-hope.md new file mode 100644 index 0000000000000..24a615a7bc526 --- /dev/null +++ b/.changeset/wise-melons-hope.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': patch +--- + +Schema-validate local storage cached session info on load diff --git a/packages/core-app-api/package.json b/packages/core-app-api/package.json index 166f39a484d4d..e1a982b7bde25 100644 --- a/packages/core-app-api/package.json +++ b/packages/core-app-api/package.json @@ -42,7 +42,8 @@ "prop-types": "^15.7.2", "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4", - "zen-observable": "^0.8.15" + "zen-observable": "^0.8.15", + "zod": "^3.11.6" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", diff --git a/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.ts b/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.ts index 4da92efbdf0a8..106034a57a210 100644 --- a/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.ts +++ b/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.ts @@ -14,25 +14,25 @@ * limitations under the License. */ -import { DefaultAuthConnector } from '../../../../lib/AuthConnector'; -import { GithubSession } from './types'; import { + AuthRequestOptions, + BackstageIdentity, OAuthApi, + ProfileInfo, SessionApi, SessionState, - ProfileInfo, - BackstageIdentity, - AuthRequestOptions, } from '@backstage/core-plugin-api'; import { Observable } from '@backstage/types'; -import { SessionManager } from '../../../../lib/AuthSessionManager/types'; +import { DefaultAuthConnector } from '../../../../lib/AuthConnector'; import { AuthSessionStore, RefreshingAuthSessionManager, StaticAuthSessionManager, } from '../../../../lib/AuthSessionManager'; -import { OAuthApiCreateOptions } from '../types'; import { OptionalRefreshSessionManagerMux } from '../../../../lib/AuthSessionManager/OptionalRefreshSessionManagerMux'; +import { SessionManager } from '../../../../lib/AuthSessionManager/types'; +import { OAuthApiCreateOptions } from '../types'; +import { GithubSession, githubSessionSchema } from './types'; export type GithubAuthResponse = { providerInfo: { @@ -105,6 +105,7 @@ export default class GithubAuth implements OAuthApi, SessionApi { sessionScopes: (session: GithubSession) => session.providerInfo.scopes, }), storageKey: `${provider.id}Session`, + schema: githubSessionSchema, sessionScopes: (session: GithubSession) => session.providerInfo.scopes, }); diff --git a/packages/core-app-api/src/apis/implementations/auth/github/index.ts b/packages/core-app-api/src/apis/implementations/auth/github/index.ts index ee4334f6fc741..b5aa1a0a25e5b 100644 --- a/packages/core-app-api/src/apis/implementations/auth/github/index.ts +++ b/packages/core-app-api/src/apis/implementations/auth/github/index.ts @@ -14,5 +14,5 @@ * limitations under the License. */ -export * from './types'; +export type { GithubSession } from './types'; export { default as GithubAuth } from './GithubAuth'; diff --git a/packages/core-app-api/src/apis/implementations/auth/github/types.ts b/packages/core-app-api/src/apis/implementations/auth/github/types.ts index e1c88b91736b0..fc289635eda09 100644 --- a/packages/core-app-api/src/apis/implementations/auth/github/types.ts +++ b/packages/core-app-api/src/apis/implementations/auth/github/types.ts @@ -15,6 +15,7 @@ */ import { ProfileInfo, BackstageIdentity } from '@backstage/core-plugin-api'; +import { z } from 'zod'; /** * Session information for GitHub auth. @@ -30,3 +31,25 @@ export type GithubSession = { profile: ProfileInfo; backstageIdentity: BackstageIdentity; }; + +export const githubSessionSchema: z.ZodSchema = z.object({ + providerInfo: z.object({ + accessToken: z.string(), + scopes: z.set(z.string()), + expiresAt: z.date().optional(), + }), + profile: z.object({ + email: z.string().optional(), + displayName: z.string().optional(), + picture: z.string().optional(), + }), + backstageIdentity: z.object({ + id: z.string(), + token: z.string(), + identity: z.object({ + type: z.literal('user'), + userEntityRef: z.string(), + ownershipEntityRefs: z.array(z.string()), + }), + }), +}); diff --git a/packages/core-app-api/src/apis/implementations/auth/saml/SamlAuth.ts b/packages/core-app-api/src/apis/implementations/auth/saml/SamlAuth.ts index 5988e81c48ae0..994772adf2ae1 100644 --- a/packages/core-app-api/src/apis/implementations/auth/saml/SamlAuth.ts +++ b/packages/core-app-api/src/apis/implementations/auth/saml/SamlAuth.ts @@ -14,24 +14,24 @@ * limitations under the License. */ -import { DirectAuthConnector } from '../../../../lib/AuthConnector'; -import { SessionManager } from '../../../../lib/AuthSessionManager/types'; import { - ProfileInfo, - BackstageIdentity, - SessionState, AuthRequestOptions, - ProfileInfoApi, + BackstageIdentity, BackstageIdentityApi, + ProfileInfo, + ProfileInfoApi, SessionApi, + SessionState, } from '@backstage/core-plugin-api'; import { Observable } from '@backstage/types'; -import { SamlSession } from './types'; +import { DirectAuthConnector } from '../../../../lib/AuthConnector'; import { AuthSessionStore, StaticAuthSessionManager, } from '../../../../lib/AuthSessionManager'; +import { SessionManager } from '../../../../lib/AuthSessionManager/types'; import { AuthApiCreateOptions } from '../types'; +import { SamlSession, samlSessionSchema } from './types'; export type SamlAuthResponse = { profile: ProfileInfo; @@ -72,6 +72,7 @@ export default class SamlAuth const authSessionStore = new AuthSessionStore({ manager: sessionManager, storageKey: `${provider.id}Session`, + schema: samlSessionSchema, }); return new SamlAuth(authSessionStore); diff --git a/packages/core-app-api/src/apis/implementations/auth/saml/types.ts b/packages/core-app-api/src/apis/implementations/auth/saml/types.ts index 724f0aa5d5b9c..2e7c67553e581 100644 --- a/packages/core-app-api/src/apis/implementations/auth/saml/types.ts +++ b/packages/core-app-api/src/apis/implementations/auth/saml/types.ts @@ -13,7 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { ProfileInfo, BackstageIdentity } from '@backstage/core-plugin-api'; + +import { BackstageIdentity, ProfileInfo } from '@backstage/core-plugin-api'; +import { z } from 'zod'; /** * Session information for SAML auth. @@ -25,3 +27,21 @@ export type SamlSession = { profile: ProfileInfo; backstageIdentity: BackstageIdentity; }; + +export const samlSessionSchema: z.ZodSchema = z.object({ + userId: z.string(), + profile: z.object({ + email: z.string().optional(), + displayName: z.string().optional(), + picture: z.string().optional(), + }), + backstageIdentity: z.object({ + id: z.string(), + token: z.string(), + identity: z.object({ + type: z.literal('user'), + userEntityRef: z.string(), + ownershipEntityRefs: z.array(z.string()), + }), + }), +}); diff --git a/packages/core-app-api/src/lib/AuthSessionManager/AuthSessionStore.test.ts b/packages/core-app-api/src/lib/AuthSessionManager/AuthSessionStore.test.ts index ded26c59e5a58..83d603517106c 100644 --- a/packages/core-app-api/src/lib/AuthSessionManager/AuthSessionStore.test.ts +++ b/packages/core-app-api/src/lib/AuthSessionManager/AuthSessionStore.test.ts @@ -13,11 +13,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + +import { withLogCollector } from '@backstage/test-utils'; +import { z } from 'zod'; import { AuthSessionStore } from './AuthSessionStore'; import { SessionManager } from './types'; const defaultOptions = { storageKey: 'my-key', + schema: z.any(), sessionScopes: (session: string) => new Set(session.split(' ')), }; @@ -147,4 +151,48 @@ describe('GheAuth AuthSessionStore', () => { store.sessionState$(); expect(manager.sessionState$).toHaveBeenCalled(); }); + + it('should schema-validate stored data', async () => { + const manager = new MockManager(); + + const firstStore = new AuthSessionStore({ + manager, + storageKey: 'a', + schema: z.boolean(), + sessionScopes: () => new Set(), + }); + const secondStore = new AuthSessionStore({ + manager, + storageKey: 'a', + schema: z.number(), + sessionScopes: () => new Set(), + }); + + firstStore.setSession(true); + await expect(firstStore.getSession({})).resolves.toBe(true); + + await expect( + withLogCollector(async () => { + await expect(secondStore.getSession({})).resolves.toBeUndefined(); + }), + ).resolves.toMatchObject({ + log: [ + expect.stringContaining( + 'Failed to load session from local storage because it did not conform to the expected schema', + ), + ], + }); + + await expect( + withLogCollector(async () => { + await secondStore.setSession('no' as any); + }), + ).resolves.toMatchObject({ + warn: [ + expect.stringContaining( + 'Failed to save session to local storage because it did not conform to the expected schema', + ), + ], + }); + }); }); diff --git a/packages/core-app-api/src/lib/AuthSessionManager/AuthSessionStore.ts b/packages/core-app-api/src/lib/AuthSessionManager/AuthSessionStore.ts index fc15da7af89dd..e1fb80d41f8ab 100644 --- a/packages/core-app-api/src/lib/AuthSessionManager/AuthSessionStore.ts +++ b/packages/core-app-api/src/lib/AuthSessionManager/AuthSessionStore.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { ZodSchema } from 'zod'; import { MutableSessionManager, SessionScopesFunc, @@ -27,6 +28,8 @@ type Options = { manager: MutableSessionManager; /** Storage key to use to store sessions */ storageKey: string; + /** The schema used to validate the stored data */ + schema: ZodSchema; /** Used to get the scope of the session */ sessionScopes?: SessionScopesFunc; /** Used to check if the session needs to be refreshed, defaults to never refresh */ @@ -42,6 +45,7 @@ type Options = { export class AuthSessionStore implements MutableSessionManager { private readonly manager: MutableSessionManager; private readonly storageKey: string; + private readonly schema: ZodSchema; private readonly sessionShouldRefreshFunc: SessionShouldRefreshFunc; private readonly helper: SessionScopeHelper; @@ -49,12 +53,14 @@ export class AuthSessionStore implements MutableSessionManager { const { manager, storageKey, + schema, sessionScopes, sessionShouldRefresh = () => false, } = options; this.manager = manager; this.storageKey = storageKey; + this.schema = schema; this.sessionShouldRefreshFunc = sessionShouldRefresh; this.helper = new SessionScopeHelper({ sessionScopes, @@ -104,7 +110,16 @@ export class AuthSessionStore implements MutableSessionManager { } return value; }); - return session; + + try { + return this.schema.parse(session); + } catch (e) { + // eslint-disable-next-line no-console + console.log( + `Failed to load session from local storage because it did not conform to the expected schema, ${e}`, + ); + throw e; + } } return undefined; @@ -117,19 +132,30 @@ export class AuthSessionStore implements MutableSessionManager { private saveSession(session: T | undefined) { if (session === undefined) { localStorage.removeItem(this.storageKey); - } else { - localStorage.setItem( - this.storageKey, - JSON.stringify(session, (_key, value) => { - if (value instanceof Set) { - return { - __type: 'Set', - __value: Array.from(value), - }; - } - return value; - }), + return; + } + + try { + this.schema.parse(session); + } catch (e) { + // eslint-disable-next-line no-console + console.warn( + `Failed to save session to local storage because it did not conform to the expected schema, ${e}`, ); + return; } + + localStorage.setItem( + this.storageKey, + JSON.stringify(session, (_key, value) => { + if (value instanceof Set) { + return { + __type: 'Set', + __value: Array.from(value), + }; + } + return value; + }), + ); } }