Skip to content

BB2-4781: Revoke prior tokens on new auth flow #1605

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JamesDemeryNava merged 18 commits into from
Jun 2, 2026
Merged

BB2-4781: Revoke prior tokens on new auth flow #1605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
6e8d9f3
Merge branch 'master' into jamesdemery/bb2-4781-revoke-prior-tokens-o…
JamesDemeryNava May 21, 2026
599e102
BB2-4781: Revoke prior tokens on new auth flow
JamesDemeryNava May 21, 2026
de1fa4d
Ensure token revocation only happens on v3 auth flows
JamesDemeryNava May 22, 2026
b0ea5ae
Merge branch 'master' into jamesdemery/bb2-4781-revoke-prior-tokens-o…
JamesDemeryNava May 22, 2026
4f2cffa
Merge branch 'master' into jamesdemery/bb2-4781-revoke-prior-tokens-o…
JamesDemeryNava May 27, 2026
f570eed
Merge branch 'master' into jamesdemery/bb2-4781-revoke-prior-tokens-o…
JamesDemeryNava May 27, 2026
8508c52
Merge branch 'master' into jamesdemery/bb2-4781-revoke-prior-tokens-o…
JamesDemeryNava May 27, 2026
4335eb3
Merge branch 'master' into jamesdemery/bb2-4781-revoke-prior-tokens-o…
JamesDemeryNava May 28, 2026
e89f80d
Merge branch 'master' into jamesdemery/bb2-4781-revoke-prior-tokens-o…
JamesDemeryNava May 28, 2026
5a993e1
Move the revocation of prior tokens to form_valid of AuthorizationVie…
JamesDemeryNava May 28, 2026
605a6ce
Merge branch 'master' into jamesdemery/bb2-4781-revoke-prior-tokens-o…
JamesDemeryNava May 29, 2026
f86556e
Address PR feedback, remove no longer needed test
JamesDemeryNava May 29, 2026
732f912
Merge branch 'master' into jamesdemery/bb2-4781-revoke-prior-tokens-o…
JamesDemeryNava Jun 1, 2026
30cca42
Address feedback - use previously existing utils function
JamesDemeryNava Jun 1, 2026
f3ab575
Fix typo
JamesDemeryNava Jun 1, 2026
50948e0
Merge branch 'master' into jamesdemery/bb2-4781-revoke-prior-tokens-o…
JamesDemeryNava Jun 2, 2026
a6de51a
Add unit test coverage
JamesDemeryNava Jun 2, 2026
fe68f98
Merge branch 'master' into jamesdemery/bb2-4781-revoke-prior-tokens-o…
JamesDemeryNava Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions apps/dot_ext/constants.py
View file
Open in desktop

