Add endpoint for JWT refresh tokens #3270

stveit · 2025-01-29T22:44:17Z

Based on #3268

Adds API endpoint for JWT refresh token. It accepts valid refresh tokens and returns an access token and a new refresh token while invalidating the old one.

Adds docs for local tokens, with caveat that its not fully implemented yet

github-actions · 2025-01-29T22:45:28Z

🦙 MegaLinter status: ✅ SUCCESS

Descriptor	Linter	Files	Fixed	Errors	Warnings	Elapsed time
✅ PYTHON	ruff	6		0	0	0.21s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

github-actions · 2025-01-29T22:53:23Z

Test results

12 files 12 suites 11m 59s ⏱️
2 196 tests 2 196 ✅ 0 💤 0 ❌
6 063 runs 6 063 ✅ 0 💤 0 ❌

Results for commit 1b61d8b.

♻️ This comment has been updated with latest results.

codecov · 2025-01-29T22:53:41Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.72%. Comparing base (51a22b1) to head (91f2633).
Report is 3 commits behind head on jwt-refresh-token-model.

Additional details and impacted files

@@                     Coverage Diff                     @@
##           jwt-refresh-token-model    #3270      +/-   ##
===========================================================
- Coverage                    60.73%   60.72%   -0.01%     
===========================================================
  Files                          606      606              
  Lines                        43769    43800      +31     
  Branches                        48       48              
===========================================================
+ Hits                         26581    26597      +16     
- Misses                       17176    17191      +15     
  Partials                        12       12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

tests/integration/jwt_refresh_endpoint_test.py

tests/conftest.py

johannaengland

Only nitpicks

python/nav/web/api/v1/views.py

tests/integration/jwt_refresh_endpoint_test.py

tests/unittests/web/jwtgen_test.py

sonarqubecloud · 2025-03-06T07:40:04Z

Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

lunkwill42

Got questions (that aren't necessarily showstoppers at all, just me being forgetful), but my major gripe is missing input validation.

Otherwise, this seems good :)

lunkwill42 · 2025-05-05T11:44:13Z

python/nav/web/api/v1/views.py

+
+    def post(self, request):
+        incoming_token = request.data.get('refresh_token')
+        token_hash = hash_token(incoming_token)


I browsed http://127.0.0.1:8000/api/refresh/ and posted an empty request, and this line crashed terribly with an AttributeError ('NoneType' object has no attribute 'encode').

Some input validation would be proper here - bad input should result in possibly a 400 Bad Request (depending on how other parts of the API behave with bad input).

Added validation that input was given and that its a string. That should be enough, I dont think its necessary to check that the string has the format of a real token - if its not valid then there will be no match for the hash in the db

lunkwill42 · 2025-05-05T11:50:34Z

python/nav/web/api/v1/urls.py

@@ -73,4 +73,5 @@
        name="prefix-usage-detail",
    ),
    re_path(r'^', include(router.urls)),
+    re_path(r'^refresh/$', views.JWTRefreshViewSet.as_view(), name='jwt-refresh'),


Just to satisfy my own curiosity: I cannot recall why some API views are added to the DRF router instance, while some URLs are added manually to the URL config.

The noticeable difference (for me) is that URLs that are added manually like this do not show up in the official list of API endpoints (swagger/OpenAPI) when just visiting /api/.

These URLs will also not be part of the versioned API. We might need a discussion on why we have this setup and whether we should keep doing it (@hmpf )

at this point I dont remember either 😛

probably had to do with it not being a modelview or something? similar to issues I had with #3354 perhaps

lunkwill42 · 2025-05-05T11:55:29Z

python/nav/web/api/v1/views.py

+        try:
+            # If hash exists in the database, then we know it is a real token
+            db_token = JWTRefreshToken.objects.get(hash=token_hash)
+        except JWTRefreshToken.DoesNotExist:
+            return Response("Invalid token", status=status.HTTP_403_FORBIDDEN)


