Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pretendo does not "normalize" nnid names properly. #88

Open
GabIsAwesome opened this issue Apr 15, 2024 · 2 comments
Open

Pretendo does not "normalize" nnid names properly. #88

GabIsAwesome opened this issue Apr 15, 2024 · 2 comments

Comments

@GabIsAwesome
Copy link

By using the NNIDLT website (https://nnidlt.murilo.eu.org), which uses Nintendo Network's account server endpoint "/v1/api/admin/mapped_ids?input_type=user_id&output_type=pid&input=[NNID]", if you put a NNID that's called "Barack", and then type it as "bar.ack" or "bar-ack", it will still work because it normalizes NNID names, even if "bar.ack" or "bar-ack" doesn't actually exist.

Pretendo's account server doesn't do that. Instead, it returns that the NNID does not exist if you use the examples above on a PNID that exists on the server.

@ariankordi
Copy link

Some notes about this (apologies if some of this is obvious):

  • Seems to apply to mapped_ids, /v1/api/people/(nnid), and checking if the NNID already exists when creating an account. TBD if it applies to linking process or others (if there are any).
  • Does not apply to login, though usernames are still case insensitive there.

Meaning that you could probably get away with implementing it in mapped_ids and doesUserExist.

  • Characters it excludes are -_.

    • While there are already checks involving punctuation characters in the signup process, mapped_ids does not care about any of these, you can add hundreds of dots after an NNID in mapped_ids and it will return you the same one.
  • Fixing this behavior will imply having to go through a process of finding out which NNIDs will need to be renamed because of this which, yeah... will suck.

I can't imagine there are many of such cases though, and after all, Nintendo changed NNIDs on the backend all the time so this shouldn't be too destructive? You'd have to make a query for this in the prod database and if it's small enough then just change the IDs and email them about it or something.

@ariankordi
Copy link

ariankordi commented Apr 18, 2024

To go forth with this, you would have to:
  • Make a column for the normalized version of the username to compare against, similar to usernameLower

And optionally:

  • Find out how to search "username" with a case insensitive index instead, then remove usernameLower, as to not add too much bloat to the model. I think this would insinuate making the entire PNID model case insensitive which, isn't ideal.

I'm going to spoiler the first version of this comment below because uhhhhh I did some research and found out that using regexes in Mongo is SUPREMELY inefficient due to the fact that it bypasses indexes! And that would justify the usage of the usernameLower field.

I just realized that the way the server searches for usernames, is a bit different from what I expected... * When you create an account, a separate `usernameLower` column is created alongside your actual username, which represents it in lowercase. When you run mapped_ids, the input is compared 1:1 directly against this.

I would've expected that it does the Mongo equivalent of something like LOWER(username) in SQL, i.e., let the database query for the lowercased version of that column (or just use ILIKE or something) in real-time, but, no.

This may complicate things a bit...

  • ... So, do we just put the normalized username in a column, perhaps the same column?

I could see this approach being ever-so-slightly faster, especially given that mapped_ids is something that's constantly requested. But it's not the only way to do this, you can totally just query a regex in Mongo. I have a local instance doing that which works just fine to solve this issue.

  • OR, do we just query the username case insensitively with a regex that normalizes it, which would mean that usernameLower would no longer be required?

Both of these would require migrations to either change or remove columns, not including the process necessary to fix usernames that would collide because of this.

I'm not sure which approach should be taken!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

2 participants