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

Implement User Logins #766

Open
otobot1 opened this issue Apr 15, 2024 · 9 comments · May be fixed by #861
Open

Implement User Logins #766

otobot1 opened this issue Apr 15, 2024 · 9 comments · May be fixed by #861
Labels
priority: very high status: help wanted This issue is low-priority for maintainers. Any user is welcome to submit a pull request type: enhancement New feature or request
Milestone

Comments

@otobot1
Copy link
Member

otobot1 commented Apr 15, 2024

Is your feature request related to a problem? Please describe.
Users need to be able to login with Discord authentication to enable features such as #532. We also need a way for users who submitted ratings to the original mods list to claim their existing user and link it to their Discord ID.

Describe the solution you'd like

  1. Implement login functionality with NextAuth - much of the required server-side code is already complete.
  2. Users who submitted ratings to the original mods list via the Google forms had new CML users created for them when the old database was imported. Implement a way for these users to claim these accounts and enable login via Discord authentication.
    i. Ideally this would be an automated process that is triggered when they first try to login to the new website, but this may not be feasible so a 100% manual solution is good enough for now.
    ii. Our only identifying information for these users is their discord username and discriminator from when they submitted their last rating - nearly all users will have had their discriminators change to 0 since then as part of Discord's username changes.
    iii. If an automated solution can't be devised, instruct these users (via an information tooltip/popover somewhere on the login page and also on the FAQ page) to create a new account and then message @otobot1 on Discord with their old information. Create a basic admin page where @otobot1 (and any other admins) can manually link the old and new users.
    iv. Either way, the linking will probably require a custom provider but probably won't need a custom adapter. See this file for an example of a custom provider.
  3. Change the My Account button in the site footer to say "Login" when not logged in and "My Account" when logged in. This button and the Settings button should move to the top-right of the page, but this can be left as a low-priority follow-up issue.

Describe alternatives you've considered
N/A. This is a required feature.

Additional context
Co-supersedes #532 along with #767.

Our cookie usage will need to be explained in our Privacy Policy #534.

@otobot1 otobot1 added type: enhancement New feature or request status: help wanted This issue is low-priority for maintainers. Any user is welcome to submit a pull request priority: very high labels Apr 15, 2024
@otobot1 otobot1 added this to the v0.2.0 milestone Apr 15, 2024
This was referenced Apr 15, 2024
@ShouvikGhosh2048
Copy link
Collaborator

ShouvikGhosh2048 commented Jul 28, 2024

@otobot1 Do you have the Discord user IDs? If not, I don't think there is a way to match the old username+discriminator with their new username. (Discord does still show the last username before the Discord username change, but if the user made a username change in between their last submission and the Discord username change, I don't think we identify them.)

@ShouvikGhosh2048
Copy link
Collaborator

ShouvikGhosh2048 commented Jul 28, 2024

For the update, I guess we'll need to just update the submittedBy field in Rating?
So we just need a page which takes user IDs A and B, and just transfers all ratings by A to B. (only allowing admins).

@otobot1
Copy link
Member Author

otobot1 commented Jul 28, 2024

I don't have the Discord IDs unfortunately. What I have is a list of traditional Discord tags, like otobot1#1564. It's currently in a spreadsheet, but I can put it in a table in the database or a text file on the server. I was wondering if we could use the Originally Known As info that you can see on some people's Discord profiles (see image). This would be the last traditional Discord tag they had before getting their new display name, which would allow us to automatically approve anyone where that old tag matches one in our list. It wouldn't get everyone, but it would get some (hopefully a lot). I haven't looked into Discord's API in quite some time, so I'm not sure if this is doable.

If this winds up being a big pain, we can leave it for a v0.2.1 update.

@otobot1
Copy link
Member Author

otobot1 commented Jul 28, 2024

For the update, I guess we'll need to just update the submittedBy field in Rating?
So we just need a page which takes user IDs A and B, and just transfers all ratings by A to B. (only allowing admins).

Yep

@ShouvikGhosh2048
Copy link
Collaborator

ShouvikGhosh2048 commented Jul 28, 2024

I don't have the Discord IDs unfortunately. What I have is a list of traditional Discord tags, like otobot1#1564. It's currently in a spreadsheet, but I can put it in a table in the database or a text file on the server. I was wondering if we could use the Originally Known As info that you can see on some people's Discord profiles (see image). This would be the last traditional Discord tag they had before getting their new display name, which would allow us to automatically approve anyone where that old tag matches one in our list. It wouldn't get everyone, but it would get some (hopefully a lot). I haven't looked into Discord's API in quite some time, so I'm not sure if this is doable.