@JamesDemeryNava JamesDemeryNava Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Counts in these constants needed to be updated now that we delete prior access/refresh tokens when going through the auth flow

Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,9 @@
# Result:
'result_has_error': False,
'result_token_scopes_granted': APPLICATION_SCOPES_NON_DEMOGRAPHIC,
'result_access_token_count': 2,
'result_refresh_token_count': 2,
'result_archived_token_count': 2,
'result_access_token_count': 1,
'result_refresh_token_count': 1,
'result_archived_token_count': 3,
'result_archived_data_access_grant_count': 2,
},
'test 5: app_requires = True bene_share = False': {
Expand All @@ -365,9 +365,9 @@
# Result:
'result_has_error': False,
'result_token_scopes_granted': APPLICATION_SCOPES_FULL,
'result_access_token_count': 2,
'result_refresh_token_count': 2,
'result_archived_token_count': 4,
'result_access_token_count': 1,
'result_refresh_token_count': 1,
'result_archived_token_count': 5,
'result_archived_data_access_grant_count': 3,
},
# Tests for request_app_requires_demographic = False
Expand Down Expand Up @@ -419,9 +419,9 @@
# Result:
'result_has_error': False,
'result_token_scopes_granted': SCOPES_JUST_EOB_AND_B,
'result_access_token_count': 2,
'result_refresh_token_count': 2,
'result_archived_token_count': 8,
'result_access_token_count': 1,
'result_refresh_token_count': 1,
'result_archived_token_count': 9,
'result_archived_data_access_grant_count': 6,
},
'test 11: app_requires = None bene_share = None request just PATIENT and A': {
Expand All @@ -445,9 +445,9 @@
# Result:
'result_has_error': False,
'result_token_scopes_granted': SCOPES_JUST_PATIENT_AND_A,
'result_access_token_count': 2,
'result_refresh_token_count': 2,
'result_archived_token_count': 10,
'result_access_token_count': 1,
'result_refresh_token_count': 1,
'result_archived_token_count': 11,
'result_archived_data_access_grant_count': 7,
},
'test 13: app_requires = False bene_share = True request just PATIENT and A': {
Expand Down
70 changes: 36 additions & 34 deletions apps/dot_ext/tests/test_beneficiary_demographic_scope_changes.py
View file
Open in desktop

@JamesDemeryNava JamesDemeryNava May 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought there would need to be changes to this file, but in the ended it wound up just being using the HTTPStatus library rather than plain integer status codes.

@ryan-morosa ryan-morosa May 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya I feel like using the HTTPStatus library should be a standard going forward

Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
import json
from apps.test import BaseApiTest
from django.core.management import call_command
from django.http import HttpRequest
from django.urls import reverse
from http import HTTPStatus
from unittest import mock

# from oauth2_provider.compat import parse_qs, urlparse
from urllib.parse import parse_qs, urlparse

from django.core.management import call_command
from django.http import HttpRequest
from django.urls import reverse
from oauth2_provider.models import AccessToken, RefreshToken
from rest_framework.test import APIClient
from waffle.testutils import override_switch
from apps.authorization.models import DataAccessGrant, ArchivedDataAccessGrant
from apps.dot_ext.models import ArchivedToken, Application
from http import HTTPStatus
from unittest import mock

from apps.authorization.models import ArchivedDataAccessGrant, DataAccessGrant
from apps.dot_ext.models import Application, ArchivedToken
from apps.test import BaseApiTest


class TestBeneficiaryDemographicScopesChanges(BaseApiTest):
Expand Down Expand Up @@ -127,7 +129,7 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
)

# Assert auth request was successful
self.assertEqual(status_code, 200)
self.assertEqual(status_code, HTTPStatus.OK)

# Assert scope in response content
self.assertEqual(response_scopes, sorted(APPLICATION_SCOPES_FULL))
Expand All @@ -138,7 +140,7 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
# Assert access to userinfo end point?
client.credentials(HTTP_AUTHORIZATION='Bearer ' + token_1.token)
response = client.get('/v1/connect/userinfo')
self.assertEqual(response.status_code, 200)
self.assertEqual(response.status_code, HTTPStatus.OK)

# ------ TEST #2: Test refresh of token_1
refresh_request_data = {
Expand All @@ -152,7 +154,7 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
content = json.loads(response.content.decode('utf-8'))

# Assert successful
self.assertEqual(response.status_code, 200)
self.assertEqual(response.status_code, HTTPStatus.OK)

# Assert response scopes
response_scopes = sorted(content['scope'].split())
Expand All @@ -166,7 +168,7 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
# Assert access to userinfo end point?
client.credentials(HTTP_AUTHORIZATION='Bearer ' + token.token)
response = client.get('/v1/connect/userinfo')
self.assertEqual(response.status_code, 200)
self.assertEqual(response.status_code, HTTPStatus.OK)

# Verify token counts expected.
self.assertEqual(AccessToken.objects.count(), 1)
Expand Down Expand Up @@ -197,7 +199,7 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
# Assert NO access to userinfo end point?
client.credentials(HTTP_AUTHORIZATION='Bearer ' + token_3.token)
response = client.get('/v1/connect/userinfo')
self.assertEqual(response.status_code, 403)
self.assertEqual(response.status_code, HTTPStatus.FORBIDDEN)

# Verify token counts expected.
self.assertEqual(AccessToken.objects.count(), 1)
Expand All @@ -215,7 +217,7 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
# Test access to userinfo end point? NO ACCESS!
response = client.get('/v1/connect/userinfo')
content = json.loads(response.content)
self.assertEqual(response.status_code, 401)
self.assertEqual(response.status_code, HTTPStatus.UNAUTHORIZED)
self.assertEqual(content.get('detail', None), 'Authentication credentials were not provided.')

# ------ TEST #5: Test token_1 from TEST #1 token refresh? NO ACCESS!
Expand All @@ -241,7 +243,7 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
)