Well, I'm sort of thinking that as long as we can verify that this token was signed by our key, it should be treated as valid (until its claims or dates are determined to be invalid, that is).

At the moment, I've entirely forgotten why we need to keep a reference to issued refresh tokens in the database, I just remember pointing out that storing the hash should be enough.

Only thing I can think of at the moment is that we need it to keep an audit trail?

The idea is that we need some way to revoke tokens that may have fallen in the wrong hands. If we accept all tokens that are signed by us and an "attacker" gets their hands on a refresh token, then they can keep generating tokens forever. The only way to stop them would be to change our keys so the signature doesn't validate anymore. Storing the hash is a type of whitelisting that can easily be revoked by deleting the hash in the db (which can be done via the planned frontend)

A side effect of storing the hash is that we don't need to validate the signature in this endpoint, checking the hash is enough

lunkwill42 · 2025-05-05T11:59:07Z

doc/reference/jwt.rst

+  ssh-keygen -t rsa -b 4096 -m PEM -f jwtRS256.key
+  # Don't add passphrase
+  openssl rsa -in jwtRS256.key -pubout -outform PEM -out jwtRS256.key.pub


How come openssl cannot also be use to generate the key?

Might be possible, this is how I've been doing it and it has worked well. I'll see if I can find the openssl equivalent just to have fewer dependencies here

openssl genrsa -out jwtRS256.key 4096 seems to be the equivalent

unittests and integration JWT tests need keys for validating and generating tokens

tests/conftest.py

stveit · 2025-05-06T07:53:13Z

shit got all messed up after merging #3268 so I needed to force push to remove unwanted commits

Adds some basic validation of the incoming token. No need to really check the format or anything, if its not a valid token in any way it will fail when we check the hash against the database. The important part is that the incoming token is a string that can actually be hashed

sonarqubecloud · 2025-05-06T12:48:46Z

Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

stveit added the JWT label Jan 29, 2025

stveit self-assigned this Jan 29, 2025

stveit force-pushed the jwt-refresh-token-endpoint branch from 257a05c to b384528 Compare January 29, 2025 23:16

stveit force-pushed the jwt-refresh-token-model branch from f3f4c12 to d16dda5 Compare January 29, 2025 23:26

stveit force-pushed the jwt-refresh-token-endpoint branch from b384528 to c3c9e16 Compare January 29, 2025 23:28

stveit force-pushed the jwt-refresh-token-model branch from d16dda5 to c2568eb Compare January 29, 2025 23:35

stveit force-pushed the jwt-refresh-token-endpoint branch 2 times, most recently from c3c9e16 to ec2ead6 Compare January 29, 2025 23:47

stveit mentioned this pull request Jan 31, 2025

Add refresh endpoint for JWT tokens #2570

Closed

stveit force-pushed the jwt-refresh-token-model branch 3 times, most recently from 1417b06 to f118619 Compare February 4, 2025 01:56

stveit force-pushed the jwt-refresh-token-endpoint branch 3 times, most recently from 66952f5 to 2190bad Compare February 4, 2025 03:17

stveit force-pushed the jwt-refresh-token-model branch from f118619 to 49c7894 Compare February 4, 2025 18:56

stveit force-pushed the jwt-refresh-token-endpoint branch 2 times, most recently from 0f0ed15 to 825d9f3 Compare February 6, 2025 07:02

stveit force-pushed the jwt-refresh-token-model branch from 49c7894 to c87e7a1 Compare February 6, 2025 07:46

stveit force-pushed the jwt-refresh-token-endpoint branch from 825d9f3 to c35d5b6 Compare February 6, 2025 07:47

stveit force-pushed the jwt-refresh-token-model branch from c87e7a1 to 50e46b4 Compare February 6, 2025 07:57

stveit force-pushed the jwt-refresh-token-endpoint branch 2 times, most recently from 2889fe6 to 315df0c Compare February 6, 2025 08:25

stveit force-pushed the jwt-refresh-token-model branch from 50e46b4 to 15b2279 Compare February 21, 2025 10:09

