feat: implementing fixes for wildcard matching for callback URLs

Signed-off-by: benjdlambert <ben@blam.sh>

plugins/auth-backend/src/service/OidcRouter.test.ts

@@ -70,6 +70,15 @@ describe('OidcRouter', () => {
 
     const mockAuth = mockServices.auth.mock();
     const mockHttpAuth = mockServices.httpAuth.mock();
+    const mockConfig = mockServices.rootConfig({
+      data: {
+        auth: {
+          experimentalDynamicClientRegistration: {
+            enabled: true,
+          },
+        },
+      },
+    });
 
     const oidcService = OidcService.create({
       auth: mockAuth,
@@ -77,6 +86,7 @@ describe('OidcRouter', () => {
       baseUrl: 'http://localhost:7000',
       userInfo: userInfoDatabase,
       oidc: oidcDatabase,
+      config: mockConfig,
     });
 
     const oidcRouter = OidcRouter.create({
@@ -88,7 +98,7 @@ describe('OidcRouter', () => {
       userInfo: userInfoDatabase,
       oidc: oidcDatabase,
       httpAuth: mockHttpAuth,
-      enableDynamicClientRegistration: true,
+      config: mockConfig,
     });
 
     return {
@@ -303,7 +313,7 @@ describe('OidcRouter', () => {
           .expect(302);
 
         expect(response.header.location).toMatch(
-          /^http:\/\/localhost:3000\/auth\/sessions\/[a-f0-9-]+$/,
+          /^http:\/\/localhost:3000\/oauth2\/authorize\/[a-f0-9-]+$/,
         );
       });
 

plugins/auth-backend/src/service/OidcRouter.ts

@@ -20,6 +20,7 @@ import {
   AuthService,
   HttpAuthService,
   LoggerService,
+  RootConfigService,
 } from '@backstage/backend-plugin-api';
 import { TokenIssuer } from '../identity/types';
 import { UserInfoDatabase } from '../database/UserInfoDatabase';
@@ -33,7 +34,7 @@ export class OidcRouter {
     private readonly auth: AuthService,
     private readonly appUrl: string,
     private readonly httpAuth: HttpAuthService,
-    private readonly enableDynamicClientRegistration: boolean,
+    private readonly config: RootConfigService,
   ) {}
 
   static create(options: {
@@ -45,7 +46,7 @@ export class OidcRouter {
     userInfo: UserInfoDatabase;
     oidc: OidcDatabase;
     httpAuth: HttpAuthService;
-    enableDynamicClientRegistration: boolean;
+    config: RootConfigService;
   }) {
     return new OidcRouter(
       OidcService.create(options),
@@ -53,7 +54,7 @@ export class OidcRouter {
       options.auth,
       options.appUrl,
       options.httpAuth,
-      options.enableDynamicClientRegistration,
+      options.config,
     );
   }
 
@@ -97,7 +98,11 @@ export class OidcRouter {
       res.json(userInfo);
     });
 
-    if (this.enableDynamicClientRegistration) {
+    if (
+      this.config.getOptionalBoolean(
+        'auth.experimentalDynamicClientRegistration.enabled',
+      )
+    ) {
       // Authorization endpoint
       // https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
       // Handles the initial authorization request from the client, validates parameters,
@@ -140,7 +145,7 @@ export class OidcRouter {
           // the plugin is mounted somewhere else?
           // support slashes in baseUrl?
           const authSessionRedirectUrl = new URL(
-            `/auth/sessions/${result.id}`,
+            `/oauth2/authorize/${result.id}`,
             this.appUrl,
           );
 

plugins/auth-backend/src/service/OidcService.test.ts

@@ -63,6 +63,8 @@ describe('OidcService', () => {
       getUserInfo: jest.fn(),
     } as unknown as jest.Mocked<UserInfoDatabase>;
 
+    const mockConfig = mockServices.rootConfig.mock();
+
     return {
       service: OidcService.create({
         auth: mockAuth,
@@ -70,11 +72,13 @@ describe('OidcService', () => {
         baseUrl: 'http://mock-base-url',
         userInfo: mockUserInfo,
         oidc: oidcDatabase,
+        config: mockConfig,
       }),
       mocks: {
         auth: mockAuth,
         tokenIssuer: mockTokenIssuer,
         userInfo: mockUserInfo,
+        config: mockConfig,
       },
     };
   }
@@ -216,6 +220,44 @@ describe('OidcService', () => {
         expect(client.clientSecret).toBeDefined();
       });
 
+      it('should throw an error for invalid redirect URI', async () => {
+        const {
+          service,
+          mocks: { config },
+        } = await createOidcService(databaseId);
+
+        config.getOptionalStringArray.mockReturnValue([
+          'https://example.com/*',
+        ]);
+
+        await expect(
+          service.registerClient({
+            clientName: 'Test Client',
+            redirectUris: ['https://invalid.com/callback'],
+          }),
+        ).rejects.toThrow('Invalid redirect_uri');
+      });
+
+      it('should create a new client with valid redirect URI', async () => {
+        const {
+          service,
+          mocks: { config },
+        } = await createOidcService(databaseId);
+
+        config.getOptionalStringArray.mockReturnValue(['cursor://*']);
+
+        const client = await service.registerClient({
+          clientName: 'Test Client',
+          redirectUris: ['cursor://callback'],
+        });
+
+        expect(client).toEqual(
+          expect.objectContaining({
+            redirectUris: ['cursor://callback'],
+          }),
+        );
+      });
+
       it('should create a client with default values', async () => {
         const { service } = await createOidcService(databaseId);
 

plugins/auth-backend/src/service/OidcService.ts

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { AuthService } from '@backstage/backend-plugin-api';
+import { AuthService, RootConfigService } from '@backstage/backend-plugin-api';
 import { TokenIssuer } from '../identity/types';
 import { UserInfoDatabase } from '../database/UserInfoDatabase';
 import {
@@ -33,6 +33,7 @@ export class OidcService {
     private readonly baseUrl: string,
     private readonly userInfo: UserInfoDatabase,
     private readonly oidc: OidcDatabase,
+    private readonly config: RootConfigService,
   ) {}
 
   static create(options: {
@@ -41,6 +42,7 @@ export class OidcService {
     baseUrl: string;
     userInfo: UserInfoDatabase;
     oidc: OidcDatabase;
+    config: RootConfigService;
   }) {
     return new OidcService(
       options.auth,
@@ -48,6 +50,7 @@ export class OidcService {
       options.baseUrl,
       options.userInfo,
       options.oidc,
+      options.config,
     );
   }
 
@@ -116,8 +119,21 @@ export class OidcService {
     const generatedClientId = crypto.randomUUID();
     const generatedClientSecret = crypto.randomUUID();
 
-    // todo(blam): add validation for redirectUris here.
-    // should be a list of urls and / or allowed schemes or something.
+    const allowedRedirectUriPatterns = this.config.getOptionalStringArray(
+      'auth.experimentalDynamicClientRegistration.allowedRedirectUriPatterns',
+    );
+
+    if (allowedRedirectUriPatterns) {
+      for (const redirectUri of opts.redirectUris ?? []) {
+        if (
+          !allowedRedirectUriPatterns.some(pattern =>
+            new RegExp(pattern).test(redirectUri),
+          )
+        ) {
+          throw new InputError('Invalid redirect_uri');
+        }
+      }
+    }
 
     return await this.oidc.createClient({
       clientId: generatedClientId,

plugins/auth-backend/src/service/router.ts

@@ -162,10 +162,7 @@ export async function createRouter(
     oidc,
     logger,
     httpAuth,
-    enableDynamicClientRegistration:
-      config.getOptionalBoolean(
-        'auth.experimentalDynamicClientRegistration.enabled',
-      ) ?? false,
+    config,
   });
 
   router.use(oidcRouter.getRouter());