# Assert auth request was successful
self.assertEqual(status_code, 200)
self.assertEqual(status_code, HTTPStatus.OK)

# Assert scope in response content
self.assertEqual(response_scopes, sorted(APPLICATION_SCOPES_FULL))
Expand All @@ -252,7 +254,7 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
# Assert access to userinfo end point?
client.credentials(HTTP_AUTHORIZATION='Bearer ' + token_6.token)
response = client.get('/v1/connect/userinfo')
self.assertEqual(response.status_code, 200)
self.assertEqual(response.status_code, HTTPStatus.OK)

# ------ TEST #7: Test token_3 from TEST #3 again. It should still have access, but no permission with status=403.

Expand All @@ -262,13 +264,13 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
# Test access to userinfo end point?
response = client.get('/v1/connect/userinfo')
content = json.loads(response.content)
self.assertEqual(response.status_code, 403)
self.assertEqual(content.get('detail', None), 'You do not have permission to perform this action.')
self.assertEqual(response.status_code, HTTPStatus.UNAUTHORIZED)
self.assertEqual(content.get('detail', None), 'Authentication credentials were not provided.')

# Verify token counts expected.
self.assertEqual(AccessToken.objects.count(), 2)
self.assertEqual(RefreshToken.objects.count(), 2)
self.assertEqual(ArchivedToken.objects.count(), 2)
self.assertEqual(AccessToken.objects.count(), 1)
self.assertEqual(RefreshToken.objects.count(), 1)
self.assertEqual(ArchivedToken.objects.count(), 3)

# Verify grant counts expected.
self.assertEqual(DataAccessGrant.objects.count(), 1)
Expand All @@ -280,15 +282,15 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):

# Perform partial authorization request, with out application getting an access token.
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
self.assertEqual(response.status_code, 302)
self.assertEqual(response.status_code, HTTPStatus.FOUND)

# Setup token_3 in APIClient from previous step. It should be removed now?
client.credentials(HTTP_AUTHORIZATION='Bearer ' + token_3.token)

# Test access to userinfo end point?
response = client.get('/v1/connect/userinfo')
content = json.loads(response.content)
self.assertEqual(response.status_code, 401)
self.assertEqual(response.status_code, HTTPStatus.UNAUTHORIZED)
self.assertEqual(content.get('detail', None), 'Authentication credentials were not provided.')

# Verify token counts expected.
Expand All @@ -309,7 +311,7 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
)

# Assert auth request was successful
self.assertEqual(status_code, 200)
self.assertEqual(status_code, HTTPStatus.OK)

# Verify token counts expected.
self.assertEqual(AccessToken.objects.count(), 1)
Expand All @@ -323,14 +325,14 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
# Assert access to userinfo end point?
client.credentials(HTTP_AUTHORIZATION='Bearer ' + token_9.token)
response = client.get('/v1/connect/userinfo')
self.assertEqual(response.status_code, 200)
self.assertEqual(response.status_code, HTTPStatus.OK)

# Beneficiary chooses the DENY button choice on consent page
payload['allow'] = False

# Perform partial authorization request, with out application getting an access token.
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
self.assertEqual(response.status_code, 302)
self.assertEqual(response.status_code, HTTPStatus.FOUND)

# Verify token counts expected.
self.assertEqual(AccessToken.objects.count(), 1)
Expand All @@ -346,7 +348,7 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
# when the allow parameter is false
client.credentials(HTTP_AUTHORIZATION='Bearer ' + token_9.token)
response = client.get('/v1/connect/userinfo')
self.assertEqual(response.status_code, 200)
self.assertEqual(response.status_code, HTTPStatus.OK)

# BB2-4270: Remove prior active tokens so tests below are not looking for multiple active tokens
# which is an impossible state
Expand All @@ -360,12 +362,12 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
payload['allow'] = True

