From a480f670c714c05fa77f26785ef79b44fdddbe1b Mon Sep 17 00:00:00 2001
From: Johan Haals <johan.haals@gmail.com>
Date: Thu, 24 Feb 2022 14:38:16 +0100
Subject: [PATCH] core-plugin-api: Require OAuth provider id

Signed-off-by: Johan Haals <johan.haals@gmail.com>
---
 .changeset/silver-chairs-compete.md                       | 5 +++++
 .../implementations/OAuthRequestApi/MockOAuthApi.test.ts  | 8 ++++----
 .../OAuthRequestApi/OAuthRequestManager.test.ts           | 1 +
 .../OAuthRequestApi/OAuthRequestManager.ts                | 6 ------
 .../src/apis/definitions/OAuthRequestApi.ts               | 8 ++------
 5 files changed, 12 insertions(+), 16 deletions(-)
 create mode 100644 .changeset/silver-chairs-compete.md

diff --git a/.changeset/silver-chairs-compete.md b/.changeset/silver-chairs-compete.md
new file mode 100644
index 0000000000..356cd2043b
--- /dev/null
+++ b/.changeset/silver-chairs-compete.md
@@ -0,0 +1,5 @@
+---
+'@backstage/core-plugin-api': minor
+---
+
+**BREAKING**: OAuth provider id is now required when passing a provider to `createAuthRequester`.
diff --git a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts
index 76cf01d20a..a64fadeb3e 100644
--- a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts
+++ b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/MockOAuthApi.test.ts
@@ -23,13 +23,13 @@ describe('MockOAuthApi', () => {
 
     const authHandler1 = jest.fn().mockImplementation(() => authResult);
     const requester1 = mock.createAuthRequester({
-      provider: { icon: () => null, title: 'Test' },
+      provider: { icon: () => null, title: 'Test', id: 'test-provider' },
       onAuthRequest: authHandler1,
     });
 
     const authHandler2 = jest.fn().mockResolvedValue('other');
     const requester2 = mock.createAuthRequester({
-      provider: { icon: () => null, title: 'Test' },
+      provider: { icon: () => null, title: 'Test', id: 'test-provider' },
       onAuthRequest: authHandler2,
     });
 
@@ -66,13 +66,13 @@ describe('MockOAuthApi', () => {
 
     const authHandler1 = jest.fn();
     const requester1 = mock.createAuthRequester({
-      provider: { icon: () => null, title: 'Test' },
+      provider: { icon: () => null, title: 'Test', id: 'test-provider' },
       onAuthRequest: authHandler1,
     });
 
     const authHandler2 = jest.fn();
     const requester2 = mock.createAuthRequester({
-      provider: { icon: () => null, title: 'Test' },
+      provider: { icon: () => null, title: 'Test', id: 'test-provider' },
       onAuthRequest: authHandler2,
     });
 
diff --git a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts
index 3110197ad9..4dd6af715e 100644
--- a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts
+++ b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.test.ts
@@ -25,6 +25,7 @@ describe('OAuthRequestManager', () => {
 
     const requester = manager.createAuthRequester({
       provider: {
+        id: 'my-provider',
         title: 'My Provider',
         icon: () => null,
       },
diff --git a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts
index f9d9270b2a..5d467be7b4 100644
--- a/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts
+++ b/packages/core-app-api/src/apis/implementations/OAuthRequestApi/OAuthRequestManager.ts
@@ -39,12 +39,6 @@ export class OAuthRequestManager implements OAuthRequestApi {
   private handlerCount = 0;
 
   createAuthRequester<T>(options: OAuthRequesterOptions<T>): OAuthRequester<T> {
-    if (!options.provider.id) {
-      // eslint-disable-next-line no-console
-      console.warn(
-        'DEPRECATION WARNING: Not passing a provider id to createAuthRequester is deprecated, it will be required in the future',
-      );
-    }
     const handler = new OAuthPendingRequests<T>();
 
     const index = this.handlerCount;
diff --git a/packages/core-plugin-api/src/apis/definitions/OAuthRequestApi.ts b/packages/core-plugin-api/src/apis/definitions/OAuthRequestApi.ts
index ccca2c50e3..75bf3a3864 100644
--- a/packages/core-plugin-api/src/apis/definitions/OAuthRequestApi.ts
+++ b/packages/core-plugin-api/src/apis/definitions/OAuthRequestApi.ts
@@ -27,10 +27,8 @@ import { AuthProviderInfo } from './auth';
 export type OAuthRequesterOptions<TOAuthResponse> = {
   /**
    * Information about the auth provider, which will be forwarded to auth requests.
-   *
-   * Not passing in an `id` is deprecated, and it will be required in the future.
    */
-  provider: Omit<AuthProviderInfo, 'id'> & { id?: string };
+  provider: AuthProviderInfo;
 
   /**
    * Implementation of the auth flow, which will be called synchronously when
@@ -71,10 +69,8 @@ export type OAuthRequester<TAuthResponse> = (
 export type PendingOAuthRequest = {
   /**
    * Information about the auth provider, as given in the AuthRequesterOptions
-   *
-   * Not passing in an `id` is deprecated, and it will be required in the future.
    */
-  provider: Omit<AuthProviderInfo, 'id'> & { id?: string };
+  provider: AuthProviderInfo;
 
   /**
    * Rejects the request, causing all pending AuthRequester calls to fail with "RejectedError".