From 3fa3f257ba0055700750ae6a3c81b25bc71222db Mon Sep 17 00:00:00 2001 From: Zainab Amir Date: Thu, 25 Mar 2021 15:43:59 +0500 Subject: [PATCH] Handle ratelimit error for registration endpoint (#212) As part of adding ratelimit to registration we need to make sure MFE works and shows error messages as expected. VAN-302 --- src/register/RegistrationFailure.jsx | 16 +++++++-- src/register/data/constants.js | 4 +++ src/register/data/sagas.js | 8 +++-- src/register/data/tests/sagas.test.js | 37 +++++++++++++++++--- src/register/messages.jsx | 5 +++ src/register/tests/RegistrationPage.test.jsx | 15 +++++++- 6 files changed, 75 insertions(+), 10 deletions(-) diff --git a/src/register/RegistrationFailure.jsx b/src/register/RegistrationFailure.jsx index 59b22a1c..13eb6b02 100644 --- a/src/register/RegistrationFailure.jsx +++ b/src/register/RegistrationFailure.jsx @@ -1,10 +1,12 @@ import React, { useEffect } from 'react'; import PropTypes from 'prop-types'; + import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Alert } from '@edx/paragon'; -import { INTERNAL_SERVER_ERROR } from '../login/data/constants'; -import { DEFAULT_STATE, PENDING_STATE } from '../data/constants'; + +import { FORBIDDEN_REQUEST, INTERNAL_SERVER_ERROR } from './data/constants'; import messages from './messages'; +import { DEFAULT_STATE, PENDING_STATE } from '../data/constants'; const RegistrationFailureMessage = (props) => { const errorMessage = props.errors; @@ -27,7 +29,15 @@ const RegistrationFailureMessage = (props) => { ); userErrors.push(serverError); break; - + case FORBIDDEN_REQUEST: + userErrors.push( + ( +
  • + {props.intl.formatMessage(messages['register.rate.limit.reached.message'])} +
    • + ), + ); + break; default: Object.keys(errorMessage).forEach((key) => { if (key !== 'error_code') { diff --git a/src/register/data/constants.js b/src/register/data/constants.js index 33c05fa6..f6d923c7 100644 --- a/src/register/data/constants.js +++ b/src/register/data/constants.js @@ -1,3 +1,7 @@ +// Registration Error Codes +export const INTERNAL_SERVER_ERROR = 'internal-server-error'; +export const FORBIDDEN_REQUEST = 'forbidden-request'; + export const YEAR_OF_BIRTH_OPTIONS = (() => { const currentYear = new Date().getFullYear(); const years = []; diff --git a/src/register/data/sagas.js b/src/register/data/sagas.js index 624898d1..271e4c3a 100644 --- a/src/register/data/sagas.js +++ b/src/register/data/sagas.js @@ -1,5 +1,6 @@ import { call, put, takeEvery } from 'redux-saga/effects'; +import { camelCaseObject } from '@edx/frontend-platform'; import { logError, logInfo } from '@edx/frontend-platform/logging'; // Actions @@ -13,10 +14,10 @@ import { fetchRealtimeValidationsSuccess, fetchRealtimeValidationsFailure, } from './actions'; +import { INTERNAL_SERVER_ERROR } from './constants'; // Services import { getFieldsValidations, registerRequest } from './service'; -import { INTERNAL_SERVER_ERROR } from '../../login/data/constants'; export function* handleNewUserRegistration(action) { try { @@ -29,10 +30,13 @@ export function* handleNewUserRegistration(action) { success, )); } catch (e) { - const statusCodes = [400, 409, 403]; + const statusCodes = [400, 409]; if (e.response && statusCodes.includes(e.response.status)) { yield put(registerNewUserFailure(e.response.data)); logInfo(e); + } else if (e.response.status === 403) { + yield put(registerNewUserFailure(camelCaseObject(e.response.data))); + logInfo(e); } else { yield put(registerNewUserFailure({ errorCode: INTERNAL_SERVER_ERROR })); logError(e); diff --git a/src/register/data/tests/sagas.test.js b/src/register/data/tests/sagas.test.js index 2b5cd2ab..541923db 100644 --- a/src/register/data/tests/sagas.test.js +++ b/src/register/data/tests/sagas.test.js @@ -8,7 +8,7 @@ import { } from '../sagas'; import * as api from '../service'; import initializeMockLogging from '../../../setupTest'; -import { INTERNAL_SERVER_ERROR } from '../../../data/constants'; +import { FORBIDDEN_REQUEST, INTERNAL_SERVER_ERROR } from '../constants'; const { loggingService } = initializeMockLogging(); @@ -176,7 +176,7 @@ describe('handleNewUserRegistration', () => { }); it('should call service and dispatch error action', async () => { - const loginErrorResponse = { + const registerErrorResponse = { response: { status: 400, data: { @@ -185,7 +185,7 @@ describe('handleNewUserRegistration', () => { }, }; const registerRequest = jest.spyOn(api, 'registerRequest') - .mockImplementation(() => Promise.reject(loginErrorResponse)); + .mockImplementation(() => Promise.reject(registerErrorResponse)); const dispatched = []; await runSaga( @@ -198,7 +198,36 @@ describe('handleNewUserRegistration', () => { expect(loggingService.logInfo).toHaveBeenCalled(); expect(dispatched).toEqual([ actions.registerNewUserBegin(), - actions.registerNewUserFailure(loginErrorResponse.response.data), + actions.registerNewUserFailure(registerErrorResponse.response.data), + ]); + registerRequest.mockClear(); + }); + + it('should handle rate limit error code', async () => { + const registerErrorResponse = { + response: { + status: 403, + data: { + errorCode: FORBIDDEN_REQUEST, + }, + }, + }; + + const registerRequest = jest.spyOn(api, 'registerRequest') + .mockImplementation(() => Promise.reject(registerErrorResponse)); + + const dispatched = []; + await runSaga( + { dispatch: (action) => dispatched.push(action) }, + handleNewUserRegistration, + params, + ); + + expect(registerRequest).toHaveBeenCalledTimes(1); + expect(loggingService.logInfo).toHaveBeenCalled(); + expect(dispatched).toEqual([ + actions.registerNewUserBegin(), + actions.registerNewUserFailure(registerErrorResponse.response.data), ]); registerRequest.mockClear(); }); diff --git a/src/register/messages.jsx b/src/register/messages.jsx index 17194d2c..25156129 100644 --- a/src/register/messages.jsx +++ b/src/register/messages.jsx @@ -51,6 +51,11 @@ const messages = defineMessages({ defaultMessage: 'Email (required)', description: 'Label that appears above email field on register page', }, + 'register.rate.limit.reached.message': { + id: 'register.rate.limit.reached.message', + defaultMessage: 'Too many failed registration attempts. Try again later.', + description: 'Error message that appears when an anonymous user has made too many failed registration attempts', + }, 'email.validation.message': { id: 'email.validation.message', defaultMessage: 'Please enter your email.', diff --git a/src/register/tests/RegistrationPage.test.jsx b/src/register/tests/RegistrationPage.test.jsx index 64bb94a7..89f98099 100644 --- a/src/register/tests/RegistrationPage.test.jsx +++ b/src/register/tests/RegistrationPage.test.jsx @@ -12,8 +12,8 @@ import RegistrationPage from '../RegistrationPage'; import { RenderInstitutionButton } from '../../common-components'; import RegistrationFailureMessage from '../RegistrationFailure'; import { COMPLETE_STATE, PENDING_STATE } from '../../data/constants'; -import { INTERNAL_SERVER_ERROR } from '../../login/data/constants'; import { fetchRealtimeValidations, registerNewUser } from '../data/actions'; +import { FORBIDDEN_REQUEST, INTERNAL_SERVER_ERROR } from '../data/constants'; jest.mock('@edx/frontend-platform/analytics'); @@ -434,6 +434,19 @@ describe('RegistrationPageTests', () => { expect(tree.toJSON()).toMatchSnapshot(); }); + it('should match registration api rate limit error message', () => { + props = { + errors: { + errorCode: FORBIDDEN_REQUEST, + }, + }; + + const registrationPage = mount(reduxWrapper()); + expect(registrationPage.find('div.alert-heading').length).toEqual(1); + const expectedMessage = 'We couldn\'t create your account.Too many failed registration attempts. Try again later.'; + expect(registrationPage.find('div.alert').first().text()).toEqual(expectedMessage); + }); + it('should match internal server error message', () => { props = { errors: {