If this winds up being a big pain, we can leave it for a v0.2.1 update.

How does the site work right now if those users aren't in the database? Ratings need a user (not null) according the prisma schema.

@ShouvikGhosh2048
Copy link
Collaborator

ShouvikGhosh2048 commented Jul 28, 2024

I was setting up Discord sign in and got a Prisma error - the model User doesn't have email & emailVerified fields. Should I add those, or do you not want to store that data?

@otobot1
Copy link
Member Author

otobot1 commented Jul 28, 2024

I don't have the Discord IDs unfortunately. What I have is a list of traditional Discord tags, like otobot1#1564. It's currently in a spreadsheet, but I can put it in a table in the database or a text file on the server. I was wondering if we could use the Originally Known As info that you can see on some people's Discord profiles (see image). This would be the last traditional Discord tag they had before getting their new display name, which would allow us to automatically approve anyone where that old tag matches one in our list. It wouldn't get everyone, but it would get some (hopefully a lot). I haven't looked into Discord's API in quite some time, so I'm not sure if this is doable.
If this winds up being a big pain, we can leave it for a v0.2.1 update.

How does the site work right now if those users aren't in the database? Ratings need a user (not null) according the prisma schema.

The users are currently in the database in the User table, but once v0.2.0 goes live new people will be able to create users. So, we would need to store the list of "legacy users" in a new table or a text file or something to support any automatic linking of new and legacy users.

@otobot1
Copy link
Member Author

otobot1 commented Jul 28, 2024

I was setting up Discord sign in and got a Prisma error - the model User doesn't have email & emailVerified fields. Should I add those, or do you not want to store that data?

I'd rather not store it, as we don't need it for our purposes. It's been nearly a year since I looked at the Auth stuff, but I did create a custom Prisma Adapter and Discord Provider that should take care of that error. I'm pretty sure I got it all working. If you're writing server-side code, import the session using the getServerAuthSession wrapper function. If you're writing client-side code, call useSession and the SessionProvider in /pages/_app.tsx will provide a session if the user is logged in, or null if not. Either way, the session object you get is defined in the session callback in the authOptions object.

@otobot1
Copy link
Member Author

otobot1 commented Aug 24, 2024

So I had a bit more of a thought about how to handle unlinked legacy users, and I also noticed that the accountStatus column in the User table has a possible value of Unlinked. My idea:

  • Unlinked users are only tracked by their accountStatus.
  • Logged in users are able to claim any unlinked user.
    • All users see a blurb on the home page telling them that, if they submitted ratings via the google forms, they can claim their old user.
      • When the user clicks the link in the blurb, they go to the claim-user page.
    • The claim-user page lets the user search through all unlinked users and claim one.
      • When the claim is submitted, an entry is created in a new UserClaim database table.
        • Each row in the table should link to the requesting user and the claimed user.
        • As a single user may claim or be claimed by multiple users, also have a numerical autoincrementing id column
      • After calling the API and creating the claim entry in the database, the claim-user page updates to show:
        • Their claim has been submitted successfully.
        • They need to contact one of the verifiers on Discord to verify their claim.
          • Include the same permanent link to the Discord that is in the website footer.
          • Direct them to post a screenshot of their claim in the #claim-user channel (I will create this when I have a chance).
        • The claim id.
        • The claimed user's id, discordUsername, and discordDiscriminator.
          • This can be a followup issue, but we should change the User table ids to be autoincrementing numbers instead of UUIDs so that they are more human-readable.
            • This will require changes to the db-import repo, and a re-import of the old data to the production db.
            • I believe NextAuth requires the User id to be a string, but we should be able to enforce it being a numerical string by removing the default value from the Prisma schema and always providing a default value ourselves in the Prisma adapter.
  • Logged in users with the Admin or Super_Admin permission are able to access a claim verification page.
    • Ideally this page will show all claims by default and allow searching by claim id.
    • When the claim is verified:
      • All foreign keys in the database pointing to the claimed user are changed to point to the requesting user.
      • A info level log entry is printed to file using the serverLogger specifying:
        • The requesting user's id.
        • The claimed user's id, discordUsername, and discordDiscriminator.
        • The verifying user's id.
        • The list of foreign keys updated.
      • Finally, the claim and claimed user are deleted from the database.

Please let me know if you have any questions/comments on this!

@ShouvikGhosh2048 ShouvikGhosh2048 linked a pull request Sep 1, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: very high status: help wanted This issue is low-priority for maintainers. Any user is welcome to submit a pull request type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants