From 0610f6c59af1a20dfc4012b97052896bd197f489 Mon Sep 17 00:00:00 2001 From: Logan Bertram Date: Wed, 11 Sep 2024 11:12:17 -0400 Subject: [PATCH 01/10] Initial attempt --- apps/dot_ext/urls.py | 1 + apps/dot_ext/v2/urls.py | 1 + apps/dot_ext/views/__init__.py | 2 +- apps/dot_ext/views/authorization.py | 24 ++++++++++++++++++++++++ 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/apps/dot_ext/urls.py b/apps/dot_ext/urls.py index 16a7ba683..99fbed41d 100644 --- a/apps/dot_ext/urls.py +++ b/apps/dot_ext/urls.py @@ -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"), ] diff --git a/apps/dot_ext/v2/urls.py b/apps/dot_ext/v2/urls.py index b02112ba3..91ba471b3 100755 --- a/apps/dot_ext/v2/urls.py +++ b/apps/dot_ext/v2/urls.py @@ -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"), ] diff --git a/apps/dot_ext/views/__init__.py b/apps/dot_ext/views/__init__.py index bce5a9c09..da038284b 100644 --- a/apps/dot_ext/views/__init__.py +++ b/apps/dot_ext/views/__init__.py @@ -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 diff --git a/apps/dot_ext/views/authorization.py b/apps/dot_ext/views/authorization.py index 770fa395e..f531dfdda 100644 --- a/apps/dot_ext/views/authorization.py +++ b/apps/dot_ext/views/authorization.py @@ -354,6 +354,30 @@ 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): + try: + app = validate_app_is_active(request) + except (InvalidClientError, InvalidGrantError) as error: + return json_response_from_oauth2_error(error) + + token = get_access_token_model().objects.get( + token=request.POST.get("token")) + try: + dag = DataAccessGrant.objects.get( + beneficiary=token.user, + application=app + ) + dag.delete() + except DataAccessGrant.DoesNotExist: + pass + + return super().post(request, args, kwargs) + + @method_decorator(csrf_exempt, name="dispatch") class IntrospectTokenView(DotIntrospectTokenView): From 963a7d0d5ac50f7ab7d6b28d0c6411ac00baf0a7 Mon Sep 17 00:00:00 2001 From: Logan Bertram Date: Tue, 8 Oct 2024 11:19:12 -0400 Subject: [PATCH 02/10] Added test --- apps/dot_ext/tests/test_authorization.py | 55 ++++++++++++++++++++++++ apps/dot_ext/views/authorization.py | 4 +- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/apps/dot_ext/tests/test_authorization.py b/apps/dot_ext/tests/test_authorization.py index 3cc3ee263..6b40b593a 100644 --- a/apps/dot_ext/tests/test_authorization.py +++ b/apps/dot_ext/tests/test_authorization.py @@ -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 = { + 'token': tkn, + 'client_id': application.client_id, + 'client_secret': application.client_secret_plain, + } + c = Client() + rev_response = c.post('/v1/o/revoke/', data=revoke_request_data) + self.assertEqual(rev_response.status_code, 200) + # check DAG deletion + dags_count = DataAccessGrant.objects.count() + self.assertEqual(dags_count, 0) + def test_refresh_with_revoked_token(self): redirect_uri = 'http://localhost' # create a user diff --git a/apps/dot_ext/views/authorization.py b/apps/dot_ext/views/authorization.py index f531dfdda..55ad76daa 100644 --- a/apps/dot_ext/views/authorization.py +++ b/apps/dot_ext/views/authorization.py @@ -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"] From 4dc1e7b2bb17996e8e7def63b22e029f7d6c50e1 Mon Sep 17 00:00:00 2001 From: Logan Bertram Date: Wed, 9 Oct 2024 01:27:35 -0400 Subject: [PATCH 03/10] Added to well-known configs and fhir capability statement and in swagger. --- apps/dot_ext/views/authorization.py | 15 ++++++++++----- apps/fhir/bluebutton/utils.py | 5 +++++ apps/wellknown/views/openid.py | 1 + static/openapi.yaml | 10 ++++++++++ 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/apps/dot_ext/views/authorization.py b/apps/dot_ext/views/authorization.py index 55ad76daa..cbb893783 100644 --- a/apps/dot_ext/views/authorization.py +++ b/apps/dot_ext/views/authorization.py @@ -359,23 +359,28 @@ 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) - token = get_access_token_model().objects.get( - token=request.POST.get("token")) + try: + body = json.loads(request.body.decode("UTF-8")) + token = at_model.objects.get(token=body.get("token")) + except Exception: + token = at_model.objects.get( + token=request.POST.get("token")) try: dag = DataAccessGrant.objects.get( beneficiary=token.user, application=app ) dag.delete() - except DataAccessGrant.DoesNotExist: - pass + except DataAccessGrant.DoesNotExist as error: + return json_response_from_oauth2_error(error) - return super().post(request, args, kwargs) + return HttpResponse(content="OK", status=200) @method_decorator(csrf_exempt, name="dispatch") diff --git a/apps/fhir/bluebutton/utils.py b/apps/fhir/bluebutton/utils.py index d5fcbb438..291bb95cf 100644 --- a/apps/fhir/bluebutton/utils.py +++ b/apps/fhir/bluebutton/utils.py @@ -636,12 +636,16 @@ def build_oauth_resource(request, v2=False, format_type="json"): %s + + %s + """ % ( endpoints["token_endpoint"], endpoints["authorization_endpoint"], + endpoints["revocation_endpoint"], ) else: # json @@ -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"]}, ], } ] diff --git a/apps/wellknown/views/openid.py b/apps/wellknown/views/openid.py index ef54710ad..68f3ac9b2 100644 --- a/apps/wellknown/views/openid.py +++ b/apps/wellknown/views/openid.py @@ -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 + \ diff --git a/static/openapi.yaml b/static/openapi.yaml index 1edadace9..2e73796a0 100755 --- a/static/openapi.yaml +++ b/static/openapi.yaml @@ -1034,6 +1034,8 @@ components: type: string authorization_endpoint: type: string + revocation_endpoint: + type: string token_endpoint: type: string userinfo_endpoint: @@ -1063,6 +1065,8 @@ components: type: string authorization_endpoint: type: string + revocation_endpoint: + type: string token_endpoint: type: string userinfo_endpoint: @@ -3917,6 +3921,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: @@ -3935,6 +3940,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: @@ -4335,6 +4341,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: @@ -4770,6 +4778,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: From c9a5c1d4cccc479528f42d98560d0cb57280e16a Mon Sep 17 00:00:00 2001 From: Logan Bertram Date: Wed, 9 Oct 2024 01:32:17 -0400 Subject: [PATCH 04/10] Fix openapi yaml typo --- static/openapi.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/openapi.yaml b/static/openapi.yaml index 2e73796a0..3ea3d4ed0 100755 --- a/static/openapi.yaml +++ b/static/openapi.yaml @@ -4342,7 +4342,7 @@ components: - url: authorize valueUri: 'https://sandbox.bluebutton.cms.gov/v1/o/authorize/' - url: revoke - valueUri: 'https://sandbox.bluebutton.cms.gov/v1/o/revoke/' + valueUri: 'https://sandbox.bluebutton.cms.gov/v1/o/revoke/' V2FhirMetadataExample: value: From 52b2f9654ada6315729f77e357b7d878946c3efa Mon Sep 17 00:00:00 2001 From: Logan Bertram Date: Wed, 9 Oct 2024 01:45:15 -0400 Subject: [PATCH 05/10] Added 404 condition for token not found --- apps/dot_ext/views/authorization.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/dot_ext/views/authorization.py b/apps/dot_ext/views/authorization.py index cbb893783..6b95bf0f1 100644 --- a/apps/dot_ext/views/authorization.py +++ b/apps/dot_ext/views/authorization.py @@ -21,6 +21,7 @@ ) from oauth2_provider.models import get_application_model from oauthlib.oauth2.rfc6749.errors import InvalidClientError, InvalidGrantError +from rest_framework import status from urllib.parse import urlparse, parse_qs from apps.dot_ext.scopes import CapabilitiesScopes @@ -371,6 +372,10 @@ def post(self, request, *args, **kwargs): except Exception: token = at_model.objects.get( token=request.POST.get("token")) + + if token is None: + return HttpResponse("Token was Not Found. Please check the value and try again.", + status=status.HTTP_404_NOT_FOUND) try: dag = DataAccessGrant.objects.get( beneficiary=token.user, From fc105d9c5e6fc9b7a972c3552a0649514a4745a9 Mon Sep 17 00:00:00 2001 From: Logan Bertram Date: Wed, 9 Oct 2024 09:29:59 -0400 Subject: [PATCH 06/10] Improved 404 and error handling --- apps/dot_ext/views/authorization.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/apps/dot_ext/views/authorization.py b/apps/dot_ext/views/authorization.py index 6b95bf0f1..86afc7644 100644 --- a/apps/dot_ext/views/authorization.py +++ b/apps/dot_ext/views/authorization.py @@ -367,15 +367,16 @@ def post(self, request, *args, **kwargs): return json_response_from_oauth2_error(error) try: - body = json.loads(request.body.decode("UTF-8")) - token = at_model.objects.get(token=body.get("token")) + tkn = json.loads(request.body.decode("UTF-8")).get("token") except Exception: - token = at_model.objects.get( - token=request.POST.get("token")) + tkn = request.POST.get("token") - if token is None: - return HttpResponse("Token was Not Found. Please check the value and try again.", + try: + token = at_model.objects.get(token=tkn) + except at_model.DoesNotExist: + return HttpResponse(f"Token {tkn} was Not Found. Please check the value and try again.", status=status.HTTP_404_NOT_FOUND) + try: dag = DataAccessGrant.objects.get( beneficiary=token.user, From 5b56fad4d740f9c25bb22cfceebfe08189964aeb Mon Sep 17 00:00:00 2001 From: Logan Bertram Date: Wed, 9 Oct 2024 09:43:49 -0400 Subject: [PATCH 07/10] 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> --- apps/dot_ext/views/authorization.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/dot_ext/views/authorization.py b/apps/dot_ext/views/authorization.py index 86afc7644..fa5b624a2 100644 --- a/apps/dot_ext/views/authorization.py +++ b/apps/dot_ext/views/authorization.py @@ -23,7 +23,7 @@ from oauthlib.oauth2.rfc6749.errors import InvalidClientError, InvalidGrantError from rest_framework import status from urllib.parse import urlparse, parse_qs - +import html from apps.dot_ext.scopes import CapabilitiesScopes import apps.logging.request_logger as bb2logging @@ -374,7 +374,8 @@ def post(self, request, *args, **kwargs): try: token = at_model.objects.get(token=tkn) except at_model.DoesNotExist: - return HttpResponse(f"Token {tkn} was Not Found. Please check the value and try again.", + escaped_tkn = html.escape(tkn) + return HttpResponse(f"Token {escaped_tkn} was Not Found. Please check the value and try again.", status=status.HTTP_404_NOT_FOUND) try: From e06ff9cecb097beca9a5b68c72c8712aa2af033d Mon Sep 17 00:00:00 2001 From: Logan Bertram Date: Wed, 9 Oct 2024 09:59:09 -0400 Subject: [PATCH 08/10] Empty change to kick jenkins test flake --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 903baa2b2..e8285f23b 100644 --- a/README.md +++ b/README.md @@ -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. \ No newline at end of file +This project is free and open source software under the Apache 2 license. See LICENSE for more information. From 56a972b153d60fdce51c8986bd4d3eee0972062e Mon Sep 17 00:00:00 2001 From: Logan Bertram Date: Thu, 10 Oct 2024 12:29:10 -0400 Subject: [PATCH 09/10] Fix token escape --- apps/dot_ext/views/authorization.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/dot_ext/views/authorization.py b/apps/dot_ext/views/authorization.py index fa5b624a2..88dba5ff1 100644 --- a/apps/dot_ext/views/authorization.py +++ b/apps/dot_ext/views/authorization.py @@ -371,10 +371,11 @@ def post(self, request, *args, **kwargs): except Exception: tkn = request.POST.get("token") + escaped_tkn = html.escape(tkn) + try: token = at_model.objects.get(token=tkn) except at_model.DoesNotExist: - escaped_tkn = html.escape(tkn) return HttpResponse(f"Token {escaped_tkn} was Not Found. Please check the value and try again.", status=status.HTTP_404_NOT_FOUND) @@ -384,8 +385,8 @@ def post(self, request, *args, **kwargs): application=app ) dag.delete() - except DataAccessGrant.DoesNotExist as error: - return json_response_from_oauth2_error(error) + except DataAccessGrant.DoesNotExist: + log.debug(f"Token deleted, but DAG lookup failed for token {escaped_tkn}.") return HttpResponse(content="OK", status=200) From a5906e3a94d46c4085617b957e10b3bbb12f61cc Mon Sep 17 00:00:00 2001 From: Logan Bertram Date: Tue, 15 Oct 2024 11:28:02 -0400 Subject: [PATCH 10/10] Conform to Oauth expectations better --- apps/dot_ext/tests/test_authorization.py | 12 ++++++------ apps/dot_ext/views/authorization.py | 21 +++++++++------------ 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/apps/dot_ext/tests/test_authorization.py b/apps/dot_ext/tests/test_authorization.py index 6b40b593a..e159074c1 100644 --- a/apps/dot_ext/tests/test_authorization.py +++ b/apps/dot_ext/tests/test_authorization.py @@ -621,17 +621,17 @@ def test_revoke_endpoint(self): self.assertEqual(response.status_code, 200) # extract token and use it to make a revoke request tkn = response.json()['access_token'] - revoke_request_data = { - 'token': tkn, - 'client_id': application.client_id, - 'client_secret': application.client_secret_plain, - } + 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) + 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' diff --git a/apps/dot_ext/views/authorization.py b/apps/dot_ext/views/authorization.py index 88dba5ff1..f0abc7e26 100644 --- a/apps/dot_ext/views/authorization.py +++ b/apps/dot_ext/views/authorization.py @@ -21,7 +21,6 @@ ) from oauth2_provider.models import get_application_model from oauthlib.oauth2.rfc6749.errors import InvalidClientError, InvalidGrantError -from rest_framework import status from urllib.parse import urlparse, parse_qs import html from apps.dot_ext.scopes import CapabilitiesScopes @@ -366,18 +365,16 @@ def post(self, request, *args, **kwargs): except (InvalidClientError, InvalidGrantError) as error: return json_response_from_oauth2_error(error) - try: - tkn = json.loads(request.body.decode("UTF-8")).get("token") - except Exception: - tkn = request.POST.get("token") - - escaped_tkn = html.escape(tkn) + 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: - return HttpResponse(f"Token {escaped_tkn} was Not Found. Please check the value and try again.", - status=status.HTTP_404_NOT_FOUND) + log.debug(f"Token {escaped_tkn} was not found.") try: dag = DataAccessGrant.objects.get( @@ -385,10 +382,10 @@ def post(self, request, *args, **kwargs): application=app ) dag.delete() - except DataAccessGrant.DoesNotExist: - log.debug(f"Token deleted, but DAG lookup failed for token {escaped_tkn}.") + except Exception: + log.debug(f"DAG lookup failed for token {escaped_tkn}.") - return HttpResponse(content="OK", status=200) + return super().post(request, args, kwargs) @method_decorator(csrf_exempt, name="dispatch")