From 39645e56ac32fe9dfedf5dac7a8114cf186eb35a Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 1 Dec 2021 16:02:09 +0100 Subject: [PATCH] chore: reworking the auth providers to decorate the identity from the token that is returned from the different providers Co-authored-by: Johan Haals Signed-off-by: blam --- .../src/layout/SignInPage/providers.tsx | 4 +-- .../src/lib/flow/authFlowHelpers.test.ts | 15 +++++++++ .../src/lib/oauth/OAuthAdapter.test.ts | 10 ++++-- .../src/lib/oauth/OAuthAdapter.ts | 33 ++++++++++++------- plugins/auth-backend/src/lib/oauth/types.ts | 18 +++++++--- .../src/providers/atlassian/provider.ts | 5 ++- .../src/providers/aws-alb/provider.test.ts | 14 ++++++-- .../src/providers/aws-alb/provider.ts | 3 +- .../src/providers/decorateWithIdentity.ts | 2 +- .../src/providers/github/provider.ts | 22 +------------ .../src/providers/saml/provider.ts | 11 ++++++- 11 files changed, 89 insertions(+), 48 deletions(-) diff --git a/packages/core-components/src/layout/SignInPage/providers.tsx b/packages/core-components/src/layout/SignInPage/providers.tsx index 2abe6b3d51..23d6c95b3c 100644 --- a/packages/core-components/src/layout/SignInPage/providers.tsx +++ b/packages/core-components/src/layout/SignInPage/providers.tsx @@ -30,7 +30,7 @@ import { import { commonProvider } from './commonProvider'; import { guestProvider } from './guestProvider'; import { customProvider } from './customProvider'; -import { IdentityApiProxy } from './IdentityApiProxy'; +import { IdentityApiSignOutProxy } from './IdentityApiSignOutProxy'; const PROVIDER_STORAGE_KEY = '@backstage/core:SignInPage:provider'; @@ -92,7 +92,7 @@ export const useSignInProviders = ( const handleWrappedResult = useCallback( (identityApi: IdentityApi) => { onSignInSuccess( - IdentityApiProxy.from({ + IdentityApiSignOutProxy.from({ identityApi, signOut: async () => { localStorage.removeItem(PROVIDER_STORAGE_KEY); diff --git a/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts b/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts index 8b4b95d59b..07c8196dd2 100644 --- a/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts +++ b/plugins/auth-backend/src/lib/flow/authFlowHelpers.test.ts @@ -52,6 +52,11 @@ describe('oauth helpers', () => { backstageIdentity: { id: 'a', token: 'a.b.c', + identity: { + type: 'user', + ownershipEntityRefs: [], + userEntityRef: 'a', + }, }, }, }; @@ -107,6 +112,11 @@ describe('oauth helpers', () => { backstageIdentity: { id: 'a', token: 'a.b.c', + identity: { + type: 'user', + ownershipEntityRefs: [], + userEntityRef: 'a', + }, }, }, }; @@ -149,6 +159,11 @@ describe('oauth helpers', () => { backstageIdentity: { id: 'a', token: 'a.b.c', + identity: { + type: 'user', + ownershipEntityRefs: [], + userEntityRef: 'a', + }, }, }, }; diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts index 5e52e60f46..91711f67ad 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts @@ -31,7 +31,8 @@ const mockResponseData = { }, backstageIdentity: { id: 'foo', - token: '', + token: + 'eyblob.eyJzdWIiOiJqaW1teW1hcmt1bSIsImVudCI6WyJ1c2VyOmRlZmF1bHQvamltbXltYXJrdW0iXX0=.eyblob', }, }; @@ -218,7 +219,12 @@ describe('OAuthAdapter', () => { ...mockResponseData, backstageIdentity: { id: mockResponseData.backstageIdentity.id, - token: 'my-id-token', + token: mockResponseData.backstageIdentity.token, + identity: { + ownershipEntityRefs: ['user:default/jimmymarkum'], + type: 'user', + userEntityRef: 'jimmymarkum', + }, }, }); }); diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index d6a825f3fd..970fe95b55 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -37,6 +37,7 @@ import { OAuthRefreshRequest, OAuthState, } from './types'; +import { decorateWithIdentity } from '../../providers/decorateWithIdentity'; export const THOUSAND_DAYS_MS = 1000 * 24 * 60 * 60 * 1000; export const TEN_MINUTES_MS = 600 * 1000; @@ -150,12 +151,12 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { this.setRefreshTokenCookie(res, refreshToken); } - await this.populateIdentity(response.backstageIdentity); + const identity = await this.populateIdentity(response.backstageIdentity); // post message back to popup if successful return postMessageResponse(res, appOrigin, { type: 'authorization_response', - response, + response: { ...response, backstageIdentity: identity }, }); } catch (error) { const { name, message } = isError(error) @@ -209,7 +210,9 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { forwardReq as OAuthRefreshRequest, ); - await this.populateIdentity(response.backstageIdentity); + const backstageIdentity = await this.populateIdentity( + response.backstageIdentity, + ); if ( response.providerInfo.refreshToken && @@ -218,7 +221,7 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { this.setRefreshTokenCookie(res, response.providerInfo.refreshToken); } - res.status(200).json(response); + res.status(200).json({ ...response, backstageIdentity }); } catch (error) { throw new AuthenticationError('Refresh failed', error); } @@ -228,18 +231,24 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { * If the response from the OAuth provider includes a Backstage identity, we * make sure it's populated with all the information we can derive from the user ID. */ - private async populateIdentity(identity?: BackstageIdentityResponse) { + private async populateIdentity( + identity?: Omit, + ): Promise { if (!identity) { - return; + return undefined; } - if (!identity.token) { - identity.token = await this.options.tokenIssuer.issueToken({ - claims: { sub: identity.id }, - }); - } else if (!identity.token && identity.token) { - identity.token = identity.token; + if (identity.token) { + return decorateWithIdentity(identity); } + + const token = await this.options.tokenIssuer.issueToken({ + claims: { sub: identity.id }, + }); + + console.log(token); + + return decorateWithIdentity({ ...identity, token }); } private setNonceCookie = (res: express.Response, nonce: string) => { diff --git a/plugins/auth-backend/src/lib/oauth/types.ts b/plugins/auth-backend/src/lib/oauth/types.ts index 7912dd16a5..f1ff9e763d 100644 --- a/plugins/auth-backend/src/lib/oauth/types.ts +++ b/plugins/auth-backend/src/lib/oauth/types.ts @@ -16,8 +16,11 @@ import express from 'express'; import { Profile as PassportProfile } from 'passport'; -import { AuthResponse, RedirectInfo } from '../../providers/types'; - +import { + AuthResponse, + RedirectInfo, + BackstageIdentityResponse, +} from '../../providers/types'; /** * Common options for passport.js-based OAuth providers */ @@ -47,7 +50,12 @@ export type OAuthResult = { refreshToken?: string; }; -export type OAuthResponse = AuthResponse; +export type OAuthResponse = Omit< + AuthResponse, + 'backstageIdentity' +> & { + backstageIdentity?: Omit; +}; export type OAuthProviderInfo = { /** @@ -108,7 +116,7 @@ export interface OAuthHandlers { * @param {express.Request} req */ handler(req: express.Request): Promise<{ - response: AuthResponse; + response: OAuthResponse; refreshToken?: string; }>; @@ -117,7 +125,7 @@ export interface OAuthHandlers { * @param {string} refreshToken * @param {string} scope */ - refresh?(req: OAuthRefreshRequest): Promise>; + refresh?(req: OAuthRefreshRequest): Promise; /** * (Optional) Sign out of the auth provider. diff --git a/plugins/auth-backend/src/providers/atlassian/provider.ts b/plugins/auth-backend/src/providers/atlassian/provider.ts index e19f29a3a4..99e1ebd2d8 100644 --- a/plugins/auth-backend/src/providers/atlassian/provider.ts +++ b/plugins/auth-backend/src/providers/atlassian/provider.ts @@ -45,6 +45,7 @@ import express from 'express'; import { TokenIssuer } from '../../identity'; import { CatalogIdentityClient } from '../../lib/catalog'; import { Logger } from 'winston'; +import { decorateWithIdentity } from '../decorateWithIdentity'; export type AtlassianAuthProviderOptions = OAuthProviderOptions & { scopes: string; @@ -136,7 +137,7 @@ export class AtlassianAuthProvider implements OAuthHandlers { }; if (this.signInResolver) { - response.backstageIdentity = await this.signInResolver( + const resolverResponse = await this.signInResolver( { result, profile, @@ -147,6 +148,8 @@ export class AtlassianAuthProvider implements OAuthHandlers { logger: this.logger, }, ); + + response.backstageIdentity = decorateWithIdentity(resolverResponse); } return response; diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts index 3f72004d48..9e4aed2030 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts @@ -122,7 +122,11 @@ describe('AwsAlbAuthProvider', () => { profile: makeProfileInfo(fullProfile), }), signInResolver: async () => { - return { id: 'user.name', token: 'TOKEN' }; + return { + id: 'user.name', + token: + 'eyblob.eyJzdWIiOiJqaW1teW1hcmt1bSIsImVudCI6WyJ1c2VyOmRlZmF1bHQvamltbXltYXJrdW0iXX0=.eyblob', + }; }, }); @@ -133,7 +137,13 @@ describe('AwsAlbAuthProvider', () => { expect(mockResponse.json).toHaveBeenCalledWith({ backstageIdentity: { id: 'user.name', - token: 'TOKEN', + token: + 'eyblob.eyJzdWIiOiJqaW1teW1hcmt1bSIsImVudCI6WyJ1c2VyOmRlZmF1bHQvamltbXltYXJrdW0iXX0=.eyblob', + identity: { + ownershipEntityRefs: ['user:default/jimmymarkum'], + type: 'user', + userEntityRef: 'jimmymarkum', + }, }, profile: { displayName: 'User Name', diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.ts b/plugins/auth-backend/src/providers/aws-alb/provider.ts index b0b9070d50..027d5190de 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.ts @@ -32,6 +32,7 @@ import { CatalogIdentityClient } from '../../lib/catalog'; import { Profile as PassportProfile } from 'passport'; import { makeProfileInfo } from '../../lib/passport'; import { AuthenticationError } from '@backstage/errors'; +import { decorateWithIdentity } from '../decorateWithIdentity'; export const ALB_JWT_HEADER = 'x-amzn-oidc-data'; export const ALB_ACCESSTOKEN_HEADER = 'x-amzn-oidc-accesstoken'; @@ -198,7 +199,7 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { accessToken: result.accessToken, expiresInSeconds: result.expiresInSeconds, }, - backstageIdentity, + backstageIdentity: decorateWithIdentity(backstageIdentity), profile, }; } diff --git a/plugins/auth-backend/src/providers/decorateWithIdentity.ts b/plugins/auth-backend/src/providers/decorateWithIdentity.ts index dab1480db4..76fa97bd81 100644 --- a/plugins/auth-backend/src/providers/decorateWithIdentity.ts +++ b/plugins/auth-backend/src/providers/decorateWithIdentity.ts @@ -33,7 +33,7 @@ export function decorateWithIdentity( identity: { type: 'user', userEntityRef: sub, - ownershipEntityRefs: ent ?? [sub], + ownershipEntityRefs: ent ?? [], }, }; } diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index eb15afa4ab..e9d019f4ac 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -32,7 +32,6 @@ import { AuthHandler, SignInResolver, StateEncoder, - BackstageIdentityResponse, } from '../types'; import { OAuthAdapter, @@ -46,6 +45,7 @@ import { } from '../../lib/oauth'; import { CatalogIdentityClient } from '../../lib/catalog'; import { TokenIssuer } from '../../identity'; +import { decorateWithIdentity } from '../decorateWithIdentity'; type PrivateInfo = { refreshToken?: string; @@ -168,25 +168,6 @@ export class GithubAuthProvider implements OAuthHandlers { }; if (this.signInResolver) { - const decorateWithIdentity = ( - signInResolverResponse: Omit, - ): BackstageIdentityResponse => { - function parseJwtPayload(token: string) { - const [_header, payload, _signature] = token.split('.'); - return JSON.parse(Buffer.from(payload, 'base64').toString()); - } - const { sub, ent } = parseJwtPayload(signInResolverResponse.token); - return { - ...signInResolverResponse, - identity: { - type: 'user', - userEntityRef: sub, - ownershipEntityRefs: ent ?? [sub], - }, - }; - // parse token - // build identity - }; const signInResolverResult = await this.signInResolver( { result, @@ -198,7 +179,6 @@ export class GithubAuthProvider implements OAuthHandlers { logger: this.logger, }, ); - response.backstageIdentity = decorateWithIdentity(signInResolverResult); } diff --git a/plugins/auth-backend/src/providers/saml/provider.ts b/plugins/auth-backend/src/providers/saml/provider.ts index 3ab00ce401..f760c9e8ee 100644 --- a/plugins/auth-backend/src/providers/saml/provider.ts +++ b/plugins/auth-backend/src/providers/saml/provider.ts @@ -36,8 +36,13 @@ import { import { postMessageResponse } from '../../lib/flow'; import { TokenIssuer } from '../../identity/types'; import { isError } from '@backstage/errors'; +<<<<<<< HEAD import { CatalogIdentityClient } from '../../lib/catalog'; import { Logger } from 'winston'; +import { decorateWithIdentity } from '../decorateWithIdentity'; +======= +import { decorateWithIdentity } from '../decorateWithIdentity'; +>>>>>>> chore: reworking the auth providers to decorate the identity from the token that is returned from the different providers /** @public */ export type SamlAuthResult = { @@ -105,7 +110,7 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { }; if (this.signInResolver) { - response.backstageIdentity = await this.signInResolver( + const signInResponse = await this.signInResolver( { result, profile, @@ -116,8 +121,12 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { logger: this.logger, }, ); + + response.backstageIdentity = decorateWithIdentity(signInResponse); } + + return postMessageResponse(res, this.appUrl, { type: 'authorization_response', response,