diff --git a/backend/backend/config/f-u/backendrc.js b/backend/backend/config/f-u/backendrc.js index 299129f9..6fade514 100644 --- a/backend/backend/config/f-u/backendrc.js +++ b/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', }, diff --git a/frontend/app/diff.txt b/frontend/app/diff.txt deleted file mode 100644 index e69de29b..00000000 diff --git a/frontend/app/src/app/modules/auth/auth.provider.spec.ts b/frontend/app/src/app/modules/auth/auth.provider.spec.ts index 70e9e634..49a691fb 100644 --- a/frontend/app/src/app/modules/auth/auth.provider.spec.ts +++ b/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 { @@ -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(); diff --git a/frontend/app/src/app/modules/auth/auth.provider.ts b/frontend/app/src/app/modules/auth/auth.provider.ts index 84de231c..b6c35986 100644 --- a/frontend/app/src/app/modules/auth/auth.provider.ts +++ b/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 { - try { - if (revokeTokens) { - await this.revokeTokens(); - } + const token = + this.tokenSubject.value; + 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 { const configuration = await this.configuration; - if ( - !configuration.revocationEndpoint - ) { - /* - * No revocation endpoint is configured by the backend. - * Remove the local token instead. - */ - await this.storage.removeItem( - TOKEN_RESPONSE_KEY, - ); - + if (!configuration.revocationEndpoint) { return; } + const requests: RevokeTokenRequestJson[] = []; + if (token.refreshToken) { - const refreshRequest: - RevokeTokenRequestJson = { + requests.push({ 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: - RevokeTokenRequestJson = { + requests.push({ token: token.accessToken, @@ -1153,20 +1187,55 @@ export class AuthProvider { client_id: this.authConfig.client_id, - }; + }); - await this.tokenHandler - .performRevokeTokenRequest( - configuration, + let firstError: unknown; - new RevokeTokenRequest( - accessRequest, - ), - ); + for (const request of requests) { + try { + await this.tokenHandler + .performRevokeTokenRequest( + configuration, - await this.storage.removeItem( - TOKEN_RESPONSE_KEY, - ); + new RevokeTokenRequest( + request, + ), + ); + } catch (error) { + firstError ??= error; + } + } + + if (firstError !== undefined) { + throw firstError; + } + } + + /** + * Rejects if the promise does not settle within the given time. + */ + private withTimeout( + promise: Promise, + timeoutMs: number, + ): Promise { + let timer: + ReturnType | undefined; + + const timeout = + new Promise((_resolve, reject) => { + timer = setTimeout(() => { + reject( + new Error('Token Revocation Timed Out'), + ); + }, timeoutMs); + }); + + return Promise.race([ + promise, + timeout, + ]).finally(() => { + clearTimeout(timer); + }); } private async handleAuthorizationCallback( diff --git a/frontend/app/src/environments/environment.ts b/frontend/app/src/environments/environment.ts index fc1777e9..101c192c 100644 --- a/frontend/app/src/environments/environment.ts +++ b/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: 'http://localhost:3000', + // backend_url: 'https://mobile.server.uni-frankfurt.de', + backend_url: 'http://localhost:3000', app_host: 'mobile.app.uni-frankfurt.de', custom_url_scheme: 'de.unifrankfurt.app', backend_version: '999.0.0',