stveit mentioned this pull request Feb 21, 2025

Add model for JWT refresh token #3268

Merged

stveit force-pushed the jwt-refresh-token-model branch 2 times, most recently from 14e45e7 to 4805ebf Compare February 21, 2025 11:43

stveit force-pushed the jwt-refresh-token-model branch from 4805ebf to 63b9bc5 Compare February 21, 2025 12:15

stveit force-pushed the jwt-refresh-token-endpoint branch from 315df0c to d1bfbc3 Compare February 21, 2025 12:17

github-advanced-security bot found potential problems Feb 21, 2025

View reviewed changes

tests/integration/jwt_refresh_endpoint_test.py Fixed Show fixed Hide fixed

stveit force-pushed the jwt-refresh-token-endpoint branch 2 times, most recently from a29801e to 8403bc7 Compare February 25, 2025 08:59

github-advanced-security bot found potential problems Feb 25, 2025

View reviewed changes

tests/conftest.py Fixed Show fixed Hide fixed

stveit force-pushed the jwt-refresh-token-endpoint branch 2 times, most recently from 83c5074 to 511b5e1 Compare February 25, 2025 11:33

johannaengland approved these changes Mar 3, 2025

View reviewed changes

stveit requested a review from lunkwill42 April 29, 2025 09:48

stveit force-pushed the jwt-refresh-token-model branch from a3db8c2 to 5f15bb6 Compare May 5, 2025 11:52

lunkwill42 requested changes May 5, 2025

View reviewed changes

Base automatically changed from jwt-refresh-token-model to master May 5, 2025 14:45

stveit added 4 commits May 6, 2025 09:47

Make RSA keys available for all tests

1535222

unittests and integration JWT tests need keys for validating and generating tokens

Add endpoint for JWT refresh tokens

404a1b0

Add documentation for local JWT tokens

5f23c21

fixup! Add endpoint for JWT refresh tokens

a869e1d

stveit force-pushed the jwt-refresh-token-endpoint branch from 91f2633 to a869e1d Compare May 6, 2025 07:48

github-advanced-security bot found potential problems May 6, 2025

View reviewed changes

tests/conftest.py Fixed Show fixed Hide fixed

stveit added 2 commits May 6, 2025 10:23

fixup! Add documentation for local JWT tokens

1b61d8b

Add endpoint for JWT refresh tokens #3270

Are you sure you want to change the base?

Add endpoint for JWT refresh tokens #3270

Uh oh!

Conversation

stveit commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦙 MegaLinter status: ✅ SUCCESS

Uh oh!

github-actions bot commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test results

Uh oh!

codecov bot commented Jan 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

johannaengland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Mar 6, 2025

Quality Gate passed

Uh oh!

lunkwill42 left a comment

Choose a reason for hiding this comment

Uh oh!

lunkwill42 May 5, 2025

Choose a reason for hiding this comment

Uh oh!

stveit May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lunkwill42 May 5, 2025

Choose a reason for hiding this comment

Uh oh!

stveit May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lunkwill42 May 5, 2025

Choose a reason for hiding this comment

Uh oh!

stveit May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lunkwill42 May 5, 2025

Choose a reason for hiding this comment

Uh oh!

stveit May 6, 2025

Choose a reason for hiding this comment

Uh oh!

stveit May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stveit commented May 6, 2025

Uh oh!

sonarqubecloud bot commented May 6, 2025

Quality Gate passed

Uh oh!

Uh oh!

stveit commented Jan 29, 2025 •

edited

Loading

github-actions bot commented Jan 29, 2025 •

edited

Loading

github-actions bot commented Jan 29, 2025 •

edited

Loading

codecov bot commented Jan 29, 2025 •

edited

Loading

stveit May 6, 2025 •

edited

Loading

stveit May 6, 2025 •

edited

Loading

stveit May 6, 2025 •

edited

Loading

stveit May 6, 2025 •

edited

Loading