From 78baf7f932583fb73c0fe9a9f151e495fa29bad5 Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Mon, 22 Jun 2020 18:02:19 +0200 Subject: [PATCH 01/24] backend - fix types + refactoring --- .../src/lib/OAuthProvider.test.ts | 64 +++++++++----- plugins/auth-backend/src/lib/OAuthProvider.ts | 69 ++++++++------- .../src/lib/PassportStrategyHelper.test.ts | 4 +- .../src/lib/PassportStrategyHelper.ts | 86 +++++++++++-------- .../src/providers/github/provider.ts | 40 ++++++--- .../src/providers/google/provider.ts | 68 +++++++++------ .../src/providers/saml/provider.ts | 76 ++++++++++------ plugins/auth-backend/src/providers/types.ts | 79 ++++++++++++----- 8 files changed, 314 insertions(+), 172 deletions(-) diff --git a/plugins/auth-backend/src/lib/OAuthProvider.test.ts b/plugins/auth-backend/src/lib/OAuthProvider.test.ts index 48952e6dad..d84226214d 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,19 @@ 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', + }, + userIdToken: 'a.b.c', }, }; const jsonData = JSON.stringify(data); @@ -111,8 +136,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 +186,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 +344,10 @@ 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, + userIdToken: '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..1c88dae693 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,23 @@ export class OAuthProvider implements AuthProviderRouteHandlers { this.setRefreshTokenCookie(res, refreshToken); } - user.userIdToken = await this.options.tokenIssuer.issueToken({ - claims: { sub: user.profile.email }, + const userIdToken = await this.options.tokenIssuer.issueToken({ + claims: { sub: response.profile.email }, }); + const fullResponse: AuthResponse = { + ...response, + userIdToken, + }; // 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 +172,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 +210,19 @@ 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 userIdToken = await this.options.tokenIssuer.issueToken({ + claims: { sub: response.profile.email }, }); + const fullResponse: AuthResponse = { + ...response, + userIdToken, + }; - 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..bb6d1b5d42 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 = ''; + let email: string | undefined = undefined; if (profile.emails) { 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..867bd280a2 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,27 @@ 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 userIdToken = await this.tokenIssuer.issueToken({ + claims: { sub: userId }, + }); return postMessageResponse(res, 'http://localhost:3000', { - type: 'auth-result', - payload: user, + type: 'authorization_response', + response: { + providerInfo: {}, + profile, + userIdToken, + }, }); } catch (error) { return postMessageResponse(res, 'http://localhost:3000', { - type: 'auth-result', + type: 'authorization_response', error: { name: error.name, message: error.message, @@ -82,7 +107,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 +116,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 +133,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..2101fc19bb 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,18 @@ export type AuthProviderFactory = ( issuer: TokenIssuer, ) => AuthProviderRouteHandlers; -export type AuthInfoBase = { +export type AuthResponse = { + providerInfo: ProviderInfo; + profile: ProfileInfo; + userIdToken: string; +}; + +export type OAuthResponse = Omit< + AuthResponse, + 'userIdToken' +>; + +export type OAuthProviderInfo = { /** * An access token issued for the signed in user. */ @@ -203,34 +219,35 @@ 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. */ refreshToken: string; }; +// {type: 'authorization_response', response: {...}} + /** * 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 +259,22 @@ export type RedirectInfo = { status?: number; }; +/* +metadata: + name: john +spec: + profile: + email: john.doe@example.com + displayName: John Doe + picture: https://example.com/avatars/john.doe + + identities: + - type: google + email: john.doe@gmail.com + */ + +// 1. Link to identity in catalog / within backstage +// 2. Display login information to user, i.e. sidebar popup 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 = { From a5785b80faf4946f47f2875e15783f7abd034040 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Tue, 23 Jun 2020 15:45:48 +0200 Subject: [PATCH 02/24] Identity and profile work, wip --- .../src/apis/definitions/IdentityApi.ts | 6 + .../core-api/src/apis/definitions/auth.ts | 104 ++++++++++-------- .../auth/github/GithubAuth.test.ts | 2 +- .../implementations/auth/github/GithubAuth.ts | 46 ++++++-- .../apis/implementations/auth/github/types.ts | 13 ++- .../auth/google/GoogleAuth.test.ts | 44 ++++---- .../implementations/auth/google/GoogleAuth.ts | 68 +++++++----- .../apis/implementations/auth/google/types.ts | 13 ++- packages/core-api/src/app/App.tsx | 4 + packages/core-api/src/app/AppIdentity.ts | 17 ++- packages/core-api/src/app/types.ts | 5 +- .../layout/Sidebar/Settings/UserProfile.tsx | 72 ++---------- .../src/layout/SignInPage/customProvider.tsx | 3 + .../src/layout/SignInPage/googleProvider.tsx | 38 ++++--- .../src/layout/SignInPage/guestProvider.tsx | 12 +- .../src/lib/OAuthProvider.test.ts | 10 +- plugins/auth-backend/src/lib/OAuthProvider.ts | 14 ++- .../src/providers/saml/provider.ts | 7 +- plugins/auth-backend/src/providers/types.ts | 16 ++- plugins/auth-backend/src/service/router.ts | 6 - 20 files changed, 288 insertions(+), 212 deletions(-) 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..dc2136a806 100644 --- a/packages/core-api/src/app/App.tsx +++ b/packages/core-api/src/app/App.tsx @@ -293,6 +293,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/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/plugins/auth-backend/src/lib/OAuthProvider.test.ts b/plugins/auth-backend/src/lib/OAuthProvider.test.ts index d84226214d..f92875fd48 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.test.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.test.ts @@ -116,7 +116,10 @@ describe('OAuthProvider Utils', () => { profile: { email: 'foo@bar.com', }, - userIdToken: 'a.b.c', + backstageIdentity: { + id: 'a', + idToken: 'a.b.c', + }, }, }; const jsonData = JSON.stringify(data); @@ -346,7 +349,10 @@ describe('OAuthProvider', () => { expect(mockResponse.send).toHaveBeenCalledTimes(1); expect(mockResponse.send).toHaveBeenCalledWith({ ...mockResponseData, - userIdToken: 'my-id-token', + backstageIdentity: { + id: mockResponseData.profile.email, + idToken: 'my-id-token', + }, }); }); diff --git a/plugins/auth-backend/src/lib/OAuthProvider.ts b/plugins/auth-backend/src/lib/OAuthProvider.ts index 1c88dae693..dac31af451 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.ts @@ -147,12 +147,13 @@ export class OAuthProvider implements AuthProviderRouteHandlers { this.setRefreshTokenCookie(res, refreshToken); } - const userIdToken = await this.options.tokenIssuer.issueToken({ - claims: { sub: response.profile.email }, + const id = response.profile.email; + const idToken = await this.options.tokenIssuer.issueToken({ + claims: { sub: id }, }); const fullResponse: AuthResponse = { ...response, - userIdToken, + backstageIdentity: { id, idToken }, }; // post message back to popup if successful @@ -212,12 +213,13 @@ export class OAuthProvider implements AuthProviderRouteHandlers { // get new access_token const response = await this.providerHandlers.refresh(refreshToken, scope); - const userIdToken = await this.options.tokenIssuer.issueToken({ - claims: { sub: response.profile.email }, + const id = response.profile.email; + const idToken = await this.options.tokenIssuer.issueToken({ + claims: { sub: id }, }); const fullResponse: AuthResponse = { ...response, - userIdToken, + backstageIdentity: { id, idToken }, }; res.send(fullResponse); diff --git a/plugins/auth-backend/src/providers/saml/provider.ts b/plugins/auth-backend/src/providers/saml/provider.ts index 867bd280a2..68b84c7e33 100644 --- a/plugins/auth-backend/src/providers/saml/provider.ts +++ b/plugins/auth-backend/src/providers/saml/provider.ts @@ -84,8 +84,9 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { response: { userId, profile }, } = await executeFrameHandlerStrategy(req, this.strategy); - const userIdToken = await this.tokenIssuer.issueToken({ - claims: { sub: userId }, + const id = userId; + const idToken = await this.tokenIssuer.issueToken({ + claims: { sub: id }, }); return postMessageResponse(res, 'http://localhost:3000', { @@ -93,7 +94,7 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { response: { providerInfo: {}, profile, - userIdToken, + backstageIdentity: { id, idToken }, }, }); } catch (error) { diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 2101fc19bb..87ea88230c 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -192,14 +192,26 @@ export type AuthProviderFactory = ( export type AuthResponse = { providerInfo: ProviderInfo; profile: ProfileInfo; - userIdToken: string; + backstageIdentity: BackstageIdentity; }; export type OAuthResponse = Omit< AuthResponse, - 'userIdToken' + '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. 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: { From 593cc2c6e6ef0f06c22abcfd8dc6d2d5bee12033 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 23 Jun 2020 16:06:26 +0200 Subject: [PATCH 03/24] core-api: update web message response handling in loginPopup to expect RFC format --- packages/core-api/src/lib/loginPopup.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) 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(); }; From cc3664faf16df70e3c3b33ed856d3594d0dfd7b7 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 23 Jun 2020 18:34:33 +0200 Subject: [PATCH 04/24] core: fix sign-in hot reloading issues and unmounted component access --- packages/core-api/src/app/App.tsx | 19 ++++--------------- .../Settings/OAuthProviderSettings.tsx | 11 +++++++++-- .../Sidebar/Settings/OIDCProviderSettings.tsx | 11 +++++++++-- .../core/src/layout/SignInPage/providers.tsx | 3 ++- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/packages/core-api/src/app/App.tsx b/packages/core-api/src/app/App.tsx index dc2136a806..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 }) => { 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/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) { From a6815e3543c23773f871858e7d315ea54bfec193 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 23 Jun 2020 19:21:15 +0200 Subject: [PATCH 05/24] core-api: update loginPopup tests --- packages/core-api/src/lib/loginPopup.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) 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', From 005ca191a829876c2e4f7577b7ce377e247fc83e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 23 Jun 2020 19:29:04 +0200 Subject: [PATCH 06/24] storybook: update identity api mock --- packages/storybook/.storybook/apis.js | 1 + 1 file changed, 1 insertion(+) 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 () => {}, }); From cddfe9bd58aa668f7cfe65fa42a8c51f446c4e4c Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Tue, 23 Jun 2020 20:11:16 +0200 Subject: [PATCH 07/24] feat(techdocs-core): add python formatter --- .../mkdocs/container/techdocs-core/requirements.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/plugins/techdocs/mkdocs/container/techdocs-core/requirements.txt b/plugins/techdocs/mkdocs/container/techdocs-core/requirements.txt index b3667f94ec..2a13d1a9da 100644 --- a/plugins/techdocs/mkdocs/container/techdocs-core/requirements.txt +++ b/plugins/techdocs/mkdocs/container/techdocs-core/requirements.txt @@ -1 +1,9 @@ +# The "base" version of the Mkdocs project. +# Note: if you update this, also update `install_requires` in setup.py +# https://github.com/mkdocs/mkdocs mkdocs==1.1.2 + +# The linter using for Python +# Note: This requires Python 3.6+ to run, but can format Python 2 code too. +# https://github.com/psf/black +black==19.10b0 From 3f79513fdd359b68c0b8687a00f9d7780907e87b Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Tue, 23 Jun 2020 20:16:36 +0200 Subject: [PATCH 08/24] chore(techdocs-core): lower indentation of notice --- plugins/techdocs/mkdocs/container/Dockerfile | 20 +++++++------- .../mkdocs/container/techdocs-core/setup.py | 26 +++++++++---------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/plugins/techdocs/mkdocs/container/Dockerfile b/plugins/techdocs/mkdocs/container/Dockerfile index 1ae66589b6..ce54f53983 100644 --- a/plugins/techdocs/mkdocs/container/Dockerfile +++ b/plugins/techdocs/mkdocs/container/Dockerfile @@ -1,17 +1,17 @@ -# Copyright 2020 Spotify AB +# 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 +# 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 +# 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. +# 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. FROM python:3.7.7-alpine3.12 diff --git a/plugins/techdocs/mkdocs/container/techdocs-core/setup.py b/plugins/techdocs/mkdocs/container/techdocs-core/setup.py index faa8b8c46e..a259ae0615 100644 --- a/plugins/techdocs/mkdocs/container/techdocs-core/setup.py +++ b/plugins/techdocs/mkdocs/container/techdocs-core/setup.py @@ -1,17 +1,17 @@ """ - * 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. +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. """ from setuptools import setup, find_packages From 8f91501b07aaba6eab12c5b1352d0edcb159c785 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Tue, 23 Jun 2020 20:14:09 +0200 Subject: [PATCH 09/24] chore(mkdocs/container): remove initial newline --- plugins/techdocs/mkdocs/container/Dockerfile | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/techdocs/mkdocs/container/Dockerfile b/plugins/techdocs/mkdocs/container/Dockerfile index ce54f53983..8df86eadb0 100644 --- a/plugins/techdocs/mkdocs/container/Dockerfile +++ b/plugins/techdocs/mkdocs/container/Dockerfile @@ -1,4 +1,3 @@ - # Copyright 2020 Spotify AB # # Licensed under the Apache License, Version 2.0 (the "License"); From 97e9e325e54c800c1a8f4f18972c88bd400cb32a Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Tue, 23 Jun 2020 20:31:07 +0200 Subject: [PATCH 10/24] docs(techdocs-core): add README --- .../mkdocs/container/techdocs-core/README.md | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 plugins/techdocs/mkdocs/container/techdocs-core/README.md diff --git a/plugins/techdocs/mkdocs/container/techdocs-core/README.md b/plugins/techdocs/mkdocs/container/techdocs-core/README.md new file mode 100644 index 0000000000..04606088d4 --- /dev/null +++ b/plugins/techdocs/mkdocs/container/techdocs-core/README.md @@ -0,0 +1,47 @@ +# techdocs-core + +This is the base [Mkdocs](https://mkdocs.org) plugin used when using Mkdocs with Spotify's TechDocs. It is written in Python and packages all of our Mkdocs defaults, such as theming, plugins, etc in a single plugin. + +## Usage + +**Installation instructions TBD.** We haven't published it to a Python registry yet. + +Once you have installed the `mkdocs-techdocs-core` plugin, you'll need to add it to your `mkdocs.yml`. + +```yaml +site_name: Backstage Docs + +nav: + - Home: index.md + - Developing a Plugin: developing-a-plugin.md + +plugins: + - techdocs-core +``` + +## Running Locally + +You can install this package locally using `pip` and the `--editable` flag used for making developing Python packages. + +```bash +pip install --editable . +``` + +You'll then have the `techdocs-core` package available to use in Mkdocs and `pip` will point the dependency to this folder. + +## Running with Docker + +In the parent `Dockerfile` we add this folder to the build and install the package locally in the container. In the future, we'll probably move away from this approach and have it download directly from a Python registry (and this folder will publish to one). + +See the `README.md` located in the `mkdocs/` folder for more details on how to build and run the Docker container. + +## Linting and Tests + +To run the tests, you'll need `tox` installed: + +```bash +pip install tox +tox +``` + +This will invoke our linter and tests. You can configure this by editing the `tox.ini` folder located in this folder. From 4c6ea359602be1223aa7320c5f3574cd0c8c7c07 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Tue, 23 Jun 2020 20:32:23 +0200 Subject: [PATCH 11/24] chore(techdocs-core): added gitignore --- plugins/techdocs/mkdocs/container/techdocs-core/.gitignore | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 plugins/techdocs/mkdocs/container/techdocs-core/.gitignore diff --git a/plugins/techdocs/mkdocs/container/techdocs-core/.gitignore b/plugins/techdocs/mkdocs/container/techdocs-core/.gitignore new file mode 100644 index 0000000000..cd5c4a44f3 --- /dev/null +++ b/plugins/techdocs/mkdocs/container/techdocs-core/.gitignore @@ -0,0 +1,2 @@ +.tox +*.egg-info From d6cf8892fe19fe009cc7698c7c0ce0b7ff5bac27 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Tue, 23 Jun 2020 20:33:29 +0200 Subject: [PATCH 12/24] docs(techdocs-core): added dependency install command --- plugins/techdocs/mkdocs/container/techdocs-core/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/techdocs/mkdocs/container/techdocs-core/README.md b/plugins/techdocs/mkdocs/container/techdocs-core/README.md index 04606088d4..b4a32a57cb 100644 --- a/plugins/techdocs/mkdocs/container/techdocs-core/README.md +++ b/plugins/techdocs/mkdocs/container/techdocs-core/README.md @@ -37,10 +37,11 @@ See the `README.md` located in the `mkdocs/` folder for more details on how to b ## Linting and Tests -To run the tests, you'll need `tox` installed: +To run the tests, you'll need `tox` installed with our dependencies installed: ```bash pip install tox +pip install -r requirements.txt tox ``` From f9ee9991a05a98535fea4680328edb274abbfaa6 Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Tue, 23 Jun 2020 20:45:22 +0200 Subject: [PATCH 13/24] docs(techdocs-core): added linting section --- .../techdocs/mkdocs/container/techdocs-core/README.md | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/plugins/techdocs/mkdocs/container/techdocs-core/README.md b/plugins/techdocs/mkdocs/container/techdocs-core/README.md index b4a32a57cb..143acf3c3b 100644 --- a/plugins/techdocs/mkdocs/container/techdocs-core/README.md +++ b/plugins/techdocs/mkdocs/container/techdocs-core/README.md @@ -35,14 +35,11 @@ In the parent `Dockerfile` we add this folder to the build and install the packa See the `README.md` located in the `mkdocs/` folder for more details on how to build and run the Docker container. -## Linting and Tests - -To run the tests, you'll need `tox` installed with our dependencies installed: +## Linting ```bash -pip install tox pip install -r requirements.txt -tox +python -m black src/ ``` -This will invoke our linter and tests. You can configure this by editing the `tox.ini` folder located in this folder. +**Note:** This will write to all Python files in `src/` with the formatted code. If you would like to only check to see if it passes, simply append the `--check` flag. From f432c9deb6a41b7e8ff82113cbee8747b4a56ace Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Tue, 23 Jun 2020 20:45:39 +0200 Subject: [PATCH 14/24] chore(techdocs-core): formatted all files in src/ --- .../mkdocs/container/techdocs-core/src/core.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/plugins/techdocs/mkdocs/container/techdocs-core/src/core.py b/plugins/techdocs/mkdocs/container/techdocs-core/src/core.py index e42a60d28b..7fce7d6dac 100644 --- a/plugins/techdocs/mkdocs/container/techdocs-core/src/core.py +++ b/plugins/techdocs/mkdocs/container/techdocs-core/src/core.py @@ -19,17 +19,17 @@ from mkdocs.theme import Theme from mkdocs.contrib.search import SearchPlugin + class TechDocsCore(BasePlugin): def on_config(self, config): - # Theme - config['theme'] = Theme(name="material") + # Theme + config["theme"] = Theme(name="material") - # Plugins - del config['plugins']['techdocs-core'] + # Plugins + del config["plugins"]["techdocs-core"] - search_plugin = SearchPlugin() - search_plugin.load_config({}) - config['plugins']['search'] = search_plugin - - return config + search_plugin = SearchPlugin() + search_plugin.load_config({}) + config["plugins"]["search"] = search_plugin + return config From 464e07d067052c34fd94e85d2c6701414bb7991c Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Tue, 23 Jun 2020 20:51:22 +0200 Subject: [PATCH 15/24] docs(mkdocs): lower indentation for container setup commands --- plugins/techdocs/mkdocs/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/techdocs/mkdocs/README.md b/plugins/techdocs/mkdocs/README.md index 4d1c11d7b0..8cf1717cf1 100644 --- a/plugins/techdocs/mkdocs/README.md +++ b/plugins/techdocs/mkdocs/README.md @@ -6,8 +6,8 @@ Welcome to MkDocs. This is the TechDocs implementation of MkDocs. ## Getting started -``` - docker build ./container -t mkdocs-container +```bash +docker build ./container -t mkdocs-container - docker run -w /content -v $(pwd)/mock-docs:/content -p 8000:8000 -it mkdocs-container serve -a 0.0.0.0:8000 +docker run -w /content -v $(pwd)/mock-docs:/content -p 8000:8000 -it mkdocs-container serve -a 0.0.0.0:8000 ``` From b53ecbff3ed45efe9be5a639ca5edb66e91bc55a Mon Sep 17 00:00:00 2001 From: Bilawal Hameed Date: Tue, 23 Jun 2020 20:52:04 +0200 Subject: [PATCH 16/24] docs(mkdocs): added open up localhost:3000 to readme --- plugins/techdocs/mkdocs/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/techdocs/mkdocs/README.md b/plugins/techdocs/mkdocs/README.md index 8cf1717cf1..5c2a7725aa 100644 --- a/plugins/techdocs/mkdocs/README.md +++ b/plugins/techdocs/mkdocs/README.md @@ -11,3 +11,5 @@ docker build ./container -t mkdocs-container docker run -w /content -v $(pwd)/mock-docs:/content -p 8000:8000 -it mkdocs-container serve -a 0.0.0.0:8000 ``` + +Then open up `http://localhost:8000` on your local machine. From afc2e3705692a9c7ab40b4217847c309ed6d7280 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 24 Jun 2020 08:47:55 +0200 Subject: [PATCH 17/24] chore(catalog): move the entity examples to catalog-model --- .../catalog-model}/examples/artist-lookup-component.yaml | 0 .../catalog-model}/examples/playback-lib-component.yaml | 0 .../catalog-model}/examples/playback-order-component.yaml | 0 .../catalog-model}/examples/podcast-api-component.yaml | 0 .../catalog-model}/examples/queue-proxy-component.yaml | 0 .../catalog-model}/examples/searcher-component.yaml | 0 .../catalog-model}/examples/shuffle-api-component.yaml | 0 .../catalog-model}/examples/www-artist-component.yaml | 0 plugins/catalog-backend/scripts/mock-data | 2 +- 9 files changed, 1 insertion(+), 1 deletion(-) rename {plugins/catalog-backend => packages/catalog-model}/examples/artist-lookup-component.yaml (100%) rename {plugins/catalog-backend => packages/catalog-model}/examples/playback-lib-component.yaml (100%) rename {plugins/catalog-backend => packages/catalog-model}/examples/playback-order-component.yaml (100%) rename {plugins/catalog-backend => packages/catalog-model}/examples/podcast-api-component.yaml (100%) rename {plugins/catalog-backend => packages/catalog-model}/examples/queue-proxy-component.yaml (100%) rename {plugins/catalog-backend => packages/catalog-model}/examples/searcher-component.yaml (100%) rename {plugins/catalog-backend => packages/catalog-model}/examples/shuffle-api-component.yaml (100%) rename {plugins/catalog-backend => packages/catalog-model}/examples/www-artist-component.yaml (100%) diff --git a/plugins/catalog-backend/examples/artist-lookup-component.yaml b/packages/catalog-model/examples/artist-lookup-component.yaml similarity index 100% rename from plugins/catalog-backend/examples/artist-lookup-component.yaml rename to packages/catalog-model/examples/artist-lookup-component.yaml diff --git a/plugins/catalog-backend/examples/playback-lib-component.yaml b/packages/catalog-model/examples/playback-lib-component.yaml similarity index 100% rename from plugins/catalog-backend/examples/playback-lib-component.yaml rename to packages/catalog-model/examples/playback-lib-component.yaml diff --git a/plugins/catalog-backend/examples/playback-order-component.yaml b/packages/catalog-model/examples/playback-order-component.yaml similarity index 100% rename from plugins/catalog-backend/examples/playback-order-component.yaml rename to packages/catalog-model/examples/playback-order-component.yaml diff --git a/plugins/catalog-backend/examples/podcast-api-component.yaml b/packages/catalog-model/examples/podcast-api-component.yaml similarity index 100% rename from plugins/catalog-backend/examples/podcast-api-component.yaml rename to packages/catalog-model/examples/podcast-api-component.yaml diff --git a/plugins/catalog-backend/examples/queue-proxy-component.yaml b/packages/catalog-model/examples/queue-proxy-component.yaml similarity index 100% rename from plugins/catalog-backend/examples/queue-proxy-component.yaml rename to packages/catalog-model/examples/queue-proxy-component.yaml diff --git a/plugins/catalog-backend/examples/searcher-component.yaml b/packages/catalog-model/examples/searcher-component.yaml similarity index 100% rename from plugins/catalog-backend/examples/searcher-component.yaml rename to packages/catalog-model/examples/searcher-component.yaml diff --git a/plugins/catalog-backend/examples/shuffle-api-component.yaml b/packages/catalog-model/examples/shuffle-api-component.yaml similarity index 100% rename from plugins/catalog-backend/examples/shuffle-api-component.yaml rename to packages/catalog-model/examples/shuffle-api-component.yaml diff --git a/plugins/catalog-backend/examples/www-artist-component.yaml b/packages/catalog-model/examples/www-artist-component.yaml similarity index 100% rename from plugins/catalog-backend/examples/www-artist-component.yaml rename to packages/catalog-model/examples/www-artist-component.yaml diff --git a/plugins/catalog-backend/scripts/mock-data b/plugins/catalog-backend/scripts/mock-data index 76ca5cf7c2..01675db363 100755 --- a/plugins/catalog-backend/scripts/mock-data +++ b/plugins/catalog-backend/scripts/mock-data @@ -14,5 +14,5 @@ for URL in \ --location \ --request POST 'localhost:7000/catalog/locations' \ --header 'Content-Type: application/json' \ - --data-raw "{\"type\": \"github\", \"target\": \"https://github.com/spotify/backstage/blob/master/plugins/catalog-backend/examples/${URL}\"}" + --data-raw "{\"type\": \"github\", \"target\": \"https://github.com/spotify/backstage/blob/master/packages/catalog-model/examples/${URL}\"}" done From b709889fb521012fe997538d2ee71eacd7682f1b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 24 Jun 2020 10:24:06 +0200 Subject: [PATCH 18/24] auth-backend: clean up comments + fix getting email from profile --- .../src/lib/PassportStrategyHelper.ts | 2 +- plugins/auth-backend/src/providers/types.ts | 22 +++++-------------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/plugins/auth-backend/src/lib/PassportStrategyHelper.ts b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts index bb6d1b5d42..46a386f419 100644 --- a/plugins/auth-backend/src/lib/PassportStrategyHelper.ts +++ b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts @@ -31,7 +31,7 @@ export const makeProfileInfo = ( const { displayName } = profile; let email: string | undefined = undefined; - if (profile.emails) { + if (profile.emails && profile.emails.length > 0) { const [firstEmail] = profile.emails; email = firstEmail.value; } diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 87ea88230c..81773bf9dc 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -238,8 +238,6 @@ export type OAuthPrivateInfo = { refreshToken: string; }; -// {type: 'authorization_response', response: {...}} - /** * 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. @@ -271,22 +269,12 @@ export type RedirectInfo = { status?: number; }; -/* -metadata: - name: john -spec: - profile: - email: john.doe@example.com - displayName: John Doe - picture: https://example.com/avatars/john.doe - - identities: - - type: google - email: john.doe@gmail.com +/** + * 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 */ - -// 1. Link to identity in catalog / within backstage -// 2. Display login information to user, i.e. sidebar popup export type ProfileInfo = { /** * Email ID of the signed in user. From 45c518272c3a962cccd0d9a890385d5c63379d28 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 24 Jun 2020 11:00:53 +0200 Subject: [PATCH 19/24] core-api: move session state tracking to auth session managers --- .../implementations/auth/github/GithubAuth.ts | 17 ++--------- .../implementations/auth/google/GoogleAuth.ts | 23 +++----------- .../AuthSessionStore.test.ts | 8 +++++ .../AuthSessionManager/AuthSessionStore.ts | 4 +++ .../RefreshingAuthSessionManager.test.ts | 30 +++++++++++++++++-- .../RefreshingAuthSessionManager.ts | 12 ++++++-- .../AuthSessionManager/SessionStateTracker.ts | 16 +++++++--- .../StaticAuthSessionManager.ts | 8 +++++ .../src/lib/AuthSessionManager/types.ts | 5 ++++ 9 files changed, 80 insertions(+), 43 deletions(-) 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 4641dd7b50..ddf03034dc 100644 --- a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts +++ b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts @@ -29,7 +29,6 @@ import { OAuthRequestApi, AuthProvider } from '../../../definitions'; import { SessionManager } from '../../../../lib/AuthSessionManager/types'; import { StaticAuthSessionManager } from '../../../../lib/AuthSessionManager'; import { Observable } from '../../../../types'; -import { SessionStateTracker } from '../../../../lib/AuthSessionManager/SessionStateTracker'; type CreateOptions = { // TODO(Rugvip): These two should be grabbed from global config when available, they're not unique to GithubAuth @@ -95,44 +94,34 @@ class GithubAuth implements OAuthApi, SessionStateApi { return new GithubAuth(sessionManager); } - private readonly sessionStateTracker = new SessionStateTracker(); - sessionState$(): Observable { - return this.sessionStateTracker.observable; + return this.sessionManager.sessionState$(); } constructor(private readonly sessionManager: SessionManager) {} async getAccessToken(scope?: string, options?: AuthRequestOptions) { - const normalizedScopes = GithubAuth.normalizeScope(scope); const session = await this.sessionManager.getSession({ ...options, - scopes: normalizedScopes, + scopes: GithubAuth.normalizeScope(scope), }); - this.sessionStateTracker.setIsSignedId(!!session); - if (session) { - return session.providerInfo.accessToken; - } - return ''; + return session?.providerInfo.accessToken ?? ''; } 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); } 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 db26d2d9a4..594116fc1a 100644 --- a/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts @@ -32,7 +32,6 @@ import { OAuthRequestApi, AuthProvider } from '../../../definitions'; import { SessionManager } from '../../../../lib/AuthSessionManager/types'; import { RefreshingAuthSessionManager } from '../../../../lib/AuthSessionManager'; import { Observable } from '../../../../types'; -import { SessionStateTracker } from '../../../../lib/AuthSessionManager/SessionStateTracker'; type CreateOptions = { // TODO(Rugvip): These two should be grabbed from global config when available, they're not unique to GoogleAuth @@ -117,10 +116,8 @@ class GoogleAuth return new GoogleAuth(sessionManager); } - private readonly sessionStateTracker = new SessionStateTracker(); - sessionState$(): Observable { - return this.sessionStateTracker.observable; + return this.sessionManager.sessionState$(); } constructor(private readonly sessionManager: SessionManager) {} @@ -129,43 +126,31 @@ class GoogleAuth scope?: string | string[], options?: AuthRequestOptions, ) { - const normalizedScopes = GoogleAuth.normalizeScopes(scope); const session = await this.sessionManager.getSession({ ...options, - scopes: normalizedScopes, + scopes: GoogleAuth.normalizeScopes(scope), }); - this.sessionStateTracker.setIsSignedId(!!session); - if (session) { - return session.providerInfo.accessToken; - } - return ''; + return session?.providerInfo.accessToken ?? ''; } async getIdToken(options: AuthRequestOptions = {}) { const session = await this.sessionManager.getSession(options); - this.sessionStateTracker.setIsSignedId(!!session); - if (session) { - return session.providerInfo.idToken; - } - return ''; + return session?.providerInfo.idToken ?? ''; } async logout() { await this.sessionManager.removeSession(); - this.sessionStateTracker.setIsSignedId(false); } 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; } diff --git a/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.test.ts b/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.test.ts index 91e55c3a48..4572b923dd 100644 --- a/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.test.ts +++ b/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.test.ts @@ -38,6 +38,7 @@ class LocalStorage { class MockManager implements SessionManager { getSession = jest.fn(); removeSession = jest.fn(); + sessionState$ = jest.fn(); } describe('GheAuth AuthSessionStore', () => { @@ -119,4 +120,11 @@ describe('GheAuth AuthSessionStore', () => { expect(localStorage.getItem('my-key')).toBe(null); }); + + it('should forward sessionState calls', () => { + const manager = new MockManager(); + const store = new AuthSessionStore({ manager, ...defaultOptions }); + store.sessionState$(); + expect(manager.sessionState$).toHaveBeenCalled(); + }); }); diff --git a/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.ts b/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.ts index 8d318f5bff..3e3a40d02a 100644 --- a/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.ts +++ b/packages/core-api/src/lib/AuthSessionManager/AuthSessionStore.ts @@ -82,6 +82,10 @@ export class AuthSessionStore implements SessionManager { await this.manager.removeSession(); } + sessionState$() { + return this.manager.sessionState$(); + } + private loadSession(): T | undefined { try { const sessionJson = localStorage.getItem(this.storageKey); diff --git a/packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts b/packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts index 793c6f1708..12e7c0a978 100644 --- a/packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts +++ b/packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.test.ts @@ -15,6 +15,7 @@ */ import { RefreshingAuthSessionManager } from './RefreshingAuthSessionManager'; +import { SessionState } from '../../apis'; const defaultOptions = { sessionScopes: (session: { scopes: Set }) => session.scopes, @@ -22,21 +23,44 @@ const defaultOptions = { }; describe('RefreshingAuthSessionManager', () => { - it('should save result form createSession', async () => { + it('should save result from createSession', async () => { const createSession = jest.fn().mockResolvedValue({ expired: false }); const refreshSession = jest.fn().mockRejectedValue(new Error('NOPE')); + const removeSession = jest.fn(); const manager = new RefreshingAuthSessionManager({ - connector: { createSession, refreshSession }, + connector: { createSession, refreshSession, removeSession }, ...defaultOptions, } as any); + const stateSubscriber = jest.fn(); + manager.sessionState$().subscribe(stateSubscriber); + await Promise.resolve(); // Wait a tick for observer to post a value + + expect(stateSubscriber.mock.calls).toEqual([[SessionState.SignedOut]]); await manager.getSession({}); expect(createSession).toBeCalledTimes(1); + expect(stateSubscriber.mock.calls).toEqual([ + [SessionState.SignedOut], + [SessionState.SignedIn], + ]); await manager.getSession({}); expect(createSession).toBeCalledTimes(1); expect(refreshSession).toBeCalledTimes(1); + expect(stateSubscriber.mock.calls).toEqual([ + [SessionState.SignedOut], + [SessionState.SignedIn], + ]); + + expect(removeSession).toHaveBeenCalledTimes(0); + await manager.removeSession(); + expect(removeSession).toHaveBeenCalledTimes(1); + expect(stateSubscriber.mock.calls).toEqual([ + [SessionState.SignedOut], + [SessionState.SignedIn], + [SessionState.SignedOut], + ]); }); it('should ask consent only if scopes have changed', async () => { @@ -130,7 +154,7 @@ describe('RefreshingAuthSessionManager', () => { expect(refreshSession).toBeCalledTimes(1); }); - it('should remove session and reload', async () => { + it('should remove session straight away', async () => { const removeSession = jest.fn(); const manager = new RefreshingAuthSessionManager({ connector: { removeSession }, diff --git a/packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts b/packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts index f7d5bcf7ca..e46c505c12 100644 --- a/packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts +++ b/packages/core-api/src/lib/AuthSessionManager/RefreshingAuthSessionManager.ts @@ -22,6 +22,7 @@ import { } from './types'; import { AuthConnector } from '../AuthConnector'; import { SessionScopeHelper, hasScopes } from './common'; +import { SessionStateTracker } from './SessionStateTracker'; type Options = { /** The connector used for acting on the auth session */ @@ -43,6 +44,7 @@ export class RefreshingAuthSessionManager implements SessionManager { private readonly helper: SessionScopeHelper; private readonly sessionScopesFunc: SessionScopesFunc; private readonly sessionShouldRefreshFunc: SessionShouldRefreshFunc; + private readonly stateTracker = new SessionStateTracker(); private refreshPromise?: Promise; private currentSession: T | undefined; @@ -109,16 +111,18 @@ export class RefreshingAuthSessionManager implements SessionManager { ...options, scopes: this.helper.getExtendedScope(this.currentSession, options.scopes), }); + this.stateTracker.setIsSignedIn(true); return this.currentSession; } async removeSession() { this.currentSession = undefined; await this.connector.removeSession(); + this.stateTracker.setIsSignedIn(false); } - async getCurrentSession() { - return this.currentSession; + sessionState$() { + return this.stateTracker.sessionState$(); } private async collapsedSessionRefresh(): Promise { @@ -129,7 +133,9 @@ export class RefreshingAuthSessionManager implements SessionManager { this.refreshPromise = this.connector.refreshSession(); try { - return await this.refreshPromise; + const session = await this.refreshPromise; + this.stateTracker.setIsSignedIn(true); + return session; } finally { delete this.refreshPromise; } diff --git a/packages/core-api/src/lib/AuthSessionManager/SessionStateTracker.ts b/packages/core-api/src/lib/AuthSessionManager/SessionStateTracker.ts index de308acb0c..c4260bb31d 100644 --- a/packages/core-api/src/lib/AuthSessionManager/SessionStateTracker.ts +++ b/packages/core-api/src/lib/AuthSessionManager/SessionStateTracker.ts @@ -16,17 +16,25 @@ import { BehaviorSubject } from '..'; import { SessionState } from '../../apis'; +import { Observable } from '../../types'; export class SessionStateTracker { - private signedIn: boolean = false; - observable = new BehaviorSubject(SessionState.SignedOut); + private readonly subject = new BehaviorSubject( + SessionState.SignedOut, + ); - setIsSignedId(isSignedIn: boolean) { + private signedIn: boolean = false; + + setIsSignedIn(isSignedIn: boolean) { if (this.signedIn !== isSignedIn) { this.signedIn = isSignedIn; - this.observable.next( + this.subject.next( this.signedIn ? SessionState.SignedIn : SessionState.SignedOut, ); } } + + sessionState$(): Observable { + return this.subject; + } } diff --git a/packages/core-api/src/lib/AuthSessionManager/StaticAuthSessionManager.ts b/packages/core-api/src/lib/AuthSessionManager/StaticAuthSessionManager.ts index 5ecbbc0c4e..e59c11421c 100644 --- a/packages/core-api/src/lib/AuthSessionManager/StaticAuthSessionManager.ts +++ b/packages/core-api/src/lib/AuthSessionManager/StaticAuthSessionManager.ts @@ -17,6 +17,7 @@ import { SessionManager, GetSessionOptions } from './types'; import { AuthConnector } from '../AuthConnector'; import { SessionScopeHelper } from './common'; +import { SessionStateTracker } from './SessionStateTracker'; type Options = { /** The connector used for acting on the auth session */ @@ -33,6 +34,7 @@ type Options = { export class StaticAuthSessionManager implements SessionManager { private readonly connector: AuthConnector; private readonly helper: SessionScopeHelper; + private readonly stateTracker = new SessionStateTracker(); private currentSession: T | undefined; @@ -60,11 +62,17 @@ export class StaticAuthSessionManager implements SessionManager { ...options, scopes: this.helper.getExtendedScope(this.currentSession, options.scopes), }); + this.stateTracker.setIsSignedIn(true); return this.currentSession; } async removeSession() { this.currentSession = undefined; await this.connector.removeSession(); + this.stateTracker.setIsSignedIn(false); + } + + sessionState$() { + return this.stateTracker.sessionState$(); } } diff --git a/packages/core-api/src/lib/AuthSessionManager/types.ts b/packages/core-api/src/lib/AuthSessionManager/types.ts index d28a751fbf..804c7121e1 100644 --- a/packages/core-api/src/lib/AuthSessionManager/types.ts +++ b/packages/core-api/src/lib/AuthSessionManager/types.ts @@ -14,6 +14,9 @@ * limitations under the License. */ +import { Observable } from '../../types'; +import { SessionState } from '../../apis'; + export type GetSessionOptions = { optional?: boolean; instantPopup?: boolean; @@ -29,6 +32,8 @@ export type SessionManager = { getSession(options: GetSessionOptions): Promise; removeSession(): Promise; + + sessionState$(): Observable; }; /** From cc0e6b0d30b4db91214088a45a11609404263a72 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 24 Jun 2020 11:30:27 +0200 Subject: [PATCH 20/24] auth-backend: leave it to the providers to figure out the backstage identity --- .../core-api/src/apis/definitions/auth.ts | 2 +- .../layout/Sidebar/Settings/UserProfile.tsx | 6 ++- .../src/lib/OAuthProvider.test.ts | 13 +++--- plugins/auth-backend/src/lib/OAuthProvider.ts | 40 ++++++++++--------- .../src/lib/PassportStrategyHelper.ts | 4 -- .../src/providers/github/provider.ts | 8 ++-- .../src/providers/google/provider.ts | 25 +++++++++--- plugins/auth-backend/src/providers/types.ts | 21 +++++----- 8 files changed, 67 insertions(+), 52 deletions(-) diff --git a/packages/core-api/src/apis/definitions/auth.ts b/packages/core-api/src/apis/definitions/auth.ts index ccd40e66ce..d5c0e9c0a4 100644 --- a/packages/core-api/src/apis/definitions/auth.ts +++ b/packages/core-api/src/apis/definitions/auth.ts @@ -173,7 +173,7 @@ export type ProfileInfo = { /** * Email ID. */ - email: string; + email?: string; /** * Display name that can be presented to the user. diff --git a/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx b/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx index 101ae7010b..3b854801c9 100644 --- a/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx +++ b/packages/core/src/layout/Sidebar/Settings/UserProfile.tsx @@ -34,14 +34,16 @@ export const UserProfile: FC<{ open: boolean; setOpen: Function }> = ({ }) => { const ref = useRef(); // for scrolling down when collapse item opens const classes = useStyles(); - const profile = useApi(identityApiRef).getProfile(); + const identityApi = useApi(identityApiRef); const handleClick = () => { setOpen(!open); setTimeout(() => ref.current?.scrollIntoView({ behavior: 'smooth' }), 300); }; - const displayName = profile.displayName ?? profile.email; + const userId = identityApi.getUserId(); + const profile = identityApi.getProfile(); + const displayName = profile.displayName ?? userId; const SignInAvatar = () => ( {displayName[0]} diff --git a/plugins/auth-backend/src/lib/OAuthProvider.test.ts b/plugins/auth-backend/src/lib/OAuthProvider.test.ts index f92875fd48..f67a91dfcd 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.test.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.test.ts @@ -23,13 +23,9 @@ import { verifyNonce, OAuthProvider, } from './OAuthProvider'; -import { - WebMessageResponse, - OAuthProviderHandlers, - OAuthResponse, -} from '../providers/types'; +import { WebMessageResponse, OAuthProviderHandlers } from '../providers/types'; -const mockResponseData: OAuthResponse = { +const mockResponseData = { providerInfo: { accessToken: 'ACCESS_TOKEN', idToken: 'ID_TOKEN', @@ -39,6 +35,9 @@ const mockResponseData: OAuthResponse = { profile: { email: 'foo@bar.com', }, + backstageIdentity: { + id: 'foo', + }, }; describe('OAuthProvider Utils', () => { @@ -350,7 +349,7 @@ describe('OAuthProvider', () => { expect(mockResponse.send).toHaveBeenCalledWith({ ...mockResponseData, backstageIdentity: { - id: mockResponseData.profile.email, + id: mockResponseData.backstageIdentity.id, idToken: 'my-id-token', }, }); diff --git a/plugins/auth-backend/src/lib/OAuthProvider.ts b/plugins/auth-backend/src/lib/OAuthProvider.ts index dac31af451..b7179659f9 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.ts @@ -18,10 +18,10 @@ import express from 'express'; import crypto from 'crypto'; import { URL } from 'url'; import { - AuthResponse, AuthProviderRouteHandlers, OAuthProviderHandlers, WebMessageResponse, + BackstageIdentity, } from '../providers/types'; import { InputError } from '@backstage/backend-common'; import { TokenIssuer } from '../identity'; @@ -147,19 +147,12 @@ export class OAuthProvider implements AuthProviderRouteHandlers { this.setRefreshTokenCookie(res, refreshToken); } - const id = response.profile.email; - const idToken = await this.options.tokenIssuer.issueToken({ - claims: { sub: id }, - }); - const fullResponse: AuthResponse = { - ...response, - backstageIdentity: { id, idToken }, - }; + await this.populateIdentity(response.backstageIdentity); // post message back to popup if successful return postMessageResponse(res, this.options.appOrigin, { type: 'authorization_response', - response: fullResponse, + response, }); } catch (error) { // post error message back to popup if failure @@ -213,21 +206,30 @@ export class OAuthProvider implements AuthProviderRouteHandlers { // get new access_token const response = await this.providerHandlers.refresh(refreshToken, scope); - const id = response.profile.email; - const idToken = await this.options.tokenIssuer.issueToken({ - claims: { sub: id }, - }); - const fullResponse: AuthResponse = { - ...response, - backstageIdentity: { id, idToken }, - }; + await this.populateIdentity(response.backstageIdentity); - res.send(fullResponse); + res.send(response); } catch (error) { res.status(401).send(`${error.message}`); } } + /** + * If the response from the OAuth provider includes a Backstage identity, we + * make sure it's populated with all the information we can derive from the user ID. + */ + private async populateIdentity(identity?: BackstageIdentity) { + if (!identity) { + return; + } + + if (!identity.idToken) { + identity.idToken = await this.options.tokenIssuer.issueToken({ + claims: { sub: identity.id }, + }); + } + } + private setNonceCookie = (res: express.Response, nonce: string) => { res.cookie(`${this.options.providerId}-nonce`, nonce, { maxAge: TEN_MINUTES_MS, diff --git a/plugins/auth-backend/src/lib/PassportStrategyHelper.ts b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts index 46a386f419..1167946560 100644 --- a/plugins/auth-backend/src/lib/PassportStrategyHelper.ts +++ b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts @@ -57,10 +57,6 @@ export const makeProfileInfo = ( } } - if (!email) { - throw new Error('No email received in profile info'); - } - return { email, picture, diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index 3ae9858bb1..01652a6d7e 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -72,15 +72,13 @@ export class GithubAuthProvider implements OAuthProviderHandlers { return await executeRedirectStrategy(req, this._strategy, options); } - async handler(req: express.Request): Promise<{ response: OAuthResponse }> { - const result = await executeFrameHandlerStrategy( + async handler(req: express.Request) { + const { response } = await executeFrameHandlerStrategy( req, this._strategy, ); - return { - response: result.response, - }; + return { response }; } } diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index f12b248021..04705b66c1 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -97,14 +97,14 @@ export class GoogleAuthProvider implements OAuthProviderHandlers { async handler( req: express.Request, ): Promise<{ response: OAuthResponse; refreshToken: string }> { - const result = await executeFrameHandlerStrategy< + const { response, privateInfo } = await executeFrameHandlerStrategy< OAuthResponse, PrivateInfo >(req, this._strategy); return { - response: result.response, - refreshToken: result.privateInfo.refreshToken, + response: await this.populateIdentity(response), + refreshToken: privateInfo.refreshToken, }; } @@ -121,7 +121,7 @@ export class GoogleAuthProvider implements OAuthProviderHandlers { params.id_token, ); - return { + return this.populateIdentity({ providerInfo: { accessToken, idToken: params.id_token, @@ -129,7 +129,22 @@ export class GoogleAuthProvider implements OAuthProviderHandlers { scope: params.scope, }, profile, - }; + }); + } + + private async populateIdentity( + response: OAuthResponse, + ): Promise { + const { profile } = response; + + if (!profile.email) { + throw new Error('Google profile contained no email'); + } + + // TODO(Rugvip): Hardcoded to the local part of the email for now + const id = profile.email.split('@')[0]; + + return { ...response, backstageIdentity: { id } }; } } diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 81773bf9dc..56d4db79fa 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -100,14 +100,20 @@ export interface OAuthProviderHandlers { */ handler( req: express.Request, - ): Promise<{ response: OAuthResponse; refreshToken?: string }>; + ): Promise<{ + response: AuthResponse; + 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. @@ -192,13 +198,10 @@ export type AuthProviderFactory = ( export type AuthResponse = { providerInfo: ProviderInfo; profile: ProfileInfo; - backstageIdentity: BackstageIdentity; + backstageIdentity?: BackstageIdentity; }; -export type OAuthResponse = Omit< - AuthResponse, - 'backstageIdentity' ->; +export type OAuthResponse = AuthResponse; export type BackstageIdentity = { /** @@ -209,7 +212,7 @@ export type BackstageIdentity = { /** * An ID token that can be used to authenticate the user within Backstage. */ - idToken: string; + idToken?: string; }; export type OAuthProviderInfo = { @@ -279,7 +282,7 @@ export type ProfileInfo = { /** * Email ID of the signed in user. */ - email: string; + email?: string; /** * Display name that can be presented to the signed in user. */ From e3a1c1944b20fbd7c8575f7e365108cdeb6d8e3a Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Tue, 23 Jun 2020 16:57:16 +0200 Subject: [PATCH 21/24] feat(mkdocs) add mkdocs monorepo plugin to techdocs core plugin --- plugins/techdocs/mkdocs/container/Dockerfile | 2 +- .../mkdocs/container/techdocs-core/src/core.py | 18 ++++++++++++++---- plugins/techdocs/mkdocs/mock-docs/mkdocs.yml | 1 + .../mkdocs/mock-docs/sub-docs/docs/index.md | 1 + .../mkdocs/mock-docs/sub-docs/mkdocs.yml | 4 ++++ 5 files changed, 21 insertions(+), 5 deletions(-) create mode 100644 plugins/techdocs/mkdocs/mock-docs/sub-docs/docs/index.md create mode 100644 plugins/techdocs/mkdocs/mock-docs/sub-docs/mkdocs.yml diff --git a/plugins/techdocs/mkdocs/container/Dockerfile b/plugins/techdocs/mkdocs/container/Dockerfile index 8df86eadb0..aefabb5c5b 100644 --- a/plugins/techdocs/mkdocs/container/Dockerfile +++ b/plugins/techdocs/mkdocs/container/Dockerfile @@ -15,7 +15,7 @@ FROM python:3.7.7-alpine3.12 RUN apk update && apk --no-cache add gcc musl-dev -RUN pip install mkdocs==1.1.2 mkdocs-material==5.3.2 +RUN pip install mkdocs==1.1.2 mkdocs-material==5.3.2 mkdocs-monorepo-plugin==0.4.5 ADD ./techdocs-core /techdocs-core RUN pip install --no-index /techdocs-core diff --git a/plugins/techdocs/mkdocs/container/techdocs-core/src/core.py b/plugins/techdocs/mkdocs/container/techdocs-core/src/core.py index 7fce7d6dac..7fc35a1529 100644 --- a/plugins/techdocs/mkdocs/container/techdocs-core/src/core.py +++ b/plugins/techdocs/mkdocs/container/techdocs-core/src/core.py @@ -19,14 +19,24 @@ from mkdocs.theme import Theme from mkdocs.contrib.search import SearchPlugin +from mkdocs_monorepo_plugin.plugin import MonorepoPlugin class TechDocsCore(BasePlugin): def on_config(self, config): - # Theme - config["theme"] = Theme(name="material") + # Theme + config['theme'] = Theme(name="material") - # Plugins - del config["plugins"]["techdocs-core"] + # Plugins + del config['plugins']['techdocs-core'] + + search_plugin = SearchPlugin() + search_plugin.load_config({}) + + monorepo_plugin = MonorepoPlugin() + monorepo_plugin.load_config({}) + + config['plugins']['search'] = search_plugin + config['plugins']['monorepo'] = monorepo_plugin search_plugin = SearchPlugin() search_plugin.load_config({}) diff --git a/plugins/techdocs/mkdocs/mock-docs/mkdocs.yml b/plugins/techdocs/mkdocs/mock-docs/mkdocs.yml index b14d82c4de..3da0280f2b 100644 --- a/plugins/techdocs/mkdocs/mock-docs/mkdocs.yml +++ b/plugins/techdocs/mkdocs/mock-docs/mkdocs.yml @@ -2,6 +2,7 @@ site_name: 'mock-docs' nav: - Home: index.md + - SubDocs: '!include ./sub-docs/mkdocs.yml' plugins: - techdocs-core diff --git a/plugins/techdocs/mkdocs/mock-docs/sub-docs/docs/index.md b/plugins/techdocs/mkdocs/mock-docs/sub-docs/docs/index.md new file mode 100644 index 0000000000..9cb6199605 --- /dev/null +++ b/plugins/techdocs/mkdocs/mock-docs/sub-docs/docs/index.md @@ -0,0 +1 @@ +### This is a md file in another docs folder using the [MkDocs Monorepo Plugin](https://github.com/spotify/mkdocs-monorepo-plugin) diff --git a/plugins/techdocs/mkdocs/mock-docs/sub-docs/mkdocs.yml b/plugins/techdocs/mkdocs/mock-docs/sub-docs/mkdocs.yml new file mode 100644 index 0000000000..09504c1b31 --- /dev/null +++ b/plugins/techdocs/mkdocs/mock-docs/sub-docs/mkdocs.yml @@ -0,0 +1,4 @@ +site_name: subdocs + +nav: + - Home 2: "index.md" From 91022ff1eca0b93e06b808a8d3ae90c2c8a65ec2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 24 Jun 2020 11:45:48 +0200 Subject: [PATCH 22/24] catalog-model: let the guest take care of some services --- packages/catalog-model/examples/artist-lookup-component.yaml | 2 +- packages/catalog-model/examples/playback-order-component.yaml | 2 +- packages/catalog-model/examples/podcast-api-component.yaml | 2 +- packages/catalog-model/examples/searcher-component.yaml | 2 +- packages/catalog-model/examples/shuffle-api-component.yaml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/catalog-model/examples/artist-lookup-component.yaml b/packages/catalog-model/examples/artist-lookup-component.yaml index bb7d8d5767..448dd13fa3 100644 --- a/packages/catalog-model/examples/artist-lookup-component.yaml +++ b/packages/catalog-model/examples/artist-lookup-component.yaml @@ -6,4 +6,4 @@ metadata: spec: type: service lifecycle: experimental - owner: tools@example.com + owner: artists@example.com diff --git a/packages/catalog-model/examples/playback-order-component.yaml b/packages/catalog-model/examples/playback-order-component.yaml index fa9f68e77a..4f93b65edf 100644 --- a/packages/catalog-model/examples/playback-order-component.yaml +++ b/packages/catalog-model/examples/playback-order-component.yaml @@ -6,4 +6,4 @@ metadata: spec: type: service lifecycle: production - owner: tools@example.com + owner: guest diff --git a/packages/catalog-model/examples/podcast-api-component.yaml b/packages/catalog-model/examples/podcast-api-component.yaml index 2a27a0a2c5..19f9a3d4de 100644 --- a/packages/catalog-model/examples/podcast-api-component.yaml +++ b/packages/catalog-model/examples/podcast-api-component.yaml @@ -6,4 +6,4 @@ metadata: spec: type: service lifecycle: experimental - owner: tools@example.com + owner: players@example.com diff --git a/packages/catalog-model/examples/searcher-component.yaml b/packages/catalog-model/examples/searcher-component.yaml index d82f6e42c7..33d765117c 100644 --- a/packages/catalog-model/examples/searcher-component.yaml +++ b/packages/catalog-model/examples/searcher-component.yaml @@ -6,4 +6,4 @@ metadata: spec: type: service lifecycle: production - owner: tools@example.com + owner: guest diff --git a/packages/catalog-model/examples/shuffle-api-component.yaml b/packages/catalog-model/examples/shuffle-api-component.yaml index ed0517835c..275c48d6ba 100644 --- a/packages/catalog-model/examples/shuffle-api-component.yaml +++ b/packages/catalog-model/examples/shuffle-api-component.yaml @@ -6,4 +6,4 @@ metadata: spec: type: service lifecycle: production - owner: tools@example.com + owner: guest From 50c5489422420ee57d663783ed6e81e68f43ac1f Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Wed, 24 Jun 2020 12:00:24 +0200 Subject: [PATCH 23/24] fix(techdocs-core): formatting --- .../container/techdocs-core/src/core.py | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/plugins/techdocs/mkdocs/container/techdocs-core/src/core.py b/plugins/techdocs/mkdocs/container/techdocs-core/src/core.py index 7fc35a1529..37d271d261 100644 --- a/plugins/techdocs/mkdocs/container/techdocs-core/src/core.py +++ b/plugins/techdocs/mkdocs/container/techdocs-core/src/core.py @@ -21,22 +21,23 @@ from mkdocs.contrib.search import SearchPlugin from mkdocs_monorepo_plugin.plugin import MonorepoPlugin + class TechDocsCore(BasePlugin): def on_config(self, config): - # Theme - config['theme'] = Theme(name="material") + # Theme + config["theme"] = Theme(name="material") - # Plugins - del config['plugins']['techdocs-core'] - - search_plugin = SearchPlugin() - search_plugin.load_config({}) + # Plugins + del config["plugins"]["techdocs-core"] - monorepo_plugin = MonorepoPlugin() - monorepo_plugin.load_config({}) - - config['plugins']['search'] = search_plugin - config['plugins']['monorepo'] = monorepo_plugin + search_plugin = SearchPlugin() + search_plugin.load_config({}) + + monorepo_plugin = MonorepoPlugin() + monorepo_plugin.load_config({}) + + config["plugins"]["search"] = search_plugin + config["plugins"]["monorepo"] = monorepo_plugin search_plugin = SearchPlugin() search_plugin.load_config({}) From 72fed2555df5eb42eafae08251c972ead574cea8 Mon Sep 17 00:00:00 2001 From: Emma Indal Date: Wed, 24 Jun 2020 12:02:09 +0200 Subject: [PATCH 24/24] docs(sub-docs): typo --- plugins/techdocs/mkdocs/mock-docs/sub-docs/docs/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/techdocs/mkdocs/mock-docs/sub-docs/docs/index.md b/plugins/techdocs/mkdocs/mock-docs/sub-docs/docs/index.md index 9cb6199605..65c6644ef8 100644 --- a/plugins/techdocs/mkdocs/mock-docs/sub-docs/docs/index.md +++ b/plugins/techdocs/mkdocs/mock-docs/sub-docs/docs/index.md @@ -1 +1 @@ -### This is a md file in another docs folder using the [MkDocs Monorepo Plugin](https://github.com/spotify/mkdocs-monorepo-plugin) +### This is an md file in another docs folder using the [MkDocs Monorepo Plugin](https://github.com/spotify/mkdocs-monorepo-plugin)