fix: Only allow owners to delete their client credentials

packages/uma/src/routes/ClientRegistration.ts

@@ -2,6 +2,7 @@ import {
   BadRequestHttpError,
   ConflictHttpError,
   createErrorMessage,
+  ForbiddenHttpError,
   IndexedStorage,
   InternalServerError,
   joinUrl,
@@ -145,12 +146,15 @@ export class ClientRegistrationRequestHandler extends HttpHandler {
       throw new InternalServerError('URI for DELETE operation should include an id.');
     }
 
-    const matches = await this.storage.findIds(CLIENT_REGISTRATION_STORAGE_TYPE, { clientId: request.parameters.id });
+    const matches = await this.storage.find(CLIENT_REGISTRATION_STORAGE_TYPE, { clientId: request.parameters.id });
     if (matches.length === 0) {
       throw new NotFoundHttpError();
     }
+    if (matches[0].userId !== userId) {
+      throw new ForbiddenHttpError();
+    }
 
-    await this.storage.delete(CLIENT_REGISTRATION_STORAGE_TYPE, matches[0]);
+    await this.storage.delete(CLIENT_REGISTRATION_STORAGE_TYPE, matches[0].id);
 
     return { status: 204 };
   }

packages/uma/test/unit/routes/ClientRegistration.test.ts

@@ -1,6 +1,6 @@
 import {
   BadRequestHttpError,
-  ConflictHttpError,
+  ConflictHttpError, ForbiddenHttpError,
   IndexedStorage, InternalServerError,
   joinUrl, NotFoundHttpError,
   UnauthorizedHttpError
@@ -136,7 +136,7 @@ describe('ClientRegistration', (): void => {
   it('can remove a registration on DELETE requests.', async(): Promise<void> => {
     request.method = 'DELETE';
     request.parameters = { id: 'id' };
-    storage.findIds.mockResolvedValueOnce([ 'match' ]);
+    storage.find.mockResolvedValueOnce([ { id: 'match', userId: webId } ]);
 
     await expect(handler.handle({ request })).resolves.toEqual({ status: 204 });
     expect(storage.delete).toHaveBeenCalledTimes(1);
@@ -146,7 +146,6 @@ describe('ClientRegistration', (): void => {
   it('errors if there is no ID when deleting.', async(): Promise<void> => {
     request.method = 'DELETE';
     request.parameters = {};
-    storage.findIds.mockResolvedValueOnce([ 'match' ]);
 
     await expect(handler.handle({ request })).rejects.toThrow(InternalServerError);
     expect(storage.delete).toHaveBeenCalledTimes(0);
@@ -155,9 +154,18 @@ describe('ClientRegistration', (): void => {
   it('errors if the ID is unknown when deleting.', async(): Promise<void> => {
     request.method = 'DELETE';
     request.parameters = { id: 'id' };
-    storage.findIds.mockResolvedValueOnce([]);
+    storage.find.mockResolvedValueOnce([]);
 
     await expect(handler.handle({ request })).rejects.toThrow(NotFoundHttpError);
     expect(storage.delete).toHaveBeenCalledTimes(0);
   });
+
+  it('errors if the user is not the owner of the ID when deleting.', async(): Promise<void> => {
+    request.method = 'DELETE';
+    request.parameters = { id: 'id' };
+    storage.find.mockResolvedValueOnce([ { id: 'match', userId: 'someone-else' } ]);
+
+    await expect(handler.handle({ request })).rejects.toThrow(ForbiddenHttpError);
+    expect(storage.delete).toHaveBeenCalledTimes(0);
+  });
 });