From 339a4213b85daf44eb78b07a19e76ade3f4dd7b6 Mon Sep 17 00:00:00 2001 From: Renlord Yang Date: Thu, 14 Jul 2022 06:24:27 +0000 Subject: [PATCH] return cloudflare access claims directly Signed-off-by: Renlord Yang Signed-off-by: Renlord Yang --- .../cloudflare-access/provider.test.ts | 171 +++--------------- .../providers/cloudflare-access/provider.ts | 79 ++++---- 2 files changed, 66 insertions(+), 184 deletions(-) diff --git a/plugins/auth-backend/src/providers/cloudflare-access/provider.test.ts b/plugins/auth-backend/src/providers/cloudflare-access/provider.test.ts index 6163add891..21ec7371e5 100644 --- a/plugins/auth-backend/src/providers/cloudflare-access/provider.test.ts +++ b/plugins/auth-backend/src/providers/cloudflare-access/provider.test.ts @@ -21,19 +21,14 @@ import { CF_AUTH_IDENTITY, CloudflareAccessAuthProvider, } from './provider'; -import { makeProfileInfo } from '../../lib/passport'; import { AuthResolverContext } from '../types'; import fetch from 'node-fetch'; -import * as winston from 'winston'; const jwtMock = jwtVerify as jest.Mocked; const mockJwt = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6IktFWV9JRCIsImlzcyI6IklTU1VFUl9VUkwifQ.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IlVzZXIgTmFtZSIsImlhdCI6MTUxNjIzOTAyMn0.uMCSBGhij1xn5pnot8XgD-huQuTIBOFGs6kkW_p_X94'; const mockClaims = { sub: '1234567890', - name: 'User Name', - family_name: 'Name', - given_name: 'User', email: 'user.name@email.test', iat: 1632833760, exp: 1632833763, @@ -52,27 +47,6 @@ const mockCfIdentity = { ], }; -// For cases where fetch resolves, but not ok and; -// fetch failed and returns rejected promise -const identityFailResponse = { - backstageIdentity: { - token: - 'eyblob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvamltbXltYXJrdW0iLCJlbnQiOlsidXNlcjpkZWZhdWx0L2ppbW15bWFya3VtIl19.eyblob', - identity: { - ownershipEntityRefs: ['user:default/jimmymarkum'], - type: 'user', - userEntityRef: 'user:default/jimmymarkum', - }, - }, - profile: { - displayName: 'User Name', - email: 'user.name@email.test', - }, - providerInfo: { - expiresInSeconds: mockClaims.exp - mockClaims.iat, - }, -}; - const identityOkResponse = { backstageIdentity: { identity: { @@ -84,9 +58,7 @@ const identityOkResponse = { 'eyblob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvamltbXltYXJrdW0iLCJlbnQiOlsidXNlcjpkZWZhdWx0L2ppbW15bWFya3VtIl19.eyblob', }, profile: { - displayName: 'foo', - email: 'foo@bar.com', - picture: undefined, + email: 'user.name@email.test', }, providerInfo: { cfAccessIdentityProfile: { @@ -101,6 +73,7 @@ const identityOkResponse = { id: '123', name: 'foo', }, + claims: mockClaims, expiresInSeconds: 3, }, }; @@ -160,13 +133,15 @@ describe('CloudflareAccessAuthProvider', () => { status: jest.fn(), } as unknown as express.Response; - const mockFetch = fetch as unknown as jest.MockedFn; + const mockFetch = fetch as unknown as jest.Mocked; const provider = new CloudflareAccessAuthProvider({ teamName: 'foobar', resolverContext: {} as AuthResolverContext, - authHandler: async ({ fullProfile }) => ({ - profile: makeProfileInfo(fullProfile), + authHandler: async ({ claims }) => ({ + profile: { + email: claims.email, + }, }), signInResolver: async () => { return { @@ -175,49 +150,22 @@ describe('CloudflareAccessAuthProvider', () => { }; }, cache: mockCacheClient, - logger: winston.createLogger({ - transports: [new winston.transports.File({ filename: '/dev/null' })], - }), }); describe('when JWT is valid', () => { - describe('resolves when passed in header', () => { - it('returns cfidentity also when get-identity succeeds', async () => { - jwtMock.mockReturnValue(Promise.resolve({ payload: mockClaims })); - mockFetch.mockReturnValueOnce( - Promise.resolve({ - ok: true, - status: 200, - json: () => { - return mockCfIdentity; - }, - }), - ); - await provider.refresh(mockRequestWithJwtHeader, mockResponse); - expect(mockResponse.json).toHaveBeenCalledWith(identityOkResponse); - }); - - it('does not return cfidentity when get-identity returns bad status code', async () => { - // when fetch resolves, but response code is bad - mockFetch.mockReturnValueOnce( - Promise.resolve({ - ok: false, - status: 400, - json: () => { - return { err: 'bad request' }; - }, - }), - ); - await provider.refresh(mockRequestWithJwtHeader, mockResponse); - expect(mockResponse.json).toHaveBeenCalledWith(identityFailResponse); - }); - - it('does not return cfidentity when get-identity fetch fails', async () => { - // when fetch rejects - mockFetch.mockReturnValueOnce(Promise.reject()); - await provider.refresh(mockRequestWithJwtHeader, mockResponse); - expect(mockResponse.json).toHaveBeenCalledWith(identityFailResponse); - }); + it('returns cfidentity also when get-identity succeeds', async () => { + jwtMock.mockReturnValue(Promise.resolve({ payload: mockClaims })); + mockFetch.mockReturnValueOnce( + Promise.resolve({ + ok: true, + status: 200, + json: () => { + return mockCfIdentity; + }, + }), + ); + await provider.refresh(mockRequestWithJwtHeader, mockResponse); + expect(mockResponse.json).toHaveBeenCalledWith(identityOkResponse); }); it('should resolve when passed in cookie', async () => { @@ -234,46 +182,6 @@ describe('CloudflareAccessAuthProvider', () => { ); await provider.refresh(mockRequestWithJwtCookie, mockResponse); expect(mockResponse.json).toHaveBeenCalledWith(identityOkResponse); - - // when mockFetch rejects - mockFetch.mockReturnValueOnce(Promise.reject()); - await provider.refresh(mockRequestWithJwtCookie, mockResponse); - expect(mockResponse.json).toHaveBeenCalledWith(identityFailResponse); - }); - - it('should return a response error when response code is not 200', async () => { - // when get-identity api responds and responds with status 400 - mockFetch.mockResolvedValueOnce( - Promise.resolve({ - ok: false, - status: 400, - json: () => { - return Promise.resolve({ - err: 'bad request', - }); - }, - }), - ); - jwtMock.mockReturnValueOnce(Promise.resolve({ payload: mockClaims })); - await provider.refresh(mockRequestWithJwtCookie, mockResponse); - expect(mockResponse.json).toHaveBeenCalledWith({ - backstageIdentity: { - token: - 'eyblob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvamltbXltYXJrdW0iLCJlbnQiOlsidXNlcjpkZWZhdWx0L2ppbW15bWFya3VtIl19.eyblob', - identity: { - ownershipEntityRefs: ['user:default/jimmymarkum'], - type: 'user', - userEntityRef: 'user:default/jimmymarkum', - }, - }, - profile: { - displayName: 'User Name', - email: 'user.name@email.test', - }, - providerInfo: { - expiresInSeconds: mockClaims.exp - mockClaims.iat, - }, - }); }); it('should resolve an identity and populate access groups when there are groups', async () => { @@ -302,37 +210,14 @@ describe('CloudflareAccessAuthProvider', () => { ); jwtMock.mockReturnValueOnce(Promise.resolve({ payload: mockClaims })); await provider.refresh(mockRequestWithJwtCookie, mockResponse); - expect(mockResponse.json).toHaveBeenCalledWith({ - backstageIdentity: { - token: - 'eyblob.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvamltbXltYXJrdW0iLCJlbnQiOlsidXNlcjpkZWZhdWx0L2ppbW15bWFya3VtIl19.eyblob', - identity: { - ownershipEntityRefs: ['user:default/jimmymarkum'], - type: 'user', - userEntityRef: 'user:default/jimmymarkum', - }, - }, - profile: { - displayName: 'foo', - email: 'foo@bar.com', - picture: undefined, - }, - providerInfo: { - expiresInSeconds: mockClaims.exp - mockClaims.iat, - cfAccessIdentityProfile: { - name: 'foo', - id: '123', - email: 'foo@bar.com', - groups: [ - { - id: '123', - email: 'foo@bar.com', - name: 'foo', - }, - ], - }, - }, - }); + expect(mockResponse.json).toHaveBeenCalledWith(identityOkResponse); + }); + + it('should throw an error when get-identity fails', async () => { + mockFetch.mockReturnValue(Promise.reject()); + await expect( + provider.refresh(mockRequestWithJwtCookie, mockResponse), + ).rejects.toThrowError(); }); }); diff --git a/plugins/auth-backend/src/providers/cloudflare-access/provider.ts b/plugins/auth-backend/src/providers/cloudflare-access/provider.ts index e5254a2fe0..b862e3c6de 100644 --- a/plugins/auth-backend/src/providers/cloudflare-access/provider.ts +++ b/plugins/auth-backend/src/providers/cloudflare-access/provider.ts @@ -20,24 +20,26 @@ import { AuthResponse, SignInResolver, } from '../types'; +import fetch, { Headers } from 'node-fetch'; import express from 'express'; import * as _ from 'lodash'; -import fetch, { Headers, Response as FetchResponse } from 'node-fetch'; import { jwtVerify, createRemoteJWKSet } from 'jose'; -import { Profile as PassportProfile } from 'passport'; -import { AuthenticationError, ResponseError } from '@backstage/errors'; +import { + AuthenticationError, + ResponseError, + ForwardedError, +} from '@backstage/errors'; import { CacheClient } from '@backstage/backend-common'; import { createAuthProviderIntegration } from '../createAuthProviderIntegration'; import { prepareBackstageIdentityResponse } from '../prepareBackstageIdentityResponse'; -import { makeProfileInfo } from '../../lib/passport'; import { commonByEmailResolver } from '../resolvers'; -import { Logger } from 'winston'; // JWT Web Token definitions are in the URL below // https://developers.cloudflare.com/cloudflare-one/identity/users/validating-json/ export const CF_JWT_HEADER = 'cf-access-jwt-assertion'; export const CF_AUTH_IDENTITY = 'cf-access-authenticated-user-email'; const COOKIE_AUTH_NAME = 'CF_Authorization'; +const CACHE_PREFIX = 'providers/cloudflare-access/profile-v1'; /** * Default cache TTL @@ -59,7 +61,6 @@ export type Options = { authHandler: AuthHandler; signInResolver: SignInResolver; resolverContext: AuthResolverContext; - logger: Logger; cache?: CacheClient; }; @@ -116,8 +117,8 @@ type CloudflareAccessIdentityProfile = { }; export type CloudflareAccessResult = { - fullProfile: PassportProfile; - cfIdentity?: CloudflareAccessIdentityProfile; + claims: CloudflareAccessClaims; + cfIdentity: CloudflareAccessIdentityProfile; expiresInSeconds?: number; }; @@ -133,6 +134,10 @@ export type CloudflareAccessProviderInfo = { * Cloudflare access identity profile with cloudflare access groups */ cfAccessIdentityProfile?: CloudflareAccessIdentityProfile; + /** + * Cloudflare access claims + */ + claims: CloudflareAccessClaims; }; export type CloudflareAccessResponse = @@ -144,7 +149,6 @@ export class CloudflareAccessAuthProvider implements AuthProviderRouteHandlers { private readonly authHandler: AuthHandler; private readonly signInResolver: SignInResolver; private readonly jwtKeySet: any; - private readonly logger: Logger; private readonly cache?: CacheClient; constructor(options: Options) { @@ -157,7 +161,6 @@ export class CloudflareAccessAuthProvider implements AuthProviderRouteHandlers { `https://${this.teamName}.cloudflareaccess.com/cdn-cgi/access/certs`, ), ); - this.logger = options.logger; this.cache = options.cache; } @@ -186,19 +189,17 @@ export class CloudflareAccessAuthProvider implements AuthProviderRouteHandlers { headers.set(CF_JWT_HEADER, jwt); headers.set('cookie', `${COOKIE_AUTH_NAME}=${jwt}`); try { - const res: FetchResponse = await fetch( + const res = await fetch( `https://${this.teamName}.cloudflareaccess.com/cdn-cgi/access/get-identity`, { headers }, ); if (!res.ok) { - // Cast to work around https://github.com/backstage/backstage/issues/12166 - throw await ResponseError.fromResponse(res as unknown as Response); + throw ResponseError.fromResponse(res); } const cfIdentity = await res.json(); return cfIdentity as unknown as CloudflareAccessIdentityProfile; } catch (err) { - this.logger.error(`getIdentityProfile failed: ${err}`); - return Promise.reject(err); + throw new ForwardedError('getIdentityProfile failed', err); } } @@ -225,42 +226,33 @@ export class CloudflareAccessAuthProvider implements AuthProviderRouteHandlers { const verifyResult = await jwtVerify(jwt, this.jwtKeySet, { issuer: `https://${this.teamName}.cloudflareaccess.com`, }); - const cfAccessResultStr = await this.cache?.get(jwt); + const sub = verifyResult.payload.sub; + const cfAccessResultStr = await this.cache?.get(`${CACHE_PREFIX}/${sub}`); if (typeof cfAccessResultStr === 'string') { return JSON.parse(cfAccessResultStr) as CloudflareAccessResult; } const claims = verifyResult.payload as CloudflareAccessClaims; // Builds a passport profile from JWT claims first - const username = claims.email.split('@')[0].toLowerCase(); - const fullProfile: PassportProfile = { - provider: 'cfAccess', - id: claims.sub, - displayName: _.startCase(username.replace('.', ' ')), - username: username, - emails: [{ value: claims.email }], - }; - const cfAccessResult: CloudflareAccessResult = { - fullProfile, - expiresInSeconds: claims.exp - claims.iat, - }; try { // If we successfully fetch the get-identity endpoint, // We supplement the passport profile with richer user identity // information here. const cfIdentity = await this.getIdentityProfile(jwt); - fullProfile.displayName = cfIdentity.name; - fullProfile.emails = [{ value: cfIdentity.email }]; - fullProfile.username = cfIdentity.email.split('@')[0].toLowerCase(); - cfAccessResult.cfIdentity = cfIdentity; // Stores a stringified JSON object in cfaccess provider cache only when // we complete all steps - this.cache?.set(jwt, JSON.stringify(cfAccessResult)); + const cfAccessResult: CloudflareAccessResult = { + claims, + cfIdentity, + expiresInSeconds: claims.exp - claims.iat, + }; + this.cache?.set(`${CACHE_PREFIX}/${sub}`, JSON.stringify(cfAccessResult)); + return cfAccessResult; } catch (err) { - this.logger.error( - `failed to populate access identity information: ${err}`, + throw new ForwardedError( + 'Failed to populate access identity information', + err, ); } - return cfAccessResult; } private async handleResult( @@ -278,6 +270,7 @@ export class CloudflareAccessAuthProvider implements AuthProviderRouteHandlers { return { providerInfo: { expiresInSeconds: result.expiresInSeconds, + claims: result.claims, cfAccessIdentityProfile: result.cfIdentity, }, backstageIdentity: prepareBackstageIdentityResponse(backstageIdentity), @@ -314,7 +307,7 @@ export const cfAccess = createAuthProviderIntegration({ */ cache?: CacheClient; }) { - return ({ config, logger, resolverContext }) => { + return ({ config, resolverContext }) => { const teamName = config.getString('teamName'); if (!options.signIn.resolver) { @@ -326,16 +319,20 @@ export const cfAccess = createAuthProviderIntegration({ const authHandler: AuthHandler = options?.authHandler ? options.authHandler - : async ({ fullProfile }) => ({ - profile: makeProfileInfo(fullProfile), - }); + : async ({ claims, cfIdentity }) => { + return { + profile: { + email: claims.email, + displayName: cfIdentity.name, + }, + }; + }; return new CloudflareAccessAuthProvider({ teamName, signInResolver: options?.signIn.resolver, authHandler, resolverContext, - logger, ...(options.cache && { cache: options.cache }), }); };