Skip to content

Commit

Permalink
BB2-3321: Oauth revoke endpoint (#1251)
Browse files Browse the repository at this point in the history
* Initial attempt

* Added test

* Added to well-known configs and fhir capability statement and in swagger.

* Fix openapi yaml typo

* Added 404 condition for token not found

* Improved 404 and error handling

* Fix code scanning alert no. 51: Reflected server-side cross-site scripting

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

* Empty change to kick jenkins test flake

* Fix token escape

* Conform to Oauth expectations better

---------

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: bwang-icf <[email protected]>
  • Loading branch information
3 people authored Oct 29, 2024
1 parent 2ece510 commit 101bffb
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 5 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,4 +167,4 @@ by their respective authors.
License
-------

This project is free and open source software under the Apache 2 license. See LICENSE for more information.
This project is free and open source software under the Apache 2 license. See LICENSE for more information.
55 changes: 55 additions & 0 deletions apps/dot_ext/tests/test_authorization.py
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,61 @@ def test_dag_expiration_exists(self):
expiration_date_string = strftime('%Y-%m-%d %H:%M:%SZ', expiration_date.timetuple())
self.assertEqual(tkn["access_grant_expiration"][:-4], expiration_date_string[:-4])

def test_revoke_endpoint(self):
redirect_uri = 'http://localhost'
# create a user
self._create_user('anna', '123456')
capability_a = self._create_capability('Capability A', [])
capability_b = self._create_capability('Capability B', [])
# create an application and add capabilities
application = self._create_application(
'an app',
grant_type=Application.GRANT_AUTHORIZATION_CODE,
client_type=Application.CLIENT_CONFIDENTIAL,
redirect_uris=redirect_uri)
application.scope.add(capability_a, capability_b)
# user logs in
request = HttpRequest()
self.client.login(request=request, username='anna', password='123456')
# post the authorization form with only one scope selected
payload = {
'client_id': application.client_id,
'response_type': 'code',
'redirect_uri': redirect_uri,
'scope': ['capability-a'],
'expires_in': 86400,
'allow': True,
}
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
self.client.logout()
self.assertEqual(response.status_code, 302)
# now extract the authorization code and use it to request an access_token
query_dict = parse_qs(urlparse(response['Location']).query)
authorization_code = query_dict.pop('code')
token_request_data = {
'grant_type': 'authorization_code',
'code': authorization_code,
'redirect_uri': redirect_uri,
'client_id': application.client_id,
'client_secret': application.client_secret_plain,
}
c = Client()
response = c.post('/v1/o/token/', data=token_request_data)
self.assertEqual(response.status_code, 200)
# extract token and use it to make a revoke request
tkn = response.json()['access_token']
revoke_request_data = f"token={tkn}&client_id={application.client_id}&client_secret={application.client_secret_plain}"
content_type = "application/x-www-form-urlencoded"
c = Client()
rev_response = c.post('/v1/o/revoke/', data=revoke_request_data, content_type=content_type)
self.assertEqual(rev_response.status_code, 200)
# check DAG deletion
dags_count = DataAccessGrant.objects.count()
self.assertEqual(dags_count, 0)
# check token deletion
tkn_count = AccessToken.objects.filter(token=tkn).count()
self.assertEqual(tkn_count, 0)

def test_refresh_with_revoked_token(self):
redirect_uri = 'http://localhost'
# create a user
Expand Down
1 change: 1 addition & 0 deletions apps/dot_ext/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
),
path("token/", views.TokenView.as_view(), name="token"),
path("revoke_token/", views.RevokeTokenView.as_view(), name="revoke-token"),
path("revoke/", views.RevokeView.as_view(), name="revoke"),
path("introspect/", views.IntrospectTokenView.as_view(), name="introspect"),
]

Expand Down
1 change: 1 addition & 0 deletions apps/dot_ext/v2/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
),
path("token/", views.TokenView.as_view(), name="token-v2"),
path("revoke_token/", views.RevokeTokenView.as_view(), name="revoke-token-v2"),
path("revoke/", views.RevokeView.as_view(), name="revoke-v2"),
path("introspect/", views.IntrospectTokenView.as_view(), name="introspect-v2"),
]

Expand Down
2 changes: 1 addition & 1 deletion apps/dot_ext/views/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
from .application import ApplicationRegistration, ApplicationUpdate, ApplicationDelete # NOQA
from .authorization import AuthorizationView, ApprovalView, TokenView, RevokeTokenView, IntrospectTokenView # NOQA
from .authorization import AuthorizationView, ApprovalView, TokenView, RevokeTokenView, RevokeView, IntrospectTokenView # NOQA
40 changes: 37 additions & 3 deletions apps/dot_ext/views/authorization.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from oauth2_provider.models import get_application_model
from oauthlib.oauth2.rfc6749.errors import InvalidClientError, InvalidGrantError
from urllib.parse import urlparse, parse_qs

import html
from apps.dot_ext.scopes import CapabilitiesScopes
import apps.logging.request_logger as bb2logging

