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

Add a provider for Discord #312

Open
eltariel opened this issue Aug 22, 2020 · 6 comments
Open

Add a provider for Discord #312

eltariel opened this issue Aug 22, 2020 · 6 comments

Comments

@eltariel
Copy link

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
Copy link
Member

bnfinet commented Aug 24, 2020

@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
Copy link
Member

bnfinet commented Aug 24, 2020

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
Copy link
Author

eltariel commented Aug 25, 2020

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
Copy link

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
Copy link

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: #528

@loganintech
Copy link

@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.

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

No branches or pull requests

4 participants