diff --git a/packages/core-api/src/apis/implementations/auth/ObservableSession.tsx b/packages/core-api/src/apis/implementations/auth/ObservableSession.tsx deleted file mode 100644 index b080f64f08..0000000000 --- a/packages/core-api/src/apis/implementations/auth/ObservableSession.tsx +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { BehaviorSubject } from '../../../lib'; -import { Observable } from '../../../types'; - -export class ObservableSession { - private session: T | undefined; - private readonly subject = new BehaviorSubject(undefined); - - session$(): Observable { - return this.subject; - } - - getSession(): T | undefined { - return this.session; - } - - setSession(session?: T): void { - this.session = session; - this.subject.next(session); - } -} diff --git a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts index bc93af8fc7..e9e40c2eab 100644 --- a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts +++ b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts @@ -21,7 +21,8 @@ import { OAuthApi, AccessTokenOptions } from '../../../definitions/auth'; import { OAuthRequestApi, AuthProvider } from '../../../definitions'; import { SessionManager } from '../../../../lib/AuthSessionManager/types'; import { StaticAuthSessionManager } from '../../../../lib/AuthSessionManager'; -import { ObservableSession } from '../ObservableSession'; +import { BehaviorSubject } from '../../../../lib'; +import { Observable } from '../../../../types'; type CreateOptions = { // TODO(Rugvip): These two should be grabbed from global config when available, they're not unique to GithubAuth @@ -47,7 +48,7 @@ const DEFAULT_PROVIDER = { icon: GithubIcon, }; -class GithubAuth extends ObservableSession implements OAuthApi { +class GithubAuth implements OAuthApi { static create({ apiOrigin, basePath, @@ -79,17 +80,23 @@ class GithubAuth extends ObservableSession implements OAuthApi { return new GithubAuth(sessionManager); } - constructor(private readonly sessionManager: SessionManager) { - super(); + private readonly subject = new BehaviorSubject( + undefined, + ); + + session$(): Observable { + return this.subject; } + constructor(private readonly sessionManager: SessionManager) {} + async getAccessToken(scope?: string, options?: AccessTokenOptions) { const normalizedScopes = GithubAuth.normalizeScope(scope); const session = await this.sessionManager.getSession({ ...options, scopes: normalizedScopes, }); - this.setSession(session); + this.subject.next(!!session); if (session) { return session.accessToken; } @@ -98,7 +105,7 @@ class GithubAuth extends ObservableSession implements OAuthApi { async logout() { await this.sessionManager.removeSession(); - this.setSession(); + this.subject.next(false); } static normalizeScope(scope?: string): Set { diff --git a/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts b/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts index 8de65ce01b..0675c60e82 100644 --- a/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts @@ -29,7 +29,8 @@ import { import { OAuthRequestApi, AuthProvider } from '../../../definitions'; import { SessionManager } from '../../../../lib/AuthSessionManager/types'; import { RefreshingAuthSessionManager } from '../../../../lib/AuthSessionManager'; -import { ObservableSession } from '../ObservableSession'; +import { BehaviorSubject } from '../../../../lib'; +import { Observable } from '../../../../types'; type CreateOptions = { // TODO(Rugvip): These two should be grabbed from global config when available, they're not unique to GoogleAuth @@ -58,8 +59,7 @@ const DEFAULT_PROVIDER = { const SCOPE_PREFIX = 'https://www.googleapis.com/auth/'; -class GoogleAuth extends ObservableSession - implements OAuthApi, OpenIdConnectApi, ProfileInfoApi { +class GoogleAuth implements OAuthApi, OpenIdConnectApi, ProfileInfoApi { static create({ apiOrigin, basePath, @@ -101,10 +101,16 @@ class GoogleAuth extends ObservableSession return new GoogleAuth(sessionManager); } - constructor(private readonly sessionManager: SessionManager) { - super(); + private readonly subject = new BehaviorSubject( + undefined, + ); + + session$(): Observable { + return this.subject; } + constructor(private readonly sessionManager: SessionManager) {} + async getAccessToken( scope?: string | string[], options?: AccessTokenOptions, @@ -114,7 +120,7 @@ class GoogleAuth extends ObservableSession ...options, scopes: normalizedScopes, }); - this.setSession(session); + this.subject.next(!!session); if (session) { return session.accessToken; } @@ -123,7 +129,7 @@ class GoogleAuth extends ObservableSession async getIdToken(options: IdTokenOptions = {}) { const session = await this.sessionManager.getSession(options); - this.setSession(session); + this.subject.next(!!session); if (session) { return session.idToken; } @@ -132,7 +138,7 @@ class GoogleAuth extends ObservableSession async logout() { await this.sessionManager.removeSession(); - this.setSession(); + this.subject.next(false); } async getProfile(options: ProfileInfoOptions = {}) { diff --git a/packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts b/packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts index 3df21af3f3..f7d5bcf7ca 100644 --- a/packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts +++ b/packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts @@ -113,8 +113,8 @@ export class RefreshingAuthSessionManager implements SessionManager { } async removeSession() { + this.currentSession = undefined; await this.connector.removeSession(); - window.location.reload(); // TODO(Rugvip): make this work without reload? } async getCurrentSession() { diff --git a/packages/core-api/src/lib/AuthSessionManager/StaticAuthSessionManager.ts b/packages/core-api/src/lib/AuthSessionManager/StaticAuthSessionManager.ts index 6e6db47a99..5ecbbc0c4e 100644 --- a/packages/core-api/src/lib/AuthSessionManager/StaticAuthSessionManager.ts +++ b/packages/core-api/src/lib/AuthSessionManager/StaticAuthSessionManager.ts @@ -64,7 +64,7 @@ export class StaticAuthSessionManager implements SessionManager { } async removeSession() { + this.currentSession = undefined; await this.connector.removeSession(); - window.location.reload(); // TODO(Rugvip): make this work without reload? } } diff --git a/packages/core/src/layout/Sidebar/Sidebar.stories.tsx b/packages/core/src/layout/Sidebar/Sidebar.stories.tsx index a450a09d88..906317fe9c 100644 --- a/packages/core/src/layout/Sidebar/Sidebar.stories.tsx +++ b/packages/core/src/layout/Sidebar/Sidebar.stories.tsx @@ -22,6 +22,7 @@ import { SidebarDivider, SidebarSearchField, SidebarSpace, + SidebarUserSettings, } from '.'; import HomeOutlinedIcon from '@material-ui/icons/HomeOutlined'; import AddCircleOutlineIcon from '@material-ui/icons/AddCircleOutline'; @@ -54,5 +55,6 @@ export const SampleSidebar = () => ( + ); diff --git a/packages/core/src/layout/Sidebar/UserSettings.tsx b/packages/core/src/layout/Sidebar/UserSettings.tsx index 28c4f100ae..722d2d3f4f 100644 --- a/packages/core/src/layout/Sidebar/UserSettings.tsx +++ b/packages/core/src/layout/Sidebar/UserSettings.tsx @@ -33,6 +33,8 @@ import { import { Avatar, IconButton, makeStyles, Tooltip } from '@material-ui/core'; import PowerButton from '@material-ui/icons/PowerSettingsNew'; +type AppAuthProviders = Provider[]; + type Provider = { title: string; api: any; @@ -44,7 +46,8 @@ type Provider = { const useProviders = () => { const googleAuth = useApi(googleAuthApiRef); const githubAuth = useApi(githubAuthApiRef); - const [providers, setProviders] = useState([ + // TODO(soapraj): List all the providers supported by the app + const [providers, setProviders] = useState([ { title: 'Google', api: googleAuth, @@ -60,43 +63,40 @@ const useProviders = () => { }, ]); - useEffect(() => { - // On page load we check the status of sign-in/sign-out for all the providers - // by making a optional getIdToken or getAccessToken request. - const setIsSignedIn = async () => { - const signInChecks = await Promise.all( - providers.map(provider => { - return provider.identity - ? provider.api.getIdToken({ optional: true }) - : provider.api.getAccessToken('', { optional: true }); - }), - ); + // On page load we check the status of sign-in/sign-out for all the providers + // by making a optional getIdToken or getAccessToken request. + const setIsSignedIn = async () => { + const signInChecks = await Promise.all( + providers.map(provider => { + return provider.identity + ? provider.api.getIdToken({ optional: true }) + : provider.api.getAccessToken('', { optional: true }); + }), + ); - signInChecks.map((result, i) => { - providers[i].isSignedIn = !!result; - }); + signInChecks.map((result, i) => { + providers[i].isSignedIn = !!result; + }); - setProviders(providers); - }; + setProviders(providers); + }; - setIsSignedIn(); - - // Any sign-in/sign-out activity on any provider is observed here + // Any sign-in/sign-out activity on any provider is observed here + const observeProviderState = () => { providers.map((provider, index) => { - provider.api.session$().subscribe((session: any) => { - if (session) { - const currentProvider = providers[index]; - currentProvider.isSignedIn = true; - setProviders([ - ...providers.slice(0, index), - currentProvider, - ...providers.slice(index + 1, providers.length), - ]); - } + provider.api.session$().subscribe((signedInState: boolean) => { + const mutatedProvider = providers[index]; + mutatedProvider.isSignedIn = signedInState; + providers[index] = mutatedProvider; + setProviders(providers.slice()); }); }); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }; + + useEffect(() => { + setIsSignedIn(); + observeProviderState(); + }, [setIsSignedIn, observeProviderState]); return providers; }; @@ -116,17 +116,19 @@ export function SidebarUserSettings() { const [profile, setProfile] = useState(); const classes = useStyles(); - // TODO(soapraj): List all the providers supported by the app and let user log in from here - // TODO(soapraj): How to observe if the user is logged in useEffect(() => { const identityProvider = providers.find( (provider: Provider) => provider.identity, ); - identityProvider?.api - .getProfile({ optional: true }) - .then((userProfile: ProfileInfo) => { - setProfile(userProfile); - }); + if (identityProvider?.isSignedIn) { + identityProvider?.api + .getProfile({ optional: true }) + .then((userProfile: ProfileInfo) => { + setProfile(userProfile); + }); + } else { + setProfile(undefined); + } }, [providers, open]); const handleClick = () => { @@ -141,30 +143,27 @@ export function SidebarUserSettings() { // Handle main auth info that is shown on the collapsible SidebarItem let avatar; - let displayName; + let displayName = 'Guest'; if (profile) { const email = profile.email; const name = profile.name; const imageUrl = profile.picture; - const avatarFallback = email.charAt(0).toUpperCase() + email.slice(1); const emailTrimmed = email.split('@')[0]; const displayEmail = emailTrimmed.charAt(0).toUpperCase() + emailTrimmed.slice(1); displayName = name ?? displayEmail; avatar = imageUrl - ? () => - : () => ( - - {avatarFallback[0]} - - ); + ? () => ( + + ) + : () => ; } return ( <>