From 52e79a003238ffd0a7950b89a765787687ffbc53 Mon Sep 17 00:00:00 2001 From: Brian Leathem Date: Wed, 18 Nov 2020 18:23:59 -0800 Subject: [PATCH] Addressed PR feedback from @Rugvip --- .../src/apis/implementations/auth/index.ts | 1 - .../implementations/auth/oidc/Oidc.test.ts | 152 --------------- .../apis/implementations/auth/oidc/Oidc.ts | 183 ------------------ .../apis/implementations/auth/oidc/index.ts | 18 -- .../apis/implementations/auth/oidc/types.ts | 28 --- packages/core/src/api-wrappers/defaultApis.ts | 3 +- .../src/providers/oidc/provider.ts | 15 +- 7 files changed, 5 insertions(+), 395 deletions(-) delete mode 100644 packages/core-api/src/apis/implementations/auth/oidc/Oidc.test.ts delete mode 100644 packages/core-api/src/apis/implementations/auth/oidc/Oidc.ts delete mode 100644 packages/core-api/src/apis/implementations/auth/oidc/index.ts delete mode 100644 packages/core-api/src/apis/implementations/auth/oidc/types.ts diff --git a/packages/core-api/src/apis/implementations/auth/index.ts b/packages/core-api/src/apis/implementations/auth/index.ts index 5adfc6f621..7ef78d19cf 100644 --- a/packages/core-api/src/apis/implementations/auth/index.ts +++ b/packages/core-api/src/apis/implementations/auth/index.ts @@ -23,4 +23,3 @@ export * from './saml'; export * from './auth0'; export * from './microsoft'; export * from './onelogin'; -export * from './oidc'; diff --git a/packages/core-api/src/apis/implementations/auth/oidc/Oidc.test.ts b/packages/core-api/src/apis/implementations/auth/oidc/Oidc.test.ts deleted file mode 100644 index 4286fe7e5d..0000000000 --- a/packages/core-api/src/apis/implementations/auth/oidc/Oidc.test.ts +++ /dev/null @@ -1,152 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * 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 OpenIdConnect from './Oidc'; - -const theFuture = new Date(Date.now() + 3600000); -const thePast = new Date(Date.now() - 10); - -const PREFIX = 'https://www.googleapis.com/auth/'; - -const scopeTransform = (x: string[]) => x; - -describe('OpenIdConnect', () => { - it('should get refreshed access token', async () => { - const getSession = jest.fn().mockResolvedValue({ - providerInfo: { accessToken: 'access-token', expiresAt: theFuture }, - }); - const openIdConnect = new OpenIdConnect({ - sessionManager: { getSession } as any, - scopeTransform, - }); - - expect(await openIdConnect.getAccessToken('my-scope my-scope2')).toBe( - 'access-token', - ); - expect(getSession).toBeCalledTimes(1); - expect(getSession.mock.calls[0][0].scopes).toEqual( - new Set(['my-scope', 'my-scope2']), - ); - }); - - it('should transform scopes', async () => { - const getSession = jest.fn().mockResolvedValue({ - providerInfo: { accessToken: 'access-token', expiresAt: theFuture }, - }); - const openIdConnect = new OpenIdConnect({ - sessionManager: { getSession } as any, - scopeTransform: scopes => scopes.map(scope => `my-prefix/${scope}`), - }); - - expect(await openIdConnect.getAccessToken('my-scope')).toBe('access-token'); - expect(getSession).toBeCalledTimes(1); - expect(getSession.mock.calls[0][0].scopes).toEqual( - new Set(['my-prefix/my-scope']), - ); - }); - - it('should get refreshed id token', async () => { - const getSession = jest.fn().mockResolvedValue({ - providerInfo: { idToken: 'id-token', expiresAt: theFuture }, - }); - const openIdConnect = new OpenIdConnect({ - sessionManager: { getSession } as any, - scopeTransform, - }); - - expect(await openIdConnect.getIdToken()).toBe('id-token'); - expect(getSession).toBeCalledTimes(1); - }); - - it('should get optional id token', async () => { - const getSession = jest.fn().mockResolvedValue({ - providerInfo: { idToken: 'id-token', expiresAt: theFuture }, - }); - const openIdConnect = new OpenIdConnect({ - sessionManager: { getSession } as any, - scopeTransform, - }); - - expect(await openIdConnect.getIdToken({ optional: true })).toBe('id-token'); - expect(getSession).toBeCalledTimes(1); - }); - - it('should share popup closed errors', async () => { - const error = new Error('NOPE'); - error.name = 'RejectedError'; - const getSession = jest - .fn() - .mockResolvedValueOnce({ - providerInfo: { - accessToken: 'access-token', - expiresAt: theFuture, - scopes: new Set([`${PREFIX}not-enough`]), - }, - }) - .mockRejectedValue(error); - const openIdConnect = new OpenIdConnect({ - sessionManager: { getSession } as any, - scopeTransform, - }); - - // Make sure we have a session before we do the double request, so that we get past the !this.currentSession check - await expect(openIdConnect.getAccessToken()).resolves.toBe('access-token'); - - const promise1 = openIdConnect.getAccessToken('more'); - const promise2 = openIdConnect.getAccessToken('more'); - await expect(promise1).rejects.toBe(error); - await expect(promise2).rejects.toBe(error); - expect(getSession).toBeCalledTimes(3); - }); - - it('should wait for all session refreshes', async () => { - const initialSession = { - providerInfo: { - idToken: 'token1', - expiresAt: theFuture, - scopes: new Set(), - }, - }; - const getSession = jest - .fn() - .mockResolvedValueOnce(initialSession) - .mockResolvedValue({ - providerInfo: { - idToken: 'token2', - expiresAt: theFuture, - scopes: new Set(), - }, - }); - const openIdConnect = new OpenIdConnect({ - sessionManager: { getSession } as any, - scopeTransform, - }); - - // Grab the expired session first - await expect(openIdConnect.getIdToken()).resolves.toBe('token1'); - expect(getSession).toBeCalledTimes(1); - - initialSession.providerInfo.expiresAt = thePast; - - const promise1 = openIdConnect.getIdToken(); - const promise2 = openIdConnect.getIdToken(); - const promise3 = openIdConnect.getIdToken(); - await expect(promise1).resolves.toBe('token2'); - await expect(promise2).resolves.toBe('token2'); - await expect(promise3).resolves.toBe('token2'); - expect(getSession).toBeCalledTimes(4); // De-duping of session requests happens in client - }); -}); diff --git a/packages/core-api/src/apis/implementations/auth/oidc/Oidc.ts b/packages/core-api/src/apis/implementations/auth/oidc/Oidc.ts deleted file mode 100644 index deb8331432..0000000000 --- a/packages/core-api/src/apis/implementations/auth/oidc/Oidc.ts +++ /dev/null @@ -1,183 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * 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 OpenIdConnectIcon from '@material-ui/icons/AcUnit'; -import { DefaultAuthConnector } from '../../../../lib/AuthConnector'; -import { RefreshingAuthSessionManager } from '../../../../lib/AuthSessionManager'; -import { SessionManager } from '../../../../lib/AuthSessionManager/types'; -import { Observable } from '../../../../types'; -import { - AuthRequestOptions, - BackstageIdentity, - OAuthApi, - OpenIdConnectApi, - ProfileInfo, - ProfileInfoApi, - SessionState, - SessionApi, - BackstageIdentityApi, -} from '../../../definitions/auth'; -import { OpenIdConnectSession } from './types'; -import { OAuthApiCreateOptions } from '../types'; - -type Options = { - sessionManager: SessionManager; - scopeTransform: (scopes: string[]) => string[]; -}; - -type CreateOptions = OAuthApiCreateOptions & { - scopeTransform?: (scopes: string[]) => string[]; -}; - -export type OpenIdConnectResponse = { - providerInfo: { - accessToken: string; - idToken: string; - scope: string; - expiresInSeconds: number; - }; - profile: ProfileInfo; - backstageIdentity: BackstageIdentity; -}; - -const DEFAULT_PROVIDER = { - id: 'oidc', - title: 'Open ID Connect Identity Provider', - icon: OpenIdConnectIcon, -}; - -class OpenIdConnect - implements - OAuthApi, - OpenIdConnectApi, - ProfileInfoApi, - BackstageIdentityApi, - SessionApi { - static create({ - discoveryApi, - environment = 'development', - provider = DEFAULT_PROVIDER, - oauthRequestApi, - defaultScopes = [], - scopeTransform = x => x, - }: CreateOptions) { - const connector = new DefaultAuthConnector({ - discoveryApi, - environment, - provider, - oauthRequestApi: oauthRequestApi, - sessionTransform(res: OpenIdConnectResponse): OpenIdConnectSession { - return { - ...res, - providerInfo: { - idToken: res.providerInfo.idToken, - accessToken: res.providerInfo.accessToken, - scopes: OpenIdConnect.normalizeScopes( - scopeTransform, - res.providerInfo.scope, - ), - expiresAt: new Date( - Date.now() + res.providerInfo.expiresInSeconds * 1000, - ), - }, - }; - }, - }); - - const sessionManager = new RefreshingAuthSessionManager({ - connector, - defaultScopes: new Set(defaultScopes), - sessionScopes: (session: OpenIdConnectSession) => - session.providerInfo.scopes, - sessionShouldRefresh: (session: OpenIdConnectSession) => { - const expiresInSec = - (session.providerInfo.expiresAt.getTime() - Date.now()) / 1000; - return expiresInSec < 60 * 5; - }, - }); - - return new OpenIdConnect({ sessionManager, scopeTransform }); - } - - private readonly sessionManager: SessionManager; - private readonly scopeTransform: (scopes: string[]) => string[]; - - constructor(options: Options) { - this.sessionManager = options.sessionManager; - this.scopeTransform = options.scopeTransform; - } - - async signIn() { - await this.getAccessToken(); - } - - async signOut() { - await this.sessionManager.removeSession(); - } - - sessionState$(): Observable { - return this.sessionManager.sessionState$(); - } - - async getAccessToken( - scope?: string | string[], - options?: AuthRequestOptions, - ) { - const normalizedScopes = OpenIdConnect.normalizeScopes( - this.scopeTransform, - scope, - ); - const session = await this.sessionManager.getSession({ - ...options, - scopes: normalizedScopes, - }); - return session?.providerInfo.accessToken ?? ''; - } - - async getIdToken(options: AuthRequestOptions = {}) { - const session = await this.sessionManager.getSession(options); - return session?.providerInfo.idToken ?? ''; - } - - async getBackstageIdentity( - options: AuthRequestOptions = {}, - ): Promise { - const session = await this.sessionManager.getSession(options); - return session?.backstageIdentity; - } - - async getProfile(options: AuthRequestOptions = {}) { - const session = await this.sessionManager.getSession(options); - return session?.profile; - } - - private static normalizeScopes( - scopeTransform: (scopes: string[]) => string[], - scopes?: string | string[], - ): Set { - if (!scopes) { - return new Set(); - } - - const scopeList = Array.isArray(scopes) - ? scopes - : scopes.split(/[\s|,]/).filter(Boolean); - - return new Set(scopeTransform(scopeList)); - } -} - -export default OpenIdConnect; diff --git a/packages/core-api/src/apis/implementations/auth/oidc/index.ts b/packages/core-api/src/apis/implementations/auth/oidc/index.ts deleted file mode 100644 index cf9add0757..0000000000 --- a/packages/core-api/src/apis/implementations/auth/oidc/index.ts +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * 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. - */ - -export { default as OpenIdConnect } from './Oidc'; -export * from './types'; diff --git a/packages/core-api/src/apis/implementations/auth/oidc/types.ts b/packages/core-api/src/apis/implementations/auth/oidc/types.ts deleted file mode 100644 index ae9696c229..0000000000 --- a/packages/core-api/src/apis/implementations/auth/oidc/types.ts +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * 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 { ProfileInfo, BackstageIdentity } from '../../../definitions'; - -export type OpenIdConnectSession = { - providerInfo: { - idToken: string; - accessToken: string; - scopes: Set; - expiresAt: Date; - }; - profile: ProfileInfo; - backstageIdentity: BackstageIdentity; -}; diff --git a/packages/core/src/api-wrappers/defaultApis.ts b/packages/core/src/api-wrappers/defaultApis.ts index adc5fce30a..9b3d26fb4d 100644 --- a/packages/core/src/api-wrappers/defaultApis.ts +++ b/packages/core/src/api-wrappers/defaultApis.ts @@ -47,7 +47,6 @@ import { oneloginAuthApiRef, OneLoginAuth, oidcApiRef, - OpenIdConnect, } from '@backstage/core-api'; export const defaultApis = [ @@ -162,6 +161,6 @@ export const defaultApis = [ oauthRequestApi: oauthRequestApiRef, }, factory: ({ discoveryApi, oauthRequestApi }) => - OpenIdConnect.create({ discoveryApi, oauthRequestApi }), + OAuth2.create({ discoveryApi, oauthRequestApi }), }), ]; diff --git a/plugins/auth-backend/src/providers/oidc/provider.ts b/plugins/auth-backend/src/providers/oidc/provider.ts index ed1488851e..71ea2c4103 100644 --- a/plugins/auth-backend/src/providers/oidc/provider.ts +++ b/plugins/auth-backend/src/providers/oidc/provider.ts @@ -46,9 +46,7 @@ type PrivateInfo = { }; export type OidcAuthProviderOptions = OAuthProviderOptions & { - authorizationUrl: string; - tokenUrl: string; - adUrl: string; + metadataUrl: string; tokenSignedResponseAlg?: string; }; @@ -118,8 +116,7 @@ export class OidcAuthProvider implements OAuthHandlers { private async setupStrategy( options: OidcAuthProviderOptions, ): Promise> { - const issuer = await Issuer.discover(options.adUrl); - // console.log('Discovered issuer %s %O', issuer.issuer, issuer.metadata); + const issuer = await Issuer.discover(options.metadataUrl); const client = new issuer.Client({ client_id: options.clientId, client_secret: options.clientSecret, @@ -191,9 +188,7 @@ export const createOidcProvider: AuthProviderFactory = ({ const clientId = envConfig.getString('clientId'); const clientSecret = envConfig.getString('clientSecret'); const callbackUrl = `${globalConfig.baseUrl}/${providerId}/handler/frame`; - const authorizationUrl = envConfig.getString('authorizationUrl'); - const adUrl = envConfig.getString('adUrl'); - const tokenUrl = envConfig.getString('tokenUrl'); + const metadataUrl = envConfig.getString('metadataUrl'); const tokenSignedResponseAlg = envConfig.getString( 'tokenSignedResponseAlg', ); @@ -202,10 +197,8 @@ export const createOidcProvider: AuthProviderFactory = ({ clientId, clientSecret, callbackUrl, - authorizationUrl, - tokenUrl, tokenSignedResponseAlg, - adUrl, + metadataUrl, }); return OAuthAdapter.fromConfig(globalConfig, provider, {