Skip to content

Commit cc945bc

Browse files
stiwarisemanticbitsShivam Tiwari
stiwarisemanticbits
and
Shivam Tiwari
authored
BB2-4142: Make POST /authorize work by disabling CSRF (#1376)
* Working POST on authorize with csrf disabled * Fixed test * Remove State FF * Fixed tests * add state * add state --------- Co-authored-by: Shivam Tiwari <shivam.tiwari@icf.com>
1 parent bf5e833 commit cc945bc

File tree

9 files changed

+56
-4
lines changed

9 files changed

+56
-4
lines changed

‎apps/authorization/tests/test_data_access_grant.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ def setup_test_application_with_user(self, test_user, application_name="an app")
244244
"scope": ["capability-a"],
245245
"expires_in": 86400,
246246
"allow": True,
247+
"state": "0123456789abcdef",
247248
}
248249
response = self.client.post(response["Location"], data=payload)
249250
self.assertEqual(response.status_code, 302)
@@ -286,6 +287,7 @@ def test_no_action_on_reapproval(self):
286287
"scope": ["capability-a"],
287288
"expires_in": 86400,
288289
"allow": True,
290+
"state": "0123456789abcdef",
289291
}
290292
response = self.client.post(response["Location"], data=payload)
291293

@@ -401,6 +403,7 @@ def test_permission_deny_on_app_or_org_disabled(self):
401403
"scope": ["capability-a"],
402404
"expires_in": 86400,
403405
"allow": True,
406+
"state": "0123456789abcdef",
404407
}
405408

406409
response = self.client.post(response["Location"], data=payload)

‎apps/dot_ext/tests/test_authorization.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ def test_post_with_valid_non_standard_scheme_granttype_authcode_clienttype_publi
6464
'scope': ['capability-a'],
6565
'expires_in': 86400,
6666
'allow': True,
67+
"state": "0123456789abcdef",
6768
'code_challenge': code_challenge,
6869
'code_challenge_method': 'S256',
6970
}
@@ -118,6 +119,7 @@ def test_post_with_invalid_non_standard_scheme_granttype_authcode_clienttype_pub
118119
'scope': ['capability-a'],
119120
'expires_in': 86400,
120121
'allow': True,
122+
"state": "0123456789abcdef",
121123
}
122124
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
123125
self.assertEqual(response.status_code, 400)
@@ -159,6 +161,7 @@ def test_post_with_valid_non_standard_scheme_granttype_authcode_clienttype_confi
159161
'scope': ['capability-a'],
160162
'expires_in': 86400,
161163
'allow': True,
164+
"state": "0123456789abcdef",
162165
'code_challenge': code_challenge,
163166
'code_challenge_method': 'S256',
164167
}
@@ -222,6 +225,7 @@ def test_post_with_invalid_non_standard_scheme_granttype_authcode_clienttype_con
222225
'scope': ['capability-a'],
223226
'expires_in': 86400,
224227
'allow': True,
228+
"state": "0123456789abcdef",
225229
}
226230
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
227231
self.assertEqual(response.status_code, 400)
@@ -250,6 +254,7 @@ def test_refresh_token(self):
250254
'scope': ['capability-a'],
251255
'expires_in': 86400,
252256
'allow': True,
257+
"state": "0123456789abcdef",
253258
}
254259
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
255260
self.client.logout()
@@ -305,6 +310,7 @@ def test_refresh_with_expired_token(self):
305310
'scope': ['capability-a'],
306311
'expires_in': 86400,
307312
'allow': True,
313+
"state": "0123456789abcdef",
308314
}
309315
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
310316
self.client.logout()
@@ -361,6 +367,7 @@ def test_refresh_13_month_with_expired_grant(self):
361367
'scope': ['capability-a'],
362368
'expires_in': 86400,
363369
'allow': True,
370+
"state": "0123456789abcdef",
364371
}
365372
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
366373
self.client.logout()
@@ -425,6 +432,7 @@ def test_refresh_with_one_time_access_retrieve_app_using_refresh_token(self):
425432
'scope': ['capability-a'],
426433
'expires_in': 86400,
427434
'allow': True,
435+
"state": "0123456789abcdef",
428436
}
429437
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
430438
self.client.logout()
@@ -480,6 +488,7 @@ def test_refresh_with_one_time_access_retrieve_app_from_auth_header(self):
480488
'scope': ['capability-a'],
481489
'expires_in': 86400,
482490
'allow': True,
491+
"state": "0123456789abcdef",
483492
}
484493
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
485494
self.client.logout()
@@ -550,6 +559,7 @@ def test_dag_expiration_exists(self):
550559
'scope': ['capability-a'],
551560
'expires_in': 86400,
552561
'allow': True,
562+
"state": "0123456789abcdef",
553563
}
554564
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
555565
self.client.logout()
@@ -594,6 +604,7 @@ def test_revoke_endpoint(self):
594604
'scope': ['capability-a'],
595605
'expires_in': 86400,
596606
'allow': True,
607+
"state": "0123456789abcdef",
597608
}
598609
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
599610
self.client.logout()
@@ -649,6 +660,7 @@ def test_refresh_with_revoked_token(self):
649660
'scope': ['capability-a'],
650661
'expires_in': 86400,
651662
'allow': True,
663+
"state": "0123456789abcdef",
652664
}
653665
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
654666
self.client.logout()
@@ -715,6 +727,7 @@ def test_application_delete_after_auth(self):
715727
'scope': ['capability-a'],
716728
'expires_in': 86400,
717729
'allow': True,
730+
"state": "0123456789abcdef",
718731
}
719732
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
720733
self.client.logout()
@@ -771,6 +784,7 @@ def test_user_delete_after_auth(self):
771784
'scope': ['capability-a'],
772785
'expires_in': 86400,
773786
'allow': True,
787+
"state": "0123456789abcdef",
774788
}
775789
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
776790
self.client.logout()
@@ -831,6 +845,7 @@ def test_revoked_token_on_inactive_app(self):
831845
'scope': ['capability-a'],
832846
'expires_in': 86400,
833847
'allow': True,
848+
"state": "0123456789abcdef",
834849
}
835850
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
836851
self.client.logout()
@@ -900,6 +915,7 @@ def test_introspect_token_on_inactive_app(self):
900915
'scope': ['capability-a', 'capability-b', 'introspection'],
901916
'expires_in': 86400,
902917
'allow': True,
918+
"state": "0123456789abcdef",
903919
}
904920
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
905921
self.client.logout()
@@ -986,6 +1002,7 @@ def _execute_token_endpoint(self, token_path):
9861002
'scope': ['capability-a'],
9871003
'expires_in': 86400,
9881004
'allow': True,
1005+
"state": "0123456789abcdef",
9891006
}
9901007
response = self.client.post(reverse('oauth2_provider:authorize'), data=payload)
9911008
self.client.logout()

