Skip to content

Conversation

@JohnBostick
Copy link

@JohnBostick JohnBostick commented Jan 6, 2025

Resolves: #28

Changes:

This commit adds a check after the username/password check to ensure the submitted email matches the user's email.

The error code (022-2613 / 1105 in source code) is correct for any case where username/password/email are incorrect according to Nintendo.

This commit adds a check after the username/password check to ensure the submitted email matches the user's email. 

Fixes: PretendoNetwork#28
@JohnBostick JohnBostick changed the title Check email validity when linking existing PNID #28: Check email validity when linking existing PNID Jan 6, 2025
this didn't make it into the original commit somehow
@JohnBostick JohnBostick marked this pull request as draft January 6, 2025 20:29
middleware is used outside of initial link, so skip checking email if email header is not set
@JohnBostick JohnBostick marked this pull request as ready for review January 6, 2025 20:42
@CLAassistant
Copy link

CLAassistant commented Feb 5, 2025

CLA assistant check
All committers have signed the CLA.

@jonbarrow
Copy link
Member

So sorry for the late review, I didn't see this until now. I think this should be fine, @DaniElectra look good to you?

@JohnBostick
Copy link
Author

@jonbarrow I completely forgot about this. Maybe I should start doing more with pretendo...

return;
}

if (email != undefined && pnid.email.address !== email) {
Copy link
Member

Choose a reason for hiding this comment

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

This can be bypassed with a custom client if it doesn't add the x-nintendo-email header. I'd move this code to where the linking takes place

Copy link
Author

Choose a reason for hiding this comment

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

If my memory serves, there are many times where this route is called but an email is never specified (I believe in eShop login and Wii u logins). So the purpose of this is less to improve security (which was fine because a username/password combination is still required) and more to restore proper functionality where expected (when email+username login is used).

Copy link
Member

Choose a reason for hiding this comment

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

I'm fairly certain Dani means to move it from middleware to the route handler itself. Which, in hindsight, I agree with

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.

Linking existing PNID - Server doesn't check email validity

4 participants