From ebcd28073aba9dbb6854ed0a5d72381911c1f8a1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 17 Mar 2022 16:22:41 +0100 Subject: [PATCH] auth-backend: reintroduce behavior of some default sign-in resolvers Signed-off-by: Patrik Oldsberg --- .../src/providers/github/index.ts | 5 ++- .../src/providers/github/provider.test.ts | 41 +++++++++++++++---- .../src/providers/github/provider.ts | 29 +++++++++++++ .../src/providers/gitlab/provider.test.ts | 17 +++++--- .../src/providers/gitlab/provider.ts | 29 +++++++++++++ .../auth-backend/src/providers/saml/index.ts | 5 ++- .../src/providers/saml/provider.ts | 24 +++++++++++ 7 files changed, 136 insertions(+), 14 deletions(-) diff --git a/plugins/auth-backend/src/providers/github/index.ts b/plugins/auth-backend/src/providers/github/index.ts index a855b13b83..3b0c605f0e 100644 --- a/plugins/auth-backend/src/providers/github/index.ts +++ b/plugins/auth-backend/src/providers/github/index.ts @@ -14,5 +14,8 @@ * limitations under the License. */ -export { createGithubProvider } from './provider'; +export { + createGithubProvider, + githubUsernameEntityNameSignInResolver, +} from './provider'; export type { GithubOAuthResult, GithubProviderOptions } from './provider'; diff --git a/plugins/auth-backend/src/providers/github/provider.test.ts b/plugins/auth-backend/src/providers/github/provider.test.ts index 206df52083..37dd4115ed 100644 --- a/plugins/auth-backend/src/providers/github/provider.test.ts +++ b/plugins/auth-backend/src/providers/github/provider.test.ts @@ -21,7 +21,7 @@ import { CatalogIdentityClient } from '../../lib/catalog'; import { GithubAuthProvider, GithubOAuthResult, - githubDefaultSignInResolver, + githubUsernameEntityNameSignInResolver, } from './provider'; import * as helpers from '../../lib/passport/PassportStrategyHelper'; import { makeProfileInfo } from '../../lib/passport/PassportStrategyHelper'; @@ -46,14 +46,18 @@ describe('GithubAuthProvider', () => { }; const catalogIdentityClient = { findUser: jest.fn(), - }; + resolveCatalogMembership: async ({ + entityRefs, + }: { + entityRefs: string[]; + }) => entityRefs, + } as unknown as CatalogIdentityClient; const provider = new GithubAuthProvider({ logger: getVoidLogger(), - catalogIdentityClient: - catalogIdentityClient as unknown as CatalogIdentityClient, + catalogIdentityClient: catalogIdentityClient, tokenIssuer: tokenIssuer as unknown as TokenIssuer, - signInResolver: githubDefaultSignInResolver, + signInResolver: githubUsernameEntityNameSignInResolver, authHandler: async ({ fullProfile }) => ({ profile: makeProfileInfo(fullProfile), }), @@ -254,6 +258,7 @@ describe('GithubAuthProvider', () => { result: { fullProfile: { id: 'ipd12039', + username: 'daveboyle', provider: 'github', displayName: 'Dave Boyle', }, @@ -271,8 +276,8 @@ describe('GithubAuthProvider', () => { expect(response).toEqual({ response: { backstageIdentity: { - id: 'ipd12039', - token: 'token-for-user:default/ipd12039', + id: 'daveboyle', + token: 'token-for-user:default/daveboyle', }, providerInfo: { accessToken: 'a.b.c', @@ -287,6 +292,28 @@ describe('GithubAuthProvider', () => { }); }); + it('should fail if username is not available', async () => { + mockFrameHandler.mockResolvedValueOnce({ + result: { + fullProfile: { + id: 'ipd12039', + provider: 'github', + displayName: 'Dave Boyle', + }, + accessToken: 'a.b.c', + params: { + scope: 'read:user', + expires_in: '123', + }, + }, + privateInfo: { refreshToken: 'refresh-me' }, + }); + + await expect(provider.handler({} as any)).rejects.toThrow( + 'GitHub user profile does not contain a username', + ); + }); + it('should forward a new refresh token on refresh', async () => { const mockRefreshToken = jest.spyOn( helpers, diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index fb61a3ae97..e57722d611 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -244,6 +244,35 @@ export class GithubAuthProvider implements OAuthHandlers { } } +export const githubUsernameEntityNameSignInResolver: SignInResolver< + GithubOAuthResult +> = async (info, ctx) => { + const { fullProfile } = info.result; + + const userId = fullProfile.username; + if (!userId) { + throw new Error(`GitHub user profile does not contain a username`); + } + + const entityRef = stringifyEntityRef({ + kind: 'User', + namespace: DEFAULT_NAMESPACE, + name: userId, + }); + const ownershipEntityRefs = + await ctx.catalogIdentityClient.resolveCatalogMembership({ + entityRefs: [entityRef], + }); + const token = await ctx.tokenIssuer.issueToken({ + claims: { + sub: entityRef, + ent: ownershipEntityRefs, + }, + }); + + return { id: userId, token }; +}; + export type GithubProviderOptions = { /** * The profile transformation function used to verify and convert the auth response diff --git a/plugins/auth-backend/src/providers/gitlab/provider.test.ts b/plugins/auth-backend/src/providers/gitlab/provider.test.ts index f90de3c75b..f51ba58365 100644 --- a/plugins/auth-backend/src/providers/gitlab/provider.test.ts +++ b/plugins/auth-backend/src/providers/gitlab/provider.test.ts @@ -14,7 +14,10 @@ * limitations under the License. */ -import { GitlabAuthProvider, gitlabDefaultSignInResolver } from './provider'; +import { + GitlabAuthProvider, + gitlabUsernameEntityNameSignInResolver, +} from './provider'; import * as helpers from '../../lib/passport/PassportStrategyHelper'; import { PassportProfile } from '../../lib/passport/types'; import { OAuthResult } from '../../lib/oauth'; @@ -34,15 +37,19 @@ describe('GitlabAuthProvider', () => { }; const catalogIdentityClient = { findUser: jest.fn(), - }; + resolveCatalogMembership: async ({ + entityRefs, + }: { + entityRefs: string[]; + }) => entityRefs, + } as unknown as CatalogIdentityClient; const provider = new GitlabAuthProvider({ clientId: 'mock', clientSecret: 'mock', callbackUrl: 'mock', baseUrl: 'mock', - catalogIdentityClient: - catalogIdentityClient as unknown as CatalogIdentityClient, + catalogIdentityClient, tokenIssuer: tokenIssuer as unknown as TokenIssuer, authHandler: async ({ fullProfile }) => ({ profile: { @@ -51,7 +58,7 @@ describe('GitlabAuthProvider', () => { picture: 'http://gitlab.com/lols', }, }), - signInResolver: gitlabDefaultSignInResolver, + signInResolver: gitlabUsernameEntityNameSignInResolver, logger: getVoidLogger(), }); diff --git a/plugins/auth-backend/src/providers/gitlab/provider.ts b/plugins/auth-backend/src/providers/gitlab/provider.ts index b16757470b..91a9b5d8f6 100644 --- a/plugins/auth-backend/src/providers/gitlab/provider.ts +++ b/plugins/auth-backend/src/providers/gitlab/provider.ts @@ -62,6 +62,35 @@ export type GitlabAuthProviderOptions = OAuthProviderOptions & { logger: Logger; }; +export const gitlabUsernameEntityNameSignInResolver: SignInResolver< + OAuthResult +> = async (info, ctx) => { + const { result } = info; + + const id = result.fullProfile.username; + if (!id) { + throw new Error(`GitLab user profile does not contain a username`); + } + + const entityRef = stringifyEntityRef({ + kind: 'User', + namespace: DEFAULT_NAMESPACE, + name: id, + }); + const ownershipEntityRefs = + await ctx.catalogIdentityClient.resolveCatalogMembership({ + entityRefs: [entityRef], + }); + const token = await ctx.tokenIssuer.issueToken({ + claims: { + sub: entityRef, + ent: ownershipEntityRefs, + }, + }); + + return { id, token }; +}; + export const gitlabDefaultAuthHandler: AuthHandler = async ({ fullProfile, params, diff --git a/plugins/auth-backend/src/providers/saml/index.ts b/plugins/auth-backend/src/providers/saml/index.ts index 0aea660941..8d36e17d29 100644 --- a/plugins/auth-backend/src/providers/saml/index.ts +++ b/plugins/auth-backend/src/providers/saml/index.ts @@ -14,5 +14,8 @@ * limitations under the License. */ -export { createSamlProvider } from './provider'; +export { + createSamlProvider, + samlNameIdEntityNameSignInResolver, +} from './provider'; export type { SamlProviderOptions, SamlAuthResult } from './provider'; diff --git a/plugins/auth-backend/src/providers/saml/provider.ts b/plugins/auth-backend/src/providers/saml/provider.ts index c7b0896b49..c3b4fc59e8 100644 --- a/plugins/auth-backend/src/providers/saml/provider.ts +++ b/plugins/auth-backend/src/providers/saml/provider.ts @@ -148,6 +148,30 @@ export class SamlAuthProvider implements AuthProviderRouteHandlers { } } +export const samlNameIdEntityNameSignInResolver: SignInResolver< + SamlAuthResult +> = async (info, ctx) => { + const id = info.result.fullProfile.nameID; + + const entityRef = stringifyEntityRef({ + kind: 'User', + namespace: DEFAULT_NAMESPACE, + name: id, + }); + const ownershipEntityRefs = + await ctx.catalogIdentityClient.resolveCatalogMembership({ + entityRefs: [entityRef], + }); + const token = await ctx.tokenIssuer.issueToken({ + claims: { + sub: entityRef, + ent: ownershipEntityRefs, + }, + }); + + return { id, token }; +}; + type SignatureAlgorithm = 'sha1' | 'sha256' | 'sha512'; /** @public */