Skip to content

made attemptCount map to track # of attempts#2020

Merged
codebyemily merged 6 commits intoSCE-Development:devfrom
codebyemily:rate-limiting
Feb 6, 2026
Merged

made attemptCount map to track # of attempts#2020
codebyemily merged 6 commits intoSCE-Development:devfrom
codebyemily:rate-limiting

Conversation

@codebyemily
Copy link
Contributor

issue #2016

Copy link
Contributor

@adarshm11 adarshm11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should also make it so that after every failed code, it returns to the frontend how many attempts are left, and the frontend modal says "wrong code - X attempts remaining" or something like that

Comment on lines 16 to 20
const data = await res.json();
status.responseData = res.status;
status.error = !res.ok;
status.remainingAttempts = data.remainingAttempts;
status.message = data.error;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const data = await res.json();
status.responseData = res.status;
status.error = !res.ok;
status.remainingAttempts = data.remainingAttempts;
status.message = data.error;
if (res.status == TOO_MANY_ATTEMPTS) {
const data = await res.json();
status.responseData = data;
status.error = false;
} else {
status.error = !res.ok;
}

TOO_MANY_ATTEMPTS might not exist as an enum in the frontend container, if so we can just hardcode an enum within this function for now

const attempts = attemptCount.get(userId) ?? 0;

if (attempts >= MAX_ATTEMPTS){
logger.error('User has made too many verification attempts.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.error('User has made too many verification attempts.');
logger.error(`User ${userId} has made too many verification attempts.`);

logger.error('Error verifying payment for user:', userId);
return res.status(NOT_FOUND).send('Error verifying payment.');
return res.status(NOT_FOUND).json({
error: 'Error verifying payment.',
Copy link
Contributor

@adarshm11 adarshm11 Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

including "error" in the json body isn't necessary since it doesn't give us any insight into what happened and it isn't used on the frontend either

user.token,
confirmationCode,
);
if (apiResponse.remainingAttempts != undefined){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (apiResponse.remainingAttempts != undefined){
if (apiResponse.remainingAttempts) {

Copy link
Contributor Author

@codebyemily codebyemily Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think if remainingAttempts is sent and it turns out there are 0 attempts left it will return false, but I want to check if remainingAttempts was sent over

so if it had 0 attempts left, that would never be displayed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay in that case it's fine, let's use !== instead of != though

const attempts = attemptCount.get(userId) ?? 0;

if (attempts >= MAX_ATTEMPTS){
logger.error(`User ${userId} has made too many verification attempts.`); return res.status(TOO_MANY_REQUESTS).json({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.error(`User ${userId} has made too many verification attempts.`); return res.status(TOO_MANY_REQUESTS).json({
logger.error(`User ${userId} has made too many verification attempts.`);
return res.status(TOO_MANY_REQUESTS).json({

Copy link
Contributor

@adarshm11 adarshm11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

address the one comment then get one more approver

user.token,
confirmationCode,
);
if (apiResponse.remainingAttempts != undefined){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay in that case it's fine, let's use !== instead of != though

Copy link
Contributor

@charred-70 charred-70 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm so tuff vro

@codebyemily codebyemily merged commit 53711c8 into SCE-Development:dev Feb 6, 2026
4 checks passed
@codebyemily codebyemily deleted the rate-limiting branch February 6, 2026 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants