davido / gerrit-oauth-provider

OAuth2 authentication provider for Gerrit Code Review. Please upload changes for review to: https://gerrit-review.googlesource.com/#/admin/projects/plugins/oauth
Apache License 2.0
140 stars 84 forks source link

Consideration on generic OAuth? #134

Open objectkuan opened 4 years ago

objectkuan commented 4 years ago

I just realize that we have to apply specific oauth providers. In terms of unlisted providers or self-hosted oauth serives, is there any good solution? Or is there any consideration for not having custom oauth provider support? (Maybe there's obviously any but I haven't seen)

davido commented 4 years ago

KeyCloak (https://www.keycloak.org) can be used as self hosted OAuth provider and there are certainly other options.

I am not aware of any generic OAuth provider. LibreOffice adapted the plugin (actually forked it?) to use this plugin with LemonLDAP::NG, see this issue upstream for more details: [1].

[1] https://redmine.documentfoundation.org/issues/2852

vaceletm commented 4 years ago

And what about a generic Open ID Connect provider (like the one for Jenkins ? Would that be possible or there are things done by this plugin that are out of ODIC scope ?

davido commented 4 years ago

@vaceletm thanks for the pointer. It should be doable, of course. However, there is already a complication, see: https://github.com/jenkinsci/oic-auth-plugin/blob/master/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java#L138-L163

  if("auto".equals(automanualconfigure)) {
            // Get the well-known configuration from the specified URL
            this.wellKnownOpenIDConfigurationUrl = Util.fixEmpty(wellKnownOpenIDConfigurationUrl);
            URL url = new URL(wellKnownOpenIDConfigurationUrl);
            HttpRequest request = httpTransport.createRequestFactory().buildGetRequest(new GenericUrl(url));
            com.google.api.client.http.HttpResponse response = request.execute();

            WellKnownOpenIDConfigurationResponse config = OicSecurityRealm.JSON_FACTORY
                    .fromInputStream(response.getContent(), Charset.defaultCharset(),
                            WellKnownOpenIDConfigurationResponse.class);

            this.authorizationServerUrl = config.getAuthorizationEndpoint();
            this.tokenServerUrl = config.getTokenEndpoint();
            this.userInfoServerUrl = config.getUserinfoEndpoint();
            this.scopes = config.getScopesSupported() != null && !config.getScopesSupported().isEmpty() ? StringUtils.join(config.getScopesSupported(), " ") : "openid email";
            this.logoutFromOpenidProvider = logoutFromOpenidProvider != null;
            this.endSessionEndpoint = config.getEndSessionEndpoint();
        } else {
            this.authorizationServerUrl = authorizationServerUrl;
            this.tokenServerUrl = tokenServerUrl;
            this.userInfoServerUrl = userInfoServerUrl;
            this.scopes = Util.fixEmpty(scopes) == null ? "openid email" : scopes;
            this.wellKnownOpenIDConfigurationUrl = null;  // Remove the autoconfig URL
            this.logoutFromOpenidProvider = logoutFromOpenidProvider;
            this.endSessionEndpoint = endSessionEndpoint;
        }

For example, Google supports well-known openid-configuration for openid-connect standard: [1], and the response is here:

{
 "issuer": "https://accounts.google.com",
 "authorization_endpoint": "https://accounts.google.com/o/oauth2/v2/auth",
 "device_authorization_endpoint": "https://oauth2.googleapis.com/device/code",
 "token_endpoint": "https://oauth2.googleapis.com/token",
 "userinfo_endpoint": "https://openidconnect.googleapis.com/v1/userinfo",
 "revocation_endpoint": "https://oauth2.googleapis.com/revoke",
 "jwks_uri": "https://www.googleapis.com/oauth2/v3/certs",
 "response_types_supported": [
  "code",
  "token",
  "id_token",
  "code token",
  "code id_token",
  "token id_token",
  "code token id_token",
  "none"
 ],
[...]

But GitHub does not, see: [2].

[1] https://accounts.google.com/.well-known/openid-configuration [2] https://stackoverflow.com/a/52164558

vaceletm commented 4 years ago

Yes, it's a known issue that Github doesn't fully respect OIDC standards. Microsoft Azure doesn't respect it either BTW.

Of course having the possibility to add a generic OIDC server will not solve the issue with providers that doesn't implement the spec. And for those, there is no other way than the specific implementation anyway as, of course, each of them they don't follow the spec in different manners.

But at least it opens the possibility to all the providers that do respect the spec.

davido commented 4 years ago

Let's assume we implement a new generic Auth plugin: gerrit-oidc-plugin with support for OIDC standard.

The one obvious configuration of this plugin would be: well-known-openid-configuration-endpoint.

What would user do if they would like to use if or GitHub, Microsoft Azure and other ooidc providers, that don't implement the standard?

Should the new plugin gerrit-oidc-plugin provide a fallback solution, by allowing to provide the single endpoints, so to say to fake the missing well-known-openid-configuration-endpoint for those providers?

Another option would be to not support non standard providers at all, and defer to non generic oidcs-plugin: gerrit-oauth-provider.

vaceletm commented 4 years ago

I don't know if generic providers deserve a dedicated plugin (as it's done for Jenkins).

In my understanding it's more a provider in the way you already list them. This generic provider would be more or less the same code of the google one but with the end points configurable.

Of course if people use "generic" provider with github enpoints it won't work but since they will have the 'github' provider in the very same list it's unlikely that they will do that.

Another way to see the problem is that, a compliant providers, is "just" the generic one with harcoded urls for endpoints.

davido commented 4 years ago

Another way to see the problem is that, a compliant providers, is "just" the generic one with harcoded urls for endpoints.

What you are saying is: this plugin currenty supports say 10 different OAuth providers. It could support another 100. Or it could support a generic one, so that we could use the generic one to add all possible 100 providers that support OIDC standard.

IOW, the only configuration for generic OAuth provider in existing (or new plugin) would be: well-known-openid-configuration-endpoint and c'est tout. Right?

vaceletm commented 4 years ago

What you are saying is: this plugin currenty supports say 10 different OAuth providers. It could support another 100. Or it could support a generic one, so that we could use the generic one to add all possible 100 providers that support OIDC standard.

Correct. Specific providers are needed when:

IOW, the only configuration for generic OAuth provider in existing (or new plugin) would be: well-known-openid-configuration-endpoint and c'est tout. Right?

Presque. I'm not, by far, an expert of OIDC so I might be wrong, I'm telling from what I saw in various apps. The providers are expected to implement the well-known discovery end-point but "a lot" of generic oidc client connectors let admins configure the Authorization, Token and UserInfo end points manually. So, again in my understanding, there are 2 approaches:

Depending on the implementation (and/or willing from the maintainer...) you could have both.

mildis commented 4 years ago

Just to tell that Azure provides an endpoint for OpenID Connect configuration
https://login.microsoftonline.com/<tenant_id>/v2.0/.well-known/openid-configuration

Registered apps can get an ID token and Access token.
One can even add groups claim if oauth/<groupname> is ever implemented as an ACL.

leonard84 commented 3 years ago

Just my two cents, OpenID Connect Core can be used without OpenID Connect Discovery, which is perfectly fine by the spec. You just have to configure the endpoints/settings manually.

opsroller commented 2 years ago

Why was this ignored and never implemented? It's a very legitimate use case. Also Dex doesn't require that you use the path /dex/auth and Dex does support the /.well-known/openid-configuration

tobias-urdin commented 1 year ago

I was interested in knowing if I could get this working with authentik [1] but seems that there is no way to use a generic one as this issue states. It even provides a GitHub compatible endpoint but since URLs is hardcoded we'd need to compile the plugin ourselves.

[1] https://goauthentik.io/docs/providers/oauth2/

tobias-urdin commented 9 months ago

I ended up proposing a Authentik OAuth provider instead [1] and add support to that to link existing LDAP accounts for migrating.

[1] https://gerrit-review.googlesource.com/c/plugins/oauth/+/388034