From b9832aee3d57c40de8e77e35275eff5acfe8d1de Mon Sep 17 00:00:00 2001 From: Olivier Liechti Date: Tue, 9 Jul 2024 09:55:58 +0200 Subject: [PATCH 01/12] Fix issues described in #25551 Signed-off-by: Olivier Liechti --- .changeset/chilly-rabbits-sneeze.md | 5 +++ .../src/authenticator.test.ts | 40 +++++++++++++++++ .../src/authenticator.ts | 44 +++++++++++++++---- .../src/types.d.ts | 3 +- 4 files changed, 83 insertions(+), 9 deletions(-) create mode 100644 .changeset/chilly-rabbits-sneeze.md create mode 100644 plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts diff --git a/.changeset/chilly-rabbits-sneeze.md b/.changeset/chilly-rabbits-sneeze.md new file mode 100644 index 0000000000..cbce80ebbc --- /dev/null +++ b/.changeset/chilly-rabbits-sneeze.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend-module-atlassian-provider': patch +--- + +Fix several issues with the Atlassian auth provider (type definition, profile url, profile transformation, scopes) diff --git a/plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts b/plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts new file mode 100644 index 0000000000..b8e9e1ad75 --- /dev/null +++ b/plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts @@ -0,0 +1,40 @@ +/* + * Copyright 2024 The Backstage Authors + * + * 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 Strategy from 'passport-atlassian-oauth2'; + +describe('Strategy', () => { + it('should be an instance of', () => { + try { + console.log(Strategy); + const strategy = new Strategy( + { + authorizationURL: 'https://auth.atlassian.com/authorize', + tokenURL: 'https://auth.atlassian.com/oauth/token', + clientID: 'my-client-id', + clientSecret: 'my-client-secret', + scope: ['offline_access', 'read:jira-work', 'read:jira-user'], + }, + () => {}, + ); + console.log(strategy.name); + expect(strategy).toBeInstanceOf(Strategy); + + expect((strategy as any).name).toBe('atlassian'); + } catch (e) { + console.error(e); + } + }); +}); diff --git a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts index 0ced9cf831..ab2cd3f94a 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts @@ -20,14 +20,35 @@ import { PassportOAuthDoneCallback, PassportProfile, } from '@backstage/plugin-auth-node'; -import { Strategy as AtlassianStrategy } from 'passport-atlassian-oauth2'; +import AtlassianStrategy from 'passport-atlassian-oauth2'; + +export type AtlassianPassportProfile = { + id: string; + displayName: string; + email: string; + photo: string; + provider: string; + _json: any; +}; /** @public */ -export const atlassianAuthenticator = createOAuthAuthenticator({ - defaultProfileTransform: - PassportOAuthAuthenticatorHelper.defaultProfileTransform, +export const atlassianAuthenticator = createOAuthAuthenticator< + any, + AtlassianPassportProfile +>({ + defaultProfileTransform: async (input, context) => { + // const result = await PassportOAuthAuthenticatorHelper.defaultProfileTransform(input, context); + + return { + profile: { + displayName: input.fullProfile.displayName, + email: input.fullProfile.email, + picture: input.fullProfile.photo, + }, + }; + }, scopes: { - required: ['offline_access', 'read:jira-work', 'read:jira-user'], + required: ['offline_access', 'read:me', 'read:jira-work', 'read:jira-user'], }, initialize({ callbackUrl, config }) { const clientId = config.getString('clientId'); @@ -49,18 +70,25 @@ export const atlassianAuthenticator = createOAuthAuthenticator({ baseURL: baseUrl, authorizationURL: `${baseUrl}/authorize`, tokenURL: `${baseUrl}/oauth/token`, - profileURL: `${baseUrl}/api/v4/user`, + // profileURL: `${baseUrl}/api/v4/user`, + profileURL: 'https://api.atlassian.com/me', + scope: config.getOptionalString('additionalScopes')?.split(' ') || [], }, ( accessToken: string, refreshToken: string, params: any, - fullProfile: PassportProfile, + fullProfile: PassportProfile & { email: string; photo: string }, done: PassportOAuthDoneCallback, ) => { + const fullProfileWithEmails = { + ...fullProfile, + // avatarUrl: fullProfile.photo, + // emails: [{ value: fullProfile.email }], + }; done( undefined, - { fullProfile, params, accessToken }, + { fullProfile: fullProfileWithEmails, params, accessToken }, { refreshToken }, ); }, diff --git a/plugins/auth-backend-module-atlassian-provider/src/types.d.ts b/plugins/auth-backend-module-atlassian-provider/src/types.d.ts index 5fd47bf885..aa945f3705 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/types.d.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/types.d.ts @@ -18,8 +18,9 @@ declare module 'passport-atlassian-oauth2' { import { Request } from 'express'; import { StrategyCreated } from 'passport'; - export class Strategy { + export default class Strategy { constructor(options: any, verify: any); + name?: string; authenticate(this: StrategyCreated, req: Request, options?: any): any; } } From be5f08f3a8474cb428ddaf0f6b2fe6b61b27fdaa Mon Sep 17 00:00:00 2001 From: Olivier Liechti Date: Tue, 9 Jul 2024 10:20:17 +0200 Subject: [PATCH 02/12] Cleaning up details Signed-off-by: Olivier Liechti --- .../src/authenticator.test.ts | 3 --- .../src/authenticator.ts | 8 +------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts b/plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts index b8e9e1ad75..cb723ecece 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts @@ -29,9 +29,6 @@ describe('Strategy', () => { }, () => {}, ); - console.log(strategy.name); - expect(strategy).toBeInstanceOf(Strategy); - expect((strategy as any).name).toBe('atlassian'); } catch (e) { console.error(e); diff --git a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts index ab2cd3f94a..ff020f131d 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts @@ -70,7 +70,6 @@ export const atlassianAuthenticator = createOAuthAuthenticator< baseURL: baseUrl, authorizationURL: `${baseUrl}/authorize`, tokenURL: `${baseUrl}/oauth/token`, - // profileURL: `${baseUrl}/api/v4/user`, profileURL: 'https://api.atlassian.com/me', scope: config.getOptionalString('additionalScopes')?.split(' ') || [], }, @@ -81,14 +80,9 @@ export const atlassianAuthenticator = createOAuthAuthenticator< fullProfile: PassportProfile & { email: string; photo: string }, done: PassportOAuthDoneCallback, ) => { - const fullProfileWithEmails = { - ...fullProfile, - // avatarUrl: fullProfile.photo, - // emails: [{ value: fullProfile.email }], - }; done( undefined, - { fullProfile: fullProfileWithEmails, params, accessToken }, + { fullProfile, params, accessToken }, { refreshToken }, ); }, From f7cf3a2a875f833c63dbab687b77b28eeb991f67 Mon Sep 17 00:00:00 2001 From: Olivier Liechti Date: Tue, 9 Jul 2024 10:30:26 +0200 Subject: [PATCH 03/12] Cleaning up details Signed-off-by: Olivier Liechti --- .../auth-backend-module-atlassian-provider/src/authenticator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts index ff020f131d..1e7b631493 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts @@ -77,7 +77,7 @@ export const atlassianAuthenticator = createOAuthAuthenticator< accessToken: string, refreshToken: string, params: any, - fullProfile: PassportProfile & { email: string; photo: string }, + fullProfile: AtlassianPassportProfile, done: PassportOAuthDoneCallback, ) => { done( From 3cdea7aa1e38893c239d71b2ac073457ffe059ff Mon Sep 17 00:00:00 2001 From: Olivier Liechti Date: Tue, 9 Jul 2024 12:12:58 +0200 Subject: [PATCH 04/12] Fix typescript typing issues Signed-off-by: Olivier Liechti --- .../src/authenticator.ts | 32 +++++++++++-------- .../src/module.ts | 7 ++-- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts index 1e7b631493..8a1b952d15 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts @@ -16,9 +16,10 @@ import { createOAuthAuthenticator, + OAuthAuthenticatorResult, PassportOAuthAuthenticatorHelper, PassportOAuthDoneCallback, - PassportProfile, + ProfileInfo, } from '@backstage/plugin-auth-node'; import AtlassianStrategy from 'passport-atlassian-oauth2'; @@ -32,19 +33,20 @@ export type AtlassianPassportProfile = { }; /** @public */ -export const atlassianAuthenticator = createOAuthAuthenticator< - any, - AtlassianPassportProfile ->({ - defaultProfileTransform: async (input, context) => { - // const result = await PassportOAuthAuthenticatorHelper.defaultProfileTransform(input, context); +export const atlassianAuthenticator = createOAuthAuthenticator({ + defaultProfileTransform: async ( + input: OAuthAuthenticatorResult, + ) => { + const atlassianProfile = input.fullProfile as AtlassianPassportProfile; + + const profile: ProfileInfo = { + displayName: atlassianProfile.displayName, + email: atlassianProfile.email, + picture: atlassianProfile.photo, + }; return { - profile: { - displayName: input.fullProfile.displayName, - email: input.fullProfile.email, - picture: input.fullProfile.photo, - }, + profile, }; }, scopes: { @@ -98,10 +100,12 @@ export const atlassianAuthenticator = createOAuthAuthenticator< }, async authenticate(input, helper) { - return helper.authenticate(input); + const result = await helper.authenticate(input); + return result as OAuthAuthenticatorResult; }, async refresh(input, helper) { - return helper.refresh(input); + const result = await helper.refresh(input); + return result as OAuthAuthenticatorResult; }, }); diff --git a/plugins/auth-backend-module-atlassian-provider/src/module.ts b/plugins/auth-backend-module-atlassian-provider/src/module.ts index 9acb665ede..2080da3065 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/module.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/module.ts @@ -19,7 +19,10 @@ import { commonSignInResolvers, createOAuthProviderFactory, } from '@backstage/plugin-auth-node'; -import { atlassianAuthenticator } from './authenticator'; +import { + AtlassianPassportProfile, + atlassianAuthenticator, +} from './authenticator'; import { atlassianSignInResolvers } from './resolvers'; /** @public */ @@ -34,7 +37,7 @@ export const authModuleAtlassianProvider = createBackendModule({ async init({ providers }) { providers.registerProvider({ providerId: 'atlassian', - factory: createOAuthProviderFactory({ + factory: createOAuthProviderFactory({ authenticator: atlassianAuthenticator, signInResolverFactories: { ...atlassianSignInResolvers, From efda5a301dbe9a4c40e9194559d32cc606734aeb Mon Sep 17 00:00:00 2001 From: Olivier Liechti Date: Tue, 9 Jul 2024 15:08:24 +0200 Subject: [PATCH 05/12] Export Atlassian OAuth Profile type Signed-off-by: Olivier Liechti --- .changeset/nasty-balloons-attack.md | 5 +++++ plugins/auth-backend-module-atlassian-provider/src/index.ts | 3 +-- plugins/auth-backend/src/providers/atlassian/provider.ts | 3 ++- 3 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 .changeset/nasty-balloons-attack.md diff --git a/.changeset/nasty-balloons-attack.md b/.changeset/nasty-balloons-attack.md new file mode 100644 index 0000000000..60229b4a25 --- /dev/null +++ b/.changeset/nasty-balloons-attack.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +Use new Atlassian Passport profile in type definition diff --git a/plugins/auth-backend-module-atlassian-provider/src/index.ts b/plugins/auth-backend-module-atlassian-provider/src/index.ts index cc71c9b7e3..224037b868 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/index.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/index.ts @@ -13,13 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - /** * The atlassian-provider backend module for the auth plugin. * * @packageDocumentation */ - export { atlassianAuthenticator } from './authenticator'; +export type { AtlassianPassportProfile } from './authenticator'; export { authModuleAtlassianProvider as default } from './module'; export { atlassianSignInResolvers } from './resolvers'; diff --git a/plugins/auth-backend/src/providers/atlassian/provider.ts b/plugins/auth-backend/src/providers/atlassian/provider.ts index 22db26fd85..d30164bd6e 100644 --- a/plugins/auth-backend/src/providers/atlassian/provider.ts +++ b/plugins/auth-backend/src/providers/atlassian/provider.ts @@ -26,6 +26,7 @@ import { import { OAuthResult } from '../../lib/oauth'; import { createAuthProviderIntegration } from '../createAuthProviderIntegration'; import { AuthHandler } from '../types'; +import { AtlassianPassportProfile } from '@backstage/plugin-auth-backend-module-atlassian-provider'; /** * Auth provider integration for Atlassian auth @@ -47,7 +48,7 @@ export const atlassian = createAuthProviderIntegration({ resolver: SignInResolver; }; }) { - return createOAuthProviderFactory({ + return createOAuthProviderFactory({ authenticator: atlassianAuthenticator, profileTransform: adaptLegacyOAuthHandler(options?.authHandler), signInResolver: adaptLegacyOAuthSignInResolver(options?.signIn?.resolver), From 55c1a729ac4cdd84afd9ac4bb081b9b1c2ad7274 Mon Sep 17 00:00:00 2001 From: Olivier Liechti Date: Tue, 9 Jul 2024 16:29:50 +0200 Subject: [PATCH 06/12] Simplify the bug fix, by implementing logic in the PassportHelper Signed-off-by: Olivier Liechti --- .changeset/stale-beds-sneeze.md | 6 +++ .../src/authenticator.ts | 37 +++---------------- .../src/index.ts | 1 - .../src/providers/atlassian/provider.ts | 3 +- .../auth-node/src/passport/PassportHelpers.ts | 7 ++++ plugins/auth-node/src/passport/types.ts | 2 + 6 files changed, 22 insertions(+), 34 deletions(-) create mode 100644 .changeset/stale-beds-sneeze.md diff --git a/.changeset/stale-beds-sneeze.md b/.changeset/stale-beds-sneeze.md new file mode 100644 index 0000000000..e2fc5acdf2 --- /dev/null +++ b/.changeset/stale-beds-sneeze.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-auth-backend-module-atlassian-provider': patch +'@backstage/plugin-auth-node': patch +--- + +Fix issues with Atlassian OAuth provider diff --git a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts index 8a1b952d15..23a4a52197 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts @@ -16,39 +16,16 @@ import { createOAuthAuthenticator, - OAuthAuthenticatorResult, PassportOAuthAuthenticatorHelper, PassportOAuthDoneCallback, - ProfileInfo, + PassportProfile, } from '@backstage/plugin-auth-node'; import AtlassianStrategy from 'passport-atlassian-oauth2'; -export type AtlassianPassportProfile = { - id: string; - displayName: string; - email: string; - photo: string; - provider: string; - _json: any; -}; - /** @public */ export const atlassianAuthenticator = createOAuthAuthenticator({ - defaultProfileTransform: async ( - input: OAuthAuthenticatorResult, - ) => { - const atlassianProfile = input.fullProfile as AtlassianPassportProfile; - - const profile: ProfileInfo = { - displayName: atlassianProfile.displayName, - email: atlassianProfile.email, - picture: atlassianProfile.photo, - }; - - return { - profile, - }; - }, + defaultProfileTransform: + PassportOAuthAuthenticatorHelper.defaultProfileTransform, scopes: { required: ['offline_access', 'read:me', 'read:jira-work', 'read:jira-user'], }, @@ -79,7 +56,7 @@ export const atlassianAuthenticator = createOAuthAuthenticator({ accessToken: string, refreshToken: string, params: any, - fullProfile: AtlassianPassportProfile, + fullProfile: PassportProfile, done: PassportOAuthDoneCallback, ) => { done( @@ -100,12 +77,10 @@ export const atlassianAuthenticator = createOAuthAuthenticator({ }, async authenticate(input, helper) { - const result = await helper.authenticate(input); - return result as OAuthAuthenticatorResult; + return helper.authenticate(input); }, async refresh(input, helper) { - const result = await helper.refresh(input); - return result as OAuthAuthenticatorResult; + return helper.refresh(input); }, }); diff --git a/plugins/auth-backend-module-atlassian-provider/src/index.ts b/plugins/auth-backend-module-atlassian-provider/src/index.ts index 224037b868..8f090b76d4 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/index.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/index.ts @@ -19,6 +19,5 @@ * @packageDocumentation */ export { atlassianAuthenticator } from './authenticator'; -export type { AtlassianPassportProfile } from './authenticator'; export { authModuleAtlassianProvider as default } from './module'; export { atlassianSignInResolvers } from './resolvers'; diff --git a/plugins/auth-backend/src/providers/atlassian/provider.ts b/plugins/auth-backend/src/providers/atlassian/provider.ts index d30164bd6e..22db26fd85 100644 --- a/plugins/auth-backend/src/providers/atlassian/provider.ts +++ b/plugins/auth-backend/src/providers/atlassian/provider.ts @@ -26,7 +26,6 @@ import { import { OAuthResult } from '../../lib/oauth'; import { createAuthProviderIntegration } from '../createAuthProviderIntegration'; import { AuthHandler } from '../types'; -import { AtlassianPassportProfile } from '@backstage/plugin-auth-backend-module-atlassian-provider'; /** * Auth provider integration for Atlassian auth @@ -48,7 +47,7 @@ export const atlassian = createAuthProviderIntegration({ resolver: SignInResolver; }; }) { - return createOAuthProviderFactory({ + return createOAuthProviderFactory({ authenticator: atlassianAuthenticator, profileTransform: adaptLegacyOAuthHandler(options?.authHandler), signInResolver: adaptLegacyOAuthSignInResolver(options?.signIn?.resolver), diff --git a/plugins/auth-node/src/passport/PassportHelpers.ts b/plugins/auth-node/src/passport/PassportHelpers.ts index d7b554d56a..8531309917 100644 --- a/plugins/auth-node/src/passport/PassportHelpers.ts +++ b/plugins/auth-node/src/passport/PassportHelpers.ts @@ -42,12 +42,19 @@ export class PassportHelpers { email = firstEmail.value; } + // This is the case for Atlassian + if (profile.email) { + email = profile.email; + } + let picture: string | undefined = undefined; if (profile.avatarUrl) { picture = profile.avatarUrl; } else if (profile.photos && profile.photos.length > 0) { const [firstPhoto] = profile.photos; picture = firstPhoto.value; + } else if (profile.photo) { + picture = profile.photo; // This is the case for Atlassian } let displayName: string | undefined = diff --git a/plugins/auth-node/src/passport/types.ts b/plugins/auth-node/src/passport/types.ts index 4a3f41b6b2..d5681fa9e3 100644 --- a/plugins/auth-node/src/passport/types.ts +++ b/plugins/auth-node/src/passport/types.ts @@ -19,6 +19,8 @@ import { Profile } from 'passport'; /** @public */ export type PassportProfile = Profile & { avatarUrl?: string; + email?: string; + photo?: string; }; /** @public */ From 487c100100365db39a89814f9dd127f8c17ec40e Mon Sep 17 00:00:00 2001 From: Olivier Liechti Date: Tue, 9 Jul 2024 16:41:18 +0200 Subject: [PATCH 07/12] Simplify the bug fix, by implementing logic in the PassportHelper Signed-off-by: Olivier Liechti --- .../auth-backend-module-atlassian-provider/src/module.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/plugins/auth-backend-module-atlassian-provider/src/module.ts b/plugins/auth-backend-module-atlassian-provider/src/module.ts index 2080da3065..9acb665ede 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/module.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/module.ts @@ -19,10 +19,7 @@ import { commonSignInResolvers, createOAuthProviderFactory, } from '@backstage/plugin-auth-node'; -import { - AtlassianPassportProfile, - atlassianAuthenticator, -} from './authenticator'; +import { atlassianAuthenticator } from './authenticator'; import { atlassianSignInResolvers } from './resolvers'; /** @public */ @@ -37,7 +34,7 @@ export const authModuleAtlassianProvider = createBackendModule({ async init({ providers }) { providers.registerProvider({ providerId: 'atlassian', - factory: createOAuthProviderFactory({ + factory: createOAuthProviderFactory({ authenticator: atlassianAuthenticator, signInResolverFactories: { ...atlassianSignInResolvers, From e1eda89df32a9d2b6987c87e8351288379332210 Mon Sep 17 00:00:00 2001 From: Olivier Liechti Date: Tue, 9 Jul 2024 18:58:55 +0200 Subject: [PATCH 08/12] Add api-report.md Signed-off-by: Olivier Liechti --- plugins/auth-node/api-report.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/auth-node/api-report.md b/plugins/auth-node/api-report.md index dc1d86308a..a3750819f9 100644 --- a/plugins/auth-node/api-report.md +++ b/plugins/auth-node/api-report.md @@ -552,6 +552,8 @@ export type PassportOAuthResult = { // @public (undocumented) export type PassportProfile = Profile & { avatarUrl?: string; + email?: string; + photo?: string; }; // @public From 9076843ffd67153f71b1ff9df0eb9af68ba96236 Mon Sep 17 00:00:00 2001 From: Olivier Liechti Date: Tue, 9 Jul 2024 19:48:38 +0200 Subject: [PATCH 09/12] Adatp test to the payload returned by the actual Atlassian strategy (vs its parent) Signed-off-by: Olivier Liechti --- .../src/module.test.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/plugins/auth-backend-module-atlassian-provider/src/module.test.ts b/plugins/auth-backend-module-atlassian-provider/src/module.test.ts index 7b92218d56..7e83aed179 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/module.test.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/module.test.ts @@ -64,10 +64,12 @@ describe('authModuleAtlassianProvider', () => { expect(startUrl.pathname).toBe('/authorize'); expect(Object.fromEntries(startUrl.searchParams)).toEqual({ response_type: 'code', + audience: 'api.atlassian.com', client_id: 'my-client-id', + prompt: 'consent', redirect_uri: `http://localhost:${server.port()}/api/auth/atlassian/handler/frame`, state: expect.any(String), - scope: 'offline_access read:jira-work read:jira-user', + scope: 'offline_access read:me read:jira-work read:jira-user', }); expect(decodeOAuthState(startUrl.searchParams.get('state')!)).toEqual({ @@ -92,10 +94,7 @@ describe('authModuleAtlassianProvider', () => { development: { clientId: 'my-client-id', clientSecret: 'my-client-secret', - additionalScopes: [ - 'read:filter:jira', - 'read:jira-work', // already required - ], + additionalScopes: 'read:filter:jira read:jira-work', // 2nd is already required }, }, }, @@ -123,11 +122,14 @@ describe('authModuleAtlassianProvider', () => { expect(startUrl.origin).toBe('https://auth.atlassian.com'); expect(startUrl.pathname).toBe('/authorize'); expect(Object.fromEntries(startUrl.searchParams)).toEqual({ + audience: 'api.atlassian.com', response_type: 'code', client_id: 'my-client-id', + prompt: 'consent', redirect_uri: `http://localhost:${server.port()}/api/auth/atlassian/handler/frame`, state: expect.any(String), - scope: 'offline_access read:jira-work read:jira-user read:filter:jira', + scope: + 'offline_access read:me read:jira-work read:jira-user read:filter:jira', }); expect(decodeOAuthState(startUrl.searchParams.get('state')!)).toEqual({ From aca86a6a007fa8c9c431f92b685f58c591f25c73 Mon Sep 17 00:00:00 2001 From: Olivier Liechti Date: Wed, 10 Jul 2024 08:11:02 +0200 Subject: [PATCH 10/12] Address review comments Signed-off-by: Olivier Liechti --- .changeset/nasty-balloons-attack.md | 5 ----- .changeset/stale-beds-sneeze.md | 3 +-- plugins/auth-node/src/passport/PassportHelpers.ts | 9 ++++----- 3 files changed, 5 insertions(+), 12 deletions(-) delete mode 100644 .changeset/nasty-balloons-attack.md diff --git a/.changeset/nasty-balloons-attack.md b/.changeset/nasty-balloons-attack.md deleted file mode 100644 index 60229b4a25..0000000000 --- a/.changeset/nasty-balloons-attack.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/plugin-auth-backend': patch ---- - -Use new Atlassian Passport profile in type definition diff --git a/.changeset/stale-beds-sneeze.md b/.changeset/stale-beds-sneeze.md index e2fc5acdf2..695e09a624 100644 --- a/.changeset/stale-beds-sneeze.md +++ b/.changeset/stale-beds-sneeze.md @@ -1,6 +1,5 @@ --- -'@backstage/plugin-auth-backend-module-atlassian-provider': patch '@backstage/plugin-auth-node': patch --- -Fix issues with Atlassian OAuth provider +Fix issues with Atlassian OAuth provider: retrieve the email and photo that were not in arrays but rather in single props. diff --git a/plugins/auth-node/src/passport/PassportHelpers.ts b/plugins/auth-node/src/passport/PassportHelpers.ts index 8531309917..d1ddee20a7 100644 --- a/plugins/auth-node/src/passport/PassportHelpers.ts +++ b/plugins/auth-node/src/passport/PassportHelpers.ts @@ -40,10 +40,8 @@ export class PassportHelpers { if (profile.emails && profile.emails.length > 0) { const [firstEmail] = profile.emails; email = firstEmail.value; - } - - // This is the case for Atlassian - if (profile.email) { + } else if (profile.email) { + // This is the case for Atlassian email = profile.email; } @@ -54,7 +52,8 @@ export class PassportHelpers { const [firstPhoto] = profile.photos; picture = firstPhoto.value; } else if (profile.photo) { - picture = profile.photo; // This is the case for Atlassian + // This is the case for Atlassian + picture = profile.photo; } let displayName: string | undefined = From c48f2cc68df4065d1d19fbbf09e167fe89492600 Mon Sep 17 00:00:00 2001 From: Olivier Liechti Date: Wed, 10 Jul 2024 10:42:16 +0200 Subject: [PATCH 11/12] Cleanup test Signed-off-by: Olivier Liechti --- .../src/authenticator.test.ts | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts b/plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts index cb723ecece..ab0a4840f9 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts @@ -17,21 +17,16 @@ import Strategy from 'passport-atlassian-oauth2'; describe('Strategy', () => { it('should be an instance of', () => { - try { - console.log(Strategy); - const strategy = new Strategy( - { - authorizationURL: 'https://auth.atlassian.com/authorize', - tokenURL: 'https://auth.atlassian.com/oauth/token', - clientID: 'my-client-id', - clientSecret: 'my-client-secret', - scope: ['offline_access', 'read:jira-work', 'read:jira-user'], - }, - () => {}, - ); - expect((strategy as any).name).toBe('atlassian'); - } catch (e) { - console.error(e); - } + const strategy = new Strategy( + { + authorizationURL: 'https://auth.atlassian.com/authorize', + tokenURL: 'https://auth.atlassian.com/oauth/token', + clientID: 'my-client-id', + clientSecret: 'my-client-secret', + scope: ['offline_access', 'read:jira-work', 'read:jira-user'], + }, + () => {}, + ); + expect((strategy as any).name).toBe('atlassian'); }); }); From 3d9abab684c528f9547814713bf1a93531059417 Mon Sep 17 00:00:00 2001 From: Olivier Liechti Date: Wed, 10 Jul 2024 11:10:56 +0200 Subject: [PATCH 12/12] Pass an empty array to the 'scope' prop required by Atlassian strategy, but which value is not actually used Signed-off-by: Olivier Liechti --- .../auth-backend-module-atlassian-provider/src/authenticator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts index 23a4a52197..d4f46b5a8c 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts @@ -50,7 +50,7 @@ export const atlassianAuthenticator = createOAuthAuthenticator({ authorizationURL: `${baseUrl}/authorize`, tokenURL: `${baseUrl}/oauth/token`, profileURL: 'https://api.atlassian.com/me', - scope: config.getOptionalString('additionalScopes')?.split(' ') || [], + scope: [], // the Atlassian strategy requires a scope, but Backstage passes the right set of scopes when calling OAuth2Strategy.prototype.authenticate }, ( accessToken: string,