Expand Down Expand Up @@ -120,12 +120,12 @@ def sensitive_info_check(self, request):
def get_template_names(self):
flag = get_waffle_flag_model().get("limit_data_access")
if waffle.switch_is_active('require-scopes'):
if flag.rollout or (flag.id is not None and flag.is_active_for_user(self.application.user)):
if flag.rollout or (flag.id is not None and self.application and flag.is_active_for_user(self.application.user)):
return ["design_system/new_authorize_v2.html"]
else:
return ["design_system/authorize_v2.html"]
else:
if flag.rollout or (flag.id is not None and flag.is_active_for_user(self.user)):
if flag.rollout or (flag.id is not None and self.user and flag.is_active_for_user(self.user)):
return ["design_system/new_authorize_v2.html"]
else:
return ["design_system/authorize.html"]
Expand Down Expand Up @@ -354,6 +354,40 @@ def post(self, request, *args, **kwargs):
return super().post(request, args, kwargs)


@method_decorator(csrf_exempt, name="dispatch")
class RevokeView(DotRevokeTokenView):

@method_decorator(sensitive_post_parameters("password"))
def post(self, request, *args, **kwargs):
at_model = get_access_token_model()
try:
app = validate_app_is_active(request)
except (InvalidClientError, InvalidGrantError) as error:
return json_response_from_oauth2_error(error)

tkn = request.POST.get('token')
if tkn is not None:
escaped_tkn = html.escape(tkn)
else:
escaped_tkn = ""

try:
token = at_model.objects.get(token=tkn)
except at_model.DoesNotExist:
log.debug(f"Token {escaped_tkn} was not found.")

try:
dag = DataAccessGrant.objects.get(
beneficiary=token.user,
application=app
)
dag.delete()
except Exception:
log.debug(f"DAG lookup failed for token {escaped_tkn}.")

return super().post(request, args, kwargs)


@method_decorator(csrf_exempt, name="dispatch")
class IntrospectTokenView(DotIntrospectTokenView):

Expand Down
5 changes: 5 additions & 0 deletions apps/fhir/bluebutton/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -636,12 +636,16 @@ def build_oauth_resource(request, v2=False, format_type="json"):
<extension url="authorize">
<valueUri>%s</valueUri>
</extension>
<extension url="revoke">
<valueUri>%s</valueUri>
</extension>
</extension>
</security>
""" % (
endpoints["token_endpoint"],
endpoints["authorization_endpoint"],
endpoints["revocation_endpoint"],
)

else: # json
Expand Down Expand Up @@ -680,6 +684,7 @@ def build_oauth_resource(request, v2=False, format_type="json"):
"url": "authorize",
"valueUri": endpoints["authorization_endpoint"],
},
{"url": "revoke", "valueUri": endpoints["revocation_endpoint"]},
],
}
]
Expand Down
1 change: 1 addition & 0 deletions apps/wellknown/views/openid.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ def build_endpoint_info(data=OrderedDict(), v2=False, issuer=""):
data["issuer"] = issuer
data["authorization_endpoint"] = issuer + \
reverse('oauth2_provider:authorize' if not v2 else 'oauth2_provider_v2:authorize-v2')
data["revocation_endpoint"] = issuer + reverse('oauth2_provider:revoke')
data["token_endpoint"] = issuer + \
reverse('oauth2_provider:token' if not v2 else 'oauth2_provider_v2:token-v2')
data["userinfo_endpoint"] = issuer + \
Expand Down
10 changes: 10 additions & 0 deletions static/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,8 @@ components:
type: string
authorization_endpoint:
type: string
revocation_endpoint:
type: string
token_endpoint:
type: string
userinfo_endpoint:
Expand Down Expand Up @@ -746,6 +748,8 @@ components:
type: string
authorization_endpoint:
type: string
revocation_endpoint:
type: string
token_endpoint:
type: string
userinfo_endpoint:
Expand Down Expand Up @@ -812,6 +816,7 @@ components:
value:
issuer: 'https://sandbox.bluebutton.cms.gov'
authorization_endpoint: 'https://sandbox.bluebutton.cms.gov/v1/o/authorize/'
revocation_endpoint: 'https://sandbox.bluebutton.cms.gov/v1/o/revoke/'
token_endpoint: 'https://sandbox.bluebutton.cms.gov/v1/o/token/'
userinfo_endpoint: 'https://sandbox.bluebutton.cms.gov/v1/connect/userinfo'
ui_locales_supported:
Expand All @@ -830,6 +835,7 @@ components:
value:
issuer: 'https://sandbox.bluebutton.cms.gov'
authorization_endpoint: 'https://sandbox.bluebutton.cms.gov/v2/o/authorize/'
revocation_endpoint: 'https://sandbox.bluebutton.cms.gov/v2/o/revoke/'
token_endpoint: 'https://sandbox.bluebutton.cms.gov/v2/o/token/'
userinfo_endpoint: 'https://sandbox.bluebutton.cms.gov/v2/connect/userinfo'
ui_locales_supported:
Expand Down Expand Up @@ -1230,6 +1236,8 @@ components:
valueUri: 'https://sandbox.bluebutton.cms.gov/v1/o/token/'
- url: authorize
valueUri: 'https://sandbox.bluebutton.cms.gov/v1/o/authorize/'
- url: revoke
valueUri: 'https://sandbox.bluebutton.cms.gov/v1/o/revoke/'

V2FhirMetadataExample:
value:
Expand Down Expand Up @@ -1665,6 +1673,8 @@ components:
valueUri: 'https://sandbox.bluebutton.cms.gov/v2/o/token/'
- url: authorize
valueUri: 'https://sandbox.bluebutton.cms.gov/v2/o/authorize/'
- url: revoke
valueUri: 'https://sandbox.bluebutton.cms.gov/v2/o/revoke/'

V1UserInfoExample:
value:
Expand Down

0 comments on commit 101bffb

Please sign in to comment.