Addressed PR feedback from @Rugvip

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';

packages/core-api/src/apis/implementations/auth/oidc/Oidc.test.ts

@@ -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
-  });
-});

packages/core-api/src/apis/implementations/auth/oidc/Oidc.ts

@@ -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<OpenIdConnectSession>;
-  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<OpenIdConnectSession>;
-  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<SessionState> {
-    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<BackstageIdentity | undefined> {
-    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<string> {
-    if (!scopes) {
-      return new Set();
-    }
-
-    const scopeList = Array.isArray(scopes)
-      ? scopes
-      : scopes.split(/[\s|,]/).filter(Boolean);
-
-    return new Set(scopeTransform(scopeList));
-  }
-}
-
-export default OpenIdConnect;

packages/core-api/src/apis/implementations/auth/oidc/index.ts

@@ -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';

packages/core-api/src/apis/implementations/auth/oidc/types.ts

@@ -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<string>;
-    expiresAt: Date;
-  };
-  profile: ProfileInfo;
-  backstageIdentity: BackstageIdentity;
-};

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 }),
   }),
 ];

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<OidcStrategy<UserinfoResponse, Client>> {
-    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, {