Skip to content

Commit c444fa8

Browse files
bwang-icfCopilotannamontare-nava
authored andcommitted
added validation around code_challenge_method to reject non S256 values (#1599)
* added validation around code_challenge_method to reject non S256 values * intermediate commit * Fix authorize POST param handling in code_challenge_method validation Agent-Logs-Url: https://github.com/CMSgov/bluebutton-web-server/sessions/ec2649c7-d427-4f59-ad18-098613b75e4e Co-authored-by: bwang-icf <178809349+bwang-icf@users.noreply.github.com> * Remove stale commented lines in authorization GET handler Agent-Logs-Url: https://github.com/CMSgov/bluebutton-web-server/sessions/ec2649c7-d427-4f59-ad18-098613b75e4e Co-authored-by: bwang-icf <178809349+bwang-icf@users.noreply.github.com> * cleanup of previous pkce setup * removing unnecessary error handling * adding updated tests with pkce params as part of enforcing pkce params on auth * adding extra tests for pkce param verification before login * removing unnecessary change for this pr * reverting code_challenge_method back to post_auth * removing invalid code challenge method printing --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwang-icf <178809349+bwang-icf@users.noreply.github.com> Co-authored-by: annamontare-nava <267455234+annamontare-nava@users.noreply.github.com>
1 parent a8914c4 commit c444fa8

10 files changed

Lines changed: 274 additions & 79 deletions

File tree

apps/authorization/tests/test_data_access_grant.py

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@
3535
class TestDataAccessGrant(BaseApiTest):
3636
@staticmethod
3737
def _create_authorization_header(client_id, client_secret):
38-
return 'Basic {0}'.format(base64.b64encode('{0}:{1}'.format(client_id, client_secret).encode('utf-8')).decode('utf-8'))
38+
return 'Basic {0}'.format(
39+
base64.b64encode('{0}:{1}'.format(client_id, client_secret).encode('utf-8')).decode('utf-8')
40+
)
3941

4042
def test_create_update_delete(self):
4143
# 1. Test create and default expiration_date
@@ -76,7 +78,9 @@ def test_create_update_delete(self):
7678
dag.delete()
7779

7880
# Verify it does exist and archived.
79-
arch_dag = ArchivedDataAccessGrant.objects.get(beneficiary__username='test_beneficiary', application__name='test_app')
81+
arch_dag = ArchivedDataAccessGrant.objects.get(
82+
beneficiary__username='test_beneficiary', application__name='test_app'
83+
)
8084

8185
# Verify expiration_date copied OK.
8286
self.assertEqual('2030-01-15 00:00:00+00:00', str(arch_dag.expiration_date))
@@ -190,6 +194,7 @@ def test_delete_authenticated_user_grant(self):
190194

191195
def setup_test_application_with_user(self, test_user, application_name='an app'):
192196
redirect_uri = 'http://localhost'
197+
code_challenge = 'sZrievZsrYqxdnu2NVD603EiYBM18CuzZpwB-pOSZjo'
193198
capability_a = self._create_capability('Capability A', [])
194199
capability_b = self._create_capability('Capability B', [])
195200
# create an application and add capabilities
@@ -207,6 +212,9 @@ def setup_test_application_with_user(self, test_user, application_name='an app')
207212
'client_id': application.client_id,
208213
'response_type': 'code',
209214
'redirect_uri': redirect_uri,
215+
'state': '0123456789abcdef',
216+
'code_challenge': code_challenge,
217+
'code_challenge_method': 'S256',
210218
}
211219
response = self.client.get('/v1/o/authorize', data=payload)
212220
# post the authorization form with only one scope selected
@@ -218,6 +226,8 @@ def setup_test_application_with_user(self, test_user, application_name='an app')
218226
'expires_in': 86400,
219227
'allow': True,
220228
'state': '0123456789abcdef',
229+
'code_challenge': code_challenge,
230+
'code_challenge_method': 'S256',
221231
}
222232
response = self.client.post(response['Location'], data=payload)
223233
self.assertEqual(response.status_code, 302)
@@ -229,6 +239,7 @@ def setup_test_application_with_user(self, test_user, application_name='an app')
229239
'code': authorization_code,
230240
'redirect_uri': redirect_uri,
231241
'client_id': application.client_id,
242+
'code_verifier': 'test123456789123456789123456789123456789123456789',
232243
}
233244
response = self.client.post(reverse('oauth2_provider:token'), data=token_request_data)
234245
fhir_id = json.loads(response.content)['patient']
@@ -237,6 +248,7 @@ def setup_test_application_with_user(self, test_user, application_name='an app')
237248

238249
def test_no_action_on_reapproval(self):
239250
redirect_uri = 'http://localhost'
251+
code_challenge = 'sZrievZsrYqxdnu2NVD603EiYBM18CuzZpwB-pOSZjo'
240252

241253
user = self._create_user('anna', '123456')
242254
application, fhir_id = self.setup_test_application_with_user(user)
@@ -248,6 +260,9 @@ def test_no_action_on_reapproval(self):
248260
'client_id': application.client_id,
249261
'response_type': 'code',
250262
'redirect_uri': redirect_uri,
263+
'state': '0123456789abcdef',
264+
'code_challenge': code_challenge,
265+
'code_challenge_method': 'S256',
251266
}
252267
response = self.client.get('/v1/o/authorize', data=payload)
253268
# post the authorization form with only one scope selected
@@ -259,6 +274,8 @@ def test_no_action_on_reapproval(self):
259274
'expires_in': 86400,
260275
'allow': True,
261276
'state': '0123456789abcdef',
277+
'code_challenge': code_challenge,
278+
'code_challenge_method': 'S256',
262279
}
263280
response = self.client.post(response['Location'], data=payload)
264281

@@ -271,6 +288,7 @@ def test_no_action_on_reapproval(self):
271288
'code': authorization_code,
272289
'redirect_uri': redirect_uri,
273290
'client_id': application.client_id,
291+
'code_verifier': 'test123456789123456789123456789123456789123456789',
274292
}
275293
response = self.client.post(reverse('oauth2_provider:token'), data=token_request_data)
276294
self.assertEqual(response.status_code, 200)
@@ -342,6 +360,7 @@ def test_permission_deny_on_app_or_org_disabled(self):
342360
to an application or applications under a user (organization)
343361
"""
344362
redirect_uri = 'http://localhost'
363+
code_challenge = 'sZrievZsrYqxdnu2NVD603EiYBM18CuzZpwB-pOSZjo'
345364
# create a user
346365
user = self._create_user('anna', '123456')
347366
capability_a = self._create_capability('Capability A', [])
@@ -363,6 +382,9 @@ def test_permission_deny_on_app_or_org_disabled(self):
363382
'client_id': application.client_id,
364383
'response_type': 'code',
365384
'redirect_uri': redirect_uri,
385+
'state': '0123456789abcdef',
386+
'code_challenge': code_challenge,
387+
'code_challenge_method': 'S256',
366388
}
367389
response = self.client.get('/v1/o/authorize', data=payload)
368390
payload = {
@@ -373,6 +395,8 @@ def test_permission_deny_on_app_or_org_disabled(self):
373395
'expires_in': 86400,
374396
'allow': True,
375397
'state': '0123456789abcdef',
398+
'code_challenge': code_challenge,
399+
'code_challenge_method': 'S256',
376400
}
377401

378402
response = self.client.post(response['Location'], data=payload)

0 commit comments

Comments
 (0)