Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions openedx/core/djangoapps/user_authn/views/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ def post(self, request):
HttpResponse: 403 operation not allowed
"""
should_be_rate_limited = getattr(request, 'limited', False)
if should_be_rate_limited:
if should_be_rate_limited and not pipeline.running(request):
return JsonResponse({'error_code': 'forbidden-request'}, status=403)

if is_require_third_party_auth_enabled() and not pipeline.running(request):
Expand Down Expand Up @@ -875,7 +875,7 @@ def country_handler(self, request):
}

@method_decorator(
ratelimit(key=REAL_IP_KEY, rate=settings.REGISTRATION_VALIDATION_RATELIMIT, method='POST', block=True)
ratelimit(key=REAL_IP_KEY, rate=settings.REGISTRATION_VALIDATION_RATELIMIT, method='POST', block=False)
)
def post(self, request):
"""
Expand All @@ -897,6 +897,9 @@ def post(self, request):
can get extra verification checks if entered along with others,
like when the password may not equal the username.
"""
if getattr(request, 'limited', False) and not pipeline.running(request):
return Response(status=403)

field_key = request.data.get('form_field_key')
validation_decisions = {}

Expand Down
75 changes: 75 additions & 0 deletions openedx/core/djangoapps/user_authn/views/tests/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -2068,6 +2068,53 @@ def _assert_social_auth_provider_present(self, field_settings, backend):
"defaultValue": backend.name
})

@override_settings(
REGISTRATION_RATELIMIT='1/d',
CACHES={
'default': {
'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
'LOCATION': 'registration_ratelimit_tpa',
}
}
)
def test_rate_limiting_exempted_for_saml_pipeline(self):
"""
Confirm that a registration POST with an active SAML/TPA pipeline is not
blocked by IP-based rate limiting, even after the limit is exhausted.
"""
# Exhaust the rate limit (limit is 1/d, so one request uses the allowance)
self.client.post(self.url, {
"email": "first@example.com",
"name": self.NAME,
"username": "firstuser",
"password": self.PASSWORD,
"honor_code": "true",
})
# Without a pipeline, the next POST from the same IP is rate limited
response = self.client.post(self.url, {
"email": self.EMAIL,
"name": self.NAME,
"username": self.USERNAME,
"password": self.PASSWORD,
"honor_code": "true",
})
assert response.status_code == 403
assert response.json().get('error_code') == 'forbidden-request'

# With an active SAML/TPA pipeline, the same IP is not blocked by rate limiting
with simulate_running_pipeline(
'openedx.core.djangoapps.user_authn.views.register.pipeline', 'tpa-saml'
):
response = self.client.post(self.url, {
"email": self.EMAIL,
"name": self.NAME,
"username": self.USERNAME,
"password": self.PASSWORD,
"honor_code": "true",
})
self.assertHttpOK(response)
self.assertNotEqual(response.json().get('error_code'), 'forbidden-request')


@ddt.ddt
class RegistrationViewTestV2(RegistrationViewTestV1):
Expand Down Expand Up @@ -3028,6 +3075,34 @@ def test_rate_limiting_registration_view(self):
response = self.request_without_auth('post', self.path)
assert response.status_code == 403

@override_settings(
REGISTRATION_VALIDATION_RATELIMIT='1/d',
CACHES={
'default': {
'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
'LOCATION': 'validation_ratelimit_tpa',
}
}
)
def test_rate_limiting_exempted_for_saml_pipeline(self):
"""
Confirm that validation requests with an active SAML/TPA pipeline are not
blocked by IP-based rate limiting, even after the limit is exhausted.
"""
# Exhaust the rate limit (limit is 1/d, so one request uses the allowance)
self.request_without_auth('post', self.path)
# Without a pipeline, the next request from the same IP is rate limited
response = self.request_without_auth('post', self.path)
assert response.status_code == 403

# With an active SAML/TPA pipeline, the same IP is not blocked by rate limiting
with simulate_running_pipeline(
'openedx.core.djangoapps.user_authn.views.register.pipeline', 'saml-idp'
):
response = self.request_without_auth('post', self.path)
self.assertHttpOK(response)
assert response.json().get('validation_decisions') == {}

def test_single_field_validation(self):
"""
Test that if `is_authn_mfe` is provided in request along with form_field_key, only
Expand Down
Loading