‎apps/dot_ext/tests/test_beneficiary_demographic_scope_changes.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,9 @@ def test_bene_demo_scopes_change(self):
9393
'response_type': 'code',
9494
'redirect_uri': 'http://example.it',
9595
'expires_in': 86400,
96-
'allow': True}
96+
'allow': True,
97+
"state": "0123456789abcdef",
98+
}
9799

98100
request_scopes = APPLICATION_SCOPES_FULL
99101
# Scopes to be requested in the authorization request

‎apps/dot_ext/tests/test_verify_bfd_headers.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ def _create_test_token(self, user, application):
6363
"scope": application.scopes().split(" "),
6464
"expires_in": 86400,
6565
"allow": True,
66+
"state": "0123456789abcdef",
6667
"share_demographic_scopes": True
6768
}
6869
if application.authorization_grant_type == Application.GRANT_IMPLICIT:

‎apps/dot_ext/tests/test_views.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ def test_post_with_restricted_scopes_issues_token_with_same_scopes(self):
155155
"scope": ["capability-a"],
156156
"expires_in": 86400,
157157
"allow": True,
158+
"state": "0123456789abcdef",
158159
}
159160
response = self._authorize_and_request_token(payload, application)
160161
self.assertEqual(response.status_code, 200)
@@ -228,6 +229,7 @@ def test_post_with_share_demographic_scopes(self):
228229
"redirect_uri": "http://example.it",
229230
"expires_in": 86400,
230231
"allow": True,
232+
"state": "0123456789abcdef",
231233
}
232234

233235
# Does the application choose to require demographic info?
@@ -373,6 +375,7 @@ def _create_test_token(self, user, application):
373375
"scope": application.scopes().split(" "),
374376
"expires_in": 86400,
375377
"allow": True,
378+
"state": "0123456789abcdef",
376379
}
377380
if application.authorization_grant_type == Application.GRANT_IMPLICIT:
378381
payload["response_type"] = "token"

