diff --git a/packages/core-api/src/apis/definitions/IdentityApi.ts b/packages/core-api/src/apis/definitions/IdentityApi.ts index dbc36ad967..13f4cd24bc 100644 --- a/packages/core-api/src/apis/definitions/IdentityApi.ts +++ b/packages/core-api/src/apis/definitions/IdentityApi.ts @@ -14,6 +14,7 @@ * limitations under the License. */ import { createApiRef } from '../ApiRef'; +import { ProfileInfo } from './auth'; /** * The Identity API used to identify and get information about the signed in user. @@ -29,6 +30,11 @@ export type IdentityApi = { */ getUserId(): string; + /** + * The profile of the signed in user. + */ + getProfile(): ProfileInfo; + /** * An OpenID Connect ID Token which proves the identity of the signed in user. * diff --git a/packages/core-api/src/apis/definitions/auth.ts b/packages/core-api/src/apis/definitions/auth.ts index c884bf975b..ccd40e66ce 100644 --- a/packages/core-api/src/apis/definitions/auth.ts +++ b/packages/core-api/src/apis/definitions/auth.ts @@ -37,13 +37,13 @@ import { Observable } from '../..'; */ export type OAuthScope = string | string[]; -export type AccessTokenOptions = { +export type AuthRequestOptions = { /** * If this is set to true, the user will not be prompted to log in, - * and an empty access token will be returned if there is no existing session. + * and an empty response will be returned if there is no existing session. * - * This can be used to perform a check whether the user is logged in with a set of scopes, - * or if you don't want to force a user to be logged in, but provide functionality if they already are. + * This can be used to perform a check whether the user is logged in, or if you don't + * want to force a user to be logged in, but provide functionality if they already are. * * @default false */ @@ -88,7 +88,7 @@ export type OAuthApi = { */ getAccessToken( scope?: OAuthScope, - options?: AccessTokenOptions, + options?: AuthRequestOptions, ): Promise; /** @@ -97,29 +97,6 @@ export type OAuthApi = { logout(): Promise; }; -export type IdTokenOptions = { - /** - * If this is set to true, the user will not be prompted to log in, - * and an empty id token will be returned if there is no existing session. - * - * This can be used to perform a check whether the user is logged in, or if you don't - * want to force a user to be logged in, but provide functionality if they already are. - * - * @default false - */ - optional?: boolean; - - /** - * If this is set to true, the request will bypass the regular oauth login modal - * and open the login popup directly. - * - * The method must be called synchronously from a user action for this to work in all browsers. - * - * @default false - */ - instantPopup?: boolean; -}; - /** * This API provides access to OpenID Connect credentials. It lets you request ID tokens, * which can be passed to backend services to prove the user's identity. @@ -136,7 +113,7 @@ export type OpenIdConnectApi = { * to log in. The returned promise will not resolve until the user has successfully logged in. * The returned promise can be rejected, but only if the user rejects the login request. */ - getIdToken(options?: IdTokenOptions): Promise; + getIdToken(options?: AuthRequestOptions): Promise; /** * Log out the user's session. This will reload the page. @@ -144,38 +121,65 @@ export type OpenIdConnectApi = { logout(): Promise; }; -export type ProfileInfoOptions = { - /** - * If this is set to true, the user will not be prompted to log in, - * and an empty profile will be returned if there is no existing session. - * - * This can be used to perform a check whether the user is logged in, or if you don't - * want to force a user to be logged in, but provide functionality if they already are. - * - * @default false - */ - optional?: boolean; -}; - /** * This API provides access to profile information of the user from an auth provider. */ export type ProfileInfoApi = { - getProfile(options?: ProfileInfoOptions): Promise; + /** + * Get profile information for the user as supplied by this auth provider. + * + * If the optional flag is not set, a session is guaranteed to be returned, while if + * the optional flag is set, the session may be undefined. See @AuthRequestOptions for more details. + */ + getProfile(options?: AuthRequestOptions): Promise; }; /** - * Profile information of the user from an auth provider. + * This API provides access to the user's identity within Backstage. + * + * An auth provider that implements this interface can be used to sign-in to backstage. It is + * not intended to be used directly from a plugin, but instead serves as a connection between + * this authentication method and the app's @IdentityApi + */ +export type BackstageIdentityApi = { + /** + * Get the user's identity within Backstage. This should normally not be called directly, + * use the @IdentityApi instead. + * + * If the optional flag is not set, a session is guaranteed to be returned, while if + * the optional flag is set, the session may be undefined. See @AuthRequestOptions for more details. + */ + getBackstageIdentity( + options?: AuthRequestOptions, + ): Promise; +}; + +export type BackstageIdentity = { + /** + * The backstage user ID. + */ + id: string; + + /** + * An ID token that can be used to authenticate the user within Backstage. + */ + idToken: string; +}; + +/** + * Profile information of the user. */ export type ProfileInfo = { /** * Email ID. */ email: string; + /** * Display name that can be presented to the user. */ - name?: string; + displayName?: string; + /** * URL to an avatar image of the user. */ @@ -207,7 +211,11 @@ export type SessionStateApi = { * email and expiration information. Do not rely on any other fields, as they might not be present. */ export const googleAuthApiRef = createApiRef< - OAuthApi & OpenIdConnectApi & ProfileInfoApi & SessionStateApi + OAuthApi & + OpenIdConnectApi & + ProfileInfoApi & + BackstageIdentityApi & + SessionStateApi >({ id: 'core.auth.google', description: 'Provides authentication towards Google APIs and identities', @@ -219,7 +227,9 @@ export const googleAuthApiRef = createApiRef< * See https://developer.github.com/apps/building-oauth-apps/understanding-scopes-for-oauth-apps/ * for a full list of supported scopes. */ -export const githubAuthApiRef = createApiRef({ +export const githubAuthApiRef = createApiRef< + OAuthApi & ProfileInfoApi & BackstageIdentityApi & SessionStateApi +>({ id: 'core.auth.github', description: 'Provides authentication towards Github APIs', }); diff --git a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.test.ts b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.test.ts index 7c8e6ce9f0..3ff29cc6ef 100644 --- a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.test.ts +++ b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.test.ts @@ -20,7 +20,7 @@ describe('GithubAuth', () => { it('should get access token', async () => { const getSession = jest .fn() - .mockResolvedValue({ accessToken: 'access-token' }); + .mockResolvedValue({ providerInfo: { accessToken: 'access-token' } }); const githubAuth = new GithubAuth({ getSession } as any); expect(await githubAuth.getAccessToken()).toBe('access-token'); 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 f4a7092f79..4641dd7b50 100644 --- a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts +++ b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts @@ -19,9 +19,11 @@ import { DefaultAuthConnector } from '../../../../lib/AuthConnector'; import { GithubSession } from './types'; import { OAuthApi, - AccessTokenOptions, SessionStateApi, SessionState, + ProfileInfo, + BackstageIdentity, + AuthRequestOptions, } from '../../../definitions/auth'; import { OAuthRequestApi, AuthProvider } from '../../../definitions'; import { SessionManager } from '../../../../lib/AuthSessionManager/types'; @@ -41,10 +43,13 @@ type CreateOptions = { }; export type GithubAuthResponse = { - accessToken: string; - idToken: string; - scope: string; - expiresInSeconds: number; + providerInfo: { + accessToken: string; + scope: string; + expiresInSeconds: number; + }; + profile: ProfileInfo; + backstageIdentity: BackstageIdentity; }; const DEFAULT_PROVIDER = { @@ -69,9 +74,14 @@ class GithubAuth implements OAuthApi, SessionStateApi { oauthRequestApi: oauthRequestApi, sessionTransform(res: GithubAuthResponse): GithubSession { return { - accessToken: res.accessToken, - scopes: GithubAuth.normalizeScope(res.scope), - expiresAt: new Date(Date.now() + res.expiresInSeconds * 1000), + ...res, + providerInfo: { + accessToken: res.providerInfo.accessToken, + scopes: GithubAuth.normalizeScope(res.providerInfo.scope), + expiresAt: new Date( + Date.now() + res.providerInfo.expiresInSeconds * 1000, + ), + }, }; }, }); @@ -79,7 +89,7 @@ class GithubAuth implements OAuthApi, SessionStateApi { const sessionManager = new StaticAuthSessionManager({ connector, defaultScopes: new Set(['user']), - sessionScopes: session => session.scopes, + sessionScopes: (session: GithubSession) => session.providerInfo.scopes, }); return new GithubAuth(sessionManager); @@ -93,7 +103,7 @@ class GithubAuth implements OAuthApi, SessionStateApi { constructor(private readonly sessionManager: SessionManager) {} - async getAccessToken(scope?: string, options?: AccessTokenOptions) { + async getAccessToken(scope?: string, options?: AuthRequestOptions) { const normalizedScopes = GithubAuth.normalizeScope(scope); const session = await this.sessionManager.getSession({ ...options, @@ -101,11 +111,25 @@ class GithubAuth implements OAuthApi, SessionStateApi { }); this.sessionStateTracker.setIsSignedId(!!session); if (session) { - return session.accessToken; + return session.providerInfo.accessToken; } return ''; } + async getBackstageIdentity( + options: AuthRequestOptions = {}, + ): Promise { + const session = await this.sessionManager.getSession(options); + this.sessionStateTracker.setIsSignedId(!!session); + return session?.backstageIdentity; + } + + async getProfile(options: AuthRequestOptions = {}) { + const session = await this.sessionManager.getSession(options); + this.sessionStateTracker.setIsSignedId(!!session); + return session?.profile; + } + async logout() { await this.sessionManager.removeSession(); this.sessionStateTracker.setIsSignedId(false); diff --git a/packages/core-api/src/apis/implementations/auth/github/types.ts b/packages/core-api/src/apis/implementations/auth/github/types.ts index 282017b80d..54c427423c 100644 --- a/packages/core-api/src/apis/implementations/auth/github/types.ts +++ b/packages/core-api/src/apis/implementations/auth/github/types.ts @@ -14,8 +14,15 @@ * limitations under the License. */ +import { ProfileInfo } from '../../..'; +import { BackstageIdentity } from '../../../definitions'; + export type GithubSession = { - accessToken: string; - scopes: Set; - expiresAt: Date; + providerInfo: { + accessToken: string; + scopes: Set; + expiresAt: Date; + }; + profile: ProfileInfo; + backstageIdentity: BackstageIdentity; }; diff --git a/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.test.ts b/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.test.ts index 1ee3575b89..5290ee798b 100644 --- a/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.test.ts +++ b/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.test.ts @@ -23,9 +23,9 @@ const PREFIX = 'https://www.googleapis.com/auth/'; describe('GoogleAuth', () => { it('should get refreshed access token', async () => { - const getSession = jest - .fn() - .mockResolvedValue({ accessToken: 'access-token', expiresAt: theFuture }); + const getSession = jest.fn().mockResolvedValue({ + providerInfo: { accessToken: 'access-token', expiresAt: theFuture }, + }); const googleAuth = new GoogleAuth({ getSession } as any); expect(await googleAuth.getAccessToken()).toBe('access-token'); @@ -33,9 +33,9 @@ describe('GoogleAuth', () => { }); it('should get refreshed id token', async () => { - const getSession = jest - .fn() - .mockResolvedValue({ idToken: 'id-token', expiresAt: theFuture }); + const getSession = jest.fn().mockResolvedValue({ + providerInfo: { idToken: 'id-token', expiresAt: theFuture }, + }); const googleAuth = new GoogleAuth({ getSession } as any); expect(await googleAuth.getIdToken()).toBe('id-token'); @@ -43,9 +43,9 @@ describe('GoogleAuth', () => { }); it('should get optional id token', async () => { - const getSession = jest - .fn() - .mockResolvedValue({ idToken: 'id-token', expiresAt: theFuture }); + const getSession = jest.fn().mockResolvedValue({ + providerInfo: { idToken: 'id-token', expiresAt: theFuture }, + }); const googleAuth = new GoogleAuth({ getSession } as any); expect(await googleAuth.getIdToken({ optional: true })).toBe('id-token'); @@ -58,9 +58,11 @@ describe('GoogleAuth', () => { const getSession = jest .fn() .mockResolvedValueOnce({ - accessToken: 'access-token', - expiresAt: theFuture, - scopes: new Set([`${PREFIX}not-enough`]), + providerInfo: { + accessToken: 'access-token', + expiresAt: theFuture, + scopes: new Set([`${PREFIX}not-enough`]), + }, }) .mockRejectedValue(error); const googleAuth = new GoogleAuth({ getSession } as any); @@ -77,17 +79,21 @@ describe('GoogleAuth', () => { it('should wait for all session refreshes', async () => { const initialSession = { - idToken: 'token1', - expiresAt: theFuture, - scopes: new Set(), + providerInfo: { + idToken: 'token1', + expiresAt: theFuture, + scopes: new Set(), + }, }; const getSession = jest .fn() .mockResolvedValueOnce(initialSession) .mockResolvedValue({ - idToken: 'token2', - expiresAt: theFuture, - scopes: new Set(), + providerInfo: { + idToken: 'token2', + expiresAt: theFuture, + scopes: new Set(), + }, }); const googleAuth = new GoogleAuth({ getSession } as any); @@ -95,7 +101,7 @@ describe('GoogleAuth', () => { await expect(googleAuth.getIdToken()).resolves.toBe('token1'); expect(getSession).toBeCalledTimes(1); - initialSession.expiresAt = thePast; + initialSession.providerInfo.expiresAt = thePast; const promise1 = googleAuth.getIdToken(); const promise2 = googleAuth.getIdToken(); 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 183ea592b4..db26d2d9a4 100644 --- a/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts @@ -20,13 +20,13 @@ import { GoogleSession } from './types'; import { OAuthApi, OpenIdConnectApi, - IdTokenOptions, - AccessTokenOptions, ProfileInfoApi, - ProfileInfoOptions, ProfileInfo, SessionStateApi, SessionState, + BackstageIdentityApi, + AuthRequestOptions, + BackstageIdentity, } from '../../../definitions/auth'; import { OAuthRequestApi, AuthProvider } from '../../../definitions'; import { SessionManager } from '../../../../lib/AuthSessionManager/types'; @@ -46,11 +46,14 @@ type CreateOptions = { }; export type GoogleAuthResponse = { + providerInfo: { + accessToken: string; + idToken: string; + scope: string; + expiresInSeconds: number; + }; profile: ProfileInfo; - accessToken: string; - idToken: string; - scope: string; - expiresInSeconds: number; + backstageIdentity: BackstageIdentity; }; const DEFAULT_PROVIDER = { @@ -62,7 +65,12 @@ const DEFAULT_PROVIDER = { const SCOPE_PREFIX = 'https://www.googleapis.com/auth/'; class GoogleAuth - implements OAuthApi, OpenIdConnectApi, ProfileInfoApi, SessionStateApi { + implements + OAuthApi, + OpenIdConnectApi, + ProfileInfoApi, + BackstageIdentityApi, + SessionStateApi { static create({ apiOrigin, basePath, @@ -78,11 +86,15 @@ class GoogleAuth oauthRequestApi: oauthRequestApi, sessionTransform(res: GoogleAuthResponse): GoogleSession { return { - profile: res.profile, - idToken: res.idToken, - accessToken: res.accessToken, - scopes: GoogleAuth.normalizeScopes(res.scope), - expiresAt: new Date(Date.now() + res.expiresInSeconds * 1000), + ...res, + providerInfo: { + idToken: res.providerInfo.idToken, + accessToken: res.providerInfo.accessToken, + scopes: GoogleAuth.normalizeScopes(res.providerInfo.scope), + expiresAt: new Date( + Date.now() + res.providerInfo.expiresInSeconds * 1000, + ), + }, }; }, }); @@ -94,9 +106,10 @@ class GoogleAuth `${SCOPE_PREFIX}userinfo.email`, `${SCOPE_PREFIX}userinfo.profile`, ]), - sessionScopes: session => session.scopes, - sessionShouldRefresh: session => { - const expiresInSec = (session.expiresAt.getTime() - Date.now()) / 1000; + sessionScopes: (session: GoogleSession) => session.providerInfo.scopes, + sessionShouldRefresh: (session: GoogleSession) => { + const expiresInSec = + (session.providerInfo.expiresAt.getTime() - Date.now()) / 1000; return expiresInSec < 60 * 5; }, }); @@ -114,7 +127,7 @@ class GoogleAuth async getAccessToken( scope?: string | string[], - options?: AccessTokenOptions, + options?: AuthRequestOptions, ) { const normalizedScopes = GoogleAuth.normalizeScopes(scope); const session = await this.sessionManager.getSession({ @@ -123,16 +136,16 @@ class GoogleAuth }); this.sessionStateTracker.setIsSignedId(!!session); if (session) { - return session.accessToken; + return session.providerInfo.accessToken; } return ''; } - async getIdToken(options: IdTokenOptions = {}) { + async getIdToken(options: AuthRequestOptions = {}) { const session = await this.sessionManager.getSession(options); this.sessionStateTracker.setIsSignedId(!!session); if (session) { - return session.idToken; + return session.providerInfo.idToken; } return ''; } @@ -142,13 +155,18 @@ class GoogleAuth this.sessionStateTracker.setIsSignedId(false); } - async getProfile(options: ProfileInfoOptions = {}) { + async getBackstageIdentity( + options: AuthRequestOptions = {}, + ): Promise { const session = await this.sessionManager.getSession(options); this.sessionStateTracker.setIsSignedId(!!session); - if (!session) { - return undefined; - } - return session.profile; + return session?.backstageIdentity; + } + + async getProfile(options: AuthRequestOptions = {}) { + const session = await this.sessionManager.getSession(options); + this.sessionStateTracker.setIsSignedId(!!session); + return session?.profile; } static normalizeScopes(scopes?: string | string[]): Set { diff --git a/packages/core-api/src/apis/implementations/auth/google/types.ts b/packages/core-api/src/apis/implementations/auth/google/types.ts index ea251c7006..3b40932980 100644 --- a/packages/core-api/src/apis/implementations/auth/google/types.ts +++ b/packages/core-api/src/apis/implementations/auth/google/types.ts @@ -14,12 +14,15 @@ * limitations under the License. */ -import { ProfileInfo } from '../../../definitions'; +import { ProfileInfo, BackstageIdentity } from '../../../definitions'; export type GoogleSession = { + providerInfo: { + idToken: string; + accessToken: string; + scopes: Set; + expiresAt: Date; + }; profile: ProfileInfo; - idToken: string; - accessToken: string; - scopes: Set; - expiresAt: Date; + backstageIdentity: BackstageIdentity; }; diff --git a/packages/core-api/src/app/App.tsx b/packages/core-api/src/app/App.tsx index a20a9b27f8..27a63ea04a 100644 --- a/packages/core-api/src/app/App.tsx +++ b/packages/core-api/src/app/App.tsx @@ -17,7 +17,6 @@ import React, { ComponentType, FC, useMemo, - useCallback, useState, ReactElement, } from 'react'; @@ -258,24 +257,14 @@ export class PrivateAppImpl implements BackstageApp { component: ComponentType; children: ReactElement; }> = ({ component: Component, children }) => { - const [done, setDone] = useState(false); + const [result, setResult] = useState(); - const onResult = useCallback( - (result: SignInResult) => { - if (done) { - throw new Error('Identity result callback was called twice'); - } - this.identityApi.setSignInResult(result); - setDone(true); - }, - [done], - ); - - if (done) { + if (result) { + this.identityApi.setSignInResult(result); return children; } - return ; + return ; }; const AppRouter: FC<{}> = ({ children }) => { @@ -293,6 +282,10 @@ export class PrivateAppImpl implements BackstageApp { if (!SignInPageComponent) { this.identityApi.setSignInResult({ userId: 'guest', + profile: { + email: 'guest@example.com', + displayName: 'Guest', + }, }); return ( diff --git a/packages/core-api/src/app/AppIdentity.ts b/packages/core-api/src/app/AppIdentity.ts index c38ece53df..69ee5d28ac 100644 --- a/packages/core-api/src/app/AppIdentity.ts +++ b/packages/core-api/src/app/AppIdentity.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { IdentityApi } from '../apis'; +import { IdentityApi, ProfileInfo } from '../apis'; import { SignInResult } from './types'; /** @@ -24,6 +24,7 @@ import { SignInResult } from './types'; export class AppIdentity implements IdentityApi { private hasIdentity = false; private userId?: string; + private profile?: ProfileInfo; private idTokenFunc?: () => Promise; private logoutFunc?: () => Promise; @@ -36,6 +37,15 @@ export class AppIdentity implements IdentityApi { return this.userId!; } + getProfile(): ProfileInfo { + if (!this.hasIdentity) { + throw new Error( + 'Tried to access IdentityApi profile before app was loaded', + ); + } + return this.profile!; + } + async getIdToken(): Promise { if (!this.hasIdentity) { throw new Error( @@ -55,6 +65,7 @@ export class AppIdentity implements IdentityApi { location.reload(); } + // This is indirectly called by the sign-in page to continue into the app. setSignInResult(result: SignInResult) { if (this.hasIdentity) { return; @@ -62,8 +73,12 @@ export class AppIdentity implements IdentityApi { if (!result.userId) { throw new Error('Invalid sign-in result, userId not set'); } + if (!result.profile) { + throw new Error('Invalid sign-in result, profile not set'); + } this.hasIdentity = true; this.userId = result.userId; + this.profile = result.profile; this.idTokenFunc = result.getIdToken; this.logoutFunc = result.logout; } diff --git a/packages/core-api/src/app/types.ts b/packages/core-api/src/app/types.ts index 33b884fd58..6b022c2736 100644 --- a/packages/core-api/src/app/types.ts +++ b/packages/core-api/src/app/types.ts @@ -18,7 +18,7 @@ import { ComponentType } from 'react'; import { IconComponent, SystemIconKey, SystemIcons } from '../icons'; import { BackstagePlugin } from '../plugin'; import { ApiHolder } from '../apis'; -import { AppTheme, ConfigApi } from '../apis/definitions'; +import { AppTheme, ConfigApi, ProfileInfo } from '../apis/definitions'; import { AppConfig } from '@backstage/config'; export type BootErrorPageProps = { @@ -31,6 +31,9 @@ export type SignInResult = { * User ID that will be returned by the IdentityApi */ userId: string; + + profile: ProfileInfo; + /** * Function used to retrieve an ID token for the signed in user. */ diff --git a/packages/core-api/src/lib/loginPopup.test.ts b/packages/core-api/src/lib/loginPopup.test.ts index 755438fead..55f1408e8d 100644 --- a/packages/core-api/src/lib/loginPopup.test.ts +++ b/packages/core-api/src/lib/loginPopup.test.ts @@ -59,7 +59,7 @@ describe('showLoginPopup', () => { // None of these should be accepted listener({ source: popupMock } as MessageEvent); listener({ origin: 'my-origin' } as MessageEvent); - listener({ data: { type: 'auth-result' } } as MessageEvent); + listener({ data: { type: 'authorization_response' } } as MessageEvent); listener({ source: popupMock, origin: 'my-origin', @@ -68,26 +68,26 @@ describe('showLoginPopup', () => { listener({ source: popupMock, origin: 'my-origin', - data: { type: 'not-auth-result', payload: {} }, + data: { type: 'not-auth-result', response: {} }, } as MessageEvent); await expect(Promise.race([payloadPromise, 'waiting'])).resolves.toBe( 'waiting', ); - const myPayload = {}; + const myResponse = {}; // This should be accepted as a valid sessions response listener({ source: popupMock, origin: 'my-origin', data: { - type: 'auth-result', - payload: myPayload, + type: 'authorization_response', + response: myResponse, }, } as MessageEvent); - await expect(payloadPromise).resolves.toBe(myPayload); + await expect(payloadPromise).resolves.toBe(myResponse); expect(openSpy).toBeCalledTimes(1); expect(addEventListenerSpy).toBeCalledTimes(1); @@ -118,7 +118,7 @@ describe('showLoginPopup', () => { source: popupMock, origin: 'my-origin', data: { - type: 'auth-result', + type: 'authorization_response', error: { message: 'NOPE', name: 'NopeError', diff --git a/packages/core-api/src/lib/loginPopup.ts b/packages/core-api/src/lib/loginPopup.ts index b4af94be49..10d6efd7a4 100644 --- a/packages/core-api/src/lib/loginPopup.ts +++ b/packages/core-api/src/lib/loginPopup.ts @@ -46,11 +46,11 @@ export type LoginPopupOptions = { type AuthResult = | { - type: 'auth-result'; - payload: any; + type: 'authorization_response'; + response: unknown; } | { - type: 'auth-result'; + type: 'authorization_response'; error: { name: string; message: string; @@ -58,12 +58,13 @@ type AuthResult = }; /** - * Show a popup pointing to a URL that starts an auth flow. + * Show a popup pointing to a URL that starts an auth flow. Implementing the receiving + * end of the postMessage mechanism outlined in https://tools.ietf.org/html/draft-sakimura-oauth-wmrm-00 * * The redirect handler of the flow should use postMessage to communicate back * to the app window. The message posted to the app must match the AuthResult type. * - * The returned promise resolves to the contents of the message that was posted from the auth popup. + * The returned promise resolves to the response of the message that was posted from the auth popup. */ export function showLoginPopup(options: LoginPopupOptions): Promise { return new Promise((resolve, reject) => { @@ -91,7 +92,7 @@ export function showLoginPopup(options: LoginPopupOptions): Promise { return; } const { data } = event; - if (data.type !== 'auth-result') { + if (data.type !== 'authorization_response') { return; } const authResult = data as AuthResult; @@ -103,7 +104,7 @@ export function showLoginPopup(options: LoginPopupOptions): Promise { // error.extra = authResult.error.extra; reject(error); } else { - resolve(authResult.payload); + resolve(authResult.response); } done(); }; diff --git a/packages/core/src/layout/Sidebar/Settings/OAuthProviderSettings.tsx b/packages/core/src/layout/Sidebar/Settings/OAuthProviderSettings.tsx index b81ba2aaf7..73659eb63d 100644 --- a/packages/core/src/layout/Sidebar/Settings/OAuthProviderSettings.tsx +++ b/packages/core/src/layout/Sidebar/Settings/OAuthProviderSettings.tsx @@ -41,22 +41,29 @@ export const OAuthProviderSettings: FC = ({ const [signedIn, setSignedIn] = useState(false); useEffect(() => { + let didCancel = false; + const checkSession = async () => { const session = await api.getAccessToken('', { optional: true }); - setSignedIn(!!session); + if (!didCancel) { + setSignedIn(!!session); + } }; let subscription: Subscription; const observeSession = () => { subscription = api .sessionState$() .subscribe((sessionState: SessionState) => { - setSignedIn(sessionState === SessionState.SignedIn); + if (!didCancel) { + setSignedIn(sessionState === SessionState.SignedIn); + } }); }; checkSession(); observeSession(); return () => { + didCancel = true; subscription.unsubscribe(); }; }, [api]); diff --git a/packages/core/src/layout/Sidebar/Settings/OIDCProviderSettings.tsx b/packages/core/src/layout/Sidebar/Settings/OIDCProviderSettings.tsx index 139d79213f..19ee00eee1 100644 --- a/packages/core/src/layout/Sidebar/Settings/OIDCProviderSettings.tsx +++ b/packages/core/src/layout/Sidebar/Settings/OIDCProviderSettings.tsx @@ -41,9 +41,13 @@ export const OIDCProviderSettings: FC = ({ const [signedIn, setSignedIn] = useState(false); useEffect(() => { + let didCancel = false; + const checkSession = async () => { const session = await api.getIdToken({ optional: true }); - setSignedIn(!!session); + if (!didCancel) { + setSignedIn(!!session); + } }; let subscription: Subscription; @@ -51,13 +55,16 @@ export const OIDCProviderSettings: FC = ({ subscription = api .sessionState$() .subscribe((sessionState: SessionState) => { - setSignedIn(sessionState === SessionState.SignedIn); + if (!didCancel) { + setSignedIn(sessionState === SessionState.SignedIn); + } }); }; checkSession(); observeSession(); return () => { + didCancel = true; subscription.unsubscribe(); }; }, [api]); diff --git a/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx b/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx index 06c36bf9ce..101ae7010b 100644 --- a/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx +++ b/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx @@ -14,19 +14,12 @@ * limitations under the License. */ -import React, { FC, useState, useRef, useEffect } from 'react'; +import React, { FC, useRef } from 'react'; import { makeStyles, Avatar, Divider } from '@material-ui/core'; -import { - ProfileInfo, - useApi, - googleAuthApiRef, - Subscription, - SessionState, -} from '@backstage/core-api'; +import { useApi, identityApiRef } from '@backstage/core-api'; import { SidebarItem } from '../Items'; import ExpandLess from '@material-ui/icons/ExpandLess'; import ExpandMore from '@material-ui/icons/ExpandMore'; -import AccountCircleIcon from '@material-ui/icons/AccountCircle'; const useStyles = makeStyles({ avatar: { @@ -39,71 +32,26 @@ export const UserProfile: FC<{ open: boolean; setOpen: Function }> = ({ open, setOpen, }) => { - const [profile, setProfile] = useState(); const ref = useRef(); // for scrolling down when collapse item opens - const googleAuth = useApi(googleAuthApiRef); const classes = useStyles(); + const profile = useApi(identityApiRef).getProfile(); const handleClick = () => { setOpen(!open); setTimeout(() => ref.current?.scrollIntoView({ behavior: 'smooth' }), 300); }; - useEffect(() => { - const fetchProfile = async () => { - await googleAuth - .getProfile({ optional: true }) - .then((userProfile?: ProfileInfo) => { - setProfile(userProfile); - }); - }; - - let subscription: Subscription; - const observeSession = () => { - subscription = googleAuth - .sessionState$() - .subscribe(async (sessionState: SessionState) => { - if (sessionState === SessionState.SignedIn) { - await fetchProfile(); - } else { - setProfile(undefined); - } - }); - }; - - fetchProfile(); - observeSession(); - return () => { - subscription.unsubscribe(); - }; - }, [googleAuth]); - - // Handle main auth info that is shown on the collapsible SidebarItem - let avatar; - let displayName = 'Guest'; - if (profile) { - const email = profile.email; - const name = profile.name; - const imageUrl = profile.picture; - const emailTrimmed = email.split('@')[0]; - const displayEmail = - emailTrimmed.charAt(0).toUpperCase() + emailTrimmed.slice(1); - displayName = name ?? displayEmail; - avatar = imageUrl - ? () => ( - - ) - : () => ; - } + const displayName = profile.displayName ?? profile.email; + const SignInAvatar = () => ( + + {displayName[0]} + + ); return ( <> - + {open ? : } diff --git a/packages/core/src/layout/SignInPage/customProvider.tsx b/packages/core/src/layout/SignInPage/customProvider.tsx index d24b054b16..c6cb9b789d 100644 --- a/packages/core/src/layout/SignInPage/customProvider.tsx +++ b/packages/core/src/layout/SignInPage/customProvider.tsx @@ -56,6 +56,9 @@ const Component: ProviderComponent = ({ onResult }) => { const handleResult = ({ userId, idToken }: Data) => { onResult({ userId, + profile: { + email: `${userId}@example.com`, + }, getIdToken: idToken ? async () => idToken : undefined, }); }; diff --git a/packages/core/src/layout/SignInPage/googleProvider.tsx b/packages/core/src/layout/SignInPage/googleProvider.tsx index 9613437af8..632b81ad17 100644 --- a/packages/core/src/layout/SignInPage/googleProvider.tsx +++ b/packages/core/src/layout/SignInPage/googleProvider.tsx @@ -18,16 +18,7 @@ import React from 'react'; import { Grid, Typography, Button } from '@material-ui/core'; import { InfoCard } from '../InfoCard/InfoCard'; import { ProviderComponent, ProviderLoader, SignInProvider } from './types'; -import { - useApi, - googleAuthApiRef, - errorApiRef, - ProfileInfo, -} from '@backstage/core-api'; - -function parseUserId(profile: ProfileInfo) { - return profile!.email.replace(/@.*/, ''); -} +import { useApi, googleAuthApiRef, errorApiRef } from '@backstage/core-api'; const Component: ProviderComponent = ({ onResult }) => { const googleAuthApi = useApi(googleAuthApiRef); @@ -35,12 +26,17 @@ const Component: ProviderComponent = ({ onResult }) => { const handleLogin = async () => { try { - await googleAuthApi.getIdToken({ instantPopup: true }); + const identity = await googleAuthApi.getBackstageIdentity({ + instantPopup: true, + }); + const profile = await googleAuthApi.getProfile(); onResult({ - userId: parseUserId(profile!), - getIdToken: () => googleAuthApi.getIdToken(), + userId: identity!.id, + profile: profile!, + getIdToken: () => + googleAuthApi.getBackstageIdentity().then(i => i!.idToken), logout: async () => { await googleAuthApi.logout(); }, @@ -69,11 +65,21 @@ const Component: ProviderComponent = ({ onResult }) => { const loader: ProviderLoader = async apis => { const googleAuthApi = apis.get(googleAuthApiRef)!; - const profile = await googleAuthApi.getProfile({ optional: true }); + const identity = await googleAuthApi.getBackstageIdentity({ + optional: true, + }); + + if (!identity) { + return undefined; + } + + const profile = await googleAuthApi.getProfile(); return { - userId: parseUserId(profile!), - getIdToken: () => googleAuthApi.getIdToken(), + userId: identity.id, + profile: profile!, + getIdToken: () => + googleAuthApi.getBackstageIdentity().then(i => i!.idToken), logout: async () => { await googleAuthApi.logout(); }, diff --git a/packages/core/src/layout/SignInPage/guestProvider.tsx b/packages/core/src/layout/SignInPage/guestProvider.tsx index 78d0191ba9..85784eb205 100644 --- a/packages/core/src/layout/SignInPage/guestProvider.tsx +++ b/packages/core/src/layout/SignInPage/guestProvider.tsx @@ -19,6 +19,14 @@ import { Grid, Typography, Button } from '@material-ui/core'; import { InfoCard } from '../InfoCard/InfoCard'; import { ProviderComponent, ProviderLoader, SignInProvider } from './types'; +const result = { + userId: 'guest', + profile: { + email: 'guest@example.com', + displayName: 'Guest', + }, +}; + const Component: ProviderComponent = ({ onResult }) => ( ( @@ -45,7 +53,7 @@ const Component: ProviderComponent = ({ onResult }) => ( ); const loader: ProviderLoader = async () => { - return { userId: 'guest' }; + return result; }; export const guestProvider: SignInProvider = { Component, loader }; diff --git a/packages/core/src/layout/SignInPage/providers.tsx b/packages/core/src/layout/SignInPage/providers.tsx index d60c8ff281..96dbc30a89 100644 --- a/packages/core/src/layout/SignInPage/providers.tsx +++ b/packages/core/src/layout/SignInPage/providers.tsx @@ -93,8 +93,9 @@ export const useSignInProviders = ( } if (result) { handleWrappedResult(result); + } else { + setLoading(false); } - setLoading(false); }) .catch(error => { if (didCancel) { diff --git a/packages/storybook/.storybook/apis.js b/packages/storybook/.storybook/apis.js index 65b2ec5aaa..3b61ae7af8 100644 --- a/packages/storybook/.storybook/apis.js +++ b/packages/storybook/.storybook/apis.js @@ -22,6 +22,7 @@ builder.add(errorApiRef, new ErrorAlerter(alertApi, new ErrorApiForwarder())); builder.add(identityApiRef, { getUserId: () => 'guest', + getProfile: () => ({ email: 'guest@example.com' }), getIdToken: () => undefined, logout: async () => {}, }); diff --git a/plugins/auth-backend/src/lib/OAuthProvider.test.ts b/plugins/auth-backend/src/lib/OAuthProvider.test.ts index 48952e6dad..f92875fd48 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.test.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.test.ts @@ -23,7 +23,23 @@ import { verifyNonce, OAuthProvider, } from './OAuthProvider'; -import { AuthResponse, OAuthProviderHandlers } from '../providers/types'; +import { + WebMessageResponse, + OAuthProviderHandlers, + OAuthResponse, +} from '../providers/types'; + +const mockResponseData: OAuthResponse = { + providerInfo: { + accessToken: 'ACCESS_TOKEN', + idToken: 'ID_TOKEN', + expiresInSeconds: 10, + scope: 'email', + }, + profile: { + email: 'foo@bar.com', + }, +}; describe('OAuthProvider Utils', () => { describe('verifyNonce', () => { @@ -38,6 +54,7 @@ describe('OAuthProvider Utils', () => { verifyNonce(mockRequest, 'providera'); }).toThrowError('Missing nonce'); }); + it('should throw error if state nonce missing', () => { const mockRequest = ({ cookies: { @@ -49,6 +66,7 @@ describe('OAuthProvider Utils', () => { verifyNonce(mockRequest, 'providera'); }).toThrowError('Missing nonce'); }); + it('should throw error if nonce mismatch', () => { const mockRequest = ({ cookies: { @@ -62,6 +80,7 @@ describe('OAuthProvider Utils', () => { verifyNonce(mockRequest, 'providera'); }).toThrowError('Invalid nonce'); }); + it('should not throw any error if nonce matches', () => { const mockRequest = ({ cookies: { @@ -85,13 +104,22 @@ describe('OAuthProvider Utils', () => { setHeader: jest.fn().mockReturnThis(), } as unknown) as express.Response; - const data: AuthResponse = { - type: 'auth-result', - payload: { - accessToken: 'ACCESS_TOKEN', - idToken: 'ID_TOKEN', - expiresInSeconds: 10, - scope: 'email', + const data: WebMessageResponse = { + type: 'authorization_response', + response: { + providerInfo: { + accessToken: 'ACCESS_TOKEN', + idToken: 'ID_TOKEN', + expiresInSeconds: 10, + scope: 'email', + }, + profile: { + email: 'foo@bar.com', + }, + backstageIdentity: { + id: 'a', + idToken: 'a.b.c', + }, }, }; const jsonData = JSON.stringify(data); @@ -111,8 +139,8 @@ describe('OAuthProvider Utils', () => { setHeader: jest.fn().mockReturnThis(), } as unknown) as express.Response; - const data: AuthResponse = { - type: 'auth-result', + const data: WebMessageResponse = { + type: 'authorization_response', error: new Error('Unknown error occured'), }; const jsonData = JSON.stringify(data); @@ -161,16 +189,12 @@ describe('OAuthProvider', () => { } async handler() { return { - user: {}, - info: { - refreshToken: 'token', - }, + response: mockResponseData, + refreshToken: 'token', }; } async refresh() { - return { - accessToken: 'token', - }; + return mockResponseData; } } const providerInstance = new MyAuthProvider(); @@ -323,11 +347,13 @@ describe('OAuthProvider', () => { await oauthProvider.refresh(mockRequest, mockResponse); expect(mockResponse.send).toHaveBeenCalledTimes(1); - expect(mockResponse.send).toHaveBeenCalledWith( - expect.objectContaining({ - accessToken: 'token', - }), - ); + expect(mockResponse.send).toHaveBeenCalledWith({ + ...mockResponseData, + backstageIdentity: { + id: mockResponseData.profile.email, + idToken: 'my-id-token', + }, + }); }); it('handles refresh without capabilities', async () => { diff --git a/plugins/auth-backend/src/lib/OAuthProvider.ts b/plugins/auth-backend/src/lib/OAuthProvider.ts index 38065c3380..dac31af451 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.ts @@ -21,6 +21,7 @@ import { AuthResponse, AuthProviderRouteHandlers, OAuthProviderHandlers, + WebMessageResponse, } from '../providers/types'; import { InputError } from '@backstage/backend-common'; import { TokenIssuer } from '../identity'; @@ -53,9 +54,9 @@ export const verifyNonce = (req: express.Request, providerId: string) => { export const postMessageResponse = ( res: express.Response, appOrigin: string, - data: AuthResponse, + response: WebMessageResponse, ) => { - const jsonData = JSON.stringify(data); + const jsonData = JSON.stringify(response); const base64Data = Buffer.from(jsonData, 'utf8').toString('base64'); res.setHeader('Content-Type', 'text/html'); @@ -96,7 +97,7 @@ export class OAuthProvider implements AuthProviderRouteHandlers { this.basePath = url.pathname; } - async start(req: express.Request, res: express.Response): Promise { + async start(req: express.Request, res: express.Response): Promise { // retrieve scopes from request const scope = req.query.scope?.toString() ?? ''; @@ -108,14 +109,15 @@ export class OAuthProvider implements AuthProviderRouteHandlers { // set a nonce cookie before redirecting to oauth provider this.setNonceCookie(res, nonce); - const options = { + const queryParameters = { scope, - accessType: 'offline', - prompt: 'consent', state: nonce, }; - const { url, status } = await this.providerHandlers.start(req, options); + const { url, status } = await this.providerHandlers.start( + req, + queryParameters, + ); res.statusCode = status || 302; res.setHeader('Location', url); @@ -126,16 +128,17 @@ export class OAuthProvider implements AuthProviderRouteHandlers { async frameHandler( req: express.Request, res: express.Response, - ): Promise { + ): Promise { try { // verify nonce cookie and state cookie on callback verifyNonce(req, this.options.providerId); - const { user, info } = await this.providerHandlers.handler(req); + const { response, refreshToken } = await this.providerHandlers.handler( + req, + ); if (!this.options.disableRefresh) { // throw error if missing refresh token - const { refreshToken } = info; if (!refreshToken) { throw new Error('Missing refresh token'); } @@ -144,19 +147,24 @@ export class OAuthProvider implements AuthProviderRouteHandlers { this.setRefreshTokenCookie(res, refreshToken); } - user.userIdToken = await this.options.tokenIssuer.issueToken({ - claims: { sub: user.profile.email }, + const id = response.profile.email; + const idToken = await this.options.tokenIssuer.issueToken({ + claims: { sub: id }, }); + const fullResponse: AuthResponse = { + ...response, + backstageIdentity: { id, idToken }, + }; // post message back to popup if successful return postMessageResponse(res, this.options.appOrigin, { - type: 'auth-result', - payload: user, + type: 'authorization_response', + response: fullResponse, }); } catch (error) { // post error message back to popup if failure return postMessageResponse(res, this.options.appOrigin, { - type: 'auth-result', + type: 'authorization_response', error: { name: error.name, message: error.message, @@ -165,27 +173,30 @@ export class OAuthProvider implements AuthProviderRouteHandlers { } } - async logout(req: express.Request, res: express.Response): Promise { + async logout(req: express.Request, res: express.Response): Promise { if (!ensuresXRequestedWith(req)) { - return res.status(401).send('Invalid X-Requested-With header'); + res.status(401).send('Invalid X-Requested-With header'); + return; } if (!this.options.disableRefresh) { // remove refresh token cookie before logout this.removeRefreshTokenCookie(res); } - return res.send('logout!'); + res.send('logout!'); } - async refresh(req: express.Request, res: express.Response): Promise { + async refresh(req: express.Request, res: express.Response): Promise { if (!ensuresXRequestedWith(req)) { - return res.status(401).send('Invalid X-Requested-With header'); + res.status(401).send('Invalid X-Requested-With header'); + return; } if (!this.providerHandlers.refresh || this.options.disableRefresh) { - return res.send( + res.send( `Refresh token not supported for provider: ${this.options.providerId}`, ); + return; } try { @@ -200,18 +211,20 @@ export class OAuthProvider implements AuthProviderRouteHandlers { const scope = req.query.scope?.toString() ?? ''; // get new access_token - const refreshInfo = await this.providerHandlers.refresh( - refreshToken, - scope, - ); + const response = await this.providerHandlers.refresh(refreshToken, scope); - refreshInfo.userIdToken = await this.options.tokenIssuer.issueToken({ - claims: { sub: refreshInfo.profile?.email }, + const id = response.profile.email; + const idToken = await this.options.tokenIssuer.issueToken({ + claims: { sub: id }, }); + const fullResponse: AuthResponse = { + ...response, + backstageIdentity: { id, idToken }, + }; - return res.send(refreshInfo); + res.send(fullResponse); } catch (error) { - return res.status(401).send(`${error.message}`); + res.status(401).send(`${error.message}`); } } diff --git a/plugins/auth-backend/src/lib/PassportStrategyHelper.test.ts b/plugins/auth-backend/src/lib/PassportStrategyHelper.test.ts index 071c949a21..d8f6d25d0f 100644 --- a/plugins/auth-backend/src/lib/PassportStrategyHelper.test.ts +++ b/plugins/auth-backend/src/lib/PassportStrategyHelper.test.ts @@ -82,8 +82,8 @@ describe('PassportStrategyHelper', () => { expect(spyAuthenticate).toBeCalledTimes(1); await expect(frameHandlerStrategyPromise).resolves.toStrictEqual( expect.objectContaining({ - user: { accessToken: 'ACCESS_TOKEN' }, - info: { refreshToken: 'REFRESH_TOKEN' }, + response: { accessToken: 'ACCESS_TOKEN' }, + privateInfo: { refreshToken: 'REFRESH_TOKEN' }, }), ); }); diff --git a/plugins/auth-backend/src/lib/PassportStrategyHelper.ts b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts index 1e8dfca5ed..46a386f419 100644 --- a/plugins/auth-backend/src/lib/PassportStrategyHelper.ts +++ b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts @@ -26,42 +26,52 @@ import { export const makeProfileInfo = ( profile: passport.Profile, - params: any, + idToken?: string, ): ProfileInfo => { - const { displayName: name } = profile; + const { displayName } = profile; - let email = ''; - if (profile.emails) { + let email: string | undefined = undefined; + if (profile.emails && profile.emails.length > 0) { const [firstEmail] = profile.emails; email = firstEmail.value; } - if (!email && params.id_token) { - try { - const decoded: { email: string } = jwtDecoder(params.id_token); - email = decoded.email; - } catch (e) { - console.error('Failed to parse id token and get profile info'); - } - } - - let picture = ''; + let picture: string | undefined = undefined; if (profile.photos) { const [firstPhoto] = profile.photos; picture = firstPhoto.value; } + if ((!email || !picture) && idToken) { + try { + const decoded: Record = jwtDecoder(idToken); + + if (!email && decoded.email) { + email = decoded.email; + } + if (!picture && decoded.picture) { + picture = decoded.picture; + } + } catch (e) { + throw new Error(`Failed to parse id token and get profile info, ${e}`); + } + } + + if (!email) { + throw new Error('No email received in profile info'); + } + return { - name, email, picture, + displayName, }; }; export const executeRedirectStrategy = async ( req: express.Request, providerStrategy: passport.Strategy, - options: any, + options: Record, ): Promise => { return new Promise(resolve => { const strategy = Object.create(providerStrategy); @@ -73,30 +83,32 @@ export const executeRedirectStrategy = async ( }); }; -export const executeFrameHandlerStrategy = async ( +export const executeFrameHandlerStrategy = async ( req: express.Request, providerStrategy: passport.Strategy, ) => { - return new Promise<{ user: any; info: any }>((resolve, reject) => { - const strategy = Object.create(providerStrategy); - strategy.success = (user: any, info: any) => { - resolve({ user, info }); - }; - strategy.fail = ( - info: { type: 'success' | 'error'; message?: string }, - // _status: number, - ) => { - reject(new Error(`Authentication rejected, ${info.message ?? ''}`)); - }; - strategy.error = (error: Error) => { - reject(new Error(`Authentication failed, ${error}`)); - }; - strategy.redirect = () => { - reject(new Error('Unexpected redirect')); - }; + return new Promise<{ response: T; privateInfo: PrivateInfo }>( + (resolve, reject) => { + const strategy = Object.create(providerStrategy); + strategy.success = (response: any, privateInfo: any) => { + resolve({ response, privateInfo }); + }; + strategy.fail = ( + info: { type: 'success' | 'error'; message?: string }, + // _status: number, + ) => { + reject(new Error(`Authentication rejected, ${info.message ?? ''}`)); + }; + strategy.error = (error: Error) => { + reject(new Error(`Authentication failed, ${error}`)); + }; + strategy.redirect = () => { + reject(new Error('Unexpected redirect')); + }; - strategy.authenticate(req, {}); - }); + strategy.authenticate(req, {}); + }, + ); }; export const executeRefreshTokenStrategy = async ( @@ -151,7 +163,7 @@ export const executeRefreshTokenStrategy = async ( export const executeFetchUserProfileStrategy = async ( providerStrategy: passport.Strategy, accessToken: string, - params: any, + idToken?: string, ): Promise => { return new Promise((resolve, reject) => { const anyStrategy = (providerStrategy as unknown) as ProviderStrategy; @@ -162,7 +174,7 @@ export const executeFetchUserProfileStrategy = async ( reject(error); } - const profile = makeProfileInfo(passportProfile, params); + const profile = makeProfileInfo(passportProfile, idToken); resolve(profile); }, ); diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index b5f9d7e46b..3ae9858bb1 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -19,16 +19,17 @@ import { Strategy as GithubStrategy } from 'passport-github2'; import { executeFrameHandlerStrategy, executeRedirectStrategy, + makeProfileInfo, } from '../../lib/PassportStrategyHelper'; import { OAuthProviderHandlers, AuthProviderConfig, RedirectInfo, - AuthInfoBase, - AuthInfoPrivate, EnvironmentProviderConfig, OAuthProviderOptions, OAuthProviderConfig, + OAuthResponse, + PassportDoneCallback, } from '../types'; import { OAuthProvider } from '../../lib/OAuthProvider'; import { @@ -44,25 +45,42 @@ export class GithubAuthProvider implements OAuthProviderHandlers { constructor(options: OAuthProviderOptions) { this._strategy = new GithubStrategy( { ...options }, - (accessToken: any, _: any, params: any, profile: any, done: any) => { + ( + accessToken: any, + _: any, + params: any, + rawProfile: any, + done: PassportDoneCallback, + ) => { + const profile = makeProfileInfo(rawProfile); done(undefined, { + providerInfo: { + accessToken, + scope: params.scope, + expiresInSeconds: params.expires_in, + }, profile, - accessToken, - scope: params.scope, - expiresInSeconds: params.expires_in, }); }, ); } - async start(req: express.Request, options: any): Promise { + async start( + req: express.Request, + options: Record, + ): Promise { return await executeRedirectStrategy(req, this._strategy, options); } - async handler( - req: express.Request, - ): Promise<{ user: AuthInfoBase; info: AuthInfoPrivate }> { - return await executeFrameHandlerStrategy(req, this._strategy); + async handler(req: express.Request): Promise<{ response: OAuthResponse }> { + const result = await executeFrameHandlerStrategy( + req, + this._strategy, + ); + + return { + response: result.response, + }; } } diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index f41fbfe787..f12b248021 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -25,14 +25,13 @@ import { } from '../../lib/PassportStrategyHelper'; import { OAuthProviderHandlers, - AuthInfoBase, - AuthInfoPrivate, RedirectInfo, AuthProviderConfig, - AuthInfoWithProfile, EnvironmentProviderConfig, OAuthProviderOptions, OAuthProviderConfig, + OAuthResponse, + PassportDoneCallback, } from '../types'; import { OAuthProvider } from '../../lib/OAuthProvider'; import passport from 'passport'; @@ -43,6 +42,10 @@ import { import { Logger } from 'winston'; import { TokenIssuer } from '../../identity'; +type PrivateInfo = { + refreshToken: string; +}; + export class GoogleAuthProvider implements OAuthProviderHandlers { private readonly _strategy: GoogleStrategy; @@ -56,18 +59,20 @@ export class GoogleAuthProvider implements OAuthProviderHandlers { accessToken: any, refreshToken: any, params: any, - profile: passport.Profile, - done: any, + rawProfile: passport.Profile, + done: PassportDoneCallback, ) => { - const profileInfo = makeProfileInfo(profile, params); + const profile = makeProfileInfo(rawProfile, params.id_token); done( undefined, { - profile: profileInfo, - idToken: params.id_token, - accessToken, - scope: params.scope, - expiresInSeconds: params.expires_in, + providerInfo: { + idToken: params.id_token, + accessToken, + scope: params.scope, + expiresInSeconds: params.expires_in, + }, + profile, }, { refreshToken, @@ -77,20 +82,33 @@ export class GoogleAuthProvider implements OAuthProviderHandlers { ); } - async start(req: express.Request, options: any): Promise { - return await executeRedirectStrategy(req, this._strategy, options); + async start( + req: express.Request, + options: Record, + ): Promise { + const providerOptions = { + ...options, + accessType: 'offline', + prompt: 'consent', + }; + return await executeRedirectStrategy(req, this._strategy, providerOptions); } async handler( req: express.Request, - ): Promise<{ user: AuthInfoBase; info: AuthInfoPrivate }> { - return await executeFrameHandlerStrategy(req, this._strategy); + ): Promise<{ response: OAuthResponse; refreshToken: string }> { + const result = await executeFrameHandlerStrategy< + OAuthResponse, + PrivateInfo + >(req, this._strategy); + + return { + response: result.response, + refreshToken: result.privateInfo.refreshToken, + }; } - async refresh( - refreshToken: string, - scope: string, - ): Promise { + async refresh(refreshToken: string, scope: string): Promise { const { accessToken, params } = await executeRefreshTokenStrategy( this._strategy, refreshToken, @@ -100,14 +118,16 @@ export class GoogleAuthProvider implements OAuthProviderHandlers { const profile = await executeFetchUserProfileStrategy( this._strategy, accessToken, - params, + params.id_token, ); return { - accessToken, - idToken: params.id_token, - expiresInSeconds: params.expires_in, - scope: params.scope, + providerInfo: { + accessToken, + idToken: params.id_token, + expiresInSeconds: params.expires_in, + scope: params.scope, + }, profile, }; } diff --git a/plugins/auth-backend/src/providers/saml/provider.ts b/plugins/auth-backend/src/providers/saml/provider.ts index 621b75078c..68b84c7e33 100644 --- a/plugins/auth-backend/src/providers/saml/provider.ts +++ b/plugins/auth-backend/src/providers/saml/provider.ts @@ -15,7 +15,11 @@ */ import express from 'express'; -import { Strategy as SamlStrategy } from 'passport-saml'; +import { + Strategy as SamlStrategy, + Profile as SamlProfile, + VerifyWithoutRequest, +} from 'passport-saml'; import { executeFrameHandlerStrategy, executeRedirectStrategy, @@ -25,6 +29,8 @@ import { AuthProviderRouteHandlers, EnvironmentProviderConfig, SAMLProviderConfig, + PassportDoneCallback, + ProfileInfo, } from '../types'; import { postMessageResponse } from '../../lib/OAuthProvider'; import { @@ -32,30 +38,39 @@ import { EnvironmentHandler, } from '../../lib/EnvironmentHandler'; import { Logger } from 'winston'; +import { TokenIssuer } from '../../identity'; + +type SamlInfo = { + userId: string; + profile: ProfileInfo; +}; export class SamlAuthProvider implements AuthProviderRouteHandlers { private readonly strategy: SamlStrategy; + private readonly tokenIssuer: TokenIssuer; constructor(options: SAMLProviderOptions) { - this.strategy = new SamlStrategy( - { ...options }, - (profile: any, done: any) => { - // TODO: There's plenty more validation and profile handling to do here, - // this provider is currently only intended to validate the provider pattern - // for non-oauth auth flows. - // TODO: This flow doesn't issue an identity token that can be used to validate - // the identity of the user in other backends, which we need in some form. - done(undefined, { - email: profile.email, - firstName: profile.firstName, - lastName: profile.lastName, - displayName: profile.displayName, - }); - }, - ); + this.tokenIssuer = options.tokenIssuer; + this.strategy = new SamlStrategy({ ...options }, (( + profile: SamlProfile, + done: PassportDoneCallback, + ) => { + // TODO: There's plenty more validation and profile handling to do here, + // this provider is currently only intended to validate the provider pattern + // for non-oauth auth flows. + // TODO: This flow doesn't issue an identity token that can be used to validate + // the identity of the user in other backends, which we need in some form. + done(undefined, { + userId: profile.ID!, + profile: { + email: profile.email!, + displayName: profile.displayName as string, + }, + }); + }) as VerifyWithoutRequest); } - async start(req: express.Request, res: express.Response): Promise { + async start(req: express.Request, res: express.Response): Promise { const { url } = await executeRedirectStrategy(req, this.strategy, {}); res.redirect(url); } @@ -63,17 +78,28 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { async frameHandler( req: express.Request, res: express.Response, - ): Promise { + ): Promise { try { - const { user } = await executeFrameHandlerStrategy(req, this.strategy); + const { + response: { userId, profile }, + } = await executeFrameHandlerStrategy(req, this.strategy); + + const id = userId; + const idToken = await this.tokenIssuer.issueToken({ + claims: { sub: id }, + }); return postMessageResponse(res, 'http://localhost:3000', { - type: 'auth-result', - payload: user, + type: 'authorization_response', + response: { + providerInfo: {}, + profile, + backstageIdentity: { id, idToken }, + }, }); } catch (error) { return postMessageResponse(res, 'http://localhost:3000', { - type: 'auth-result', + type: 'authorization_response', error: { name: error.name, message: error.message, @@ -82,7 +108,7 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { } } - async logout(_req: express.Request, res: express.Response): Promise { + async logout(_req: express.Request, res: express.Response): Promise { res.send('noop'); } } @@ -91,12 +117,14 @@ type SAMLProviderOptions = { entryPoint: string; issuer: string; path: string; + tokenIssuer: TokenIssuer; }; export function createSamlProvider( _authProviderConfig: AuthProviderConfig, providerConfig: EnvironmentProviderConfig, logger: Logger, + tokenIssuer: TokenIssuer, ) { const envProviders: EnvironmentHandlers = {}; @@ -106,6 +134,7 @@ export function createSamlProvider( entryPoint: config.entryPoint, issuer: config.issuer, path: '/auth/saml/handler/frame', + tokenIssuer, }; if (!opts.entryPoint || !opts.issuer) { diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 3660751c1c..81773bf9dc 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -89,25 +89,30 @@ export interface OAuthProviderHandlers { * @param {express.Request} req * @param options */ - start(req: express.Request, options: any): Promise; + start( + req: express.Request, + options: Record, + ): Promise; /** * Handles the redirect from the auth provider when the user has signed in. * @param {express.Request} req */ - handler(req: express.Request): Promise; + handler( + req: express.Request, + ): Promise<{ response: OAuthResponse; refreshToken?: string }>; /** * (Optional) Given a refresh token and scope fetches a new access token from the auth provider. * @param {string} refreshToken * @param {string} scope */ - refresh?(refreshToken: string, scope: string): Promise; + refresh?(refreshToken: string, scope: string): Promise; /** * (Optional) Sign out of the auth provider. */ - logout?(): Promise; + logout?(): Promise; } /** @@ -134,7 +139,7 @@ export interface AuthProviderRouteHandlers { * @param {express.Request} req * @param {express.Response} res */ - start(req: express.Request, res: express.Response): Promise; + start(req: express.Request, res: express.Response): Promise; /** * Once the user signs in or consents in the OAuth screen, the auth provider redirects to the @@ -149,7 +154,7 @@ export interface AuthProviderRouteHandlers { * @param {express.Request} req * @param {express.Response} res */ - frameHandler(req: express.Request, res: express.Response): Promise; + frameHandler(req: express.Request, res: express.Response): Promise; /** * (Optional) If the auth provider supports refresh tokens then this method handles @@ -163,7 +168,7 @@ export interface AuthProviderRouteHandlers { * @param {express.Request} req * @param {express.Response} res */ - refresh?(req: express.Request, res: express.Response): Promise; + refresh?(req: express.Request, res: express.Response): Promise; /** * (Optional) Handles sign out requests @@ -174,7 +179,7 @@ export interface AuthProviderRouteHandlers { * @param {express.Request} req * @param {express.Response} res */ - logout?(req: express.Request, res: express.Response): Promise; + logout?(req: express.Request, res: express.Response): Promise; } export type AuthProviderFactory = ( @@ -184,7 +189,30 @@ export type AuthProviderFactory = ( issuer: TokenIssuer, ) => AuthProviderRouteHandlers; -export type AuthInfoBase = { +export type AuthResponse = { + providerInfo: ProviderInfo; + profile: ProfileInfo; + backstageIdentity: BackstageIdentity; +}; + +export type OAuthResponse = Omit< + AuthResponse, + 'backstageIdentity' +>; + +export type BackstageIdentity = { + /** + * The backstage user ID. + */ + id: string; + + /** + * An ID token that can be used to authenticate the user within Backstage. + */ + idToken: string; +}; + +export type OAuthProviderInfo = { /** * An access token issued for the signed in user. */ @@ -203,14 +231,7 @@ export type AuthInfoBase = { scope: string; }; -export type AuthInfoWithProfile = AuthInfoBase & { - /** - * Profile information of the signed in user. - */ - profile: ProfileInfo | undefined; -}; - -export type AuthInfoPrivate = { +export type OAuthPrivateInfo = { /** * A refresh token issued for the signed in user. */ @@ -221,16 +242,22 @@ export type AuthInfoPrivate = { * Payload sent as a post message after the auth request is complete. * If successful then has a valid payload with Auth information else contains an error. */ -export type AuthResponse = +export type WebMessageResponse = | { - type: 'auth-result'; - payload: AuthInfoBase | AuthInfoWithProfile; + type: 'authorization_response'; + response: AuthResponse; } | { - type: 'auth-result'; + type: 'authorization_response'; error: Error; }; +export type PassportDoneCallback = ( + err?: Error, + response?: Res, + privateInfo?: Private, +) => void; + export type RedirectInfo = { /** * URL to redirect to @@ -242,6 +269,12 @@ export type RedirectInfo = { status?: number; }; +/** + * Used to display login information to user, i.e. sidebar popup. + * + * It is also temporarily used as the profile of the signed-in user's Backstage + * identity, but we want to replace that with data from identity and/org catalog service + */ export type ProfileInfo = { /** * Email ID of the signed in user. @@ -250,12 +283,12 @@ export type ProfileInfo = { /** * Display name that can be presented to the signed in user. */ - name: string; + displayName?: string; /** * URL to an image that can be used as the display image or avatar of the * signed in user. */ - picture: string; + picture?: string; }; export type RefreshTokenResponse = { diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 68b2d31032..28718e85a1 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -67,12 +67,6 @@ export async function createRouter( clientId: process.env.AUTH_GOOGLE_CLIENT_ID!, clientSecret: process.env.AUTH_GOOGLE_CLIENT_SECRET!, }, - production: { - appOrigin: 'http://localhost:3000', - secure: false, - clientId: '', - clientSecret: '', - }, }, github: { development: {