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

Add a provider for Discord #312

Open eltariel opened 4 years ago

eltariel commented 4 years ago

I've used the oidc provider to authenticate with Discord, but discord isn't properly OIDC compliant, in particular it's missing the required sub claim on the user endpoint, and the fields that are most useful for identifying a user in discord aren't exposed by default.

I've created a set of configs (here: eltariel/foundry-docker-nginx-vouch) which works reasonably well but it'd be nice to have something that works out-of-the-box.

bnfinet commented 4 years ago

@eltariel thanks for bringing the conversation here

Just to clarify, your Discord setup is able to capture the user info and pass them as headers in Nginx via X-Vouch-IdP-Claims-Email...

https://github.com/eltariel/foundry-docker-nginx-vouch/blob/master/nginx-configs/common/vouch-proxy.conf

Is that using custom javascript glue code?

bnfinet commented 4 years ago

Here's the Reddit thread regarding using Vouch Proxy + Discord to log into Foundry VTT where we began this conversation. I find that use case quite compelling.

https://www.reddit.com/r/FoundryVTT/comments/hw14rr/anyone_else_using_container/fz5jocz/?context=3

@eltariel thanks again for offering your configs!

eltariel commented 4 years ago

Just to clarify, your Discord setup is able to capture the user info and pass them as headers in Nginx via X-Vouch-IdP-Claims-Email...

Yes, that’s correct.

Some extra context: I’ve built a small asp.net app to manage authorisation based on these headers: eltariel/FoundryLanding. This uses a bunch of metadata inserted into the Foundry VTT data files to determine what a given discord user has access to for each foundry instance. This is referenced as the top level domain but also gets injected into each of the subdomains used for the fvtt instances so that it can override the login flow.

https://github.com/eltariel/foundry-docker-nginx-vouch/blob/master/nginx-configs/common/vouch-proxy.conf

Is that using custom javascript glue code?

Looking back at my config (I’m a beginner at nginx!) I believe that the configuration directives in this file are only required because I’ve accidentally double-proxied the auth helper in the fvtt domains. It’s not needed for the top-level domain to work. I could probably get rid of it altogether if I used the localhost URL in the proxy_pass directive instead of the external URL...

Hope this helps?

inklesspen commented 2 years ago

It would be nice to have a way to configure the username extraction out of the user info response via the config file, perhaps using a Go template.

By default, Discord does not return the email address. It can be asked to do so, with an additional scope, but most Discord users are accustomed to identifying users by their username and discriminator, which are usually formatted like so: MyUsername#1234. Discord guarantees that username+discriminator is unique across the platform, but each of these values can be changed at will. (All users are allowed to change the username, while "Nitro" users are also allowed to change their discriminator.)

Discord also provides an unchangeable "Snowflake" identifier as the id value: https://discord.com/developers/docs/topics/oauth2#get-current-authorization-information

Therefore, if I wanted to use the unchangeable id as username, I might specify a username format string of {{.user.id}}, but if I wanted a more readable username+discriminator combo, and were willing to accept that I might need to change the allowlist if someone changes username, I might specify {{.user.username}}#{{.user.discriminator}}.

I would imagine such a flexible id extraction system could be useful with a lot of different providers.

loganintech commented 11 months ago

Given the updates to the way Discord is formatting usernames going forward I think this PR maintains the patterns of other providers expectations for the discord platform: https://github.com/vouch/vouch-proxy/pull/528

loganintech commented 8 months ago

@bnfinet wonder if you've had a chance to review #528, or if we could start a discussion on things that may need to change for it to be merged. Discord has stabilized their approach and APIs for this.