‎apps/dot_ext/views/authorization.py‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import json
22
import logging
33
from datetime import datetime, timedelta
4+
from functools import wraps
45
from time import strftime
56

7+
from django.contrib.auth.views import redirect_to_login
68
from django.http import JsonResponse
79
from django.http.response import HttpResponse, HttpResponseBadRequest
810
from django.template.response import TemplateResponse
@@ -54,6 +56,20 @@ def get_grant_expiration(data_access_type):
5456
pass
5557

5658

59+
def require_post_state_decorator(view_func):
60+
@wraps(view_func)
61+
def _wrapped(request, *args, **kwargs):
62+
if request.method == "POST" and not request.POST.get("state"):
63+
return JsonResponse(
64+
{"status_code": 401, "message": "State required for POST requests."},
65+
status=401,
66+
)
67+
return view_func(request, *args, **kwargs)
68+
return _wrapped
69+
70+
71+
@method_decorator(csrf_exempt, name="dispatch")
72+
@method_decorator(require_post_state_decorator, name="dispatch")
5773
class AuthorizationView(DotAuthorizationView):
5874
"""
5975
Override the base authorization view from dot to
@@ -117,6 +133,14 @@ def dispatch(self, request, *args, **kwargs):
117133
if lang in ('en', 'es'):
118134
request.session['auth_language'] = lang
119135

136+
if request.method == "POST" and not request.user.is_authenticated:
137+
post_qs = request.POST.urlencode()
138+
# preserve existing query too
139+
existing_qs = request.META.get("QUERY_STRING", "")
140+
merged_qs = f"{existing_qs}&{post_qs}" if existing_qs else post_qs
141+
next_url = f"{request.path}?{merged_qs}"
142+
return redirect_to_login(next_url, login_url=self.login_url)
143+
120144
return super().dispatch(request, *args, **kwargs)
121145

122146
def sensitive_info_check(self, request):
@@ -292,6 +316,7 @@ def form_valid(self, form):
292316

293317

294318
@method_decorator(csrf_exempt, name="dispatch")
319+
@method_decorator(require_post_state_decorator, name="dispatch")
295320
class ApprovalView(AuthorizationView):
296321
"""
297322
Override the base authorization view from dot to
@@ -306,8 +331,6 @@ def __init__(self, version=1):
306331
super().__init__()
307332

308333
def dispatch(self, request, uuid, *args, **kwargs):
309-
if request.method == "POST" and request.POST.get("state") is None:
310-
return JsonResponse({"status_code": 401, "message": "State required for POST requests."}, status=401)
311334

312335
# Get auth_uuid to set again after super() return. It gets cleared out otherwise.
313336
auth_flow_dict = get_session_auth_flow_trace(request)

‎apps/logging/tests/test_audit_loggers.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,7 @@ def _creation_on_approval_token_logger(self, v2=False):
480480
"scope": ["capability-a"],
481481
"expires_in": 86400,
482482
"allow": True,
483+
"state": "0123456789abcdef",
483484
}
484485
response = self.client.post(response["Location"], data=payload)
485486
self.assertEqual(response.status_code, status.HTTP_302_FOUND)
@@ -606,6 +607,7 @@ def test_auth_flow_lang_logger(self, v2=False):
606607
"scope": ["capability-a"],
607608
"expires_in": 86400,
608609
"allow": True,
610+
"state": "0123456789abcdef",
609611
}
610612
response = self.client.post(response["Location"], data=payload)
611613
request_log_content = get_log_content(self.logger_registry,

‎apps/mymedicare_cb/tests/test_callback_slsx.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ def test_authorize_uuid(self):
185185
"scope": ["capability-a"],
186186
"expires_in": 86400,
187187
"allow": True,
188-
"state": "1234567890",
188+
"state": "0123456789abcdef",
189189
}
190190
response = self.client.post(auth_uri, data=payload)
191191
self.assertEqual(status.HTTP_302_FOUND, response.status_code)

‎apps/test.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,7 @@ def _get_access_token_authcode_confidential(
297297
"scope": ["capability-a"],
298298
"expires_in": 86400,
299299
"allow": True,
300+
"state": "0123456789abcdef",
300301
"code_challenge": code_challenge,
301302
"code_challenge_method": "S256",
302303
}

0 commit comments

Comments
 (0)