https://www.ory.sh/ logo
Join Slack
Powered by
This message was deleted.
# general
m
This message was deleted.
p
Looking at the implementation for discord social sign-in, looks to me like I might need to fork kratos to add the capability. Am I correct in this assumption?
s
depends on the provider, but if they need specific code then yes
in general we accept basically any public oauth2 provider implementation
as a PR contribution
p
Hm, there are a lot of oauth2 providers out there, and I imagine most are only going to affect a few users. I’m happy to make a PR, but this specific provider (bungie.net) would only affect very few users. Would it not make sense to allow a “custom” configuration that allows you to set the correct oauth2 endpoints and such as configuration options?
In fact, the Configuration struct in provider_config.go allows for setting those endpoints with the comment
```// AuthURL is the authorize url, typically something like: https://example.org/oauth2/auth
// Should only be used when the OAuth2 / OpenID Connect server is not supporting OpenID Connect Discovery and when
// 
provider
is set to 
generic
.
AuthURL string `json:"auth_url"````
However, selecting the provider with generic set automatically sets the oidc provider
Copy code
case addProviderName("generic"):
   return NewProviderGenericOIDC(&p, reg), nil
s
do you think that will be enough? some providers need specific code & logic IIRC
p
the generic oidc provider is hardcoded to resolve the well-known openid configuration. which makes sense. However, this makes it incompatible with setting AuthURL and related via the config when using generic. Instead of adding another arbitrary oauth2 provider that may be used by two people, it may make sense to instead check, when creating the generic provider, if it’s meant to be an oidc provider or if AuthURL & related are already set and use those in that case?
I’m not sure if we’re extracting information from oidc that isn’t available via oauth2; apart from the well-known configuration
eg. if oidc defines a flow to retrieve authenticated user information and if oidc defines a format for said user information
yeah, this is all assuming that the third party oauth2 implementation conforms to the spec
eg in discords case, which doesn’t use oidc, just oauth2, we need to make a call to /user/@me to retrieve structured user information:
Copy code
dg, err := discordgo.New(fmt.Sprintf("Bearer %s", exchange.AccessToken))
	if err != nil {
		return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("%s", err))
	}

	user, err := dg.User("@me")
	if err != nil {
		return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("%s", err))
	}
on second thought, this might be more complicated to implement in a generic way - while the authentication flow is well-defined, the following user information retrieval is entirely up to the provider
s
yeah see, that's the problem... if you have a proposal, also to just cover "most common oauth2 providers" we could do that IMO but I guess they are too different in the end
p
If this were implemented, not at all saying that’s a reasonable idea at the current point in time, you’d have to • define a configuration to tell the provider how to authenticate (eg. via Bearer token, header, etc., most likely a Bearer token though) • define a configuration to tell the provider where to retrieve user data (url, method, although reasonably this most likely would be a GET call) • define a configuration to tell the provider how to extract data from the response, similar to how the discord provider extracts this from the user call. this would likely be the tipping point where things get too convoluted for a clean implementation.
although a wide range of oauth2 implementations could likely be covered by defaulting to • GET • Bearer token auth • JSON response data and allowing the user to configure • endpoint URL • data selectors for claims
s
For similar use cases with data selectors we use jsonnet, so that could be covered by that
p
I’m a little worried about the fact that this would very likely mean a change to the Configuration data structure, although that already holds fields for provider specific configuration like azure ids or apple key data
there’s also already a Mapper field for openid, specifically for the same use case of extracting user data to hydrate the identity
ah, that’s used differently internally though it seems. yeah, might need a separate field for that
4 Views
Open in Slack
PreviousNext
Powered by