WIP: revocation

backend/backend/config/f-u/backendrc.js

@@ -28,6 +28,7 @@ const config = {
           role: '$.eduPersonPrimaryAffiliation',
           studentId: '$.employeeNumber',
         },
+        revoke: 'https://idp.ub.uni-frankfurt.de/idp/profile/oauth2/revocation',
         token: 'https://idp.ub.uni-frankfurt.de/idp/profile/oidc/token',
         userinfo: 'https://idp.ub.uni-frankfurt.de/idp/profile/oidc/userinfo',
       },

frontend/app/src/app/modules/auth/auth.provider.spec.ts

@@ -1,6 +1,7 @@
 import {
   AppAuthError,
   AuthorizationServiceConfiguration,
+  RevokeTokenRequest,
   StorageBackend,
   TokenRequestHandler,
   TokenResponse,
@@ -61,6 +62,7 @@ describe('AuthProvider', () => {
   let storageSetItem: jasmine.Spy;
   let storageRemoveItem: jasmine.Spy;
   let tokenRequest: jasmine.Spy;
+  let revokeRequest: jasmine.Spy;
 
   /** Seeds storage with the given token and loads it through the public API. */
   async function givenLoadedToken(token: TokenResponse | null): Promise<void> {
@@ -109,6 +111,7 @@ describe('AuthProvider', () => {
     tokenRequest = spyOn(internals.tokenHandler, 'performTokenRequest').and.rejectWith(
       new Error('Unexpected token request'),
     );
+    revokeRequest = spyOn(internals.tokenHandler, 'performRevokeTokenRequest').and.resolveTo(true);
   });
 
   afterEach(() => {
@@ -437,6 +440,108 @@ describe('AuthProvider', () => {
     });
   });
 
+  describe('signOut', () => {
+    /** The [token, token_type_hint, client_id] of each revoke request, in order. */
+    const revokedTokens = () =>
+      revokeRequest.calls.allArgs().map(([, request]) => {
+        const json = (request as RevokeTokenRequest).toJson();
+        return [json.token, json.token_type_hint, json.client_id];
+      });
+
+    it('revokes the refresh token, then the access token', async () => {
+      await givenLoadedToken(makeToken({accessToken: 'access', refreshToken: 'refresh'}));
+
+      await provider.signOut();
+
+      expect(revokedTokens()).toEqual([
+        ['refresh', 'refresh_token', 'test-client'],
+        ['access', 'access_token', 'test-client'],
+      ]);
+      expect(await isLoggedIn()).toBeFalse();
+      expect(storageRemoveItem).toHaveBeenCalledWith(TOKEN_RESPONSE_KEY);
+    });
+
+    it('revokes only the access token when there is no refresh token (browser)', async () => {
+      await givenLoadedToken(makeToken({accessToken: 'access', refreshToken: null}));
+
+      await provider.signOut();
+
+      expect(revokedTokens()).toEqual([['access', 'access_token', 'test-client']]);
+    });
+
+    it('logs out locally before the IdP answers', async () => {
+      await givenLoadedToken(makeToken());
+      let loggedInWhileRevoking: boolean | undefined;
+      revokeRequest.and.callFake(async () => {
+        loggedInWhileRevoking = await isLoggedIn();
+        return true;
+      });
+
+      await provider.signOut();
+
+      expect(loggedInWhileRevoking).toBeFalse();
+    });
+
+    it('still logs out and tries the access token when revoking the refresh token fails', async () => {
+      await givenLoadedToken(makeToken());
+      const events = recordEvents();
+      revokeRequest.and.returnValues(Promise.reject(new AppAuthError('Network Error')), Promise.resolve(true));
+
+      await provider.signOut();
+
+      expect(revokeRequest).toHaveBeenCalledTimes(2);
+      expect(await isLoggedIn()).toBeFalse();
+      expect(events).toEqual([AuthActions.SignOutSuccess, AuthActions.RevokeTokensFailed]);
+    });
+
+    it('does not wait for an IdP that does not answer', async () => {
+      await givenLoadedToken(makeToken());
+      revokeRequest.and.returnValue(new Promise(() => {}));
+      let done = false;
+
+      const signOut = provider.signOut().then(() => (done = true));
+      await flushPromises();
+      expect(done).toBeFalse();
+      expect(await isLoggedIn()).toBeFalse();
+
+      jasmine.clock().tick(5000);
+      await signOut;
+
+      expect(done).toBeTrue();
+    });
+
+    it('revokes nothing when no revocation endpoint is configured', async () => {
+      (provider as any).localConfiguration = new AuthorizationServiceConfiguration({
+        authorization_endpoint: 'https://idp.example.org/authorize',
+        token_endpoint: 'https://idp.example.org/token',
+        revocation_endpoint: '',
+      });
+      await givenLoadedToken(makeToken());
+
+      await provider.signOut();
+
+      expect(revokeRequest).not.toHaveBeenCalled();
+      expect(await isLoggedIn()).toBeFalse();
+    });
+
+    it('revokes nothing when asked not to', async () => {
+      await givenLoadedToken(makeToken());
+
+      await provider.signOut(undefined, false);
+
+      expect(revokeRequest).not.toHaveBeenCalled();
+      expect(await isLoggedIn()).toBeFalse();
+    });
+
+    it('revokes nothing when nobody is logged in', async () => {
+      await givenLoadedToken(null);
+
+      await provider.signOut();
+
+      expect(revokeRequest).not.toHaveBeenCalled();
+    });
+  });
+
   describe('isInvalidGrant', () => {
     it('detects invalid_grant as error message', () => {
       expect(isInvalidGrant(new AppAuthError(INVALID_GRANT))).toBeTrue();

frontend/app/src/app/modules/auth/auth.provider.ts

@@ -69,6 +69,11 @@ const MAX_EARLY_REFRESH_SECONDS = 60;
  */
 const SESSION_CHECK_RETRY_MS = 60 * 1000;
 
+/**
+ * Logout never waits longer than this for the IdP to revoke the tokens.
+ */
+const REVOKE_TIMEOUT_MS = 5000;
+
 /**
  * Largest delay setTimeout accepts (about 24.8 days).
  */
@@ -510,15 +515,24 @@ export class AuthProvider {
       ?.endSession;
   }
 
+  /**
+   * Logs the user out of the app.
+   *
+   * With revokeTokens (default), the refresh and access tokens are also
+   * revoked at the IdP (RFC 7009), so a copied token cannot be used anymore.
+   * This is best effort: the local logout always happens first and never
+   * waits longer than REVOKE_TIMEOUT_MS for the IdP.
+   * @param _state Unused.
+   * @param revokeTokens Revoke the tokens at the IdP.
+   */
   public async signOut(
     _state?: string,
-    revokeTokens = false,
+    revokeTokens = true,
   ): Promise<void> {
-    try {
+    const token =
-      if (revokeTokens) {
+      this.tokenSubject.value;
-        await this.revokeTokens();
-      }
 
+    try {
       await this.storage.removeItem(
         TOKEN_RESPONSE_KEY,
       );
@@ -542,6 +556,26 @@ export class AuthProvider {
         error,
       });
     }
+
+    if (revokeTokens && token) {
+      try {
+        await this.withTimeout(
+          this.revokeTokenResponse(token),
+          REVOKE_TIMEOUT_MS,
+        );
+      } catch (error) {
+        /*
+         * Offline, IdP unreachable or revocation not allowed: the user is
+         * logged out locally anyway.
+         */
+        this.notify({
+          action:
+          AuthActions.RevokeTokensFailed,
+
+          error,
+        });
+      }
+    }
   }
 
   public async revokeTokens():
@@ -1103,26 +1137,36 @@ export class AuthProvider {
       );
     }
 
+    await this.revokeTokenResponse(
+      token,
+    );
+
+    await this.storage.removeItem(
+      TOKEN_RESPONSE_KEY,
+    );
+  }
+
+  /**
+   * Revokes the refresh token (if any) and the access token at the IdP.
+   *
+   * The refresh token goes first: it is the long-lived one. Both are tried
+   * even if the first request fails; the first error is rethrown.
+   * Does nothing if the backend configures no revocation endpoint.
+   */
+  private async revokeTokenResponse(
+    token: TokenResponse,
+  ): Promise<void> {
     const configuration =
       await this.configuration;
 
-    if (
+    if (!configuration.revocationEndpoint) {
-      !configuration.revocationEndpoint
-    ) {
-      /*
-       * No revocation endpoint is configured by the backend.
-       * Remove the local token instead.
-       */
-      await this.storage.removeItem(
-        TOKEN_RESPONSE_KEY,
-      );
-
       return;
     }
 
+    const requests: RevokeTokenRequestJson[] = [];
+
     if (token.refreshToken) {
-      const refreshRequest:
+      requests.push({
-        RevokeTokenRequestJson = {
         token:
         token.refreshToken,
 
@@ -1131,20 +1175,10 @@ export class AuthProvider {
 
         client_id:
         this.authConfig.client_id,
-      };
+      });
-
-      await this.tokenHandler
-        .performRevokeTokenRequest(
-          configuration,
-
-          new RevokeTokenRequest(
-            refreshRequest,
-          ),
-        );
     }
 
-    const accessRequest:
+    requests.push({
-      RevokeTokenRequestJson = {
       token:
       token.accessToken,
 
@@ -1153,20 +1187,55 @@ export class AuthProvider {
 
       client_id:
       this.authConfig.client_id,
-    };
+    });
 
-    await this.tokenHandler
+    let firstError: unknown;
-      .performRevokeTokenRequest(
-        configuration,
 
-        new RevokeTokenRequest(
+    for (const request of requests) {
-          accessRequest,
+      try {
-        ),
+        await this.tokenHandler
-      );
+          .performRevokeTokenRequest(
+            configuration,
 
-    await this.storage.removeItem(
+            new RevokeTokenRequest(
-      TOKEN_RESPONSE_KEY,
+              request,
-    );
+            ),
+          );
+      } catch (error) {
+        firstError ??= error;
+      }
+    }
+
+    if (firstError !== undefined) {
+      throw firstError;
+    }
+  }
+
+  /**
+   * Rejects if the promise does not settle within the given time.
+   */
+  private withTimeout<T>(
+    promise: Promise<T>,
+    timeoutMs: number,
+  ): Promise<T> {
+    let timer:
+      ReturnType<typeof setTimeout> | undefined;
+
+    const timeout =
+      new Promise<never>((_resolve, reject) => {
+        timer = setTimeout(() => {
+          reject(
+            new Error('Token Revocation Timed Out'),
+          );
+        }, timeoutMs);
+      });
+
+    return Promise.race([
+      promise,
+      timeout,
+    ]).finally(() => {
+      clearTimeout(timer);
+    });
   }
 
   private async handleAuthorizationCallback(

frontend/app/src/environments/environment.ts

@@ -18,8 +18,8 @@
 // The list of which env maps to which file can be found in `.angular-cli.json`.
 
 export const environment = {
-  backend_url: 'https://mobile.server.uni-frankfurt.de',
+  // backend_url: 'https://mobile.server.uni-frankfurt.de',
-  // backend_url: 'http://localhost:3000',
+  backend_url: 'http://localhost:3000',
   app_host: 'mobile.app.uni-frankfurt.de',
   custom_url_scheme: 'de.unifrankfurt.app',
   backend_version: '999.0.0',