From d681db2b5035e0ae932c7631c889eee24d9780a2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 17 Dec 2020 15:29:44 +0100 Subject: [PATCH] core-api: add MutableSessionManager and fix AuthSessionStore not properly updating session state --- .changeset/giant-rice-jump.md | 5 +++++ .../src/lib/AuthSessionManager/AuthSessionStore.test.ts | 7 +++++++ .../src/lib/AuthSessionManager/AuthSessionStore.ts | 6 ++++-- .../lib/AuthSessionManager/StaticAuthSessionManager.ts | 9 +++++++-- packages/core-api/src/lib/AuthSessionManager/types.ts | 7 +++++++ 5 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 .changeset/giant-rice-jump.md diff --git a/.changeset/giant-rice-jump.md b/.changeset/giant-rice-jump.md new file mode 100644 index 0000000000..20a574176e --- /dev/null +++ b/.changeset/giant-rice-jump.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-api': patch +--- + +Fix for GitHub and SAML auth not properly updating session state when already logged in. diff --git a/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.test.ts b/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.test.ts index 4572b923dd..5c960f7876 100644 --- a/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.test.ts +++ b/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.test.ts @@ -36,6 +36,7 @@ class LocalStorage { } class MockManager implements SessionManager { + setSession = jest.fn(); getSession = jest.fn(); removeSession = jest.fn(); sessionState$ = jest.fn(); @@ -59,6 +60,7 @@ describe('GheAuth AuthSessionStore', () => { await expect(store.getSession({})).resolves.toBe('a b c'); expect(manager.getSession).not.toHaveBeenCalled(); + expect(manager.setSession).toHaveBeenCalledWith('a b c'); }); it('should not use session without enough scope', async () => { @@ -72,6 +74,7 @@ describe('GheAuth AuthSessionStore', () => { 'a b c d', ); expect(manager.getSession).toHaveBeenCalledTimes(1); + expect(manager.setSession).not.toHaveBeenCalled(); }); it('should not use expired session', async () => { @@ -87,6 +90,7 @@ describe('GheAuth AuthSessionStore', () => { await expect(store.getSession({})).resolves.toBe('123'); expect(manager.getSession).toHaveBeenCalledTimes(1); + expect(manager.setSession).not.toHaveBeenCalled(); }); it('should not load missing session', async () => { @@ -96,6 +100,7 @@ describe('GheAuth AuthSessionStore', () => { await expect(store.getSession({})).resolves.toBe('123'); expect(manager.getSession).toHaveBeenCalledTimes(1); + expect(manager.setSession).not.toHaveBeenCalled(); expect(localStorage.getItem('my-key')).toBe('"123"'); }); @@ -109,6 +114,7 @@ describe('GheAuth AuthSessionStore', () => { await expect(store.getSession({})).resolves.toBe('123'); expect(manager.getSession).toHaveBeenCalledTimes(1); + expect(manager.setSession).not.toHaveBeenCalled(); }); it('should clear session', () => { @@ -119,6 +125,7 @@ describe('GheAuth AuthSessionStore', () => { store.removeSession(); expect(localStorage.getItem('my-key')).toBe(null); + expect(manager.removeSession).toHaveBeenCalled(); }); it('should forward sessionState calls', () => { diff --git a/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.ts b/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.ts index e82557b1ce..224036d283 100644 --- a/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.ts +++ b/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.ts @@ -16,6 +16,7 @@ import { SessionManager, + MutableSessionManager, SessionScopesFunc, SessionShouldRefreshFunc, GetSessionOptions, @@ -24,7 +25,7 @@ import { SessionScopeHelper } from './common'; type Options = { /** The connector used for acting on the auth session */ - manager: SessionManager; + manager: MutableSessionManager; /** Storage key to use to store sessions */ storageKey: string; /** Used to get the scope of the session */ @@ -40,7 +41,7 @@ type Options = { * Session is serialized to JSON with special support for following types: Set. */ export class AuthSessionStore implements SessionManager { - private readonly manager: SessionManager; + private readonly manager: MutableSessionManager; private readonly storageKey: string; private readonly sessionShouldRefreshFunc: SessionShouldRefreshFunc; private readonly helper: SessionScopeHelper; @@ -70,6 +71,7 @@ export class AuthSessionStore implements SessionManager { const shouldRefresh = this.sessionShouldRefreshFunc(session!); if (!shouldRefresh) { + this.manager.setSession(session!); return session!; } } diff --git a/packages/core-api/src/lib/AuthSessionManager/StaticAuthSessionManager.ts b/packages/core-api/src/lib/AuthSessionManager/StaticAuthSessionManager.ts index e02b600828..e4f144b0a9 100644 --- a/packages/core-api/src/lib/AuthSessionManager/StaticAuthSessionManager.ts +++ b/packages/core-api/src/lib/AuthSessionManager/StaticAuthSessionManager.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { SessionManager, GetSessionOptions } from './types'; +import { MutableSessionManager, GetSessionOptions } from './types'; import { AuthConnector } from '../AuthConnector'; import { SessionScopeHelper } from './common'; import { SessionStateTracker } from './SessionStateTracker'; @@ -31,7 +31,7 @@ type Options = { /** * StaticAuthSessionManager manages an underlying session that does not expire. */ -export class StaticAuthSessionManager implements SessionManager { +export class StaticAuthSessionManager implements MutableSessionManager { private readonly connector: AuthConnector; private readonly helper: SessionScopeHelper; private readonly stateTracker = new SessionStateTracker(); @@ -45,6 +45,11 @@ export class StaticAuthSessionManager implements SessionManager { this.helper = new SessionScopeHelper({ sessionScopes, defaultScopes }); } + setSession(session: T | undefined): void { + this.currentSession = session; + this.stateTracker.setIsSignedIn(Boolean(session)); + } + async getSession(options: GetSessionOptions): Promise { if ( this.helper.sessionExistsAndHasScope(this.currentSession, options.scopes) diff --git a/packages/core-api/src/lib/AuthSessionManager/types.ts b/packages/core-api/src/lib/AuthSessionManager/types.ts index 804c7121e1..f57afd6760 100644 --- a/packages/core-api/src/lib/AuthSessionManager/types.ts +++ b/packages/core-api/src/lib/AuthSessionManager/types.ts @@ -36,6 +36,13 @@ export type SessionManager = { sessionState$(): Observable; }; +/** + * An extension of the session manager where the session can also be pushed from the manager. + */ +export interface MutableSessionManager extends SessionManager { + setSession(session: T | undefined): void; +} + /** * A function called to determine the scopes of a session. */