-
Notifications
You must be signed in to change notification settings - Fork 326
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
Implements a Discord provider #528
base: master
Are you sure you want to change the base?
Implements a Discord provider #528
Conversation
Discord has just announced their intent to remove discriminators, so I'm going to adjust to use the ID until their API is updated to reflect the new constraint of unique usernames. Alternatively, regardless of what they do with usernames, IDs will remain unique. https://support.discord.com/hc/en-us/articles/12620128861463 |
139b62a
to
d6c22f9
Compare
Users can change their usernames (as frequently as twice a week), so the only reliable identifier is the 'id' field (which is a Twitter-style "snowflake") |
@inklesspen That's true, and we could use that, but it's important to note that now that usernames are unique they're an equally reliable source of truth in terms of unique identification. It seems easier to me to write a list of usernames the way you would a list of emails instead of a list of 18 digit numbers. In the FAQ you listed it says
Nobody could impersonate you and improperly gain auth unless you change your username, at which point you'd also immediately lose auth. Also I believe at least three other providers use Username or Email when given and vouch compares the |
Yes, and at which point an unauthorized person could gain auth. That's my concern here. |
ce7fb2f
to
7dc4825
Compare
…implementation to reflect that
4759ead
to
6bacd9e
Compare
I understand your concern. It's the same vulnerability in the other providers that behave the same way. I've updated the provider to be configurable such that if you set |
As was discussed in #312, Discord doesn't fulfill the proper spec for unique identification. They return a
User
object withUsername
,Discriminator
, andId
. Discord guarantees a user'sUsername#Discriminator
is unique across the app, so I've used that as the unique ID to be checked. This allows thewhiteList
array to have a list of usernames in the format that Discord users are used to, as opposed to their snowflake id.I also had to bump the docker images go build versions because a dependency requires go1.19 but the images were on go1.18.
An update to Discord's username specifications brings this more inline with other providers and removes discriminators. Now the username field is unique for each user provided they've taken the steps to select one.