From 95f86e74825f16699d244dbdbfa4a80be5f77bdb Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Mon, 25 May 2020 16:05:35 +0200 Subject: [PATCH 01/21] Implemented refresh endpoint for google provider --- plugins/auth-backend/package.json | 14 ++++-- .../src/providers/google/provider.ts | 44 ++++++++++++++++++- plugins/auth-backend/src/providers/index.ts | 2 +- plugins/auth-backend/src/service/router.ts | 7 +++ yarn.lock | 40 ++++++++++++++--- 5 files changed, 94 insertions(+), 13 deletions(-) diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index 6b85bdfe1c..fe82e544e7 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -25,10 +25,16 @@ "morgan": "^1.10.0", "winston": "^3.2.1", "yn": "^4.0.0", - "passport": "0.4.1", - "passport-google-oauth20": "2.0.0", - "@types/passport": "1.0.3", - "@types/passport-google-oauth20": "2.0.3" + "passport": "^0.4.1", + "passport-google-oauth20": "^2.0.0", + "passport-oauth2-refresh": "^2.0.0", + "passport-oauth2": "^1.5.0", + "cookie-parser": "^1.4.5", + "@types/passport-oauth2-refresh": "^1.1.1", + "@types/passport": "^1.0.3", + "@types/passport-google-oauth20": "^2.0.3", + "@types/cookie-parser": "^1.4.2", + "@types/passport-oauth2": "^1.4.9" }, "devDependencies": { "@backstage/cli": "^0.1.1-alpha.6", diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 0ef7e19d3e..03f6424296 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -15,8 +15,9 @@ */ import passport from 'passport'; -import express from 'express'; +import express, { CookieOptions } from 'express'; import { Strategy as GoogleStrategy } from 'passport-google-oauth20'; +import refresh from 'passport-oauth2-refresh'; import { AuthProvider, AuthProviderRouteHandlers, @@ -25,6 +26,7 @@ import { import { postMessageResponse } from './../utils'; import { InputError } from '@backstage/backend-common'; +const THOUSAND_DAYS_MS = 1000 * 24 * 60 * 60 * 1000; export class GoogleAuthProvider implements AuthProvider, AuthProviderRouteHandlers { private readonly providerConfig: AuthProviderConfig; @@ -54,6 +56,18 @@ export class GoogleAuthProvider next: express.NextFunction, ) { return passport.authenticate('google', (_, user) => { + const { refreshToken } = user; + delete user.refreshToken; + + const options: CookieOptions = { + maxAge: THOUSAND_DAYS_MS, + secure: false, + sameSite: 'none', + path: 'localhost:3000/auth/google', + httpOnly: true, + }; + + res.cookie('grtoken', refreshToken, options); postMessageResponse(res, { type: 'auth-result', payload: user, @@ -62,7 +76,33 @@ export class GoogleAuthProvider } async logout(_req: express.Request, res: express.Response) { - res.send('logout!'); + return res.send('logout!'); + } + + async refresh(req: express.Request, res: express.Response) { + const refreshToken = req.cookies['grtoken']; + const scope = req.query.scope?.toString() ?? ''; + const params = scope ? { scope } : {}; + + if (!refreshToken) { + return res.status(401).send('Missing session cookie'); + } + + refresh.requestNewAccessToken( + 'google', + refreshToken, + params, + (err, accessToken) => { + if (err || !accessToken) { + return res.status(401).send('Failed to refresh access token'); + } + + return res.send({ + accessToken, + expiresInSeconds: 36000, + }); + }, + ); } strategy(): passport.Strategy { diff --git a/plugins/auth-backend/src/providers/index.ts b/plugins/auth-backend/src/providers/index.ts index 6658299033..68fe61c7a6 100644 --- a/plugins/auth-backend/src/providers/index.ts +++ b/plugins/auth-backend/src/providers/index.ts @@ -24,7 +24,7 @@ export const defaultRouter = (provider: AuthProviderRouteHandlers) => { router.get('/handler/frame', provider.frameHandler); router.get('/logout', provider.logout); if (provider.refresh) { - router.get('/refreshToken', provider.refresh); + router.get('/refresh', provider.refresh); } return router; }; diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index cdde95d9d0..a2e3b3e1db 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -17,6 +17,9 @@ import express from 'express'; import Router from 'express-promise-router'; import passport from 'passport'; +import cookieParser from 'cookie-parser'; +import refresh from 'passport-oauth2-refresh'; +import OAuth2Strategy from 'passport-oauth2'; import { Logger } from 'winston'; import { providers } from './../providers/config'; import { makeProvider } from '../providers'; @@ -39,6 +42,9 @@ export async function createRouter( ); logger.info(`Configuring provider: ${providerId}`); passport.use(strategy); + if (strategy instanceof OAuth2Strategy) { + refresh.use(strategy); + } providerRouters[providerId] = providerRouter; } @@ -52,6 +58,7 @@ export async function createRouter( router.use(passport.initialize()); router.use(passport.session()); + router.use(cookieParser()); for (const providerId in providerRouters) { if (providerRouters.hasOwnProperty(providerId)) { diff --git a/yarn.lock b/yarn.lock index 074279db2b..0d49abe417 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3861,6 +3861,13 @@ dependencies: "@types/node" "*" +"@types/cookie-parser@^1.4.2": + version "1.4.2" + resolved "https://registry.npmjs.org/@types/cookie-parser/-/cookie-parser-1.4.2.tgz#e4d5c5ffda82b80672a88a4281aaceefb1bd9df5" + integrity sha512-uwcY8m6SDQqciHsqcKDGbo10GdasYsPCYkH3hVegj9qAah6pX5HivOnOuI3WYmyQMnOATV39zv/Ybs0bC/6iVg== + dependencies: + "@types/express" "*" + "@types/cookiejar@*": version "2.1.1" resolved "https://registry.npmjs.org/@types/cookiejar/-/cookiejar-2.1.1.tgz#90b68446364baf9efd8e8349bb36bd3852b75b80" @@ -4181,7 +4188,7 @@ resolved "https://registry.npmjs.org/@types/parse-json/-/parse-json-4.0.0.tgz#2f8bb441434d163b35fb8ffdccd7138927ffb8c0" integrity sha512-//oorEZjL6sbPcKUaCdIGlIUeH26mgzimjBB77G6XRgnDl/L5wOnpyBGRe/Mmf5CVW3PwEBE1NjiMZ/ssFh4wA== -"@types/passport-google-oauth20@2.0.3": +"@types/passport-google-oauth20@^2.0.3": version "2.0.3" resolved "https://registry.npmjs.org/@types/passport-google-oauth20/-/passport-google-oauth20-2.0.3.tgz#f554ff6d39f395acff3f1d762e54462194dac8da" integrity sha512-6EUEGzEg4acwowvgR/yVZIj8S2Kkwc6JmlY2/wnM1wJHNz20o7s1TIGrxnah8ymLgJasYDpy95P3TMMqlmetPw== @@ -4190,7 +4197,15 @@ "@types/passport" "*" "@types/passport-oauth2" "*" -"@types/passport-oauth2@*": +"@types/passport-oauth2-refresh@^1.1.1": + version "1.1.1" + resolved "https://registry.npmjs.org/@types/passport-oauth2-refresh/-/passport-oauth2-refresh-1.1.1.tgz#cbe466d4fcac36182fd75bf55279c0b1e953c382" + integrity sha512-Tw0JvfDPv9asgFPACd9oOGCaD/0/Uyi+QF7fmrJC74cJKC6I8N8wwhJJHyfd1N2E/qaLgTh431lhOa9jicpNdg== + dependencies: + "@types/oauth" "*" + "@types/passport-oauth2" "*" + +"@types/passport-oauth2@*", "@types/passport-oauth2@^1.4.9": version "1.4.9" resolved "https://registry.npmjs.org/@types/passport-oauth2/-/passport-oauth2-1.4.9.tgz#134007c4b505a82548c9cb19094c5baeb2205c92" integrity sha512-QP0q+NVQOaIu2r0e10QWkiUA0Ya5mOBHRJN0UrI+LolMLOP1/VN4EVIpJ3xVwFo+xqNFRoFvFwJhBvKnk7kpUA== @@ -4199,7 +4214,7 @@ "@types/oauth" "*" "@types/passport" "*" -"@types/passport@*", "@types/passport@1.0.3": +"@types/passport@*", "@types/passport@^1.0.3": version "1.0.3" resolved "https://registry.npmjs.org/@types/passport/-/passport-1.0.3.tgz#e459ed6c262bf0686684d1b05901be0d0b192a9c" integrity sha512-nyztuxtDPQv9utCzU0qW7Gl8BY2Dn8BKlYAFFyxKipFxjaVd96celbkLCV/tRqqBUZ+JB8If3UfgV8347DTo3Q== @@ -7241,6 +7256,14 @@ convert-source-map@^1.4.0, convert-source-map@^1.5.0, convert-source-map@^1.6.0, dependencies: safe-buffer "~5.1.1" +cookie-parser@^1.4.5: + version "1.4.5" + resolved "https://registry.npmjs.org/cookie-parser/-/cookie-parser-1.4.5.tgz#3e572d4b7c0c80f9c61daf604e4336831b5d1d49" + integrity sha512-f13bPUj/gG/5mDr+xLmSxxDsB9DQiTIfhJS/sqjrmfAWiAN+x2O4i/XguTL9yDZ+/IFDanJ+5x7hC4CXT9Tdzw== + dependencies: + cookie "0.4.0" + cookie-signature "1.0.6" + cookie-signature@1.0.6: version "1.0.6" resolved "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.0.6.tgz#e303a882b342cc3ee8ca513a79999734dab3ae2c" @@ -15987,14 +16010,19 @@ pascalcase@^0.1.1: resolved "https://registry.npmjs.org/pascalcase/-/pascalcase-0.1.1.tgz#b363e55e8006ca6fe21784d2db22bd15d7917f14" integrity sha1-s2PlXoAGym/iF4TS2yK9FdeRfxQ= -passport-google-oauth20@2.0.0: +passport-google-oauth20@^2.0.0: version "2.0.0" resolved "https://registry.npmjs.org/passport-google-oauth20/-/passport-google-oauth20-2.0.0.tgz#0d241b2d21ebd3dc7f2b60669ec4d587e3a674ef" integrity sha512-KSk6IJ15RoxuGq7D1UKK/8qKhNfzbLeLrG3gkLZ7p4A6DBCcv7xpyQwuXtWdpyR0+E0mwkpjY1VfPOhxQrKzdQ== dependencies: passport-oauth2 "1.x.x" -passport-oauth2@1.x.x: +passport-oauth2-refresh@^2.0.0: + version "2.0.0" + resolved "https://registry.npmjs.org/passport-oauth2-refresh/-/passport-oauth2-refresh-2.0.0.tgz#7b19c77ff3cc000819c69f6ad9e318450f57b85e" + integrity sha512-yXvCB6nem/O+WThhiyI3TlPXpzSGY+9+hy9OTx9QF8e9GInplyRHxHaaOhFylKvnof9UmWHAufQFZk8cO1Fb2g== + +passport-oauth2@1.x.x, passport-oauth2@^1.5.0: version "1.5.0" resolved "https://registry.npmjs.org/passport-oauth2/-/passport-oauth2-1.5.0.tgz#64babbb54ac46a4dcab35e7f266ed5294e3c4108" integrity sha512-kqBt6vR/5VlCK8iCx1/KpY42kQ+NEHZwsSyt4Y6STiNjU+wWICG1i8ucc1FapXDGO15C5O5VZz7+7vRzrDPXXQ== @@ -16010,7 +16038,7 @@ passport-strategy@1.x.x: resolved "https://registry.npmjs.org/passport-strategy/-/passport-strategy-1.0.0.tgz#b5539aa8fc225a3d1ad179476ddf236b440f52e4" integrity sha1-tVOaqPwiWj0a0XlHbd8ja0QPUuQ= -passport@0.4.1: +passport@^0.4.1: version "0.4.1" resolved "https://registry.npmjs.org/passport/-/passport-0.4.1.tgz#941446a21cb92fc688d97a0861c38ce9f738f270" integrity sha512-IxXgZZs8d7uFSt3eqNjM9NQ3g3uQCW5avD8mRNoXV99Yig50vjuaez6dQK2qC0kVWPRTujxY0dWgGfT09adjYg== From 6cd55c3b7b0c554bdbde4b06b9b769514eb02e4c Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Mon, 25 May 2020 16:27:56 +0200 Subject: [PATCH 02/21] Always return scope --- .../auth-backend/src/providers/google/provider.ts | 8 +++++--- plugins/auth-backend/src/providers/types.ts | 15 ++++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 03f6424296..d567e363bc 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -92,14 +92,15 @@ export class GoogleAuthProvider 'google', refreshToken, params, - (err, accessToken) => { + (err, accessToken, _, params) => { if (err || !accessToken) { return res.status(401).send('Failed to refresh access token'); } - return res.send({ accessToken, - expiresInSeconds: 36000, + idToken: params.id_token, + expiresInSeconds: params.expires_in, + scope: params.scope, }); }, ); @@ -121,6 +122,7 @@ export class GoogleAuthProvider idToken: params.id_token, accessToken, refreshToken, + scope: params.scope, }); }, ); diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 4350f36200..36941c6850 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -58,16 +58,25 @@ export type AuthProviderFactory = { new (providerConfig: any): AuthProvider & AuthProviderRouteHandlers; }; -export type AuthInfo = { - profile: passport.Profile; +export type AuthInfoBase = { accessToken: string; + idToken?: string; expiresInSeconds?: number; + scope: string; +}; + +export type AuthInfoWithProfile = AuthInfoBase & { + profile: passport.Profile; +}; + +export type AuthInfoPrivate = AuthInfoWithProfile & { + refreshToken: string; }; export type AuthResponse = | { type: 'auth-result'; - payload: AuthInfo; + payload: AuthInfoBase | AuthInfoWithProfile; } | { type: 'auth-result'; From a739f2da583f5f6d88f58a6b954940699aff45df Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Mon, 25 May 2020 16:51:50 +0200 Subject: [PATCH 03/21] Fixes in response format in frontend and backend apis --- packages/backend/src/index.ts | 6 +++++- .../src/api/apis/implementations/auth/google/GoogleAuth.ts | 4 ++-- .../lib/AuthConnector/DefaultAuthConnector.ts | 2 +- plugins/auth-backend/src/providers/google/provider.ts | 4 +++- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 7af6e631a8..5b1fc54330 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -58,9 +58,13 @@ function createEnv(plugin: string): PluginEnvironment { async function main() { const app = express(); + const corsOptions: cors.CorsOptions = { + origin: 'http://localhost:3000', + credentials: true, + }; app.use(helmet()); - app.use(cors()); + app.use(cors(corsOptions)); app.use(compression()); app.use(express.json()); app.use(requestLoggingHandler()); diff --git a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts index d1d5da1a02..9f3bcec762 100644 --- a/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts +++ b/packages/core/src/api/apis/implementations/auth/google/GoogleAuth.ts @@ -40,7 +40,7 @@ type CreateOptions = { export type GoogleAuthResponse = { accessToken: string; idToken: string; - scopes: string; + scope: string; expiresInSeconds: number; }; @@ -70,7 +70,7 @@ class GoogleAuth implements OAuthApi, OpenIdConnectApi { return { idToken: res.idToken, accessToken: res.accessToken, - scopes: GoogleAuth.normalizeScopes(res.scopes), + scopes: GoogleAuth.normalizeScopes(res.scope), expiresAt: new Date(Date.now() + res.expiresInSeconds * 1000), }; }, diff --git a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts index 161015b1a8..ca8a9cd2ae 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.ts @@ -99,7 +99,7 @@ export class DefaultAuthConnector } async refreshSession(): Promise { - const res = await fetch(this.buildUrl('/token', { optional: true }), { + const res = await fetch(this.buildUrl('/refresh', { optional: true }), { headers: { 'x-requested-with': 'XMLHttpRequest', }, diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index d567e363bc..2bfb606495 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -63,7 +63,8 @@ export class GoogleAuthProvider maxAge: THOUSAND_DAYS_MS, secure: false, sameSite: 'none', - path: 'localhost:3000/auth/google', + domain: 'localhost', + path: '/auth/google', httpOnly: true, }; @@ -123,6 +124,7 @@ export class GoogleAuthProvider accessToken, refreshToken, scope: params.scope, + expiresInSeconds: params.expires_in, }); }, ); From 53c1fe0601779139799c71f8ee671b841c26ff0e Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Mon, 25 May 2020 17:46:42 +0200 Subject: [PATCH 04/21] fix lint issues --- .../auth-backend/src/providers/google/provider.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 2bfb606495..4d6183da89 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -81,23 +81,24 @@ export class GoogleAuthProvider } async refresh(req: express.Request, res: express.Response) { - const refreshToken = req.cookies['grtoken']; - const scope = req.query.scope?.toString() ?? ''; - const params = scope ? { scope } : {}; + const refreshToken = req.cookies.grtoken; if (!refreshToken) { return res.status(401).send('Missing session cookie'); } - refresh.requestNewAccessToken( + const scope = req.query.scope?.toString() ?? ''; + const refreshTokenRequestParams = scope ? { scope } : {}; + + return refresh.requestNewAccessToken( 'google', refreshToken, - params, + refreshTokenRequestParams, (err, accessToken, _, params) => { if (err || !accessToken) { return res.status(401).send('Failed to refresh access token'); } - return res.send({ + res.send({ accessToken, idToken: params.id_token, expiresInSeconds: params.expires_in, From a8184250fe3e78c60dcfda55a3eb07da16a0313c Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Mon, 25 May 2020 17:51:27 +0200 Subject: [PATCH 05/21] fix lint issues --- plugins/auth-backend/src/providers/google/provider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 4d6183da89..88b7345213 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -98,7 +98,7 @@ export class GoogleAuthProvider if (err || !accessToken) { return res.status(401).send('Failed to refresh access token'); } - res.send({ + return res.send({ accessToken, idToken: params.id_token, expiresInSeconds: params.expires_in, From 1da6ac369e8853822fe341672b04da0a919a28d0 Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Mon, 25 May 2020 23:44:20 +0200 Subject: [PATCH 06/21] fix tests and add more tests --- .../src/providers/google/provider.test.ts | 206 +++++++++++++++++- .../src/providers/google/provider.ts | 11 +- 2 files changed, 207 insertions(+), 10 deletions(-) diff --git a/plugins/auth-backend/src/providers/google/provider.test.ts b/plugins/auth-backend/src/providers/google/provider.test.ts index 845c46a4da..1726fbf33a 100644 --- a/plugins/auth-backend/src/providers/google/provider.test.ts +++ b/plugins/auth-backend/src/providers/google/provider.test.ts @@ -14,10 +14,11 @@ * limitations under the License. */ -import { GoogleAuthProvider } from './provider'; +import { GoogleAuthProvider, THOUSAND_DAYS_MS } from './provider'; import passport from 'passport'; import express from 'express'; import * as utils from './../utils'; +import refresh from 'passport-oauth2-refresh'; const googleAuthProviderConfig = { provider: 'google', @@ -51,7 +52,7 @@ describe('GoogleAuthProvider', () => { }); describe('start authentication handler', () => { - const mockResponse: any = ({} as unknown) as express.Response; + const mockResponse = ({} as unknown) as express.Response; const mockNext: express.NextFunction = jest.fn(); it('should initiate authenticate request with provided scopes', () => { @@ -116,28 +117,64 @@ describe('GoogleAuthProvider', () => { describe('redirect frame handler', () => { const mockRequest = ({} as unknown) as express.Request; const mockResponse: any = ({ - send: jest.fn(), + status: jest.fn().mockReturnThis(), + send: jest.fn().mockReturnThis(), + cookie: jest.fn().mockReturnThis(), } as unknown) as express.Response; const mockNext: express.NextFunction = jest.fn(); - it('should call authenticate and post a response ', () => { + it('should call authenticate and post a response', () => { const spyPostMessage = jest .spyOn(utils, 'postMessageResponse') .mockImplementation(() => jest.fn()); const spyPassport = jest .spyOn(passport, 'authenticate') - .mockImplementation(() => jest.fn()); + .mockImplementation((_x, callbackFunc) => { + const cb = callbackFunc as Function; + cb(null, { refreshToken: 'REFRESH_TOKEN' }); + return jest.fn(); + }); const googleAuthProvider = new GoogleAuthProvider( googleAuthProviderConfig, ); googleAuthProvider.frameHandler(mockRequest, mockResponse, mockNext); - const callbackFunc = spyPassport.mock.calls[0][1] as Function; - callbackFunc(); expect(spyPassport).toBeCalledTimes(1); expect(spyPostMessage).toBeCalledTimes(1); + expect(mockResponse.cookie).toBeCalledTimes(1); + expect(mockResponse.cookie).toBeCalledWith( + 'grtoken', + 'REFRESH_TOKEN', + expect.objectContaining({ + path: '/auth/google', + sameSite: 'none', + httpOnly: true, + maxAge: THOUSAND_DAYS_MS, + }), + ); + }); + + it('should respond with a 401 if no refresh token returned', () => { + const spyPassport = jest + .spyOn(passport, 'authenticate') + .mockImplementation((_x, callbackFunc) => { + const cb = callbackFunc as Function; + cb(null, {}); + return jest.fn(); + }); + + const googleAuthProvider = new GoogleAuthProvider( + googleAuthProviderConfig, + ); + + googleAuthProvider.frameHandler(mockRequest, mockResponse, mockNext); + expect(spyPassport).toBeCalledTimes(1); + expect(mockResponse.send).toBeCalledTimes(1); + expect(mockResponse.send).toBeCalledWith('Failed to fetch refresh token'); + expect(mockResponse.status).toBeCalledTimes(1); + expect(mockResponse.status).toBeCalledWith(401); }); }); @@ -159,4 +196,159 @@ describe('GoogleAuthProvider', () => { }).toThrow(); }); }); + + describe('refresh token handler', () => { + const mockResponse = ({ + status: jest.fn().mockReturnThis(), + send: jest.fn().mockReturnThis(), + } as unknown) as express.Response; + + describe('no refresh token cookie', () => { + it('should respond with a 401', () => { + const mockRequest = ({ + cookies: jest.fn(), + } as unknown) as express.Request; + + const googleAuthProvider = new GoogleAuthProvider( + googleAuthProviderConfig, + ); + + googleAuthProvider.refresh(mockRequest, mockResponse); + expect(mockResponse.send).toBeCalledTimes(1); + expect(mockResponse.send).toBeCalledWith('Missing session cookie'); + + expect(mockResponse.status).toBeCalledTimes(1); + expect(mockResponse.status).toBeCalledWith(401); + }); + }); + + describe('refresh token cookie, no scope', () => { + const mockRequest = ({ + cookies: { grtoken: 'REFRESH_TOKEN' }, + query: {}, + } as unknown) as express.Request; + + const googleAuthProvider = new GoogleAuthProvider( + googleAuthProviderConfig, + ); + + it('should request for a new access token and fail if no access token returned', () => { + const spyRefresh = jest + .spyOn(refresh, 'requestNewAccessToken') + .mockImplementation((_x, _y, _z, callbackFunc) => { + const cb = callbackFunc as Function; + cb(undefined, undefined, undefined, {}); + }); + + googleAuthProvider.refresh(mockRequest, mockResponse); + expect(spyRefresh).toBeCalledTimes(1); + expect(spyRefresh).toBeCalledWith( + 'google', + 'REFRESH_TOKEN', + {}, + expect.any(Function), + ); + expect(mockResponse.status).toBeCalledTimes(1); + expect(mockResponse.status).toBeCalledWith(401); + expect(mockResponse.send).toBeCalledTimes(1); + expect(mockResponse.send).toBeCalledWith( + 'Failed to refresh access token', + ); + }); + + it('should request for a new access token and return 401 if any error', () => { + const spyRefresh = jest + .spyOn(refresh, 'requestNewAccessToken') + .mockImplementation((_x, _y, _z, callbackFunc) => { + const cb = callbackFunc as Function; + cb({ error: 'ERROR' }, undefined, undefined, {}); + }); + + googleAuthProvider.refresh(mockRequest, mockResponse); + expect(spyRefresh).toBeCalledTimes(1); + expect(spyRefresh).toBeCalledWith( + 'google', + 'REFRESH_TOKEN', + {}, + expect.any(Function), + ); + expect(mockResponse.status).toBeCalledTimes(1); + expect(mockResponse.status).toBeCalledWith(401); + expect(mockResponse.send).toBeCalledTimes(1); + expect(mockResponse.send).toBeCalledWith( + 'Failed to refresh access token', + ); + }); + + it('should fetch and return a new access token', () => { + const spyRefresh = jest + .spyOn(refresh, 'requestNewAccessToken') + .mockImplementation((_x, _y, _z, callbackFunc) => { + const cb = callbackFunc as Function; + cb(undefined, 'ACCESS_TOKEN', undefined, { + expires_in: 'EXPIRES_IN', + id_token: 'ID_TOKEN', + }); + }); + + googleAuthProvider.refresh(mockRequest, mockResponse); + expect(spyRefresh).toBeCalledTimes(1); + expect(spyRefresh).toBeCalledWith( + 'google', + 'REFRESH_TOKEN', + {}, + expect.any(Function), + ); + expect(mockResponse.send).toBeCalledTimes(1); + expect(mockResponse.send).toBeCalledWith({ + accessToken: 'ACCESS_TOKEN', + idToken: 'ID_TOKEN', + expiresInSeconds: 'EXPIRES_IN', + scope: undefined, + }); + }); + }); + + describe('refresh token cookie and scope', () => { + const mockRequest = ({ + cookies: { grtoken: 'REFRESH_TOKEN' }, + query: { + scope: 'a,b', + }, + } as unknown) as express.Request; + + const googleAuthProvider = new GoogleAuthProvider( + googleAuthProviderConfig, + ); + + it('should fetch and return a new access token with scopes', () => { + const spyRefresh = jest + .spyOn(refresh, 'requestNewAccessToken') + .mockImplementation((_x, _y, _z, callbackFunc) => { + const cb = callbackFunc as Function; + cb(undefined, 'ACCESS_TOKEN', undefined, { + expires_in: 'EXPIRES_IN', + id_token: 'ID_TOKEN', + scope: 'a,b', + }); + }); + + googleAuthProvider.refresh(mockRequest, mockResponse); + expect(spyRefresh).toBeCalledTimes(1); + expect(spyRefresh).toBeCalledWith( + 'google', + 'REFRESH_TOKEN', + { scope: 'a,b' }, + expect.any(Function), + ); + expect(mockResponse.send).toBeCalledTimes(1); + expect(mockResponse.send).toBeCalledWith({ + accessToken: 'ACCESS_TOKEN', + idToken: 'ID_TOKEN', + expiresInSeconds: 'EXPIRES_IN', + scope: 'a,b', + }); + }); + }); + }); }); diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 88b7345213..51845f638b 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -26,7 +26,7 @@ import { import { postMessageResponse } from './../utils'; import { InputError } from '@backstage/backend-common'; -const THOUSAND_DAYS_MS = 1000 * 24 * 60 * 60 * 1000; +export const THOUSAND_DAYS_MS = 1000 * 24 * 60 * 60 * 1000; export class GoogleAuthProvider implements AuthProvider, AuthProviderRouteHandlers { private readonly providerConfig: AuthProviderConfig; @@ -57,6 +57,11 @@ export class GoogleAuthProvider ) { return passport.authenticate('google', (_, user) => { const { refreshToken } = user; + + if (!refreshToken) { + return res.status(401).send('Failed to fetch refresh token'); + } + delete user.refreshToken; const options: CookieOptions = { @@ -69,7 +74,7 @@ export class GoogleAuthProvider }; res.cookie('grtoken', refreshToken, options); - postMessageResponse(res, { + return postMessageResponse(res, { type: 'auth-result', payload: user, }); @@ -94,7 +99,7 @@ export class GoogleAuthProvider 'google', refreshToken, refreshTokenRequestParams, - (err, accessToken, _, params) => { + (err, accessToken, _refreshToken, params) => { if (err || !accessToken) { return res.status(401).send('Failed to refresh access token'); } From 8c71c29b60111dd80849ad712c1c26692321c1a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 26 May 2020 11:08:17 +0200 Subject: [PATCH 07/21] Add basic test harness for the router --- packages/backend/package.json | 3 +- plugins/catalog-backend/package.json | 2 + .../src/service/router.test.ts | 76 +++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 plugins/catalog-backend/src/service/router.test.ts diff --git a/packages/backend/package.json b/packages/backend/package.json index 2c7b48f65f..9aedafecbb 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -40,8 +40,7 @@ "@types/helmet": "^0.0.47", "jest": "^26.0.1", "tsc-watch": "^4.2.3", - "typescript": "^3.9.2", - "winston": "^3.2.1" + "typescript": "^3.9.2" }, "nodemonConfig": { "watch": "./dist" diff --git a/plugins/catalog-backend/package.json b/plugins/catalog-backend/package.json index 9c0b066f4f..ce2a985efa 100644 --- a/plugins/catalog-backend/package.json +++ b/plugins/catalog-backend/package.json @@ -17,6 +17,7 @@ "dependencies": { "@backstage/backend-common": "^0.1.1-alpha.6", "@types/node-fetch": "^2.5.7", + "@types/supertest": "^2.0.8", "compression": "^1.7.4", "cors": "^2.8.5", "express": "^4.17.1", @@ -40,6 +41,7 @@ "@types/uuid": "^7.0.3", "@types/yup": "^0.28.2", "jest-fetch-mock": "^3.0.3", + "supertest": "^4.0.2", "tsc-watch": "^4.2.3" }, "files": [ diff --git a/plugins/catalog-backend/src/service/router.test.ts b/plugins/catalog-backend/src/service/router.test.ts new file mode 100644 index 0000000000..c4e21c9b0e --- /dev/null +++ b/plugins/catalog-backend/src/service/router.test.ts @@ -0,0 +1,76 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import express from 'express'; +import request from 'supertest'; +import { createRouter } from './router'; +import { EntitiesCatalog, LocationsCatalog, Location } from '../catalog'; +import { getVoidLogger } from '@backstage/backend-common'; +import { DescriptorEnvelope } from '../ingestion'; + +class MockEntitiesCatalog implements EntitiesCatalog { + entities = jest.fn(); + entity = jest.fn(); +} + +class MockLocationsCatalog implements LocationsCatalog { + addLocation = jest.fn(); + removeLocation = jest.fn(); + locations = jest.fn(); + location = jest.fn(); +} + +describe('createRouter', () => { + describe('entities', () => { + it('happy path: lists entities', async () => { + const entities: DescriptorEnvelope[] = [{ apiVersion: 'a', kind: 'b' }]; + + const catalog = new MockEntitiesCatalog(); + catalog.entities.mockResolvedValueOnce(entities); + + const router = await createRouter({ + entitiesCatalog: catalog, + logger: getVoidLogger(), + }); + + const app = express().use(router); + const response = await request(app).get('/entities'); + + expect(response.status).toEqual(200); + expect(JSON.parse(response.text)).toEqual(entities); + }); + }); + + describe('locations', () => { + it('happy path: lists locations', async () => { + const locations: Location[] = [{ id: 'a', type: 'b', target: 'c' }]; + + const catalog = new MockLocationsCatalog(); + catalog.locations.mockResolvedValueOnce(locations); + + const router = await createRouter({ + locationsCatalog: catalog, + logger: getVoidLogger(), + }); + + const app = express().use(router); + const response = await request(app).get('/locations'); + + expect(response.status).toEqual(200); + expect(JSON.parse(response.text)).toEqual(locations); + }); + }); +}); From 642de90255497124768504a8a29fb8a6b5d4a199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 26 May 2020 14:48:37 +0200 Subject: [PATCH 08/21] Filtering --- .../src/catalog/DatabaseEntitiesCatalog.ts | 6 +- plugins/catalog-backend/src/catalog/types.ts | 10 +- .../src/database/Database.test.ts | 104 ++++++++++++++++++ .../catalog-backend/src/database/Database.ts | 27 ++++- .../src/service/router.test.ts | 52 ++++++++- plugins/catalog-backend/src/service/router.ts | 27 ++++- 6 files changed, 209 insertions(+), 17 deletions(-) diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts index 697da0ea35..fb19f04220 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts @@ -17,14 +17,14 @@ import { NotFoundError } from '@backstage/backend-common'; import { Database } from '../database'; import { DescriptorEnvelope } from '../ingestion/types'; -import { EntitiesCatalog } from './types'; +import { EntitiesCatalog, EntityFilters } from './types'; export class DatabaseEntitiesCatalog implements EntitiesCatalog { constructor(private readonly database: Database) {} - async entities(): Promise { + async entities(filters?: EntityFilters): Promise { const items = await this.database.transaction(tx => - this.database.entities(tx), + this.database.entities(tx, filters), ); return items.map(i => i.entity); } diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index 92c059d25b..e5beb801d0 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -18,11 +18,17 @@ import * as yup from 'yup'; import { DescriptorEnvelope } from '../ingestion'; // -// Items +// Entities // +export type EntityFilter = { + key: string; + values: (string | null)[]; +}; +export type EntityFilters = EntityFilter[]; + export type EntitiesCatalog = { - entities(): Promise; + entities(filters?: EntityFilters): Promise; entity( kind: string, name: string, diff --git a/plugins/catalog-backend/src/database/Database.test.ts b/plugins/catalog-backend/src/database/Database.test.ts index 0eb64916e5..7f02b45eee 100644 --- a/plugins/catalog-backend/src/database/Database.test.ts +++ b/plugins/catalog-backend/src/database/Database.test.ts @@ -28,6 +28,7 @@ import { DbEntityResponse, DbLocationsRow, } from './types'; +import { DescriptorEnvelope } from '../ingestion'; describe('Database', () => { let database: Knex; @@ -242,4 +243,107 @@ describe('Database', () => { ).rejects.toThrow(ConflictError); }); }); + + describe('entities', () => { + it('can get all entities with empty filters list', async () => { + const catalog = new Database(database, getVoidLogger()); + const e1: DescriptorEnvelope = { apiVersion: 'a', kind: 'b' }; + const e2: DescriptorEnvelope = { + apiVersion: 'a', + kind: 'b', + spec: { c: null }, + }; + await catalog.transaction(async tx => { + await catalog.addEntity(tx, { entity: e1 }); + await catalog.addEntity(tx, { entity: e2 }); + }); + await expect( + catalog.transaction(async tx => catalog.entities(tx, [])), + ).resolves.toEqual([ + { locationId: undefined, entity: expect.objectContaining(e1) }, + { locationId: undefined, entity: expect.objectContaining(e2) }, + ]); + }); + it('can get all specific entities for matching filters (naive case)', async () => { + const catalog = new Database(database, getVoidLogger()); + const entities: DescriptorEnvelope[] = [ + { apiVersion: 'a', kind: 'b' }, + { + apiVersion: 'a', + kind: 'b', + spec: { c: 'some' }, + }, + { + apiVersion: 'a', + kind: 'b', + spec: { c: null }, + }, + ]; + + await catalog.transaction(async tx => { + for (const entity of entities) { + await catalog.addEntity(tx, { entity }); + } + }); + + await expect( + catalog.transaction(async tx => + catalog.entities(tx, [ + { key: 'kind', values: ['b'] }, + { key: 'spec.c', values: ['some'] }, + ]), + ), + ).resolves.toEqual([ + { locationId: undefined, entity: expect.objectContaining(entities[1]) }, + ]); + }); + + it('can get all specific entities for matching filters with nulls (both missing and literal null value)', async () => { + const catalog = new Database(database, getVoidLogger()); + const entities: DescriptorEnvelope[] = [ + { apiVersion: 'a', kind: 'b' }, + { + apiVersion: 'a', + kind: 'b', + spec: { c: 'some' }, + }, + { + apiVersion: 'a', + kind: 'b', + spec: { c: null }, + }, + ]; + + await catalog.transaction(async tx => { + for (const entity of entities) { + await catalog.addEntity(tx, { entity }); + } + }); + + const rows = await catalog.transaction(async tx => + catalog.entities(tx, [ + { key: 'kind', values: ['b'] }, + { key: 'spec.c', values: [null, 'some'] }, + ]), + ); + + expect(rows.length).toEqual(3); + expect(rows).toEqual( + expect.arrayContaining([ + { + locationId: undefined, + entity: expect.objectContaining(entities[0]), + }, + { + locationId: undefined, + entity: expect.objectContaining(entities[1]), + }, + { + locationId: undefined, + entity: expect.objectContaining(entities[2]), + }, + ]), + ); + }); + }); }); diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/Database.ts index cd4ca731a3..3b616b4202 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/Database.ts @@ -23,6 +23,7 @@ import Knex from 'knex'; import lodash from 'lodash'; import { v4 as uuidv4 } from 'uuid'; import { Logger } from 'winston'; +import { EntityFilters } from '../catalog'; import { DescriptorEnvelope, EntityMeta } from '../ingestion'; import { buildEntitySearch } from './search'; import { @@ -309,10 +310,30 @@ export class Database { return { locationId: request.locationId, entity: newEntity }; } - async entities(tx: Knex.Transaction): Promise { - const rows = await tx('entities') + async entities( + tx: Knex.Transaction, + filters?: EntityFilters, + ): Promise { + let builder = tx('entities'); + for (const [index, filter] of (filters ?? []).entries()) { + builder = builder + .leftOuterJoin(`entities_search as t${index}`, function join() { + this.on('entities.id', '=', `t${index}.entity_id`).onIn( + `t${index}.value`, + filter.values.filter(x => x), + ); + if (filter.values.some(x => !x)) { + this.orOnNull(`t${index}.value`); + } + }) + .where(`t${index}.key`, '=', filter.key); + } + + const rows = await builder .orderBy('namespace', 'name') - .select(); + .select('entities.*') + .groupBy('id'); + return rows.map(row => toEntityResponse(row)); } diff --git a/plugins/catalog-backend/src/service/router.test.ts b/plugins/catalog-backend/src/service/router.test.ts index c4e21c9b0e..1b1f4c5034 100644 --- a/plugins/catalog-backend/src/service/router.test.ts +++ b/plugins/catalog-backend/src/service/router.test.ts @@ -14,12 +14,16 @@ * limitations under the License. */ +import { + errorHandler, + getVoidLogger, + InputError, +} from '@backstage/backend-common'; import express from 'express'; import request from 'supertest'; -import { createRouter } from './router'; -import { EntitiesCatalog, LocationsCatalog, Location } from '../catalog'; -import { getVoidLogger } from '@backstage/backend-common'; +import { EntitiesCatalog, Location, LocationsCatalog } from '../catalog'; import { DescriptorEnvelope } from '../ingestion'; +import { createRouter } from './router'; class MockEntitiesCatalog implements EntitiesCatalog { entities = jest.fn(); @@ -50,7 +54,26 @@ describe('createRouter', () => { const response = await request(app).get('/entities'); expect(response.status).toEqual(200); - expect(JSON.parse(response.text)).toEqual(entities); + expect(response.body).toEqual(entities); + }); + + it('parses single and multiple request parameters and passes them down', async () => { + const catalog = new MockEntitiesCatalog(); + + const router = await createRouter({ + entitiesCatalog: catalog, + logger: getVoidLogger(), + }); + + const app = express().use(router); + const response = await request(app).get('/entities?a=1&a=&a=3&b=4&c='); + + expect(response.status).toEqual(200); + expect(catalog.entities).toHaveBeenCalledWith([ + { key: 'a', values: ['1', null, '3'] }, + { key: 'b', values: ['4'] }, + { key: 'c', values: [null] }, + ]); }); }); @@ -70,7 +93,26 @@ describe('createRouter', () => { const response = await request(app).get('/locations'); expect(response.status).toEqual(200); - expect(JSON.parse(response.text)).toEqual(locations); + expect(response.body).toEqual(locations); + }); + + it('rejects malformed locations', async () => { + const location = ({ + id: 'a', + typez: 'b', + target: 'c', + } as unknown) as Location; + + const catalog = new MockLocationsCatalog(); + const router = await createRouter({ + locationsCatalog: catalog, + logger: getVoidLogger(), + }); + + const app = express().use(router); + const response = await request(app).post('/locations').send(location); + + expect(response.status).toEqual(400); }); }); }); diff --git a/plugins/catalog-backend/src/service/router.ts b/plugins/catalog-backend/src/service/router.ts index af714e4026..81533adfc5 100644 --- a/plugins/catalog-backend/src/service/router.ts +++ b/plugins/catalog-backend/src/service/router.ts @@ -14,12 +14,14 @@ * limitations under the License. */ +import { errorHandler } from '@backstage/backend-common'; import express from 'express'; import Router from 'express-promise-router'; import { Logger } from 'winston'; import { addLocationSchema, EntitiesCatalog, + EntityFilters, LocationsCatalog, } from '../catalog'; import { validateRequestBody } from './util'; @@ -34,17 +36,33 @@ export async function createRouter( options: RouterOptions, ): Promise { const { entitiesCatalog, locationsCatalog } = options; + const router = Router(); + router.use(express.json()); if (entitiesCatalog) { - // Entities - router.get('/entities', async (_req, res) => { - const entities = await entitiesCatalog.entities(); + router.get('/entities', async (req, res) => { + const filters: EntityFilters = []; + for (const [key, valueOrValues] of Object.entries(req.query)) { + const values = Array.isArray(valueOrValues) + ? valueOrValues + : [valueOrValues]; + if (values.some(v => typeof v !== 'string')) { + res.status(400).send('Complex query parameters are not supported'); + return; + } + filters.push({ + key, + values: values.map(v => v || null) as string[], + }); + } + + const entities = await entitiesCatalog.entities(filters); + res.status(200).send(entities); }); } - // Locations if (locationsCatalog) { router .post('/locations', async (req, res) => { @@ -68,5 +86,6 @@ export async function createRouter( }); } + router.use(errorHandler()); return router; } From bfeaf43c717c6365022c0322719ef1f9ce310c2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 26 May 2020 15:50:31 +0200 Subject: [PATCH 09/21] Individual entity fetches --- .../src/catalog/DatabaseEntitiesCatalog.ts | 31 +++++-- .../src/catalog/DatabaseLocationsCatalog.ts | 2 +- .../src/catalog/StaticEntitiesCatalog.ts | 10 +- plugins/catalog-backend/src/catalog/types.ts | 5 +- .../src/database/Database.test.ts | 3 +- .../catalog-backend/src/database/Database.ts | 6 +- .../src/database/DatabaseManager.test.ts | 2 +- .../src/service/router.test.ts | 92 +++++++++++++++++-- plugins/catalog-backend/src/service/router.ts | 73 +++++++++++---- 9 files changed, 180 insertions(+), 44 deletions(-) diff --git a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts index fb19f04220..9d0d8fcaf7 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseEntitiesCatalog.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { NotFoundError } from '@backstage/backend-common'; import { Database } from '../database'; import { DescriptorEnvelope } from '../ingestion/types'; import { EntitiesCatalog, EntityFilters } from './types'; @@ -29,17 +28,33 @@ export class DatabaseEntitiesCatalog implements EntitiesCatalog { return items.map(i => i.entity); } - async entity( + async entityByUid(uid: string): Promise { + const matches = await this.database.transaction(tx => + this.database.entities(tx, [{ key: 'uid', values: [uid] }]), + ); + + return matches.length ? matches[0].entity : undefined; + } + + async entityByName( kind: string, name: string, namespace: string | undefined, ): Promise { - const item = await this.database.transaction(tx => - this.database.entity(tx, kind, name, namespace), + const matches = await this.database.transaction(tx => + this.database.entities(tx, [ + { key: 'kind', values: [kind] }, + { key: 'name', values: [name] }, + { + key: 'namespace', + values: + !namespace || namespace === 'default' + ? [null, 'default'] + : [namespace], + }, + ]), ); - if (!item) { - throw new NotFoundError('Entity cannot be found'); - } - return item.entity; + + return matches.length ? matches[0].entity : undefined; } } diff --git a/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts index e9f1512074..9effed724c 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseLocationsCatalog.ts @@ -35,7 +35,7 @@ export class DatabaseLocationsCatalog implements LocationsCatalog { } async location(id: string): Promise { - const item = await this.location(id); + const item = await this.database.location(id); return item; } } diff --git a/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts b/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts index 17f7603fb7..371cdf1f76 100644 --- a/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/catalog/StaticEntitiesCatalog.ts @@ -30,7 +30,15 @@ export class StaticEntitiesCatalog implements EntitiesCatalog { return lodash.cloneDeep(this._entities); } - async entity( + async entityByUid(uid: string): Promise { + const item = this._entities.find(e => uid === e.metadata?.uid); + if (!item) { + throw new NotFoundError('Entity cannot be found'); + } + return lodash.cloneDeep(item); + } + + async entityByName( kind: string, name: string, namespace: string | undefined, diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index e5beb801d0..d5627ea86a 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -29,10 +29,11 @@ export type EntityFilters = EntityFilter[]; export type EntitiesCatalog = { entities(filters?: EntityFilters): Promise; - entity( + entityByUid(uid: string): Promise; + entityByName( kind: string, - name: string, namespace: string | undefined, + name: string, ): Promise; }; diff --git a/plugins/catalog-backend/src/database/Database.test.ts b/plugins/catalog-backend/src/database/Database.test.ts index 7f02b45eee..7e9c75d199 100644 --- a/plugins/catalog-backend/src/database/Database.test.ts +++ b/plugins/catalog-backend/src/database/Database.test.ts @@ -21,6 +21,7 @@ import { } from '@backstage/backend-common'; import Knex from 'knex'; import path from 'path'; +import { DescriptorEnvelope } from '../ingestion'; import { Database } from './Database'; import { AddDatabaseLocation, @@ -28,7 +29,6 @@ import { DbEntityResponse, DbLocationsRow, } from './types'; -import { DescriptorEnvelope } from '../ingestion'; describe('Database', () => { let database: Knex; @@ -264,6 +264,7 @@ describe('Database', () => { { locationId: undefined, entity: expect.objectContaining(e2) }, ]); }); + it('can get all specific entities for matching filters (naive case)', async () => { const catalog = new Database(database, getVoidLogger()); const entities: DescriptorEnvelope[] = [ diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/Database.ts index 3b616b4202..a3aba825f4 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/Database.ts @@ -276,7 +276,7 @@ export class Database { ? oldRow.generation : oldRow.generation + 1; const newEntity = lodash.cloneDeep(request.entity); - newEntity.metadata = Object.assign({}, request.entity.metadata, { + newEntity.metadata = Object.assign({}, newEntity.metadata, { uid: oldRow.id, etag: newEtag, generation: newGeneration, @@ -357,9 +357,7 @@ export class Database { async addLocation(location: AddDatabaseLocation): Promise { return await this.database.transaction(async tx => { const existingLocation = await tx('locations') - .where({ - target: location.target, - }) + .where({ target: location.target }) .select(); if (existingLocation?.[0]) { diff --git a/plugins/catalog-backend/src/database/DatabaseManager.test.ts b/plugins/catalog-backend/src/database/DatabaseManager.test.ts index 2da3c33a00..ca50f518cb 100644 --- a/plugins/catalog-backend/src/database/DatabaseManager.test.ts +++ b/plugins/catalog-backend/src/database/DatabaseManager.test.ts @@ -15,6 +15,7 @@ */ import { getVoidLogger } from '@backstage/backend-common'; +import Knex from 'knex'; import { ComponentDescriptor, DescriptorParser, @@ -24,7 +25,6 @@ import { import { Database } from './Database'; import { DatabaseManager } from './DatabaseManager'; import { DatabaseLocationUpdateLogStatus, DbLocationsRow } from './types'; -import Knex from 'knex'; describe('DatabaseManager', () => { describe('refreshLocations', () => { diff --git a/plugins/catalog-backend/src/service/router.test.ts b/plugins/catalog-backend/src/service/router.test.ts index 1b1f4c5034..8c29362015 100644 --- a/plugins/catalog-backend/src/service/router.test.ts +++ b/plugins/catalog-backend/src/service/router.test.ts @@ -14,11 +14,7 @@ * limitations under the License. */ -import { - errorHandler, - getVoidLogger, - InputError, -} from '@backstage/backend-common'; +import { getVoidLogger } from '@backstage/backend-common'; import express from 'express'; import request from 'supertest'; import { EntitiesCatalog, Location, LocationsCatalog } from '../catalog'; @@ -27,7 +23,8 @@ import { createRouter } from './router'; class MockEntitiesCatalog implements EntitiesCatalog { entities = jest.fn(); - entity = jest.fn(); + entityByUid = jest.fn(); + entityByName = jest.fn(); } class MockLocationsCatalog implements LocationsCatalog { @@ -77,6 +74,89 @@ describe('createRouter', () => { }); }); + describe('entityByUid', () => { + it('can fetch entity by uid', async () => { + const entity: DescriptorEnvelope = { + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'c', + }, + }; + const catalog = new MockEntitiesCatalog(); + catalog.entityByUid.mockResolvedValue(entity); + + const router = await createRouter({ + entitiesCatalog: catalog, + logger: getVoidLogger(), + }); + + const app = express().use(router); + const response = await request(app).get('/entities/by-uid/zzz'); + + expect(response.status).toEqual(200); + expect(response.body).toEqual(expect.objectContaining(entity)); + }); + + it('responds with a 404 for missing entities', async () => { + const catalog = new MockEntitiesCatalog(); + catalog.entityByUid.mockResolvedValue(undefined); + + const router = await createRouter({ + entitiesCatalog: catalog, + logger: getVoidLogger(), + }); + + const app = express().use(router); + const response = await request(app).get('/entities/by-uid/zzz'); + + expect(response.status).toEqual(404); + expect(response.text).toMatch(/uid/); + }); + }); + + describe('entityByName', () => { + it('can fetch entity by name', async () => { + const entity: DescriptorEnvelope = { + apiVersion: 'a', + kind: 'b', + metadata: { + name: 'c', + namespace: 'd', + }, + }; + const catalog = new MockEntitiesCatalog(); + catalog.entityByName.mockResolvedValue(entity); + + const router = await createRouter({ + entitiesCatalog: catalog, + logger: getVoidLogger(), + }); + + const app = express().use(router); + const response = await request(app).get('/entities/by-name/b/d/c'); + + expect(response.status).toEqual(200); + expect(response.body).toEqual(expect.objectContaining(entity)); + }); + + it('responds with a 404 for missing entities', async () => { + const catalog = new MockEntitiesCatalog(); + catalog.entityByName.mockResolvedValue(undefined); + + const router = await createRouter({ + entitiesCatalog: catalog, + logger: getVoidLogger(), + }); + + const app = express().use(router); + const response = await request(app).get('/entities/by-name//b/d/c'); + + expect(response.status).toEqual(404); + expect(response.text).toMatch(/name/); + }); + }); + describe('locations', () => { it('happy path: lists locations', async () => { const locations: Location[] = [{ id: 'a', type: 'b', target: 'c' }]; diff --git a/plugins/catalog-backend/src/service/router.ts b/plugins/catalog-backend/src/service/router.ts index 81533adfc5..c3318a7377 100644 --- a/plugins/catalog-backend/src/service/router.ts +++ b/plugins/catalog-backend/src/service/router.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { errorHandler } from '@backstage/backend-common'; +import { errorHandler, InputError } from '@backstage/backend-common'; import express from 'express'; import Router from 'express-promise-router'; import { Logger } from 'winston'; @@ -41,26 +41,36 @@ export async function createRouter( router.use(express.json()); if (entitiesCatalog) { - router.get('/entities', async (req, res) => { - const filters: EntityFilters = []; - for (const [key, valueOrValues] of Object.entries(req.query)) { - const values = Array.isArray(valueOrValues) - ? valueOrValues - : [valueOrValues]; - if (values.some(v => typeof v !== 'string')) { - res.status(400).send('Complex query parameters are not supported'); - return; + router + .get('/entities', async (req, res) => { + const filters = translateQueryToEntityFilters(req); + const entities = await entitiesCatalog.entities(filters); + res.status(200).send(entities); + }) + .get('/entities/by-uid/:uid', async (req, res) => { + const { uid } = req.params; + const entity = await entitiesCatalog.entityByUid(uid); + if (!entity) { + res.status(404).send(`No entity with uid ${uid}`); } - filters.push({ - key, - values: values.map(v => v || null) as string[], - }); - } - - const entities = await entitiesCatalog.entities(filters); - - res.status(200).send(entities); - }); + res.status(200).send(entity); + }) + .get('/entities/by-name/:kind/:namespace/:name', async (req, res) => { + const { kind, namespace, name } = req.params; + const entity = await entitiesCatalog.entityByName( + kind, + name, + namespace, + ); + if (!entity) { + res + .status(404) + .send( + `No entity with kind ${kind} namespace ${namespace} name ${name}`, + ); + } + res.status(200).send(entity); + }); } if (locationsCatalog) { @@ -89,3 +99,26 @@ export async function createRouter( router.use(errorHandler()); return router; } + +function translateQueryToEntityFilters( + request: express.Request, +): EntityFilters { + const filters: EntityFilters = []; + + for (const [key, valueOrValues] of Object.entries(request.query)) { + const values = Array.isArray(valueOrValues) + ? valueOrValues + : [valueOrValues]; + + if (values.some(v => typeof v !== 'string')) { + throw new InputError('Complex query parameters are not supported'); + } + + filters.push({ + key, + values: values.map(v => v || null) as string[], + }); + } + + return filters; +} From 8a1145b96cb283e667d5985256f053138c13df45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 26 May 2020 16:22:45 +0200 Subject: [PATCH 10/21] Fix test order --- .../src/database/Database.test.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/plugins/catalog-backend/src/database/Database.test.ts b/plugins/catalog-backend/src/database/Database.test.ts index 7e9c75d199..a82385aae4 100644 --- a/plugins/catalog-backend/src/database/Database.test.ts +++ b/plugins/catalog-backend/src/database/Database.test.ts @@ -257,12 +257,16 @@ describe('Database', () => { await catalog.addEntity(tx, { entity: e1 }); await catalog.addEntity(tx, { entity: e2 }); }); - await expect( - catalog.transaction(async tx => catalog.entities(tx, [])), - ).resolves.toEqual([ - { locationId: undefined, entity: expect.objectContaining(e1) }, - { locationId: undefined, entity: expect.objectContaining(e2) }, - ]); + const result = await catalog.transaction(async tx => + catalog.entities(tx, []), + ); + expect(result.length).toEqual(2); + expect(result).toEqual( + expect.arrayContaining([ + { locationId: undefined, entity: expect.objectContaining(e1) }, + { locationId: undefined, entity: expect.objectContaining(e2) }, + ]), + ); }); it('can get all specific entities for matching filters (naive case)', async () => { From a51ad53b00c94ba8fe0bba2a80744b19fabc367d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 26 May 2020 16:38:40 +0200 Subject: [PATCH 11/21] packages/core: group api implementations by the name of the declarations --- .../{ => AlertApi}/AlertApiForwarder.ts | 9 ++++++--- .../api/apis/implementations/AlertApi/index.ts | 17 +++++++++++++++++ .../AppThemeSelector.test.ts | 0 .../AppThemeSelector.ts | 4 ++-- .../{AppThemeSelector => AppThemeApi}/index.ts | 0 .../{ => ErrorApi}/ErrorAlerter.ts | 2 +- .../{ => ErrorApi}/ErrorApiForwarder.ts | 9 ++++++--- .../api/apis/implementations/ErrorApi/index.ts | 18 ++++++++++++++++++ .../MockOAuthApi.test.ts | 0 .../MockOAuthApi.ts | 0 .../OAuthPendingRequests.test.ts | 0 .../OAuthPendingRequests.ts | 0 .../OAuthRequestManager.test.ts | 0 .../OAuthRequestManager.ts | 0 .../index.ts | 0 .../core/src/api/apis/implementations/index.ts | 10 +++++----- .../AuthConnector/DefaultAuthConnector.test.ts | 2 +- .../RefreshingAuthSessionManager.ts | 3 +-- 18 files changed, 57 insertions(+), 17 deletions(-) rename packages/core/src/api/apis/implementations/{ => AlertApi}/AlertApiForwarder.ts (78%) create mode 100644 packages/core/src/api/apis/implementations/AlertApi/index.ts rename packages/core/src/api/apis/implementations/{AppThemeSelector => AppThemeApi}/AppThemeSelector.test.ts (100%) rename packages/core/src/api/apis/implementations/{AppThemeSelector => AppThemeApi}/AppThemeSelector.ts (94%) rename packages/core/src/api/apis/implementations/{AppThemeSelector => AppThemeApi}/index.ts (100%) rename packages/core/src/api/apis/implementations/{ => ErrorApi}/ErrorAlerter.ts (94%) rename packages/core/src/api/apis/implementations/{ => ErrorApi}/ErrorApiForwarder.ts (80%) create mode 100644 packages/core/src/api/apis/implementations/ErrorApi/index.ts rename packages/core/src/api/apis/implementations/{OAuthRequestManager => OAuthRequestApi}/MockOAuthApi.test.ts (100%) rename packages/core/src/api/apis/implementations/{OAuthRequestManager => OAuthRequestApi}/MockOAuthApi.ts (100%) rename packages/core/src/api/apis/implementations/{OAuthRequestManager => OAuthRequestApi}/OAuthPendingRequests.test.ts (100%) rename packages/core/src/api/apis/implementations/{OAuthRequestManager => OAuthRequestApi}/OAuthPendingRequests.ts (100%) rename packages/core/src/api/apis/implementations/{OAuthRequestManager => OAuthRequestApi}/OAuthRequestManager.test.ts (100%) rename packages/core/src/api/apis/implementations/{OAuthRequestManager => OAuthRequestApi}/OAuthRequestManager.ts (100%) rename packages/core/src/api/apis/implementations/{OAuthRequestManager => OAuthRequestApi}/index.ts (100%) diff --git a/packages/core/src/api/apis/implementations/AlertApiForwarder.ts b/packages/core/src/api/apis/implementations/AlertApi/AlertApiForwarder.ts similarity index 78% rename from packages/core/src/api/apis/implementations/AlertApiForwarder.ts rename to packages/core/src/api/apis/implementations/AlertApi/AlertApiForwarder.ts index c95408283c..901d3b57cc 100644 --- a/packages/core/src/api/apis/implementations/AlertApiForwarder.ts +++ b/packages/core/src/api/apis/implementations/AlertApi/AlertApiForwarder.ts @@ -13,10 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { AlertApi, AlertMessage } from '../../../'; -import { PublishSubject } from './lib'; -import { Observable } from '../../types'; +import { AlertApi, AlertMessage } from '../../../..'; +import { PublishSubject } from '../lib'; +import { Observable } from '../../../types'; +/** + * Base implementation for the AlertApi that simply forwards alerts to consumers. + */ export class AlertApiForwarder implements AlertApi { private readonly subject = new PublishSubject(); diff --git a/packages/core/src/api/apis/implementations/AlertApi/index.ts b/packages/core/src/api/apis/implementations/AlertApi/index.ts new file mode 100644 index 0000000000..12ab8bc60c --- /dev/null +++ b/packages/core/src/api/apis/implementations/AlertApi/index.ts @@ -0,0 +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. + */ + +export { AlertApiForwarder } from './AlertApiForwarder'; diff --git a/packages/core/src/api/apis/implementations/AppThemeSelector/AppThemeSelector.test.ts b/packages/core/src/api/apis/implementations/AppThemeApi/AppThemeSelector.test.ts similarity index 100% rename from packages/core/src/api/apis/implementations/AppThemeSelector/AppThemeSelector.test.ts rename to packages/core/src/api/apis/implementations/AppThemeApi/AppThemeSelector.test.ts diff --git a/packages/core/src/api/apis/implementations/AppThemeSelector/AppThemeSelector.ts b/packages/core/src/api/apis/implementations/AppThemeApi/AppThemeSelector.ts similarity index 94% rename from packages/core/src/api/apis/implementations/AppThemeSelector/AppThemeSelector.ts rename to packages/core/src/api/apis/implementations/AppThemeApi/AppThemeSelector.ts index 7f6659e641..0f554ed7f1 100644 --- a/packages/core/src/api/apis/implementations/AppThemeSelector/AppThemeSelector.ts +++ b/packages/core/src/api/apis/implementations/AppThemeApi/AppThemeSelector.ts @@ -33,7 +33,7 @@ export class AppThemeSelector implements AppThemeApi { selector.setActiveThemeId(initialThemeId); - selector.activeThemeId$().subscribe((themeId) => { + selector.activeThemeId$().subscribe(themeId => { if (themeId) { window.localStorage.setItem(STORAGE_KEY, themeId); } else { @@ -41,7 +41,7 @@ export class AppThemeSelector implements AppThemeApi { } }); - window.addEventListener('storage', (event) => { + window.addEventListener('storage', event => { if (event.key === STORAGE_KEY) { const themeId = localStorage.getItem(STORAGE_KEY) ?? undefined; selector.setActiveThemeId(themeId); diff --git a/packages/core/src/api/apis/implementations/AppThemeSelector/index.ts b/packages/core/src/api/apis/implementations/AppThemeApi/index.ts similarity index 100% rename from packages/core/src/api/apis/implementations/AppThemeSelector/index.ts rename to packages/core/src/api/apis/implementations/AppThemeApi/index.ts diff --git a/packages/core/src/api/apis/implementations/ErrorAlerter.ts b/packages/core/src/api/apis/implementations/ErrorApi/ErrorAlerter.ts similarity index 94% rename from packages/core/src/api/apis/implementations/ErrorAlerter.ts rename to packages/core/src/api/apis/implementations/ErrorApi/ErrorAlerter.ts index 81d0cc8fb8..903463d7de 100644 --- a/packages/core/src/api/apis/implementations/ErrorAlerter.ts +++ b/packages/core/src/api/apis/implementations/ErrorApi/ErrorAlerter.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { ErrorApi, ErrorContext, AlertApi } from '../../../'; +import { ErrorApi, ErrorContext, AlertApi } from '../../../..'; /** * Decorates an ErrorApi by also forwarding error messages diff --git a/packages/core/src/api/apis/implementations/ErrorApiForwarder.ts b/packages/core/src/api/apis/implementations/ErrorApi/ErrorApiForwarder.ts similarity index 80% rename from packages/core/src/api/apis/implementations/ErrorApiForwarder.ts rename to packages/core/src/api/apis/implementations/ErrorApi/ErrorApiForwarder.ts index a61fdae5c1..6729050401 100644 --- a/packages/core/src/api/apis/implementations/ErrorApiForwarder.ts +++ b/packages/core/src/api/apis/implementations/ErrorApi/ErrorApiForwarder.ts @@ -13,10 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { ErrorApi, ErrorContext } from '../../../'; -import { PublishSubject } from './lib'; -import { Observable } from '../../types'; +import { ErrorApi, ErrorContext } from '../../../..'; +import { PublishSubject } from '../lib'; +import { Observable } from '../../../types'; +/** + * Base implementation for the ErrorApi that simply forwards errors to consumers. + */ export class ErrorApiForwarder implements ErrorApi { private readonly subject = new PublishSubject<{ error: Error; diff --git a/packages/core/src/api/apis/implementations/ErrorApi/index.ts b/packages/core/src/api/apis/implementations/ErrorApi/index.ts new file mode 100644 index 0000000000..757dfd0d8f --- /dev/null +++ b/packages/core/src/api/apis/implementations/ErrorApi/index.ts @@ -0,0 +1,18 @@ +/* + * 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. + */ + +export { ErrorAlerter } from './ErrorAlerter'; +export { ErrorApiForwarder } from './ErrorApiForwarder'; diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts b/packages/core/src/api/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts similarity index 100% rename from packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.test.ts rename to packages/core/src/api/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.ts b/packages/core/src/api/apis/implementations/OAuthRequestApi/MockOAuthApi.ts similarity index 100% rename from packages/core/src/api/apis/implementations/OAuthRequestManager/MockOAuthApi.ts rename to packages/core/src/api/apis/implementations/OAuthRequestApi/MockOAuthApi.ts diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.test.ts b/packages/core/src/api/apis/implementations/OAuthRequestApi/OAuthPendingRequests.test.ts similarity index 100% rename from packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.test.ts rename to packages/core/src/api/apis/implementations/OAuthRequestApi/OAuthPendingRequests.test.ts diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts b/packages/core/src/api/apis/implementations/OAuthRequestApi/OAuthPendingRequests.ts similarity index 100% rename from packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts rename to packages/core/src/api/apis/implementations/OAuthRequestApi/OAuthPendingRequests.ts diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts b/packages/core/src/api/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts similarity index 100% rename from packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.test.ts rename to packages/core/src/api/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.ts b/packages/core/src/api/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts similarity index 100% rename from packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthRequestManager.ts rename to packages/core/src/api/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/index.ts b/packages/core/src/api/apis/implementations/OAuthRequestApi/index.ts similarity index 100% rename from packages/core/src/api/apis/implementations/OAuthRequestManager/index.ts rename to packages/core/src/api/apis/implementations/OAuthRequestApi/index.ts diff --git a/packages/core/src/api/apis/implementations/index.ts b/packages/core/src/api/apis/implementations/index.ts index 1f2b91c16d..bb77cf5bd3 100644 --- a/packages/core/src/api/apis/implementations/index.ts +++ b/packages/core/src/api/apis/implementations/index.ts @@ -19,8 +19,8 @@ // Plugins should rely on these APIs for functionality as much as possible. export * from './auth'; -export * from './AppThemeSelector'; -export * from './AlertApiForwarder'; -export * from './ErrorAlerter'; -export * from './ErrorApiForwarder'; -export * from './OAuthRequestManager'; + +export * from './AlertApi'; +export * from './AppThemeApi'; +export * from './ErrorApi'; +export * from './OAuthRequestApi'; diff --git a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts index f3698b4e15..d4f95f7907 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthConnector/DefaultAuthConnector.test.ts @@ -16,7 +16,7 @@ import ProviderIcon from '@material-ui/icons/AcUnit'; import { DefaultAuthConnector } from './DefaultAuthConnector'; -import MockOAuthApi from '../../OAuthRequestManager/MockOAuthApi'; +import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import * as loginPopup from '../loginPopup'; const anyFetch = fetch as any; diff --git a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts index 0cb0f2f1ea..cbd6d5ca46 100644 --- a/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts +++ b/packages/core/src/api/apis/implementations/lib/AuthSessionManager/RefreshingAuthSessionManager.ts @@ -20,8 +20,7 @@ import { SessionShouldRefreshFunc, } from './types'; import { AuthConnector } from '../AuthConnector'; -import { SessionScopeHelper } from './common'; -import { hasScopes } from '../../OAuthRequestManager/OAuthPendingRequests'; +import { SessionScopeHelper, hasScopes } from './common'; type Options = { /** The connector used for acting on the auth session */ From f6fc811d0a353fa4e5424720d4118315c7bbde2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 26 May 2020 16:45:24 +0200 Subject: [PATCH 12/21] Use spread --- plugins/catalog-backend/src/database/Database.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/Database.ts index a3aba825f4..614f7d09bf 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/Database.ts @@ -187,11 +187,12 @@ export class Database { } const newEntity = lodash.cloneDeep(request.entity); - newEntity.metadata = Object.assign({}, newEntity.metadata, { + newEntity.metadata = { + ...newEntity.metadata, uid: generateUid(), etag: generateEtag(), generation: 1, - }); + }; const newRow = toEntityRow(request.locationId, newEntity); await tx('entities').insert(newRow); @@ -276,11 +277,12 @@ export class Database { ? oldRow.generation : oldRow.generation + 1; const newEntity = lodash.cloneDeep(request.entity); - newEntity.metadata = Object.assign({}, newEntity.metadata, { + newEntity.metadata = { + ...newEntity.metadata, uid: oldRow.id, etag: newEtag, generation: newGeneration, - }); + }; // Preserve annotations that were set on the old version of the entity, // unless the new version overwrites them From 3a7e998a160f1ed91127c0864190a809b4dc3bed Mon Sep 17 00:00:00 2001 From: Raghunandan Date: Tue, 26 May 2020 11:00:34 +0200 Subject: [PATCH 13/21] fix review comments, more error handling, moar tests --- .../src/providers/google/provider.test.ts | 54 ++++++++++++++++--- .../src/providers/google/provider.ts | 37 ++++++++++--- 2 files changed, 77 insertions(+), 14 deletions(-) diff --git a/plugins/auth-backend/src/providers/google/provider.test.ts b/plugins/auth-backend/src/providers/google/provider.test.ts index 1726fbf33a..b692a35216 100644 --- a/plugins/auth-backend/src/providers/google/provider.test.ts +++ b/plugins/auth-backend/src/providers/google/provider.test.ts @@ -98,6 +98,7 @@ describe('GoogleAuthProvider', () => { it('should perform logout and respond with 200', () => { const mockResponse: any = ({ send: jest.fn(), + cookie: jest.fn(), } as unknown) as express.Response; const googleAuthProvider = new GoogleAuthProvider( @@ -111,6 +112,12 @@ describe('GoogleAuthProvider', () => { googleAuthProvider.logout(mockRequest, mockResponse); expect(spyResponse).toBeCalledTimes(1); expect(spyResponse).toBeCalledWith('logout!'); + expect(mockResponse.cookie).toBeCalledTimes(1); + expect(mockResponse.cookie).toBeCalledWith( + 'google-refresh-token', + '', + expect.objectContaining({ maxAge: 0 }), + ); }); }); @@ -145,7 +152,7 @@ describe('GoogleAuthProvider', () => { expect(spyPostMessage).toBeCalledTimes(1); expect(mockResponse.cookie).toBeCalledTimes(1); expect(mockResponse.cookie).toBeCalledWith( - 'grtoken', + 'google-refresh-token', 'REFRESH_TOKEN', expect.objectContaining({ path: '/auth/google', @@ -156,7 +163,7 @@ describe('GoogleAuthProvider', () => { ); }); - it('should respond with a 401 if no refresh token returned', () => { + it('should respond with a error message if no refresh token returned', () => { const spyPassport = jest .spyOn(passport, 'authenticate') .mockImplementation((_x, callbackFunc) => { @@ -165,16 +172,47 @@ describe('GoogleAuthProvider', () => { return jest.fn(); }); + const spyPostMessage = jest + .spyOn(utils, 'postMessageResponse') + .mockImplementation(() => jest.fn()); + const googleAuthProvider = new GoogleAuthProvider( googleAuthProviderConfig, ); googleAuthProvider.frameHandler(mockRequest, mockResponse, mockNext); expect(spyPassport).toBeCalledTimes(1); - expect(mockResponse.send).toBeCalledTimes(1); - expect(mockResponse.send).toBeCalledWith('Failed to fetch refresh token'); - expect(mockResponse.status).toBeCalledTimes(1); - expect(mockResponse.status).toBeCalledWith(401); + expect(spyPostMessage).toBeCalledTimes(1); + expect(spyPostMessage).toBeCalledWith(mockResponse, { + type: 'auth-result', + error: new Error('Missing refresh token'), + }); + }); + + it('should respond with a error message if auth failed', () => { + const spyPassport = jest + .spyOn(passport, 'authenticate') + .mockImplementation((_x, callbackFunc) => { + const cb = callbackFunc as Function; + cb(new Error('TokenError'), null); + return jest.fn(); + }); + + const spyPostMessage = jest + .spyOn(utils, 'postMessageResponse') + .mockImplementation(() => jest.fn()); + + const googleAuthProvider = new GoogleAuthProvider( + googleAuthProviderConfig, + ); + + googleAuthProvider.frameHandler(mockRequest, mockResponse, mockNext); + expect(spyPassport).toBeCalledTimes(1); + expect(spyPostMessage).toBeCalledTimes(1); + expect(spyPostMessage).toBeCalledWith(mockResponse, { + type: 'auth-result', + error: new Error('Google auth failed, Error: TokenError'), + }); }); }); @@ -224,7 +262,7 @@ describe('GoogleAuthProvider', () => { describe('refresh token cookie, no scope', () => { const mockRequest = ({ - cookies: { grtoken: 'REFRESH_TOKEN' }, + cookies: { 'google-refresh-token': 'REFRESH_TOKEN' }, query: {}, } as unknown) as express.Request; @@ -311,7 +349,7 @@ describe('GoogleAuthProvider', () => { describe('refresh token cookie and scope', () => { const mockRequest = ({ - cookies: { grtoken: 'REFRESH_TOKEN' }, + cookies: { 'google-refresh-token': 'REFRESH_TOKEN' }, query: { scope: 'a,b', }, diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts index 51845f638b..096260e044 100644 --- a/plugins/auth-backend/src/providers/google/provider.ts +++ b/plugins/auth-backend/src/providers/google/provider.ts @@ -55,11 +55,21 @@ export class GoogleAuthProvider res: express.Response, next: express.NextFunction, ) { - return passport.authenticate('google', (_, user) => { + return passport.authenticate('google', (err, user) => { + if (err) { + return postMessageResponse(res, { + type: 'auth-result', + error: new Error(`Google auth failed, ${err}`), + }); + } + const { refreshToken } = user; if (!refreshToken) { - return res.status(401).send('Failed to fetch refresh token'); + return postMessageResponse(res, { + type: 'auth-result', + error: new Error('Missing refresh token'), + }); } delete user.refreshToken; @@ -69,11 +79,15 @@ export class GoogleAuthProvider secure: false, sameSite: 'none', domain: 'localhost', - path: '/auth/google', + path: `/auth/${this.providerConfig.provider}`, httpOnly: true, }; - res.cookie('grtoken', refreshToken, options); + res.cookie( + `${this.providerConfig.provider}-refresh-token`, + refreshToken, + options, + ); return postMessageResponse(res, { type: 'auth-result', payload: user, @@ -82,11 +96,22 @@ export class GoogleAuthProvider } async logout(_req: express.Request, res: express.Response) { + const options: CookieOptions = { + maxAge: 0, + secure: false, + sameSite: 'none', + domain: 'localhost', + path: `/auth/${this.providerConfig.provider}`, + httpOnly: true, + }; + + res.cookie(`${this.providerConfig.provider}-refresh-token`, '', options); return res.send('logout!'); } async refresh(req: express.Request, res: express.Response) { - const refreshToken = req.cookies.grtoken; + const refreshToken = + req.cookies[`${this.providerConfig.provider}-refresh-token`]; if (!refreshToken) { return res.status(401).send('Missing session cookie'); @@ -96,7 +121,7 @@ export class GoogleAuthProvider const refreshTokenRequestParams = scope ? { scope } : {}; return refresh.requestNewAccessToken( - 'google', + this.providerConfig.provider, refreshToken, refreshTokenRequestParams, (err, accessToken, _refreshToken, params) => { From c954f9d5ec6c485f2ff5f47f19163eb0918e7d42 Mon Sep 17 00:00:00 2001 From: Ben Lambert Date: Tue, 26 May 2020 20:06:21 +0200 Subject: [PATCH 14/21] Catalog Filters (#1000) * feat(catalog-plugin): Starting to add the sidebar filters component for the different filters applicable * chore(catalog-frontend): fixing pr code review comments * feat(packages/core): export IconComponent for use in other packages * chore(catalog-frontend): Added some tests for the catalog filter component to make sure it renders the correct data * feat(catalog-frontend): added the ability for click handlers when changing selected filter * feat(catalog-frontend): store state in the page component for active filter and moving out the mock data to a better place * feat(catalog-frontend): reworking the selected state and fixing the selected text on the table when you choose the correct filter --- packages/core/src/icons/types.ts | 1 - packages/core/src/index.ts | 1 + .../CatalogFilter/CatalogFilter.test.tsx | 131 ++++++++++++++++++ .../CatalogFilter/CatalogFilter.tsx | 117 ++++++++++++++++ .../src/components/CatalogFilter/index.ts | 17 +++ .../CatalogPage/CatalogPage.test.tsx | 12 +- .../components/CatalogPage/CatalogPage.tsx | 47 ++++++- .../CatalogTable/CatalogTable.test.tsx | 14 +- .../components/CatalogTable/CatalogTable.tsx | 4 +- plugins/catalog/src/data/filters.ts | 53 +++++++ 10 files changed, 382 insertions(+), 15 deletions(-) create mode 100644 plugins/catalog/src/components/CatalogFilter/CatalogFilter.test.tsx create mode 100644 plugins/catalog/src/components/CatalogFilter/CatalogFilter.tsx create mode 100644 plugins/catalog/src/components/CatalogFilter/index.ts create mode 100644 plugins/catalog/src/data/filters.ts diff --git a/packages/core/src/icons/types.ts b/packages/core/src/icons/types.ts index 30e0ba53b2..599cb969df 100644 --- a/packages/core/src/icons/types.ts +++ b/packages/core/src/icons/types.ts @@ -16,7 +16,6 @@ import { ComponentType } from 'react'; import { SvgIconProps } from '@material-ui/core'; - export type IconComponent = ComponentType; export type SystemIconKey = 'user' | 'group'; export type SystemIcons = { [key in SystemIconKey]: IconComponent }; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 013c80e2fa..816ad92578 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -46,3 +46,4 @@ export { default as TrendLine } from './components/TrendLine'; export { FeatureCalloutCircular } from './components/FeatureDiscovery/FeatureCalloutCircular'; export * from './components/Status'; export { default as WarningPanel } from './components/WarningPanel'; +export type { IconComponent } from './icons'; diff --git a/plugins/catalog/src/components/CatalogFilter/CatalogFilter.test.tsx b/plugins/catalog/src/components/CatalogFilter/CatalogFilter.test.tsx new file mode 100644 index 0000000000..762a881302 --- /dev/null +++ b/plugins/catalog/src/components/CatalogFilter/CatalogFilter.test.tsx @@ -0,0 +1,131 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { render, fireEvent } from '@testing-library/react'; +import { wrapInThemedTestApp } from '@backstage/test-utils'; +import { CatalogFilter, CatalogFilterGroup } from './CatalogFilter'; + +describe('Catalog Filter', () => { + it('should render the different groups', async () => { + const mockGroups: CatalogFilterGroup[] = [ + { name: 'Test Group 1', items: [] }, + { name: 'Test Group 2', items: [] }, + ]; + const { findByText } = render( + wrapInThemedTestApp(), + ); + + for (const group of mockGroups) { + expect(await findByText(group.name)).toBeInTheDocument(); + } + }); + + it('should render the different items and their names', async () => { + const mockGroups: CatalogFilterGroup[] = [ + { + name: 'Test Group 1', + items: [ + { + id: 'first', + label: 'First Label', + }, + { + id: 'second', + label: 'Second Label', + }, + ], + }, + ]; + + const { findByText } = render( + wrapInThemedTestApp(), + ); + + const [group] = mockGroups; + for (const item of group.items) { + expect(await findByText(item.label)).toBeInTheDocument(); + } + }); + + it('should render the count in each item', async () => { + const mockGroups: CatalogFilterGroup[] = [ + { + name: 'Test Group 1', + items: [ + { + id: 'first', + label: 'First Label', + count: 100, + }, + { + id: 'second', + label: 'Second Label', + count: 400, + }, + ], + }, + ]; + + const { findByText } = render( + wrapInThemedTestApp(), + ); + + const [group] = mockGroups; + for (const item of group.items) { + expect(await findByText(item.count!.toString())).toBeInTheDocument(); + } + }); + + it('should fire the callback when an item is clicked', async () => { + const mockGroups: CatalogFilterGroup[] = [ + { + name: 'Test Group 1', + items: [ + { + id: 'first', + label: 'First Label', + count: 100, + }, + { + id: 'second', + label: 'Second Label', + count: 400, + }, + ], + }, + ]; + + const onSelectedChangeHandler = jest.fn(); + + const { findByText } = render( + wrapInThemedTestApp( + , + ), + ); + + const item = mockGroups[0].items[0]; + + const element = await findByText(item.label); + + fireEvent.click(element); + + expect(onSelectedChangeHandler).toHaveBeenCalledWith(item); + }); +}); diff --git a/plugins/catalog/src/components/CatalogFilter/CatalogFilter.tsx b/plugins/catalog/src/components/CatalogFilter/CatalogFilter.tsx new file mode 100644 index 0000000000..eb45d8a80b --- /dev/null +++ b/plugins/catalog/src/components/CatalogFilter/CatalogFilter.tsx @@ -0,0 +1,117 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React from 'react'; +import { + Card, + List, + ListItemIcon, + ListItemText, + MenuItem, + Typography, + Theme, + makeStyles, +} from '@material-ui/core'; +import type { IconComponent } from '@backstage/core'; + +export type CatalogFilterItem = { + id: string; + label: string; + icon?: IconComponent; + count?: number; + loading?: boolean; +}; + +export type CatalogFilterGroup = { + name: string; + items: CatalogFilterItem[]; +}; + +export type CatalogFilterProps = { + groups: CatalogFilterGroup[]; + selectedId?: string; + onSelectedChange?: (item: CatalogFilterItem) => void; +}; + +const useStyles = makeStyles(theme => ({ + root: { + backgroundColor: 'rgba(0, 0, 0, .11)', + boxShadow: 'none', + }, + title: { + margin: theme.spacing(1, 0, 0, 1), + textTransform: 'uppercase', + fontSize: 12, + fontWeight: 'bold', + }, + listIcon: { + minWidth: 30, + color: theme.palette.text.primary, + }, + menuItem: { + minHeight: theme.spacing(6), + }, + groupWrapper: { + margin: theme.spacing(1, 1, 2, 1), + }, + menuTitle: { + fontWeight: 500, + }, +})); + +export const CatalogFilter: React.FC = ({ + groups, + selectedId, + onSelectedChange, +}) => { + const classes = useStyles(); + return ( + + {groups.map(group => ( + + + {group.name} + + + + {group.items.map(item => ( + onSelectedChange?.(item)} + selected={item.id === selectedId} + className={classes.menuItem} + > + {item.icon && ( + + + + )} + + + {item.label} + + + {item.count} + + ))} + + + + ))} + + ); +}; diff --git a/plugins/catalog/src/components/CatalogFilter/index.ts b/plugins/catalog/src/components/CatalogFilter/index.ts new file mode 100644 index 0000000000..5103b16307 --- /dev/null +++ b/plugins/catalog/src/components/CatalogFilter/index.ts @@ -0,0 +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. + */ + +export { CatalogFilter } from './CatalogFilter'; diff --git a/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx b/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx index 60af5f7d24..5dee366696 100644 --- a/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx +++ b/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx @@ -17,8 +17,7 @@ import React from 'react'; import { render } from '@testing-library/react'; import CatalogPage from './CatalogPage'; -import { ThemeProvider } from '@material-ui/core'; -import { lightTheme } from '@backstage/theme'; +import { wrapInThemedTestApp } from '@backstage/test-utils'; import { ComponentFactory } from '../../data/component'; const testComponentFactory: ComponentFactory = { @@ -28,11 +27,14 @@ const testComponentFactory: ComponentFactory = { }; describe('CatalogPage', () => { + // this test right now causes some red lines in the log output when running tests + // related to some theme issues in mui-table + // https://github.com/mbrn/material-table/issues/1293 it('should render', async () => { const rendered = render( - - - , + wrapInThemedTestApp( + , + ), ); expect( await rendered.findByText('Keep track of your software'), diff --git a/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx b/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx index ec6c1cf09f..f5094542a5 100644 --- a/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx +++ b/plugins/catalog/src/components/CatalogPage/CatalogPage.tsx @@ -27,13 +27,38 @@ import { import { useAsync } from 'react-use'; import { ComponentFactory } from '../../data/component'; import CatalogTable from '../CatalogTable/CatalogTable'; -import { Button } from '@material-ui/core'; +import { + CatalogFilter, + CatalogFilterItem, +} from '../CatalogFilter/CatalogFilter'; +import { Button, makeStyles } from '@material-ui/core'; +import { filterGroups, defaultFilter } from '../../data/filters'; + +const useStyles = makeStyles(theme => ({ + contentWrapper: { + display: 'grid', + gridTemplateAreas: "'filters' 'table'", + gridTemplateColumns: '250px 1fr', + gridColumnGap: theme.spacing(2), + }, +})); type CatalogPageProps = { componentFactory: ComponentFactory; }; + const CatalogPage: FC = ({ componentFactory }) => { const { value, error, loading } = useAsync(componentFactory.getAllComponents); + const [selectedFilter, setSelectedFilter] = React.useState( + defaultFilter, + ); + + const onFilterSelected = React.useCallback( + selected => setSelectedFilter(selected), + [], + ); + + const styles = useStyles(); return (
@@ -46,11 +71,21 @@ const CatalogPage: FC = ({ componentFactory }) => { All your components - +
+
+ +
+ +
); diff --git a/plugins/catalog/src/components/CatalogTable/CatalogTable.test.tsx b/plugins/catalog/src/components/CatalogTable/CatalogTable.test.tsx index 99debb12d4..9bb1db4519 100644 --- a/plugins/catalog/src/components/CatalogTable/CatalogTable.test.tsx +++ b/plugins/catalog/src/components/CatalogTable/CatalogTable.test.tsx @@ -28,7 +28,9 @@ const components: Component[] = [ describe('CatalogTable component', () => { it('should render loading when loading prop it set to true', async () => { const rendered = render( - wrapInThemedTestApp(), + wrapInThemedTestApp( + , + ), ); const progress = await rendered.findByTestId('progress'); expect(progress).toBeInTheDocument(); @@ -38,6 +40,7 @@ describe('CatalogTable component', () => { const rendered = render( wrapInThemedTestApp( { it('should display component names when loading has finished and no error occurred', async () => { const rendered = render( wrapInThemedTestApp( - , + , ), ); + expect( + await rendered.findByText(`Owned (${components.length})`), + ).toBeInTheDocument(); expect(await rendered.findByText('component1')).toBeInTheDocument(); expect(await rendered.findByText('component2')).toBeInTheDocument(); expect(await rendered.findByText('component3')).toBeInTheDocument(); diff --git a/plugins/catalog/src/components/CatalogTable/CatalogTable.tsx b/plugins/catalog/src/components/CatalogTable/CatalogTable.tsx index 57f1dfe7f5..537deb1b30 100644 --- a/plugins/catalog/src/components/CatalogTable/CatalogTable.tsx +++ b/plugins/catalog/src/components/CatalogTable/CatalogTable.tsx @@ -63,6 +63,7 @@ const columns: TableColumn[] = [ type CatalogTableProps = { components: Component[]; + titlePreamble: string; loading: boolean; error?: any; }; @@ -70,6 +71,7 @@ const CatalogTable: FC = ({ components, loading, error, + titlePreamble, }) => { if (loading) { return ; @@ -87,7 +89,7 @@ const CatalogTable: FC = ({ ); diff --git a/plugins/catalog/src/data/filters.ts b/plugins/catalog/src/data/filters.ts new file mode 100644 index 0000000000..66eb55b478 --- /dev/null +++ b/plugins/catalog/src/data/filters.ts @@ -0,0 +1,53 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { + CatalogFilterGroup, + CatalogFilterItem, +} from '../components/CatalogFilter/CatalogFilter'; +import SettingsIcon from '@material-ui/icons/Settings'; +import StarIcon from '@material-ui/icons/Star'; + +export const filterGroups: CatalogFilterGroup[] = [ + { + name: 'Personal', + items: [ + { + id: 'owned', + label: 'Owned', + count: 123, + icon: SettingsIcon, + }, + { + id: 'starred', + label: 'Starred', + count: 10, + icon: StarIcon, + }, + ], + }, + { + name: 'Spotify', + items: [ + { + id: 'all', + label: 'All Services', + count: 123, + }, + ], + }, +]; + +export const defaultFilter: CatalogFilterItem = filterGroups[0].items[0]; From 4c4b48490bab5939d2fdfd190bbf1792ebbc5313 Mon Sep 17 00:00:00 2001 From: Niklas Ek Date: Tue, 26 May 2020 21:16:41 +0200 Subject: [PATCH 15/21] Remove WIP label on Storybook in readme (#1014) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 086c1ace60..2e22818f4b 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ Take a look at the [Getting Started](docs/getting-started/README.md) guide to le - [Architecture](docs/architecture-terminology.md) - [API references](docs/reference/README.md) - [Designing for Backstage](docs/design.md) -- [Storybook - UI components](http://storybook.backstage.io) ([WIP](https://github.com/spotify/backstage/milestone/9)) +- [Storybook - UI components](http://storybook.backstage.io) - [Contributing to Storybook](docs/getting-started/contributing-to-storybook.md) - Using Backstage components (TODO) From 1042635159e3ee80461b971326c3fbaea4affe9b Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Tue, 26 May 2020 21:20:14 +0200 Subject: [PATCH 16/21] build(deps): bump rollup from 2.3.2 to 2.10.9 (#1004) Bumps [rollup](https://github.com/rollup/rollup) from 2.3.2 to 2.10.9. - [Release notes](https://github.com/rollup/rollup/releases) - [Changelog](https://github.com/rollup/rollup/blob/master/CHANGELOG.md) - [Commits](https://github.com/rollup/rollup/compare/v2.3.2...v2.10.9) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index c23e7a42a5..c2398e238c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -18370,9 +18370,9 @@ rollup@^0.63.4: "@types/node" "*" rollup@^2.3.2: - version "2.3.2" - resolved "https://registry.npmjs.org/rollup/-/rollup-2.3.2.tgz#afa68e4f3325bcef4e150d082056bef450bcac60" - integrity sha512-p66+fbfaUUOGE84sHXAOgfeaYQMslgAazoQMp//nlR519R61213EPFgrMZa48j31jNacJwexSAR1Q8V/BwGKBA== + version "2.10.9" + resolved "https://registry.npmjs.org/rollup/-/rollup-2.10.9.tgz#17dcc6753c619efcc1be2cf61d73a87827eebdf9" + integrity sha512-dY/EbjiWC17ZCUSyk14hkxATAMAShkMsD43XmZGWjLrgFj15M3Dw2kEkA9ns64BiLFm9PKN6vTQw8neHwK74eg== optionalDependencies: fsevents "~2.1.2" From 93263f6bd103128359f86514f0df57d451327d3f Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Tue, 26 May 2020 22:59:26 +0200 Subject: [PATCH 17/21] build(deps-dev): bump @types/uuid from 7.0.3 to 8.0.0 (#944) Bumps [@types/uuid](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/uuid) from 7.0.3 to 8.0.0. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/uuid) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- plugins/catalog-backend/package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/catalog-backend/package.json b/plugins/catalog-backend/package.json index ce2a985efa..f32ffffb6c 100644 --- a/plugins/catalog-backend/package.json +++ b/plugins/catalog-backend/package.json @@ -38,7 +38,7 @@ "devDependencies": { "@backstage/cli": "^0.1.1-alpha.6", "@types/lodash": "^4.14.151", - "@types/uuid": "^7.0.3", + "@types/uuid": "^8.0.0", "@types/yup": "^0.28.2", "jest-fetch-mock": "^3.0.3", "supertest": "^4.0.2", diff --git a/yarn.lock b/yarn.lock index c2398e238c..a255b7355c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4510,10 +4510,10 @@ dependencies: source-map "^0.6.1" -"@types/uuid@^7.0.3": - version "7.0.3" - resolved "https://registry.npmjs.org/@types/uuid/-/uuid-7.0.3.tgz#45cd03e98e758f8581c79c535afbd4fc27ba7ac8" - integrity sha512-PUdqTZVrNYTNcIhLHkiaYzoOIaUi5LFg/XLerAdgvwQrUCx+oSbtoBze1AMyvYbcwzUSNC+Isl58SM4Sm/6COw== +"@types/uuid@^8.0.0": + version "8.0.0" + resolved "https://registry.npmjs.org/@types/uuid/-/uuid-8.0.0.tgz#165aae4819ad2174a17476dbe66feebd549556c0" + integrity sha512-xSQfNcvOiE5f9dyd4Kzxbof1aTrLobL278pGLKOZI6esGfZ7ts9Ka16CzIN6Y8hFHE1C7jIBZokULhK1bOgjRw== "@types/webpack-dev-server@*", "@types/webpack-dev-server@^3.10.0": version "3.10.1" From 147926fbd6a35a9583df6d9d03578c14419ac992 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Tue, 26 May 2020 23:01:05 +0200 Subject: [PATCH 18/21] build(deps): bump fork-ts-checker-webpack-plugin from 4.1.0 to 4.1.5 (#1009) Bumps [fork-ts-checker-webpack-plugin](https://github.com/TypeStrong/fork-ts-checker-webpack-plugin) from 4.1.0 to 4.1.5. - [Release notes](https://github.com/TypeStrong/fork-ts-checker-webpack-plugin/releases) - [Changelog](https://github.com/TypeStrong/fork-ts-checker-webpack-plugin/blob/master/CHANGELOG.md) - [Commits](https://github.com/TypeStrong/fork-ts-checker-webpack-plugin/compare/v4.1.0...v4.1.5) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- yarn.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/yarn.lock b/yarn.lock index a255b7355c..d535894e20 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9,7 +9,7 @@ dependencies: "@babel/highlight" "^7.0.0" -"@babel/code-frame@7.8.3", "@babel/code-frame@^7.0.0", "@babel/code-frame@^7.8.3": +"@babel/code-frame@7.8.3", "@babel/code-frame@^7.0.0", "@babel/code-frame@^7.5.5", "@babel/code-frame@^7.8.3": version "7.8.3" resolved "https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.8.3.tgz#33e25903d7481181534e12ec0a25f16b6fcf419e" integrity sha512-a9gxpmdXtZEInkCSHUJDLHZVBgb1QS0jhss4cPP93EW7s+uC5bikET2twEF3KV+7rDblJcmNvTR7VJejqd2C2g== @@ -9796,11 +9796,11 @@ fork-ts-checker-webpack-plugin@3.1.1: worker-rpc "^0.1.0" fork-ts-checker-webpack-plugin@^4.0.5: - version "4.1.0" - resolved "https://registry.npmjs.org/fork-ts-checker-webpack-plugin/-/fork-ts-checker-webpack-plugin-4.1.0.tgz#62bffe704426770fee33f15f0c0d56c86297fefd" - integrity sha512-2DLwUVUR/AdNmMD2utfmSR8r4qHRFhnfL6QQDQS5q4g5uBZzXYDgg8MXPIbu0HzyLjyvbogqjBNKILG5fufwzg== + version "4.1.5" + resolved "https://registry.npmjs.org/fork-ts-checker-webpack-plugin/-/fork-ts-checker-webpack-plugin-4.1.5.tgz#780d52c65183742d8c885fff42a9ec9ea7006672" + integrity sha512-nuD4IDqoOfkEIlS6shhjLGaLBDSNyVJulAlr5lFbPe0saGqlsTo+/HmhtIrs/cNLFtmaudL10byivhxr+Qhh4w== dependencies: - babel-code-frame "^6.22.0" + "@babel/code-frame" "^7.5.5" chalk "^2.4.1" micromatch "^3.1.10" minimatch "^3.0.4" From ec16280876c2c8c70b4af394f191423e5c955bf8 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 27 May 2020 08:51:48 +0200 Subject: [PATCH 19/21] build(deps-dev): bump typescript from 3.9.2 to 3.9.3 (#1024) Bumps [typescript](https://github.com/Microsoft/TypeScript) from 3.9.2 to 3.9.3. - [Release notes](https://github.com/Microsoft/TypeScript/releases) - [Commits](https://github.com/Microsoft/TypeScript/compare/v3.9.2...v3.9.3) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- yarn.lock | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/yarn.lock b/yarn.lock index d535894e20..fdca7768a5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -20469,15 +20469,10 @@ typedarray@^0.0.6: resolved "https://registry.npmjs.org/typedarray/-/typedarray-0.0.6.tgz#867ac74e3864187b1d3d47d996a78ec5c8830777" integrity sha1-hnrHTjhkGHsdPUfZlqeOxciDB3c= -typescript@^3.7.4: - version "3.8.3" - resolved "https://registry.npmjs.org/typescript/-/typescript-3.8.3.tgz#409eb8544ea0335711205869ec458ab109ee1061" - integrity sha512-MYlEfn5VrLNsgudQTVJeNaQFUAI7DkhnOjdpAp4T+ku1TfQClewlbSuTVHiA+8skNBgaf02TL/kLOvig4y3G8w== - -typescript@^3.9.2: - version "3.9.2" - resolved "https://registry.npmjs.org/typescript/-/typescript-3.9.2.tgz#64e9c8e9be6ea583c54607677dd4680a1cf35db9" - integrity sha512-q2ktq4n/uLuNNShyayit+DTobV2ApPEo/6so68JaD5ojvc/6GClBipedB9zNWYxRSAlZXAe405Rlijzl6qDiSw== +typescript@^3.7.4, typescript@^3.9.2: + version "3.9.3" + resolved "https://registry.npmjs.org/typescript/-/typescript-3.9.3.tgz#d3ac8883a97c26139e42df5e93eeece33d610b8a" + integrity sha512-D/wqnB2xzNFIcoBG9FG8cXRDjiqSTbG2wd8DMZeQyJlP1vfTkIxH4GKveWaEBYySKIg+USu+E+EDIR47SqnaMQ== uc.micro@^1.0.1, uc.micro@^1.0.5: version "1.0.6" From 4a93c608a333bbbff47da09ff9808111a126b176 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 27 May 2020 08:52:23 +0200 Subject: [PATCH 20/21] build(deps): bump webpack-dev-server from 3.10.3 to 3.11.0 (#1022) Bumps [webpack-dev-server](https://github.com/webpack/webpack-dev-server) from 3.10.3 to 3.11.0. - [Release notes](https://github.com/webpack/webpack-dev-server/releases) - [Changelog](https://github.com/webpack/webpack-dev-server/blob/master/CHANGELOG.md) - [Commits](https://github.com/webpack/webpack-dev-server/compare/v3.10.3...v3.11.0) Signed-off-by: dependabot-preview[bot] Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> --- yarn.lock | 133 ++++++++++++++++++++++++++---------------------------- 1 file changed, 63 insertions(+), 70 deletions(-) diff --git a/yarn.lock b/yarn.lock index fdca7768a5..772229aa98 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10903,10 +10903,10 @@ html-encoding-sniffer@^2.0.1: dependencies: whatwg-encoding "^1.0.5" -html-entities@^1.2.0, html-entities@^1.2.1: - version "1.2.1" - resolved "https://registry.npmjs.org/html-entities/-/html-entities-1.2.1.tgz#0df29351f0721163515dfb9e5543e5f6eed5162f" - integrity sha1-DfKTUfByEWNRXfueVUPl9u7VFi8= +html-entities@^1.2.0, html-entities@^1.3.1: + version "1.3.1" + resolved "https://registry.npmjs.org/html-entities/-/html-entities-1.3.1.tgz#fb9a1a4b5b14c5daba82d3e34c6ae4fe701a0e44" + integrity sha512-rhE/4Z3hIhzHAUKbW8jVcCyuT5oJCXXqhN/6mXXVCpzTmvJnoH2HL/bt3EZ6p55jbFJBeAe1ZNpL5BugLujxNA== html-escaper@^2.0.0: version "2.0.1" @@ -13958,10 +13958,10 @@ logform@^2.1.1: ms "^2.1.1" triple-beam "^1.3.0" -loglevel@^1.6.6: - version "1.6.7" - resolved "https://registry.npmjs.org/loglevel/-/loglevel-1.6.7.tgz#b3e034233188c68b889f5b862415306f565e2c56" - integrity sha512-cY2eLFrQSAfVPhCgH1s7JI73tMbg9YC3v3+ZHVW67sBS7UxWzNEk/ZBbSfLykBWHp33dqqtOv82gjhKEi81T/A== +loglevel@^1.6.8: + version "1.6.8" + resolved "https://registry.npmjs.org/loglevel/-/loglevel-1.6.8.tgz#8a25fb75d092230ecd4457270d80b54e28011171" + integrity sha512-bsU7+gc9AJ2SqpzxwU3+1fedl8zAntbtC5XYlt3s2j1hJcn2PsXSmgN8TaLG/J1/2mod4+cE/3vNL70/c1RNCA== lolex@^5.0.0: version "5.1.2" @@ -15594,7 +15594,7 @@ os-locale@^2.0.0: lcid "^1.0.0" mem "^1.1.0" -os-locale@^3.0.0, os-locale@^3.1.0: +os-locale@^3.1.0: version "3.1.0" resolved "https://registry.npmjs.org/os-locale/-/os-locale-3.1.0.tgz#a802a6ee17f24c10483ab9935719cef4ed16bf1a" integrity sha512-Z8l3R4wYWM40/52Z+S265okfFj8Kt2cC2MKY+xNi3kFs+XGI7WXu/I309QQQYbRW4ijiZ+yxs9pqEhJh0DqW3Q== @@ -16306,10 +16306,10 @@ popper.js@^1.14.1, popper.js@^1.14.4, popper.js@^1.14.7: resolved "https://registry.npmjs.org/popper.js/-/popper.js-1.16.1.tgz#2a223cb3dc7b6213d740e40372be40de43e65b1b" integrity sha512-Wb4p1J4zyFTbM+u6WuO4XstYx4Ky9Cewe4DWrel7B0w6VVICvPwdOpotjzcf6eD8TsckVnIMNONQyPIUFOUbCQ== -portfinder@^1.0.25: - version "1.0.25" - resolved "https://registry.npmjs.org/portfinder/-/portfinder-1.0.25.tgz#254fd337ffba869f4b9d37edc298059cb4d35eca" - integrity sha512-6ElJnHBbxVA1XSLgBp7G1FiCkQdlqGzuF7DswL5tcea+E8UpuvPU7beVAjjRwCioTS9ZluNbu+ZyRvgTsmqEBg== +portfinder@^1.0.26: + version "1.0.26" + resolved "https://registry.npmjs.org/portfinder/-/portfinder-1.0.26.tgz#475658d56ca30bed72ac7f1378ed350bd1b64e70" + integrity sha512-Xi7mKxJHHMI3rIUrnm/jjUgwhbYMkp/XKEcZX3aG4BrumLpq3nmoQMX+ClYnDZnZ/New7IatC1no5RX0zo1vXQ== dependencies: async "^2.6.2" debug "^3.1.1" @@ -18504,15 +18504,7 @@ schema-utils@^1.0.0: ajv-errors "^1.0.0" ajv-keywords "^3.1.0" -schema-utils@^2.0.1, schema-utils@^2.5.0, schema-utils@^2.6.0, schema-utils@^2.6.4: - version "2.6.5" - resolved "https://registry.npmjs.org/schema-utils/-/schema-utils-2.6.5.tgz#c758f0a7e624263073d396e29cd40aa101152d8a" - integrity sha512-5KXuwKziQrTVHh8j/Uxz+QUbxkaLW9X/86NBlx/gnKgtsZA2GIVMUn17qWhRFwF8jdYb3Dig5hRO/W5mZqy6SQ== - dependencies: - ajv "^6.12.0" - ajv-keywords "^3.4.1" - -schema-utils@^2.6.5, schema-utils@^2.6.6: +schema-utils@^2.0.1, schema-utils@^2.5.0, schema-utils@^2.6.0, schema-utils@^2.6.4, schema-utils@^2.6.5, schema-utils@^2.6.6: version "2.6.6" resolved "https://registry.npmjs.org/schema-utils/-/schema-utils-2.6.6.tgz#299fe6bd4a3365dc23d99fd446caff8f1d6c330c" integrity sha512-wHutF/WPSbIi9x6ctjGGk2Hvl0VOz5l3EKEuKbjPlB30mKZUzb9A5k9yEXRX3pwyqVLPvpfZZEllaFq/M718hA== @@ -18942,13 +18934,14 @@ sockjs-client@1.4.0: json3 "^3.3.2" url-parse "^1.4.3" -sockjs@0.3.19: - version "0.3.19" - resolved "https://registry.npmjs.org/sockjs/-/sockjs-0.3.19.tgz#d976bbe800af7bd20ae08598d582393508993c0d" - integrity sha512-V48klKZl8T6MzatbLlzzRNhMepEys9Y4oGFpypBFFn1gLI/QQ9HtLLyWJNbPlwGLelOVOEijUbTTJeLLI59jLw== +sockjs@0.3.20: + version "0.3.20" + resolved "https://registry.npmjs.org/sockjs/-/sockjs-0.3.20.tgz#b26a283ec562ef8b2687b44033a4eeceac75d855" + integrity sha512-SpmVOVpdq0DJc0qArhF3E5xsxvaiqGNb73XfgBpK1y3UD5gs8DSo8aCTsuT5pX8rssdc2NDIzANwP9eCAiSdTA== dependencies: faye-websocket "^0.10.0" - uuid "^3.0.1" + uuid "^3.4.0" + websocket-driver "0.6.5" socks-proxy-agent@^4.0.0: version "4.0.2" @@ -19095,10 +19088,10 @@ spdy-transport@^3.0.0: readable-stream "^3.0.6" wbuf "^1.7.3" -spdy@^4.0.1: - version "4.0.1" - resolved "https://registry.npmjs.org/spdy/-/spdy-4.0.1.tgz#6f12ed1c5db7ea4f24ebb8b89ba58c87c08257f2" - integrity sha512-HeZS3PBdMA+sZSu0qwpCxl3DeALD5ASx8pAX0jZdKXSpPWbQ6SYGnlg3BBmYLx5LtiZrmkAZfErCm2oECBcioA== +spdy@^4.0.2: + version "4.0.2" + resolved "https://registry.npmjs.org/spdy/-/spdy-4.0.2.tgz#b74f466203a3eda452c02492b91fb9e84a27677b" + integrity sha512-r46gZQZQV+Kl9oItvl1JZZqJKGr+oEkB08A6BzkiR7593/7IbtuncXHd2YoYeTsG4157ZssMu9KYvUHLcjcDoA== dependencies: debug "^4.1.0" handle-thing "^2.0.0" @@ -20892,7 +20885,7 @@ utils-merge@1.0.1, utils-merge@1.x.x: resolved "https://registry.npmjs.org/utils-merge/-/utils-merge-1.0.1.tgz#9f95710f50a267947b2ccc124741c1028427e713" integrity sha1-n5VxD1CiZ5R7LMwSR0HBAoQn5xM= -uuid@^3.0.1, uuid@^3.3.2, uuid@^3.3.3: +uuid@^3.0.1, uuid@^3.3.2, uuid@^3.3.3, uuid@^3.4.0: version "3.4.0" resolved "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz#b23e4358afa8a202fe7a100af1f5f883f02007ee" integrity sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A== @@ -21117,9 +21110,9 @@ webpack-dev-middleware@^3.7.0, webpack-dev-middleware@^3.7.2: webpack-log "^2.0.0" webpack-dev-server@^3.10.3: - version "3.10.3" - resolved "https://registry.npmjs.org/webpack-dev-server/-/webpack-dev-server-3.10.3.tgz#f35945036813e57ef582c2420ef7b470e14d3af0" - integrity sha512-e4nWev8YzEVNdOMcNzNeCN947sWJNd43E5XvsJzbAL08kGc2frm1tQ32hTJslRS+H65LCb/AaUCYU7fjHCpDeQ== + version "3.11.0" + resolved "https://registry.npmjs.org/webpack-dev-server/-/webpack-dev-server-3.11.0.tgz#8f154a3bce1bcfd1cc618ef4e703278855e7ff8c" + integrity sha512-PUxZ+oSTxogFQgkTtFndEtJIPNmml7ExwufBZ9L2/Xyyd5PnOL5UreWe5ZT7IU25DSdykL9p1MLQzmLh2ljSeg== dependencies: ansi-html "0.0.7" bonjour "^3.5.0" @@ -21129,31 +21122,31 @@ webpack-dev-server@^3.10.3: debug "^4.1.1" del "^4.1.1" express "^4.17.1" - html-entities "^1.2.1" + html-entities "^1.3.1" http-proxy-middleware "0.19.1" import-local "^2.0.0" internal-ip "^4.3.0" ip "^1.1.5" is-absolute-url "^3.0.3" killable "^1.0.1" - loglevel "^1.6.6" + loglevel "^1.6.8" opn "^5.5.0" p-retry "^3.0.1" - portfinder "^1.0.25" + portfinder "^1.0.26" schema-utils "^1.0.0" selfsigned "^1.10.7" semver "^6.3.0" serve-index "^1.9.1" - sockjs "0.3.19" + sockjs "0.3.20" sockjs-client "1.4.0" - spdy "^4.0.1" + spdy "^4.0.2" strip-ansi "^3.0.1" supports-color "^6.1.0" url "^0.11.0" webpack-dev-middleware "^3.7.2" webpack-log "^2.0.0" ws "^6.2.1" - yargs "12.0.5" + yargs "^13.3.2" webpack-hot-middleware@^2.25.0: version "2.25.0" @@ -21217,6 +21210,13 @@ webpack@^4.33.0, webpack@^4.38.0, webpack@^4.41.6: watchpack "^1.6.1" webpack-sources "^1.4.1" +websocket-driver@0.6.5: + version "0.6.5" + resolved "https://registry.npmjs.org/websocket-driver/-/websocket-driver-0.6.5.tgz#5cb2556ceb85f4373c6d8238aa691c8454e13a36" + integrity sha1-XLJVbOuF9Dc8bYI4qmkchFThOjY= + dependencies: + websocket-extensions ">=0.1.1" + websocket-driver@>=0.5.1: version "0.7.3" resolved "https://registry.npmjs.org/websocket-driver/-/websocket-driver-0.7.3.tgz#a2d4e0d4f4f116f1e6297eba58b05d430100e9f9" @@ -21490,12 +21490,7 @@ ws@^6.1.2, ws@^6.2.1: dependencies: async-limiter "~1.0.0" -ws@^7.0.0: - version "7.2.3" - resolved "https://registry.npmjs.org/ws/-/ws-7.2.3.tgz#a5411e1fb04d5ed0efee76d26d5c46d830c39b46" - integrity sha512-HTDl9G9hbkNDk98naoR/cHDws7+EyYMOdL1BmjsZXRUjf7d+MficC4B7HLUPlSiho0vg+CWKrGIt/VJBd1xunQ== - -ws@^7.2.3: +ws@^7.0.0, ws@^7.2.3: version "7.3.0" resolved "https://registry.npmjs.org/ws/-/ws-7.3.0.tgz#4b2f7f219b3d3737bc1a2fbf145d825b94d38ffd" integrity sha512-iFtXzngZVXPGgpTlP1rBqsUK82p9tKqsWRPg5L56egiljujJT3vGAYnHANvFxBieXrTFavhzhxW52jnaWV+w2w== @@ -21552,7 +21547,7 @@ y18n@^3.2.1: resolved "https://registry.npmjs.org/y18n/-/y18n-3.2.1.tgz#6d15fba884c08679c0d77e88e7759e811e07fa41" integrity sha1-bRX7qITAhnnA136I53WegR4H+kE= -"y18n@^3.2.1 || ^4.0.0", y18n@^4.0.0: +y18n@^4.0.0: version "4.0.0" resolved "https://registry.npmjs.org/y18n/-/y18n-4.0.0.tgz#95ef94f85ecc81d007c264e190a120f0a3c8566b" integrity sha512-r9S/ZyXu/Xu9q1tYlpsLIsa3EeLXXk0VwlxqTcFRfg9EhMW+17kbt9G0NrgCmhGb5vT2hyhJZLfDGx+7+5Uj/w== @@ -21601,10 +21596,10 @@ yargs-parser@^10.0.0: dependencies: camelcase "^4.1.0" -yargs-parser@^11.1.1: - version "11.1.1" - resolved "https://registry.npmjs.org/yargs-parser/-/yargs-parser-11.1.1.tgz#879a0865973bca9f6bab5cbdf3b1c67ec7d3bcf4" - integrity sha512-C6kB/WJDiaxONLJQnF8ccx9SEeoTTLek8RVbaOIsrAUS8VrBEXfmeSnCZxygc+XC2sNMBIwOOnfcxiynjHsVSQ== +yargs-parser@^13.1.2: + version "13.1.2" + resolved "https://registry.npmjs.org/yargs-parser/-/yargs-parser-13.1.2.tgz#130f09702ebaeef2650d54ce6e3e5706f7a4fb38" + integrity sha512-3lbsNRf/j+A4QuSZfDRA7HRSfWrzO0YjqTJd5kjAq37Zep1CEgaYmrH9Q3GwPiB9cHyd1Y1UwggGhJGoxipbzg== dependencies: camelcase "^5.0.0" decamelize "^1.2.0" @@ -21647,24 +21642,6 @@ yargs-parser@^9.0.2: dependencies: camelcase "^4.1.0" -yargs@12.0.5: - version "12.0.5" - resolved "https://registry.npmjs.org/yargs/-/yargs-12.0.5.tgz#05f5997b609647b64f66b81e3b4b10a368e7ad13" - integrity sha512-Lhz8TLaYnxq/2ObqHDql8dX8CJi97oHxrjUcYtzKbbykPtVW9WB+poxI+NM2UIzsMgNCZTIf0AQwsjK5yMAqZw== - dependencies: - cliui "^4.0.0" - decamelize "^1.2.0" - find-up "^3.0.0" - get-caller-file "^1.0.1" - os-locale "^3.0.0" - require-directory "^2.1.1" - require-main-filename "^1.0.1" - set-blocking "^2.0.0" - string-width "^2.0.0" - which-module "^2.0.0" - y18n "^3.2.1 || ^4.0.0" - yargs-parser "^11.1.1" - yargs@^11.0.0: version "11.1.1" resolved "https://registry.npmjs.org/yargs/-/yargs-11.1.1.tgz#5052efe3446a4df5ed669c995886cc0f13702766" @@ -21683,6 +21660,22 @@ yargs@^11.0.0: y18n "^3.2.1" yargs-parser "^9.0.2" +yargs@^13.3.2: + version "13.3.2" + resolved "https://registry.npmjs.org/yargs/-/yargs-13.3.2.tgz#ad7ffefec1aa59565ac915f82dccb38a9c31a2dd" + integrity sha512-AX3Zw5iPruN5ie6xGRIDgqkT+ZhnRlZMLMHAs8tg7nRruy2Nb+i5o9bwghAogtM08q1dpr2LVoS8KSTMYpWXUw== + dependencies: + cliui "^5.0.0" + find-up "^3.0.0" + get-caller-file "^2.0.1" + require-directory "^2.1.1" + require-main-filename "^2.0.0" + set-blocking "^2.0.0" + string-width "^3.0.0" + which-module "^2.0.0" + y18n "^4.0.0" + yargs-parser "^13.1.2" + yargs@^14.2.2: version "14.2.3" resolved "https://registry.npmjs.org/yargs/-/yargs-14.2.3.tgz#1a1c3edced1afb2a2fea33604bc6d1d8d688a414" From 6572ea3ecc777dbf0c5e146b9930ff778c7ea83d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 27 May 2020 08:53:59 +0200 Subject: [PATCH 21/21] packages/core: move LoginPage to layouts and remove it as a hardcoded route in the app (#1020) --- packages/app/src/App.tsx | 10 ++++++++-- packages/core/src/api/app/App.tsx | 5 ----- packages/core/src/index.ts | 1 + .../app => layout}/LoginPage/LoginPage.tsx | 18 ++++++++---------- .../src/{api/app => layout}/LoginPage/index.ts | 2 +- 5 files changed, 18 insertions(+), 18 deletions(-) rename packages/core/src/{api/app => layout}/LoginPage/LoginPage.tsx (92%) rename packages/core/src/{api/app => layout}/LoginPage/index.ts (93%) diff --git a/packages/app/src/App.tsx b/packages/app/src/App.tsx index b4d01e067b..b0ff8b14c7 100644 --- a/packages/app/src/App.tsx +++ b/packages/app/src/App.tsx @@ -14,9 +14,14 @@ * limitations under the License. */ -import { createApp, AlertDisplay, OAuthRequestDialog } from '@backstage/core'; +import { + createApp, + AlertDisplay, + OAuthRequestDialog, + LoginPage, +} from '@backstage/core'; import React, { FC } from 'react'; -import { BrowserRouter as Router } from 'react-router-dom'; +import { BrowserRouter as Router, Route } from 'react-router-dom'; import Root from './components/Root'; import * as plugins from './plugins'; import apis from './apis'; @@ -35,6 +40,7 @@ const App: FC<{}> = () => ( + , diff --git a/packages/core/src/api/app/App.tsx b/packages/core/src/api/app/App.tsx index 3d1faf0d56..afdb6ffc58 100644 --- a/packages/core/src/api/app/App.tsx +++ b/packages/core/src/api/app/App.tsx @@ -38,7 +38,6 @@ import { AppThemeSelector, appThemeApiRef, } from '../apis'; -import LoginPage from './LoginPage'; import { lightTheme, darkTheme } from '@backstage/theme'; import { ApiAggregator } from '../apis/ApiAggregator'; @@ -138,10 +137,6 @@ class AppImpl implements BackstageApp { FeatureFlags.registeredFeatureFlags = registeredFeatureFlags; } - routes.push( - , - ); - const rendered = ( {routes} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 816ad92578..fc6e2988bc 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -28,6 +28,7 @@ export { default as InfoCard } from './layout/InfoCard'; export { CardTab, TabbedCard } from './layout/TabbedCard'; export { default as ErrorBoundary } from './layout/ErrorBoundary'; export * from './layout/Sidebar'; +export * from './layout/LoginPage'; export { AlertDisplay } from './components/AlertDisplay'; export { default as HorizontalScrollGrid } from './components/HorizontalScrollGrid'; export { default as ProgressCard } from './components/ProgressBars/ProgressCard'; diff --git a/packages/core/src/api/app/LoginPage/LoginPage.tsx b/packages/core/src/layout/LoginPage/LoginPage.tsx similarity index 92% rename from packages/core/src/api/app/LoginPage/LoginPage.tsx rename to packages/core/src/layout/LoginPage/LoginPage.tsx index cdf0b55fd4..e7c83d563c 100644 --- a/packages/core/src/api/app/LoginPage/LoginPage.tsx +++ b/packages/core/src/layout/LoginPage/LoginPage.tsx @@ -16,10 +16,10 @@ import React, { FC, useState } from 'react'; import GitHubIcon from '@material-ui/icons/GitHub'; -import Page from '../../../layout/Page'; -import Header from '../../../layout/Header'; -import Content from '../../../layout/Content/Content'; -import ContentHeader from '../../../layout/ContentHeader/ContentHeader'; +import Page from '../Page'; +import Header from '../Header'; +import Content from '../Content/Content'; +import ContentHeader from '../ContentHeader/ContentHeader'; import { Grid, Typography, @@ -29,13 +29,13 @@ import { ListItem, Link, } from '@material-ui/core'; -import InfoCard from '../../../layout/InfoCard/InfoCard'; +import InfoCard from '../InfoCard/InfoCard'; enum AuthType { GitHub, } -const LoginPage: FC<{}> = () => { +export const LoginPage: FC<{}> = () => { const [githubUsername, setGithubUsername] = useState(String); const [githubPersonalAuthToken, setGithubPersonalAuthToken] = useState( String, @@ -72,11 +72,11 @@ const LoginPage: FC<{}> = () => { 'Content-Type': 'application/x-www-form-urlencoded', }), }) - .then((response) => { + .then(response => { if (response.status === 200) return response.json(); throw Error(`${response.status} ${response.statusText}`); }) - .then((data) => { + .then(data => { const info = { username: username, token: token, @@ -176,5 +176,3 @@ const LoginPage: FC<{}> = () => { ); }; - -export default LoginPage; diff --git a/packages/core/src/api/app/LoginPage/index.ts b/packages/core/src/layout/LoginPage/index.ts similarity index 93% rename from packages/core/src/api/app/LoginPage/index.ts rename to packages/core/src/layout/LoginPage/index.ts index 094029448c..caa94bd6d7 100644 --- a/packages/core/src/api/app/LoginPage/index.ts +++ b/packages/core/src/layout/LoginPage/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export { default } from './LoginPage'; +export { LoginPage } from './LoginPage';