From 813c6a4f20967e34af054c98693adb5257d40486 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Mon, 15 Feb 2021 22:12:49 +0100 Subject: [PATCH 1/7] add authorization to techdocs api requests --- .changeset/old-coats-suffer.md | 5 +++++ plugins/techdocs/dev/api.ts | 10 +++++++++- plugins/techdocs/dev/index.tsx | 11 ++++++++--- plugins/techdocs/src/api.ts | 24 +++++++++++++++++++++--- plugins/techdocs/src/plugin.ts | 19 +++++++++++++++---- 5 files changed, 58 insertions(+), 11 deletions(-) create mode 100644 .changeset/old-coats-suffer.md diff --git a/.changeset/old-coats-suffer.md b/.changeset/old-coats-suffer.md new file mode 100644 index 0000000000..8391f77ee0 --- /dev/null +++ b/.changeset/old-coats-suffer.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs': minor +--- + +Add authorization header on techdocs api requests diff --git a/plugins/techdocs/dev/api.ts b/plugins/techdocs/dev/api.ts index 7d0679f6f6..add1415526 100644 --- a/plugins/techdocs/dev/api.ts +++ b/plugins/techdocs/dev/api.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { DiscoveryApi } from '@backstage/core'; +import { DiscoveryApi, IdentityApi } from '@backstage/core'; import { Config } from '@backstage/config'; import { EntityName } from '@backstage/catalog-model'; import { TechDocsStorage } from '../src/api'; @@ -21,16 +21,20 @@ import { TechDocsStorage } from '../src/api'; export class TechDocsDevStorageApi implements TechDocsStorage { public configApi: Config; public discoveryApi: DiscoveryApi; + public identityApi: IdentityApi; constructor({ configApi, discoveryApi, + identityApi, }: { configApi: Config; discoveryApi: DiscoveryApi; + identityApi: IdentityApi; }) { this.configApi = configApi; this.discoveryApi = discoveryApi; + this.identityApi = identityApi; } async getApiOrigin() { @@ -45,9 +49,13 @@ export class TechDocsDevStorageApi implements TechDocsStorage { const apiOrigin = await this.getApiOrigin(); const url = `${apiOrigin}/${name}/${path}`; + const idToken = await this.identityApi.getIdToken(); const request = await fetch( `${url.endsWith('/') ? url : `${url}/`}index.html`, + { + headers: idToken ? { Authorization: `Bearer ${idToken}` } : {}, + }, ); if (request.status === 404) { diff --git a/plugins/techdocs/dev/index.tsx b/plugins/techdocs/dev/index.tsx index 4cf74c9622..3eefa0a814 100644 --- a/plugins/techdocs/dev/index.tsx +++ b/plugins/techdocs/dev/index.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import { configApiRef, discoveryApiRef } from '@backstage/core'; +import { configApiRef, discoveryApiRef, identityApiRef } from '@backstage/core'; import { createDevApp } from '@backstage/dev-utils'; import { techdocsPlugin } from '../src/plugin'; import { TechDocsDevStorageApi } from './api'; @@ -23,11 +23,16 @@ import { techdocsStorageApiRef } from '../src'; createDevApp() .registerApi({ api: techdocsStorageApiRef, - deps: { configApi: configApiRef, discoveryApi: discoveryApiRef }, - factory: ({ configApi, discoveryApi }) => + deps: { + configApi: configApiRef, + discoveryApi: discoveryApiRef, + identityApi: identityApiRef, + }, + factory: ({ configApi, discoveryApi, identityApi }) => new TechDocsDevStorageApi({ configApi, discoveryApi, + identityApi, }), }) .registerPlugin(techdocsPlugin) diff --git a/plugins/techdocs/src/api.ts b/plugins/techdocs/src/api.ts index 7f77be44e2..5518ab4be8 100644 --- a/plugins/techdocs/src/api.ts +++ b/plugins/techdocs/src/api.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { createApiRef, DiscoveryApi } from '@backstage/core'; +import { createApiRef, DiscoveryApi, IdentityApi } from '@backstage/core'; import { Config } from '@backstage/config'; import { EntityName } from '@backstage/catalog-model'; import { TechDocsMetadata } from './types'; @@ -51,16 +51,20 @@ export interface TechDocs { export class TechDocsApi implements TechDocs { public configApi: Config; public discoveryApi: DiscoveryApi; + public identityApi: IdentityApi; constructor({ configApi, discoveryApi, + identityApi, }: { configApi: Config; discoveryApi: DiscoveryApi; + identityApi: IdentityApi; }) { this.configApi = configApi; this.discoveryApi = discoveryApi; + this.identityApi = identityApi; } async getApiOrigin() { @@ -84,8 +88,11 @@ export class TechDocsApi implements TechDocs { const apiOrigin = await this.getApiOrigin(); const requestUrl = `${apiOrigin}/metadata/techdocs/${namespace}/${kind}/${name}`; + const idToken = await this.identityApi.getIdToken(); - const request = await fetch(`${requestUrl}`); + const request = await fetch(`${requestUrl}`, { + headers: idToken ? { Authorization: `Bearer ${idToken}` } : {}, + }); const res = await request.json(); return res; @@ -104,8 +111,11 @@ export class TechDocsApi implements TechDocs { const apiOrigin = await this.getApiOrigin(); const requestUrl = `${apiOrigin}/metadata/entity/${namespace}/${kind}/${name}`; + const idToken = await this.identityApi.getIdToken(); - const request = await fetch(`${requestUrl}`); + const request = await fetch(`${requestUrl}`, { + headers: idToken ? { Authorization: `Bearer ${idToken}` } : {}, + }); const res = await request.json(); return res; @@ -120,16 +130,20 @@ export class TechDocsApi implements TechDocs { export class TechDocsStorageApi implements TechDocsStorage { public configApi: Config; public discoveryApi: DiscoveryApi; + public identityApi: IdentityApi; constructor({ configApi, discoveryApi, + identityApi, }: { configApi: Config; discoveryApi: DiscoveryApi; + identityApi: IdentityApi; }) { this.configApi = configApi; this.discoveryApi = discoveryApi; + this.identityApi = identityApi; } async getApiOrigin() { @@ -152,9 +166,13 @@ export class TechDocsStorageApi implements TechDocsStorage { const apiOrigin = await this.getApiOrigin(); const url = `${apiOrigin}/docs/${namespace}/${kind}/${name}/${path}`; + const idToken = await this.identityApi.getIdToken(); const request = await fetch( `${url.endsWith('/') ? url : `${url}/`}index.html`, + { + headers: idToken ? { Authorization: `Bearer ${idToken}` } : {}, + }, ); let errorMessage = ''; diff --git a/plugins/techdocs/src/plugin.ts b/plugins/techdocs/src/plugin.ts index 0b4063c237..a945ca1494 100644 --- a/plugins/techdocs/src/plugin.ts +++ b/plugins/techdocs/src/plugin.ts @@ -35,6 +35,7 @@ import { createApiFactory, configApiRef, discoveryApiRef, + identityApiRef, createRoutableExtension, } from '@backstage/core'; import { @@ -64,20 +65,30 @@ export const techdocsPlugin = createPlugin({ apis: [ createApiFactory({ api: techdocsStorageApiRef, - deps: { configApi: configApiRef, discoveryApi: discoveryApiRef }, - factory: ({ configApi, discoveryApi }) => + deps: { + configApi: configApiRef, + discoveryApi: discoveryApiRef, + identityApi: identityApiRef, + }, + factory: ({ configApi, discoveryApi, identityApi }) => new TechDocsStorageApi({ configApi, discoveryApi, + identityApi, }), }), createApiFactory({ api: techdocsApiRef, - deps: { configApi: configApiRef, discoveryApi: discoveryApiRef }, - factory: ({ configApi, discoveryApi }) => + deps: { + configApi: configApiRef, + discoveryApi: discoveryApiRef, + identityApi: identityApiRef, + }, + factory: ({ configApi, discoveryApi, identityApi }) => new TechDocsApi({ configApi, discoveryApi, + identityApi, }), }), ], From f18b51d9326369d734362b406003f0cb4717e98a Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Thu, 18 Feb 2021 03:35:45 +0100 Subject: [PATCH 2/7] set and use access-token cookie --- .../tutorials/authenticate-api-requests.md | 6 +++- .../src/lib/oauth/OAuthAdapter.ts | 33 +++++++++++++++++++ .../techdocs-backend/src/service/router.ts | 16 ++++++++- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/contrib/docs/tutorials/authenticate-api-requests.md b/contrib/docs/tutorials/authenticate-api-requests.md index 2b8407abd2..4698293152 100644 --- a/contrib/docs/tutorials/authenticate-api-requests.md +++ b/contrib/docs/tutorials/authenticate-api-requests.md @@ -11,6 +11,7 @@ Caveat: as of writing this, Backstage does not refresh the identity token so eve ```typescript // packages/backend/src/index.ts from a create-app deployment +import cookieParser from 'cookie-parser'; import { Request, Response, NextFunction } from 'express'; import { IdentityClient } from '@backstage/plugin-auth-backend'; @@ -30,7 +31,9 @@ async function main() { next: NextFunction, ) => { try { - const token = IdentityClient.getBearerToken(req.headers.authorization); + const token = + IdentityClient.getBearerToken(req.headers.authorization) || + req.cookies['access-token']; req.user = await identity.authenticate(token); next(); } catch (error) { @@ -39,6 +42,7 @@ async function main() { }; const apiRouter = Router(); + apiRouter.use(cookieParser()); // The auth route must be publically available as it is used during login apiRouter.use('/auth', await auth(authEnv)); // Only authenticated requests are allowed to the routes below diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index 62cb8e444d..2da831950a 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -16,6 +16,7 @@ import express from 'express'; import crypto from 'crypto'; +import { JWT } from 'jose'; import { URL } from 'url'; import { AuthProviderRouteHandlers, @@ -127,6 +128,9 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { } await this.populateIdentity(response.backstageIdentity); + if (response.backstageIdentity?.idToken) { + this.setAccessTokenCookie(res, response.backstageIdentity?.idToken); + } // post message back to popup if successful return postMessageResponse(res, this.options.appOrigin, { @@ -151,6 +155,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { return; } + this.removeAccessTokenCookie(res); if (!this.options.disableRefresh) { // remove refresh token cookie before logout this.removeRefreshTokenCookie(res); @@ -190,6 +195,9 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { ); await this.populateIdentity(response.backstageIdentity); + if (response.backstageIdentity?.idToken) { + this.setAccessTokenCookie(res, response.backstageIdentity?.idToken); + } if ( response.providerInfo.refreshToken && @@ -246,6 +254,20 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { return req.cookies[`${providerId}-scope`]; }; + private setAccessTokenCookie = ( + res: express.Response, + accessToken: string, + ) => { + res.cookie(`access-token`, accessToken, { + expires: new Date(JWT.decode(accessToken).exp * 1000), + secure: this.options.secure, + sameSite: 'lax', + domain: this.options.cookieDomain, + path: '/api', + httpOnly: true, + }); + }; + private setRefreshTokenCookie = ( res: express.Response, refreshToken: string, @@ -260,6 +282,17 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { }); }; + private removeAccessTokenCookie = (res: express.Response) => { + res.cookie(`access-token`, '', { + maxAge: 0, + secure: this.options.secure, + sameSite: 'lax', + domain: this.options.cookieDomain, + path: '/api', + httpOnly: true, + }); + }; + private removeRefreshTokenCookie = (res: express.Response) => { res.cookie(`${this.options.providerId}-refresh-token`, '', { maxAge: 0, diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index ef032e9f8b..885a1a64af 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -82,9 +82,15 @@ export async function createRouter({ const { kind, namespace, name } = req.params; try { + const token = + getBearerToken(req.headers.authorization) || + req.cookies['access-token']; const entity = (await ( await fetch( `${catalogUrl}/entities/by-name/${kind}/${namespace}/${name}`, + { + headers: token ? { Authorization: `Bearer ${token}` } : {}, + }, ) ).json()) as Entity; @@ -109,7 +115,11 @@ export async function createRouter({ const catalogUrl = await discovery.getBaseUrl('catalog'); const triple = [kind, namespace, name].map(encodeURIComponent).join('/'); - const catalogRes = await fetch(`${catalogUrl}/entities/by-name/${triple}`); + const token = + getBearerToken(req.headers.authorization) || req.cookies['access-token']; + const catalogRes = await fetch(`${catalogUrl}/entities/by-name/${triple}`, { + headers: token ? { Authorization: `Bearer ${token}` } : {}, + }); if (!catalogRes.ok) { const catalogResText = await catalogRes.text(); res.status(catalogRes.status); @@ -202,3 +212,7 @@ export async function createRouter({ return router; } + +function getBearerToken(header?: string): string | undefined { + return header?.match(/(?:Bearer|Basic)\s+(\S+)/i)?.[1]; +} From 8d6e2fd36b75276644d0a266daa07d5bc7c86ca4 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Thu, 18 Feb 2021 03:43:28 +0100 Subject: [PATCH 3/7] tsc --- plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index 2da831950a..c4476752b6 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -258,8 +258,11 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { res: express.Response, accessToken: string, ) => { + const payload = JWT.decode(accessToken) as object & { + exp: number; + }; res.cookie(`access-token`, accessToken, { - expires: new Date(JWT.decode(accessToken).exp * 1000), + expires: new Date(payload?.exp ? payload?.exp * 1000 : 0), secure: this.options.secure, sameSite: 'lax', domain: this.options.cookieDomain, From afebbadd34ade5b1c9e5e57846f71efd3636e995 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Thu, 18 Feb 2021 04:05:50 +0100 Subject: [PATCH 4/7] use token instead of idToken --- plugins/techdocs/dev/api.ts | 4 ++-- plugins/techdocs/src/api.ts | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/plugins/techdocs/dev/api.ts b/plugins/techdocs/dev/api.ts index add1415526..5fa494a973 100644 --- a/plugins/techdocs/dev/api.ts +++ b/plugins/techdocs/dev/api.ts @@ -49,12 +49,12 @@ export class TechDocsDevStorageApi implements TechDocsStorage { const apiOrigin = await this.getApiOrigin(); const url = `${apiOrigin}/${name}/${path}`; - const idToken = await this.identityApi.getIdToken(); + const token = await this.identityApi.getIdToken(); const request = await fetch( `${url.endsWith('/') ? url : `${url}/`}index.html`, { - headers: idToken ? { Authorization: `Bearer ${idToken}` } : {}, + headers: token ? { Authorization: `Bearer ${token}` } : {}, }, ); diff --git a/plugins/techdocs/src/api.ts b/plugins/techdocs/src/api.ts index 5518ab4be8..6de94b63bb 100644 --- a/plugins/techdocs/src/api.ts +++ b/plugins/techdocs/src/api.ts @@ -88,10 +88,10 @@ export class TechDocsApi implements TechDocs { const apiOrigin = await this.getApiOrigin(); const requestUrl = `${apiOrigin}/metadata/techdocs/${namespace}/${kind}/${name}`; - const idToken = await this.identityApi.getIdToken(); + const token = await this.identityApi.getIdToken(); const request = await fetch(`${requestUrl}`, { - headers: idToken ? { Authorization: `Bearer ${idToken}` } : {}, + headers: token ? { Authorization: `Bearer ${token}` } : {}, }); const res = await request.json(); @@ -111,10 +111,10 @@ export class TechDocsApi implements TechDocs { const apiOrigin = await this.getApiOrigin(); const requestUrl = `${apiOrigin}/metadata/entity/${namespace}/${kind}/${name}`; - const idToken = await this.identityApi.getIdToken(); + const token = await this.identityApi.getIdToken(); const request = await fetch(`${requestUrl}`, { - headers: idToken ? { Authorization: `Bearer ${idToken}` } : {}, + headers: token ? { Authorization: `Bearer ${token}` } : {}, }); const res = await request.json(); @@ -166,12 +166,12 @@ export class TechDocsStorageApi implements TechDocsStorage { const apiOrigin = await this.getApiOrigin(); const url = `${apiOrigin}/docs/${namespace}/${kind}/${name}/${path}`; - const idToken = await this.identityApi.getIdToken(); + const token = await this.identityApi.getIdToken(); const request = await fetch( `${url.endsWith('/') ? url : `${url}/`}index.html`, { - headers: idToken ? { Authorization: `Bearer ${idToken}` } : {}, + headers: token ? { Authorization: `Bearer ${token}` } : {}, }, ); From 145205a14bf5a27cb6e2687aa986fb7c5d3f557a Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Thu, 18 Feb 2021 04:19:38 +0100 Subject: [PATCH 5/7] fix tests --- .../src/lib/oauth/OAuthAdapter.test.ts | 9 +++++-- .../src/lib/oauth/OAuthAdapter.ts | 26 +++++++++++-------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts index d2b31213f2..7d51f152f8 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts @@ -160,7 +160,7 @@ describe('OAuthAdapter', () => { expect(mockResponse.cookie).toHaveBeenCalledTimes(0); }); - it('removes refresh cookie when logging out', async () => { + it('removes access and refresh cookies when logging out', async () => { const oauthProvider = new OAuthAdapter(providerInstance, { ...oAuthProviderOptions, disableRefresh: false, @@ -176,7 +176,12 @@ describe('OAuthAdapter', () => { } as unknown) as express.Response; await oauthProvider.logout(mockRequest, mockResponse); - expect(mockResponse.cookie).toHaveBeenCalledTimes(1); + expect(mockResponse.cookie).toHaveBeenCalledTimes(2); + expect(mockResponse.cookie).toHaveBeenCalledWith( + expect.stringContaining('access-token'), + '', + expect.objectContaining({ path: '/api' }), + ); expect(mockResponse.cookie).toHaveBeenCalledWith( expect.stringContaining('test-provider-refresh-token'), '', diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index c4476752b6..02e4e26091 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -258,17 +258,21 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { res: express.Response, accessToken: string, ) => { - const payload = JWT.decode(accessToken) as object & { - exp: number; - }; - res.cookie(`access-token`, accessToken, { - expires: new Date(payload?.exp ? payload?.exp * 1000 : 0), - secure: this.options.secure, - sameSite: 'lax', - domain: this.options.cookieDomain, - path: '/api', - httpOnly: true, - }); + try { + const payload = JWT.decode(accessToken) as object & { + exp: number; + }; + res.cookie(`access-token`, accessToken, { + expires: new Date(payload?.exp ? payload?.exp * 1000 : 0), + secure: this.options.secure, + sameSite: 'lax', + domain: this.options.cookieDomain, + path: '/api', + httpOnly: true, + }); + } catch (_err) { + // Ignore + } }; private setRefreshTokenCookie = ( From 74534a0f8cdd09f23e2f781af99ffd5813f1f13c Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Sat, 20 Feb 2021 01:46:00 +0100 Subject: [PATCH 6/7] Move auth token to contrib --- .../tutorials/authenticate-api-requests.md | 39 +++++++++++++++++- .../src/lib/oauth/OAuthAdapter.test.ts | 9 +---- .../src/lib/oauth/OAuthAdapter.ts | 40 ------------------- .../techdocs-backend/src/service/router.ts | 9 ++--- plugins/techdocs/src/api.ts | 1 + 5 files changed, 44 insertions(+), 54 deletions(-) diff --git a/contrib/docs/tutorials/authenticate-api-requests.md b/contrib/docs/tutorials/authenticate-api-requests.md index 4698293152..4f4547a49e 100644 --- a/contrib/docs/tutorials/authenticate-api-requests.md +++ b/contrib/docs/tutorials/authenticate-api-requests.md @@ -13,10 +13,33 @@ Caveat: as of writing this, Backstage does not refresh the identity token so eve import cookieParser from 'cookie-parser'; import { Request, Response, NextFunction } from 'express'; +import { JWT } from 'jose'; +import { URL } from 'url'; import { IdentityClient } from '@backstage/plugin-auth-backend'; // ... +function setTokenCookie( + res: Response, + options: { token: string; secure: boolean; cookieDomain: string }, +) { + try { + const payload = JWT.decode(options.token) as object & { + exp: number; + }; + res.cookie(`token`, options.token, { + expires: new Date(payload?.exp ? payload?.exp * 1000 : 0), + secure: options.secure, + sameSite: 'lax', + domain: options.cookieDomain, + path: '/', + httpOnly: true, + }); + } catch (_err) { + // Ignore + } +} + async function main() { // ... @@ -25,6 +48,9 @@ async function main() { discovery, issuer: await discovery.getExternalBaseUrl('auth'), }); + const baseUrl = config.getString('backend.baseUrl'); + const secure = baseUrl.startsWith('https://'); + const cookieDomain = new URL(baseUrl).hostname; const authMiddleware = async ( req: Request, res: Response, @@ -33,8 +59,19 @@ async function main() { try { const token = IdentityClient.getBearerToken(req.headers.authorization) || - req.cookies['access-token']; + req.cookies['token']; req.user = await identity.authenticate(token); + if (!req.headers.authorization) { + // Authorization header may be forwarded by plugin requests + req.headers.authorization = `Bearer ${token}`; + } + if (token !== req.cookies['token']) { + setTokenCookie(res, { + token, + secure, + cookieDomain, + }); + } next(); } catch (error) { res.status(401).send(`Unauthorized`); diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts index 7d51f152f8..d2b31213f2 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts @@ -160,7 +160,7 @@ describe('OAuthAdapter', () => { expect(mockResponse.cookie).toHaveBeenCalledTimes(0); }); - it('removes access and refresh cookies when logging out', async () => { + it('removes refresh cookie when logging out', async () => { const oauthProvider = new OAuthAdapter(providerInstance, { ...oAuthProviderOptions, disableRefresh: false, @@ -176,12 +176,7 @@ describe('OAuthAdapter', () => { } as unknown) as express.Response; await oauthProvider.logout(mockRequest, mockResponse); - expect(mockResponse.cookie).toHaveBeenCalledTimes(2); - expect(mockResponse.cookie).toHaveBeenCalledWith( - expect.stringContaining('access-token'), - '', - expect.objectContaining({ path: '/api' }), - ); + expect(mockResponse.cookie).toHaveBeenCalledTimes(1); expect(mockResponse.cookie).toHaveBeenCalledWith( expect.stringContaining('test-provider-refresh-token'), '', diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index 02e4e26091..62cb8e444d 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -16,7 +16,6 @@ import express from 'express'; import crypto from 'crypto'; -import { JWT } from 'jose'; import { URL } from 'url'; import { AuthProviderRouteHandlers, @@ -128,9 +127,6 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { } await this.populateIdentity(response.backstageIdentity); - if (response.backstageIdentity?.idToken) { - this.setAccessTokenCookie(res, response.backstageIdentity?.idToken); - } // post message back to popup if successful return postMessageResponse(res, this.options.appOrigin, { @@ -155,7 +151,6 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { return; } - this.removeAccessTokenCookie(res); if (!this.options.disableRefresh) { // remove refresh token cookie before logout this.removeRefreshTokenCookie(res); @@ -195,9 +190,6 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { ); await this.populateIdentity(response.backstageIdentity); - if (response.backstageIdentity?.idToken) { - this.setAccessTokenCookie(res, response.backstageIdentity?.idToken); - } if ( response.providerInfo.refreshToken && @@ -254,27 +246,6 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { return req.cookies[`${providerId}-scope`]; }; - private setAccessTokenCookie = ( - res: express.Response, - accessToken: string, - ) => { - try { - const payload = JWT.decode(accessToken) as object & { - exp: number; - }; - res.cookie(`access-token`, accessToken, { - expires: new Date(payload?.exp ? payload?.exp * 1000 : 0), - secure: this.options.secure, - sameSite: 'lax', - domain: this.options.cookieDomain, - path: '/api', - httpOnly: true, - }); - } catch (_err) { - // Ignore - } - }; - private setRefreshTokenCookie = ( res: express.Response, refreshToken: string, @@ -289,17 +260,6 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { }); }; - private removeAccessTokenCookie = (res: express.Response) => { - res.cookie(`access-token`, '', { - maxAge: 0, - secure: this.options.secure, - sameSite: 'lax', - domain: this.options.cookieDomain, - path: '/api', - httpOnly: true, - }); - }; - private removeRefreshTokenCookie = (res: express.Response) => { res.cookie(`${this.options.providerId}-refresh-token`, '', { maxAge: 0, diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 885a1a64af..d300400c70 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -82,9 +82,7 @@ export async function createRouter({ const { kind, namespace, name } = req.params; try { - const token = - getBearerToken(req.headers.authorization) || - req.cookies['access-token']; + const token = getBearerToken(req.headers.authorization); const entity = (await ( await fetch( `${catalogUrl}/entities/by-name/${kind}/${namespace}/${name}`, @@ -115,8 +113,7 @@ export async function createRouter({ const catalogUrl = await discovery.getBaseUrl('catalog'); const triple = [kind, namespace, name].map(encodeURIComponent).join('/'); - const token = - getBearerToken(req.headers.authorization) || req.cookies['access-token']; + const token = getBearerToken(req.headers.authorization); const catalogRes = await fetch(`${catalogUrl}/entities/by-name/${triple}`, { headers: token ? { Authorization: `Bearer ${token}` } : {}, }); @@ -214,5 +211,5 @@ export async function createRouter({ } function getBearerToken(header?: string): string | undefined { - return header?.match(/(?:Bearer|Basic)\s+(\S+)/i)?.[1]; + return header?.match(/(?:Bearer)\s+(\S+)/i)?.[1]; } diff --git a/plugins/techdocs/src/api.ts b/plugins/techdocs/src/api.ts index 6de94b63bb..ea176f26eb 100644 --- a/plugins/techdocs/src/api.ts +++ b/plugins/techdocs/src/api.ts @@ -171,6 +171,7 @@ export class TechDocsStorageApi implements TechDocsStorage { const request = await fetch( `${url.endsWith('/') ? url : `${url}/`}index.html`, { + credentials: 'include', headers: token ? { Authorization: `Bearer ${token}` } : {}, }, ); From 52b5bc3e27531a7e5afcd5fc38d58f5cd0eb4a53 Mon Sep 17 00:00:00 2001 From: Erik Larsson Date: Mon, 1 Mar 2021 12:45:18 +0100 Subject: [PATCH 7/7] Remove credentials from xhr request. Add techdocs-backend changeset --- .changeset/clean-plums-crash.md | 5 +++++ .changeset/old-coats-suffer.md | 2 +- plugins/techdocs/src/api.ts | 1 - 3 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 .changeset/clean-plums-crash.md diff --git a/.changeset/clean-plums-crash.md b/.changeset/clean-plums-crash.md new file mode 100644 index 0000000000..77cd94f097 --- /dev/null +++ b/.changeset/clean-plums-crash.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs-backend': patch +--- + +Forward authorization header on backend request if present diff --git a/.changeset/old-coats-suffer.md b/.changeset/old-coats-suffer.md index 8391f77ee0..5938c60b1e 100644 --- a/.changeset/old-coats-suffer.md +++ b/.changeset/old-coats-suffer.md @@ -2,4 +2,4 @@ '@backstage/plugin-techdocs': minor --- -Add authorization header on techdocs api requests +Add authorization header on techdocs api requests. Breaking change as clients now needs the Identity API. diff --git a/plugins/techdocs/src/api.ts b/plugins/techdocs/src/api.ts index ea176f26eb..6de94b63bb 100644 --- a/plugins/techdocs/src/api.ts +++ b/plugins/techdocs/src/api.ts @@ -171,7 +171,6 @@ export class TechDocsStorageApi implements TechDocsStorage { const request = await fetch( `${url.endsWith('/') ? url : `${url}/`}index.html`, { - credentials: 'include', headers: token ? { Authorization: `Bearer ${token}` } : {}, }, );