From ea3c2f8eab736365b51274c0fbd54aeb442bb9c9 Mon Sep 17 00:00:00 2001 From: Matthew White Date: Wed, 12 Jul 2023 11:02:23 -0400 Subject: [PATCH] Add endpoint to delete current session --- docs/api.md | 22 +++++++++++++++++++++- lib/resources/sessions.js | 32 +++++++++++++++++++------------- test/integration/api/sessions.js | 29 +++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/docs/api.md b/docs/api.md index de83de85c..682e54f36 100644 --- a/docs/api.md +++ b/docs/api.md @@ -387,7 +387,7 @@ _(There is not really anything at `/v1/example`; this section only demonstrates #### Logging out [DELETE /v1/sessions/{token}] -Logging out is not strictly necessary for Web Users; all sessions expire 24 hours after they are created. But it can be a good idea, in case someone else manages to steal your token. It is also the way Public Link and App User access are revoked. To do so, issue a `DELETE` request to that token resource. +Logging out is not strictly necessary for Web Users; all sessions expire 24 hours after they are created. But it can be a good idea, in case someone else manages to steal your token. It is also the way Public Link and App User access is revoked. To do so, issue a `DELETE` request to that token resource. + Parameters + token: `lSpAIeksRu1CNZs7!qjAot2T17dPzkrw9B4iTtpj7OoIJBmXvnHM8z8Ka4QPEjR7` (string, required) - The session bearer token, obtained at login time. @@ -403,6 +403,26 @@ Logging out is not strictly necessary for Web Users; all sessions expire 24 hour + Response 403 (application/json) + Attributes (Error 403) +#### Logging out current session [DELETE /v1/sessions/current] + +This endpoint causes the current session to log itself out. Logging out is not strictly necessary for Web Users; all sessions expire 24 hours after they are created. But it can be a good idea, in case someone else manages to steal your token. + +Only the session that was used to authenticate the request is logged out. If the Actor associated with the session has other sessions as well, those are not logged out. + ++ Request + + Headers + + Authorization: Bearer lSpAIeksRu1CNZs7!qjAot2T17dPzkrw9B4iTtpj7OoIJBmXvnHM8z8Ka4QPEjR7 + ++ Response 200 (application/json) + + Attributes (Success) + ++ Response 403 (application/json) + + Attributes (Error 403) + ++ Response 404 (application/json) + + Attributes (Error 404) + ## HTTPS Basic Authentication [/v1/example] Standard HTTP Basic Authentication is allowed, but **_strongly discouraged_**. This is because the server must verify your password with every single request, which is very slow to compute: typically, this will add hundreds of milliseconds to each request. For some one-off tasks and in cases where there is no other choice, it is reasonable to choose Basic authentication, but wherever possible we strongly encourage the use of any other authentication method. diff --git a/lib/resources/sessions.js b/lib/resources/sessions.js index 08e198590..30406e3b3 100644 --- a/lib/resources/sessions.js +++ b/lib/resources/sessions.js @@ -48,24 +48,30 @@ module.exports = (service, endpoint) => { service.get('/sessions/restore', endpoint((_, { auth }) => auth.session.orElse(Problem.user.notFound()))); + const logOut = (Sessions, auth, session) => + auth.canOrReject('session.end', session.actor) + .then(() => Sessions.terminate(session)) + .then(() => (_, response) => { + // revoke the cookie associated w the session, if the session was used to + // terminate itself. + // TODO: repetitive w above. + if (session.token === auth.session.map((s) => s.token).orNull()) + response.cookie('__Host-session', 'null', { path: '/', expires: new Date(0), + httpOnly: true, secure: true, sameSite: 'strict' }); + + return success; + }); + + service.delete('/sessions/current', endpoint(({ Sessions }, { auth }) => + auth.session.map(session => logOut(Sessions, auth, session)) + .orElse(Problem.user.notFound()))); + // here we always throw a 403 even if the token doesn't exist to prevent // information leakage. // TODO: but a timing attack still exists here. :( service.delete('/sessions/:token', endpoint(({ Sessions }, { auth, params }) => Sessions.getByBearerToken(params.token) .then(getOrReject(Problem.user.insufficientRights())) - .then((session) => auth.canOrReject('session.end', session.actor) - .then(() => Sessions.terminate(session)) - .then(() => (_, response) => { - // revoke the cookie associated w the session, if the session was used to - // terminate itself. - // TODO: repetitive w above. - if (session.token === auth.session.map((s) => s.token).orNull()) - response.cookie('__Host-session', 'null', { path: '/', expires: new Date(0), - httpOnly: true, secure: true, sameSite: 'strict' }); - - return success; - })))); - + .then(session => logOut(Sessions, auth, session)))); }; diff --git a/test/integration/api/sessions.js b/test/integration/api/sessions.js index 004c663d6..885bc7fc4 100644 --- a/test/integration/api/sessions.js +++ b/test/integration/api/sessions.js @@ -269,6 +269,35 @@ describe('api: /sessions', () => { }))))); }); + describe('/current DELETE', () => { + it('should return a 404 if no token was specified', testService(service => + service.delete('/v1/sessions/current') + .expect(404))); + + it('should invalidate the token if successful', testService(async (service) => { + const { body: session } = await service.post('/v1/sessions') + .send({ email: 'alice@getodk.org', password: 'alice' }) + .expect(200); + const { token } = session; + const { body } = await service.delete('/v1/sessions/current') + .set('Authorization', `Bearer ${token}`) + .expect(200); + body.should.eql({ success: true }); + await service.get('/v1/users/current') + .set('Authorization', `Bearer ${token}`) + .expect(401); + })); + + it('should not allow app users to delete their own sessions', testService(async (service) => { + const asBob = await service.login('bob'); + const { body: appUser } = await asBob.post('/v1/projects/1/app-users') + .send({ displayName: 'test app user' }) + .expect(200); + await service.delete(`/v1/key/${appUser.token}/sessions/current`) + .expect(403); + })); + }); + // this isn't exactly the right place for this but i just want to check the // whole stack in addition to the unit tests. describe('cookie CSRF auth', () => {