Skip to content

Commit 41ef966

Browse files
committed
fix: SAML registration requests being rate limited
1 parent 66d1187 commit 41ef966

2 files changed

Lines changed: 82 additions & 2 deletions

File tree

openedx/core/djangoapps/user_authn/views/register.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ def post(self, request):
577577
HttpResponse: 403 operation not allowed
578578
"""
579579
should_be_rate_limited = getattr(request, 'limited', False)
580-
if should_be_rate_limited:
580+
if should_be_rate_limited and not pipeline.running(request):
581581
return JsonResponse({'error_code': 'forbidden-request'}, status=403)
582582

583583
if is_require_third_party_auth_enabled() and not pipeline.running(request):
@@ -875,7 +875,7 @@ def country_handler(self, request):
875875
}
876876

877877
@method_decorator(
878-
ratelimit(key=REAL_IP_KEY, rate=settings.REGISTRATION_VALIDATION_RATELIMIT, method='POST', block=True)
878+
ratelimit(key=REAL_IP_KEY, rate=settings.REGISTRATION_VALIDATION_RATELIMIT, method='POST', block=False)
879879
)
880880
def post(self, request):
881881
"""
@@ -897,6 +897,11 @@ def post(self, request):
897897
can get extra verification checks if entered along with others,
898898
like when the password may not equal the username.
899899
"""
900+
if getattr(request, 'limited', False) and not (
901+
pipeline.running(request) and pipeline.get(request)['backend'] == 'tpa-saml'
902+
):
903+
return Response(status=403)
904+
900905
field_key = request.data.get('form_field_key')
901906
validation_decisions = {}
902907

openedx/core/djangoapps/user_authn/views/tests/test_register.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2068,6 +2068,53 @@ def _assert_social_auth_provider_present(self, field_settings, backend):
20682068
"defaultValue": backend.name
20692069
})
20702070

2071+
@override_settings(
2072+
REGISTRATION_RATELIMIT='1/d',
2073+
CACHES={
2074+
'default': {
2075+
'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
2076+
'LOCATION': 'registration_ratelimit_tpa',
2077+
}
2078+
}
2079+
)
2080+
def test_rate_limiting_exempted_for_saml_pipeline(self):
2081+
"""
2082+
Confirm that a registration POST with an active SAML/TPA pipeline is not
2083+
blocked by IP-based rate limiting, even after the limit is exhausted.
2084+
"""
2085+
# Exhaust the rate limit (limit is 1/d, so one request uses the allowance)
2086+
self.client.post(self.url, {
2087+
"email": "first@example.com",
2088+
"name": self.NAME,
2089+
"username": "firstuser",
2090+
"password": self.PASSWORD,
2091+
"honor_code": "true",
2092+
})
2093+
# Without a pipeline, the next POST from the same IP is rate limited
2094+
response = self.client.post(self.url, {
2095+
"email": self.EMAIL,
2096+
"name": self.NAME,
2097+
"username": self.USERNAME,
2098+
"password": self.PASSWORD,
2099+
"honor_code": "true",
2100+
})
2101+
assert response.status_code == 403
2102+
assert response.json().get('error_code') == 'forbidden-request'
2103+
2104+
# With an active SAML/TPA pipeline, the same IP is not blocked by rate limiting
2105+
with simulate_running_pipeline(
2106+
'openedx.core.djangoapps.user_authn.views.register.pipeline', 'tpa-saml'
2107+
):
2108+
response = self.client.post(self.url, {
2109+
"email": self.EMAIL,
2110+
"name": self.NAME,
2111+
"username": self.USERNAME,
2112+
"password": self.PASSWORD,
2113+
"honor_code": "true",
2114+
})
2115+
self.assertHttpOK(response)
2116+
self.assertNotEqual(response.json().get('error_code'), 'forbidden-request')
2117+
20712118

20722119
@ddt.ddt
20732120
class RegistrationViewTestV2(RegistrationViewTestV1):
@@ -3028,6 +3075,34 @@ def test_rate_limiting_registration_view(self):
30283075
response = self.request_without_auth('post', self.path)
30293076
assert response.status_code == 403
30303077

3078+
@override_settings(
3079+
REGISTRATION_VALIDATION_RATELIMIT='1/d',
3080+
CACHES={
3081+
'default': {
3082+
'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
3083+
'LOCATION': 'validation_ratelimit_tpa',
3084+
}
3085+
}
3086+
)
3087+
def test_rate_limiting_exempted_for_saml_pipeline(self):
3088+
"""
3089+
Confirm that validation requests with an active SAML/TPA pipeline are not
3090+
blocked by IP-based rate limiting, even after the limit is exhausted.
3091+
"""
3092+
# Exhaust the rate limit (limit is 1/d, so one request uses the allowance)
3093+
self.request_without_auth('post', self.path)
3094+
# Without a pipeline, the next request from the same IP is rate limited
3095+
response = self.request_without_auth('post', self.path)
3096+
assert response.status_code == 403
3097+
3098+
# With an active SAML/TPA pipeline, the same IP is not blocked by rate limiting
3099+
with simulate_running_pipeline(
3100+
'openedx.core.djangoapps.user_authn.views.register.pipeline', 'tpa-saml'
3101+
):
3102+
response = self.request_without_auth('post', self.path)
3103+
self.assertHttpOK(response)
3104+
assert response.json().get('validation_decisions') == {}
3105+
30313106
def test_single_field_validation(self):
30323107
"""
30333108
Test that if `is_authn_mfe` is provided in request along with form_field_key, only

0 commit comments

Comments
 (0)