vouch / vouch-proxy

an SSO and OAuth / OIDC login solution for Nginx using the auth_request module
MIT License
2.92k stars 327 forks source link

Implements a Discord provider #528

Open loganintech opened 1 year ago

loganintech commented 1 year ago

As was discussed in https://github.com/vouch/vouch-proxy/issues/312, Discord doesn't fulfill the proper spec for unique identification. They return a User object with Username, Discriminator, and Id. Discord guarantees a user's Username#Discriminator is unique across the app, so I've used that as the unique ID to be checked. This allows the whiteList 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.

loganintech commented 1 year ago

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

inklesspen commented 8 months ago

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")

loganintech commented 8 months ago

@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

Usernames are unique to each user and no two users can share the same username

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 User struct's Username prop for verification it makes more sense to me to use the unique usernames.

inklesspen commented 8 months ago

Nobody could impersonate you and improperly gain auth unless you change your username, at which point you'd also immediately lose auth.

Yes, and at which point an unauthorized person could gain auth. That's my concern here.

loganintech commented 7 months ago

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 discord_use_ids to true the provider will check the IDs instead of the Username.