# Perform authorization request
token_10, refresh_token_10, status_code, response_scopes, access_token_scopes = self._authorize_and_request_token(
payload, application
token_10, refresh_token_10, status_code, response_scopes, access_token_scopes = (
self._authorize_and_request_token(payload, application)
)

# Assert auth request was successful
self.assertEqual(status_code, 200)
self.assertEqual(status_code, HTTPStatus.OK)

# Verify token counts expected.
self.assertEqual(AccessToken.objects.count(), 1)
Expand All @@ -379,15 +381,15 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
# Assert access to userinfo end point?
client.credentials(HTTP_AUTHORIZATION='Bearer ' + token_10.token)
response = client.get('/v1/connect/userinfo')
self.assertEqual(response.status_code, 200)
self.assertEqual(response.status_code, HTTPStatus.OK)

# Application changes choice to require demographic scopes
application.require_demographic_scopes = False
application.save()

# Perform partial authorization request, with out application getting an access token.
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
self.assertEqual(response.status_code, 302)
self.assertEqual(response.status_code, HTTPStatus.FOUND)

# Verify token counts expected.
self.assertEqual(AccessToken.objects.count(), 0)
Expand All @@ -400,7 +402,7 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):

# Perform partial authorization request, with out application getting an access token.
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
self.assertEqual(response.status_code, 302)
self.assertEqual(response.status_code, HTTPStatus.FOUND)

# Verify token counts expected.
self.assertEqual(AccessToken.objects.count(), 0)
Expand All @@ -414,5 +416,5 @@ def test_bene_demo_scopes_change(self, mock_get_and_update):
# Assert access to userinfo end point?
client.credentials(HTTP_AUTHORIZATION='Bearer ' + token_10.token)
response = client.get('/v1/connect/userinfo')
self.assertEqual(response.status_code, 401)
self.assertEqual(response.status_code, HTTPStatus.UNAUTHORIZED)
self.assertEqual(content.get('detail', None), 'Authentication credentials were not provided.')
35 changes: 33 additions & 2 deletions apps/dot_ext/tests/test_utils.py
View file
Open in desktop

@JamesDemeryNava JamesDemeryNava Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now NFC after removing tests for a function that was added then removed.

Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
from unittest.mock import MagicMock, patch

from django.test import TestCase

from apps.versions import VersionNotMatched
from apps.dot_ext.constants import SUPPORTED_VERSION_TEST_CASES
from apps.dot_ext.utils import get_api_version_number_from_url, validate_latin_extended_string
from apps.dot_ext.utils import (
get_api_version_number_from_url,
remove_application_user_pair_tokens_data_access,
validate_latin_extended_string,
)
from apps.versions import VersionNotMatched


class TestDOTUtils(TestCase):
Expand Down Expand Up @@ -33,3 +39,28 @@ def test_latin_extended_failure(self):

for text in invalid_inputs:
assert not validate_latin_extended_string(text)

@patch('apps.dot_ext.utils.AccessToken')
@patch('apps.dot_ext.utils.DataAccessGrant')
@patch('apps.dot_ext.utils.RefreshToken')
def test_remove_application_user_pair_tokens_data_access_delete_access_tokens_not_grant(
self, mock_refresh_token, mock_data_access_grant, mock_access_token
) -> None:
application = MagicMock()
user = MagicMock()
access_token_queryset = MagicMock()
mock_access_token.objects.filter.return_value = access_token_queryset
data_access_grant_queryset = MagicMock()
mock_data_access_grant.objects.filter.return_value = data_access_grant_queryset
refresh_token_queryset = MagicMock()
mock_refresh_token.objects.filter.return_value = refresh_token_queryset

remove_application_user_pair_tokens_data_access(application, user, False, True)

mock_access_token.objects.filter.assert_called_once_with(application=application, user=user)
access_token_queryset.delete.assert_called_once()

data_access_grant_queryset.delete.assert_not_called()

mock_refresh_token.objects.filter.assert_called_once_with(application=application, user=user)
refresh_token_queryset.delete.assert_called_once()
Loading
Loading