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 support for Omniauth OpenID Connect login. #2953

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

molnarpe
Copy link

Enables OpenID Connect OmniAuth authentication method in GitLab. OmniAuth OIDC documentation

@kkimurak
Copy link
Contributor

Possibly duplicate work of #2927 (except the naming of variables, documentation and so on).
The important difference is that #2927 does not include the process to reflect the parameters in the configuration file, so it may not work (though I have not tried it).

Value for client_auth_method is 'query' in the example, but default value is 'basic' as described in [GitLab OmniAuth OIDC documentation, step 4.](https://docs.gitlab.com/ee/administration/auth/oidc.html).
@molnarpe
Copy link
Author

I found the following differences between my request (#2953) and #2927:

OAUTH_OIDC_RESPONSE_TYPE=${OAUTH_OIDC_RESPONSE_TYPE:-'code'}
OAUTH_OIDC_ISSUER=${OAUTH_OIDC_ISSUER:-}
OAUTH_OIDC_DISCOVERY=${OAUTH_OIDC_DISCOVERY:-true}
OAUTH_OIDC_CLIENT_AUTH_METHOD=${OAUTH_OIDC_CLIENT_AUTH_METHOD:-'basic'}
Copy link
Collaborator

@sachilles sachilles Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In https://docs.gitlab.com/ee/administration/auth/oidc.html the default value for client_auth_method (OAUTH_OIDC_CLIENT_AUTH_METHOD) is 'query'. Does tis matter?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In https://docs.gitlab.com/ee/administration/auth/oidc.html under step 4, supported values for client_auth_method are listed: basic, jwt_bearer, mtls, Any other value posts the client ID and secret in the request body. If not specified, this value defaults to basic. However query is written in the examples. Finally, I decided to use the basic value.

Copy link
Collaborator

@sachilles sachilles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@molnarpe Thanks for your PR and your helpful explanations. Hopefully my questions aren't to silly?

README.md Outdated
Comment on lines 735 to 749
| GitLab setting | environment variable |
|----------------|----------------------|
| `label` | `OAUTH_OIDC_LABEL` |
| `icon` | `OAUTH_OIDC_ICON` |
| `scope`| `OAUTH_OIDC_SCOPE` |
| `response_type` | `OAUTH_OIDC_RESPONSE_TYPE` |
| `issuer` | `OAUTH_OIDC_ISSUER` |
| `discovery` | `OAUTH_OIDC_DISCOVERY` |
| `client_auth_method` | `OAUTH_OIDC_CLIENT_AUTH_METHOD` |
| `uid_field` | `OAUTH_OIDC_UID_FIELD` |
| `send_scope_to_token_endpoint` | `OAUTH_OIDC_SEND_SCOPE_TO_TOKEN_EP` |
| `pkce` | `OAUTH_OIDC_PKCE` |
| `client_options.identifier` | `OAUTH_OIDC_CLIENT_ID` |
| `client_options.secret` | `OAUTH_OIDC_CLIENT_SECRET` |
| `client_options.redirect_uri` | `OAUTH_OIDC_REDIRECT_URI` |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you be so kind and add the default values defined in assets/runtime/functions? I guess, this will be helpful.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the README.md.

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

Successfully merging this pull request may close these issues.

None yet

3 participants