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/.changeset/stale-beds-sneeze.md b/.changeset/stale-beds-sneeze.md new file mode 100644 index 0000000000..695e09a624 --- /dev/null +++ b/.changeset/stale-beds-sneeze.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-node': patch +--- + +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-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..ab0a4840f9 --- /dev/null +++ b/plugins/auth-backend-module-atlassian-provider/src/authenticator.test.ts @@ -0,0 +1,32 @@ +/* + * 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', () => { + 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'); + }); +}); diff --git a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts index 0ced9cf831..d4f46b5a8c 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/authenticator.ts @@ -20,14 +20,14 @@ import { PassportOAuthDoneCallback, PassportProfile, } from '@backstage/plugin-auth-node'; -import { Strategy as AtlassianStrategy } from 'passport-atlassian-oauth2'; +import AtlassianStrategy from 'passport-atlassian-oauth2'; /** @public */ export const atlassianAuthenticator = createOAuthAuthenticator({ defaultProfileTransform: PassportOAuthAuthenticatorHelper.defaultProfileTransform, 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,7 +49,8 @@ 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: [], // the Atlassian strategy requires a scope, but Backstage passes the right set of scopes when calling OAuth2Strategy.prototype.authenticate }, ( accessToken: string, diff --git a/plugins/auth-backend-module-atlassian-provider/src/index.ts b/plugins/auth-backend-module-atlassian-provider/src/index.ts index cc71c9b7e3..8f090b76d4 100644 --- a/plugins/auth-backend-module-atlassian-provider/src/index.ts +++ b/plugins/auth-backend-module-atlassian-provider/src/index.ts @@ -13,13 +13,11 @@ * 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 { authModuleAtlassianProvider as default } from './module'; export { atlassianSignInResolvers } from './resolvers'; 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({ 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; } } 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 diff --git a/plugins/auth-node/src/passport/PassportHelpers.ts b/plugins/auth-node/src/passport/PassportHelpers.ts index d7b554d56a..d1ddee20a7 100644 --- a/plugins/auth-node/src/passport/PassportHelpers.ts +++ b/plugins/auth-node/src/passport/PassportHelpers.ts @@ -40,6 +40,9 @@ export class PassportHelpers { if (profile.emails && profile.emails.length > 0) { const [firstEmail] = profile.emails; email = firstEmail.value; + } else if (profile.email) { + // This is the case for Atlassian + email = profile.email; } let picture: string | undefined = undefined; @@ -48,6 +51,9 @@ export class PassportHelpers { } else if (profile.photos && profile.photos.length > 0) { const [firstPhoto] = profile.photos; picture = firstPhoto.value; + } else if (profile.photo) { + // This is the case for Atlassian + picture = profile.photo; } 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 */