Skip to content

Commit

Permalink
query/sessions: return 401 for invalid Bearer token (#1379)
Browse files Browse the repository at this point in the history
Currently passing a `Authorization: Bearer ...` header with a token that fails the `isValidToken()` check will return a 500 response.

To recreate:

```
$ curl https://dev.getodk.cloud/v1/projects/1 --header 'Authorization: Bearer hi'
{"message":"Internal Server Error"}
```
vs
```
$ curl https://dev.getodk.cloud/v1/projects/1 --header 'Authorization: Bearer aaaaaaaabbbbbbbbccccccccddddddddaaaaaaaabbbbbbbbccccccccdddddddd'
{"message":"Could not authenticate with the provided credentials.","code":401.2}
```

From Sentry (https://getodk.sentry.io/issues/6194482982/?environment=production&project=5724763&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D&referrer=issue-stream&statsPeriod=30d&stream_index=2), when the supplied token is invalid, `authHandler` can throw.
  • Loading branch information
alxndrsn authored Jan 30, 2025
1 parent 20e27f3 commit 973462b
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/model/query/sessions.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const getByBearerToken = (token) => ({ maybeOne }) => (isValidToken(token) ? may
select ${_unjoiner.fields} from sessions
join actors on actors.id=sessions."actorId"
where token=${token} and sessions."expiresAt" > now()`)
.then(map(_unjoiner)) : Option.none());
.then(map(_unjoiner)) : Promise.resolve(Option.none()));

const terminateByActorId = (actorId, current = undefined) => ({ run }) =>
run(sql`DELETE FROM sessions WHERE "actorId"=${actorId}
Expand Down
5 changes: 5 additions & 0 deletions test/integration/other/bearer-auth.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
const { testService } = require('../setup');

describe('bearer authentication', () => {
it('should reject 401 if supplied token does not match the expected format', testService((service) => // gh329
service.get('/v1/users/current')
.set('Authorization', 'Bearer a')
.expect(401)));

it('should reject 401 if the header format is wrong', testService((service) => // gh329
service.get('/v1/users/current')
.set('Authorization', 'Bearer: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa')
Expand Down

0 comments on commit 973462b

Please sign in to comment.