Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Add config option for always using "userinfo endpoint" for OIDC (#7658)
Browse files Browse the repository at this point in the history
This allows for connecting to certain IdPs, e.g. GitLab.
  • Loading branch information
BBBSnowball authored Oct 1, 2020
1 parent 0b68577 commit 05ee048
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 15 deletions.
1 change: 1 addition & 0 deletions changelog.d/7658.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add a configuration option for always using the "userinfo endpoint" for OpenID Connect. This fixes support for some identity providers, e.g. GitLab. Contributed by Benjamin Koch.
41 changes: 32 additions & 9 deletions docs/openid.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,13 +238,36 @@ Synapse config:

```yaml
oidc_config:
enabled: true
issuer: "https://id.twitch.tv/oauth2/"
client_id: "your-client-id" # TO BE FILLED
client_secret: "your-client-secret" # TO BE FILLED
client_auth_method: "client_secret_post"
user_mapping_provider:
config:
localpart_template: '{{ user.preferred_username }}'
display_name_template: '{{ user.name }}'
enabled: true
issuer: "https://id.twitch.tv/oauth2/"
client_id: "your-client-id" # TO BE FILLED
client_secret: "your-client-secret" # TO BE FILLED
client_auth_method: "client_secret_post"
user_mapping_provider:
config:
localpart_template: "{{ user.preferred_username }}"
display_name_template: "{{ user.name }}"
```

### GitLab

1. Create a [new application](https://gitlab.com/profile/applications).
2. Add the `read_user` and `openid` scopes.
3. Add this Callback URL: `[synapse public baseurl]/_synapse/oidc/callback`

Synapse config:

```yaml
oidc_config:
enabled: true
issuer: "https://gitlab.com/"
client_id: "your-client-id" # TO BE FILLED
client_secret: "your-client-secret" # TO BE FILLED
client_auth_method: "client_secret_post"
scopes: ["openid", "read_user"]
user_profile_method: "userinfo_endpoint"
user_mapping_provider:
config:
localpart_template: '{{ user.nickname }}'
display_name_template: '{{ user.name }}'
```
8 changes: 8 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1714,6 +1714,14 @@ oidc_config:
#
#skip_verification: true

# Whether to fetch the user profile from the userinfo endpoint. Valid
# values are: "auto" or "userinfo_endpoint".
#
# Defaults to "auto", which fetches the userinfo endpoint if "openid" is included
# in `scopes`. Uncomment the following to always fetch the userinfo endpoint.
#
#user_profile_method: "userinfo_endpoint"

# Uncomment to allow a user logging in via OIDC to match a pre-existing account instead
# of failing. This could be used if switching from password logins to OIDC. Defaults to false.
#
Expand Down
9 changes: 9 additions & 0 deletions synapse/config/oidc_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ def read_config(self, config, **kwargs):
self.oidc_userinfo_endpoint = oidc_config.get("userinfo_endpoint")
self.oidc_jwks_uri = oidc_config.get("jwks_uri")
self.oidc_skip_verification = oidc_config.get("skip_verification", False)
self.oidc_user_profile_method = oidc_config.get("user_profile_method", "auto")
self.oidc_allow_existing_users = oidc_config.get("allow_existing_users", False)

ump_config = oidc_config.get("user_mapping_provider", {})
Expand Down Expand Up @@ -159,6 +160,14 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
#
#skip_verification: true
# Whether to fetch the user profile from the userinfo endpoint. Valid
# values are: "auto" or "userinfo_endpoint".
#
# Defaults to "auto", which fetches the userinfo endpoint if "openid" is included
# in `scopes`. Uncomment the following to always fetch the userinfo endpoint.
#
#user_profile_method: "userinfo_endpoint"
# Uncomment to allow a user logging in via OIDC to match a pre-existing account instead
# of failing. This could be used if switching from password logins to OIDC. Defaults to false.
#
Expand Down
11 changes: 7 additions & 4 deletions synapse/handlers/oidc_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ def __init__(self, hs: "HomeServer"):
self.hs = hs
self._callback_url = hs.config.oidc_callback_url # type: str
self._scopes = hs.config.oidc_scopes # type: List[str]
self._user_profile_method = hs.config.oidc_user_profile_method # type: str
self._client_auth = ClientAuth(
hs.config.oidc_client_id,
hs.config.oidc_client_secret,
Expand Down Expand Up @@ -196,11 +197,11 @@ def _validate_metadata(self):
% (m["response_types_supported"],)
)

# If the openid scope was not requested, we need a userinfo endpoint to fetch user infos
# Ensure there's a userinfo endpoint to fetch from if it is required.
if self._uses_userinfo:
if m.get("userinfo_endpoint") is None:
raise ValueError(
'provider has no "userinfo_endpoint", even though it is required because the "openid" scope is not requested'
'provider has no "userinfo_endpoint", even though it is required'
)
else:
# If we're not using userinfo, we need a valid jwks to validate the ID token
Expand All @@ -220,8 +221,10 @@ def _uses_userinfo(self) -> bool:
``access_token`` with the ``userinfo_endpoint``.
"""

# Maybe that should be user-configurable and not inferred?
return "openid" not in self._scopes
return (
"openid" not in self._scopes
or self._user_profile_method == "userinfo_endpoint"
)

async def load_metadata(self) -> OpenIDProviderMetadata:
"""Load and validate the provider metadata.
Expand Down
10 changes: 8 additions & 2 deletions tests/handlers/test_oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,15 @@ def test_validate_config(self):
h._validate_metadata,
)

# Tests for configs that the userinfo endpoint
# Tests for configs that require the userinfo endpoint
self.assertFalse(h._uses_userinfo)
h._scopes = [] # do not request the openid scope
self.assertEqual(h._user_profile_method, "auto")
h._user_profile_method = "userinfo_endpoint"
self.assertTrue(h._uses_userinfo)

# Revert the profile method and do not request the "openid" scope.
h._user_profile_method = "auto"
h._scopes = []
self.assertTrue(h._uses_userinfo)
self.assertRaisesRegex(ValueError, "userinfo_endpoint", h._validate_metadata)

Expand Down

0 comments on commit 05ee048

Please sign in to comment.