From 6bee3ce985a5de59dd87d6d230e997bafb40822c Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Fri, 15 May 2020 14:25:07 +0200 Subject: [PATCH 1/8] feat: deduplicate locations on insert --- plugins/catalog-backend/package.json | 1 + .../src/catalog/DatabaseCatalog.test.ts | 16 +++++++++++++ .../src/catalog/DatabaseCatalog.ts | 24 +++++++++++++++---- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/plugins/catalog-backend/package.json b/plugins/catalog-backend/package.json index 629f4c6d18..477cf5e54f 100644 --- a/plugins/catalog-backend/package.json +++ b/plugins/catalog-backend/package.json @@ -21,6 +21,7 @@ "helmet": "^3.22.0", "knex": "^0.21.1", "morgan": "^1.10.0", + "sqlite3": "^4.2.0", "uuid": "^8.0.0", "winston": "^3.2.1", "yaml": "^1.9.2", diff --git a/plugins/catalog-backend/src/catalog/DatabaseCatalog.test.ts b/plugins/catalog-backend/src/catalog/DatabaseCatalog.test.ts index 98a5e9acd4..e244831bfd 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseCatalog.test.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseCatalog.test.ts @@ -60,4 +60,20 @@ describe('DatabaseCatalog', () => { /Found no location/, ); }); + + it('instead of adding second location with the same target, returns existing one', async () => { + // Prepare + const catalog = new DatabaseCatalog(database, logger); + const input = { type: 'a', target: 'b' }; + const output1 = await catalog.addLocation(input); + + // Try to insert the same location + const output2 = await catalog.addLocation(input); + const locations = await catalog.locations(); + + // Output is the same + expect(output2).toEqual(output1); + // Locations contain only one record + expect(locations).toEqual([output1]); + }); }); diff --git a/plugins/catalog-backend/src/catalog/DatabaseCatalog.ts b/plugins/catalog-backend/src/catalog/DatabaseCatalog.ts index 87ff5a503f..acf5e02272 100644 --- a/plugins/catalog-backend/src/catalog/DatabaseCatalog.ts +++ b/plugins/catalog-backend/src/catalog/DatabaseCatalog.ts @@ -53,7 +53,7 @@ export class DatabaseCatalog implements Catalog { name: descriptor.metadata.name, }; - await this.database.transaction(async (tx) => { + await this.database.transaction(async tx => { // TODO(freben): Currently, several locations can compete for the same component const count = await tx('components') .where({ name: component.name }) @@ -74,7 +74,9 @@ export class DatabaseCatalog implements Catalog { // eslint-disable-next-line @typescript-eslint/no-unused-vars async component(name: string): Promise { - const items = await this.database('components').where({ name }).select(); + const items = await this.database('components') + .where({ name }) + .select(); if (!items.length) { throw new NotFoundError(`Found no component with name ${name}`); } @@ -82,6 +84,10 @@ export class DatabaseCatalog implements Catalog { } async addLocation(location: AddLocationRequest): Promise { + const existingLocation = await this.locationByTarget(location.target); + if (existingLocation) { + return existingLocation; + } const id = uuidv4(); const { type, target } = location; await this.database('locations').insert({ id, type, target }); @@ -89,7 +95,9 @@ export class DatabaseCatalog implements Catalog { } async removeLocation(id: string): Promise { - const result = await this.database('locations').where({ id }).del(); + const result = await this.database('locations') + .where({ id }) + .del(); if (!result) { throw new NotFoundError(`Found no location with ID ${id}`); @@ -97,13 +105,21 @@ export class DatabaseCatalog implements Catalog { } async location(id: string): Promise { - const items = await this.database('locations').where({ id }).select(); + const items = await this.database('locations') + .where({ id }) + .select(); if (!items.length) { throw new NotFoundError(`Found no location with ID ${id}`); } return items[0]; } + private async locationByTarget( + target: string, + ): Promise { + return (await this.locations()).find(l => l.target === target); + } + async locations(): Promise { return await this.database('locations').select(); } From 4ca8e3370e079592b8cd8506c735b5b96a2ab805 Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Mon, 18 May 2020 13:10:10 +0200 Subject: [PATCH 2/8] fix: moved into transaction for atomicity --- .../catalog-backend/src/database/Database.ts | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/Database.ts index fba8728fcd..9b37980c04 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/Database.ts @@ -60,19 +60,31 @@ export class Database { } async addLocation(location: AddDatabaseLocation): Promise { - const existingLocation = await this.locationByTarget(location.target); - if (existingLocation) { - return existingLocation; - } + return await this.database.transaction(async tx => { + const existingLocation = await tx('locations') + .where({ + target: location.target, + }) + .select(); - const id = uuidv4(); - const { type, target } = location; - await this.database('locations').insert({ - id, - type, - target, + if (existingLocation?.[0]) { + return existingLocation[0]; + } + + const id = uuidv4(); + const { type, target } = location; + await tx('locations').insert({ + id, + type, + target, + }); + + return ( + await tx('locations') + .where({ id }) + .select() + )?.[0]; }); - return await this.location(id); } async removeLocation(id: string): Promise { From 6084eeeb4343b05d638cddca269358c1c0461329 Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Mon, 18 May 2020 17:52:51 +0200 Subject: [PATCH 3/8] fix: leftovers --- plugins/catalog-backend/src/database/Database.ts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/Database.ts index 9b37980c04..04755e8f4d 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/Database.ts @@ -28,7 +28,7 @@ export class Database { constructor(private readonly database: Knex) {} async addOrUpdateComponent(component: AddDatabaseComponent): Promise { - await this.database.transaction(async tx => { + await this.database.transaction(async (tx) => { // TODO(freben): Currently, several locations can compete for the same component // TODO(freben): If locationId is unset in the input, it won't be overwritten - should we instead replace with null? const count = await tx('components') @@ -60,7 +60,7 @@ export class Database { } async addLocation(location: AddDatabaseLocation): Promise { - return await this.database.transaction(async tx => { + return await this.database.transaction(async (tx) => { const existingLocation = await tx('locations') .where({ target: location.target, @@ -80,9 +80,7 @@ export class Database { }); return ( - await tx('locations') - .where({ id }) - .select() + await tx('locations').where({ id }).select() )?.[0]; }); } @@ -107,12 +105,6 @@ export class Database { return items[0]; } - private async locationByTarget( - target: string, - ): Promise { - return (await this.locations()).find(l => l.target === target); - } - async locations(): Promise { return await this.database('locations').select(); } From 8bd06d9882ef932f95fc094b7710300b47afa03e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 18 May 2020 20:09:35 +0200 Subject: [PATCH 4/8] packages/core: add unimplemented auth api definitions --- .../core/src/api/apis/definitions/auth.ts | 149 ++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 packages/core/src/api/apis/definitions/auth.ts diff --git a/packages/core/src/api/apis/definitions/auth.ts b/packages/core/src/api/apis/definitions/auth.ts new file mode 100644 index 0000000000..8d44dda00b --- /dev/null +++ b/packages/core/src/api/apis/definitions/auth.ts @@ -0,0 +1,149 @@ +/* + * 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 { ApiRef } from '../ApiRef'; + +/** + * An array of scopes, or a scope string formatted according to the + * auth provider, which is typically a space separated list. + * + * See the documentation for each auth provider for the list of scopes + * supported by each provider. + */ +export type OAuthScopeLike = string | string[]; + +/** + * This file contains declarations for common interfaces of auth-related APIs. + * The declarations should be used to signal which type of authentication and + * authorization methods each separate auth provider supports. + * + * For example, a Google OAuth provider that supports OAuth 2 and OpenID Connect, + * would be declared as follows: + * + * const googleAuthApiRef = new ApiRef({ ... }) + */ + +export type AccessTokenOptions = { + // If this is set to true, the user will not be prompted to log in, + // and an empty access token will be returned if there is no existing session. + // + // This can be used to perform a check whether the user is logged in with a set of scopes, + // or if you don't want to force a user to be logged in, but provide functionality if they already are. + optional?: boolean; + + // If this is set to true, the request will bypass the regular oauth login modal + // and open the login popup directly. + // + // The method must be called synchronously from a user action for this to work in all browsers. + instantPopup?: boolean; +}; + +/** + * This API provides access to OAuth 2 credentials. It lets you request access tokens, + * which can be used to act on behalf of the user when talking to APIs. + */ +export type OAuthApi = { + /** + * Requests an OAuth 2 Access Token, optionally with a set of scopes. The access token allows + * you to make requests on behalf of the user, and the copes may grant you broader access, depending + * on the auth provider. + * + * Each auth provider has separate handling of scope, so you need to look at the documentation + * for each one to know what scope you need to request. + * + * This method is cheap and should be called each time an access token is used. Do not for example + * store the access token in React component state, as that could cause the token to expire. Instead + * fetch a new access token for each request. + * + * Be sure to include all required scopes when requesting an access token. When testing your implementation + * it is best to log out the Backstage session and then visit your plugin page directly, as + * you might already have some required scopes in your existing session. Not requesting the correct + * scopes can lead to 403 or other authorization errors, which can be tricky to debug. + * + * If the user has not yet granted access to the provider and the set of requested scopes, the user + * will be prompted to log in. The returned promise will not resolve until the user has + * successfully logged in. The returned promise can be rejected, but only if the user rejects the login request. + */ + getAccessToken(scope?: OAuthScopeLike): Promise; + + /** + * Log out the user's session. This will reload the page. + */ + logout(): Promise; +}; + +export type IdTokenOptions = { + // If this is set to true, the user will not be prompted to log in, + // and an empty id token will be returned if there is no existing session. + // + // This can be used to perform a check whether the user is logged in, or if you don't + // want to force a user to be logged in, but provide functionality if they already are. + optional?: boolean; + + // If this is set to true, the request will bypass the regular oauth login modal + // and open the login popup directly. + // + // The method must be called synchronously from a user action for this to work in all browsers. + instantPopup?: boolean; +}; + +/** + * This API provides access to OpenID Connect credentials. It lets you request ID tokens, + * which can be passed to backend services to prove the user's identity. + */ +export type OpenIdConnectApi = { + /** + * Requests an OpenID Connect ID Token. + * + * This method is cheap and should be called each time an ID token is used. Do not for example + * store the id token in React component state, as that could cause the token to expire. Instead + * fetch a new id token for each request. + * + * If the user has not yet logged in to Google inside Backstage, the user will be prompted + * to log in. The returned promise will not resolve until the user has successfully logged in. + * The returned promise can be rejected, but only if the user rejects the login request. + */ + getIdToken(options?: IdTokenOptions): Promise; + + /** + * Log out the user's session. This will reload the page. + */ + logout(): Promise; +}; + +/** + * Provides authentication towards Google APIs and identities. + * + * See https://developers.google.com/identity/protocols/googlescopes for a full list of supported scopes. + * + * Note that the ID token payload is only guaranteed to contain the user's numerical Google ID, + * email and expiration information. Do not rely on any other fields, as they might not be present. + */ +export const googleAuthApiRef = new ApiRef({ + id: 'core.auth.google', + description: 'Provides authentication towards Google APIs and identities', +}); + +/** + * Provides authentication towards Github APIs. + * + * See https://developer.github.com/apps/building-oauth-apps/understanding-scopes-for-oauth-apps/ + * for a full list of supported scopes. + */ +export const githubAuthApiRef = new ApiRef({ + id: 'core.auth.github', + description: 'Provides authentication towards Github APIs', +}); From b9bd2fd52363ef92c49990ef0818c9f1f3e0ea4f Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Tue, 19 May 2020 11:03:01 +0200 Subject: [PATCH 5/8] fix: throw if error --- plugins/catalog-backend/src/database/Database.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/catalog-backend/src/database/Database.ts b/plugins/catalog-backend/src/database/Database.ts index 04755e8f4d..39e075efb8 100644 --- a/plugins/catalog-backend/src/database/Database.ts +++ b/plugins/catalog-backend/src/database/Database.ts @@ -81,7 +81,7 @@ export class Database { return ( await tx('locations').where({ id }).select() - )?.[0]; + )![0]; }); } From 534607ec5d9076950ea8867114ab006f876f5e17 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 May 2020 11:46:00 +0200 Subject: [PATCH 6/8] pacakges/core: auth api difinitions review feedback --- .../core/src/api/apis/definitions/auth.ts | 72 +++++++++++-------- 1 file changed, 44 insertions(+), 28 deletions(-) diff --git a/packages/core/src/api/apis/definitions/auth.ts b/packages/core/src/api/apis/definitions/auth.ts index 8d44dda00b..092b496488 100644 --- a/packages/core/src/api/apis/definitions/auth.ts +++ b/packages/core/src/api/apis/definitions/auth.ts @@ -16,15 +16,6 @@ import { ApiRef } from '../ApiRef'; -/** - * An array of scopes, or a scope string formatted according to the - * auth provider, which is typically a space separated list. - * - * See the documentation for each auth provider for the list of scopes - * supported by each provider. - */ -export type OAuthScopeLike = string | string[]; - /** * This file contains declarations for common interfaces of auth-related APIs. * The declarations should be used to signal which type of authentication and @@ -36,18 +27,35 @@ export type OAuthScopeLike = string | string[]; * const googleAuthApiRef = new ApiRef({ ... }) */ +/** + * An array of scopes, or a scope string formatted according to the + * auth provider, which is typically a space separated list. + * + * See the documentation for each auth provider for the list of scopes + * supported by each provider. + */ +export type OAuthScope = string | string[]; + export type AccessTokenOptions = { - // If this is set to true, the user will not be prompted to log in, - // and an empty access token will be returned if there is no existing session. - // - // This can be used to perform a check whether the user is logged in with a set of scopes, - // or if you don't want to force a user to be logged in, but provide functionality if they already are. + /** + * If this is set to true, the user will not be prompted to log in, + * and an empty access token will be returned if there is no existing session. + * + * This can be used to perform a check whether the user is logged in with a set of scopes, + * or if you don't want to force a user to be logged in, but provide functionality if they already are. + * + * @default false + */ optional?: boolean; - // If this is set to true, the request will bypass the regular oauth login modal - // and open the login popup directly. - // - // The method must be called synchronously from a user action for this to work in all browsers. + /** + * If this is set to true, the request will bypass the regular oauth login modal + * and open the login popup directly. + * + * The method must be called synchronously from a user action for this to work in all browsers. + * + * @default false + */ instantPopup?: boolean; }; @@ -77,7 +85,7 @@ export type OAuthApi = { * will be prompted to log in. The returned promise will not resolve until the user has * successfully logged in. The returned promise can be rejected, but only if the user rejects the login request. */ - getAccessToken(scope?: OAuthScopeLike): Promise; + getAccessToken(scope?: OAuthScope): Promise; /** * Log out the user's session. This will reload the page. @@ -86,17 +94,25 @@ export type OAuthApi = { }; export type IdTokenOptions = { - // If this is set to true, the user will not be prompted to log in, - // and an empty id token will be returned if there is no existing session. - // - // This can be used to perform a check whether the user is logged in, or if you don't - // want to force a user to be logged in, but provide functionality if they already are. + /** + * If this is set to true, the user will not be prompted to log in, + * and an empty id token will be returned if there is no existing session. + * + * This can be used to perform a check whether the user is logged in, or if you don't + * want to force a user to be logged in, but provide functionality if they already are. + * + * @default false + */ optional?: boolean; - // If this is set to true, the request will bypass the regular oauth login modal - // and open the login popup directly. - // - // The method must be called synchronously from a user action for this to work in all browsers. + /** + * If this is set to true, the request will bypass the regular oauth login modal + * and open the login popup directly. + * + * The method must be called synchronously from a user action for this to work in all browsers. + * + * @default false + */ instantPopup?: boolean; }; From 2752bc412009d6e8fc2177cef0562a55492b929e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 19 May 2020 11:48:16 +0200 Subject: [PATCH 7/8] package/core: removed unused internal interface in OAuthPendingRequests (#917) --- .../OAuthRequestManager/OAuthPendingRequests.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts index 96aada1864..e8dd0e12ef 100644 --- a/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts +++ b/packages/core/src/api/apis/implementations/OAuthRequestManager/OAuthPendingRequests.ts @@ -30,20 +30,12 @@ export type PendingRequest = { reject: (reason: Error) => void; }; -export type OAuthPendingRequestsApi = { - request(scopes: OAuthScopes): Promise; - resolve(scopes: OAuthScopes, result: ResultType): void; - reject(error: Error): void; - pending(): Observable>; -}; - /** * The OAuthPendingRequests class is a utility for managing and observing * a stream of requests for oauth scopes for a single provider, and resolving * them correctly once requests are fulfilled. */ -export class OAuthPendingRequests - implements OAuthPendingRequestsApi { +export class OAuthPendingRequests { private requests: RequestQueueEntry[] = []; private subject = new BehaviorSubject>( this.getCurrentPending(), From 9a7d5518d52acb99ff52ffa361623cbb1398cb35 Mon Sep 17 00:00:00 2001 From: nikek Date: Tue, 19 May 2020 13:49:40 +0200 Subject: [PATCH 8/8] make chromatic workflow work for forks --- .github/workflows/chromatic-storybook-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/chromatic-storybook-test.yml b/.github/workflows/chromatic-storybook-test.yml index 34c3c9f7d6..e38b494a73 100644 --- a/.github/workflows/chromatic-storybook-test.yml +++ b/.github/workflows/chromatic-storybook-test.yml @@ -18,5 +18,5 @@ jobs: - uses: chromaui/action@v1 with: token: ${{ secrets.GITHUB_TOKEN }} - projectToken: ${{ secrets.CHROMATIC_PROJECT_TOKEN }} + projectToken: 9tzak77m9nj storybookBuildDir: 'packages/storybook/dist'