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

Add SSO attribute requirements for OIDC providers #9609

Merged
merged 18 commits into from
Mar 16, 2021
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/9609.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added SsoAttributeRequirements to OpenID Connect-based SSO providers. Contributed by Hubbe King.
HubbeKing marked this conversation as resolved.
Show resolved Hide resolved
19 changes: 19 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1872,6 +1872,19 @@ saml2_config:
# When rendering, the Jinja2 templates are given a 'user' variable,
# which is set to the claims returned by the UserInfo Endpoint and/or
# in the ID Token.
# It is possible to configure Synapse to only allow logins if certain attributes
# match particular values in the OIDC userinfo. The requirements can be listed under
# `attribute_requirements` as shown below. All of the listed attributes must
# match for the login to be permitted. Note that fields listed may need to be added
# to the userinfo return in the OIDC provider, by extending or adding a scope.
# If the attributes listed are not part of the default scopes, these scopes may
# also need to be added to the `scopes` section of the OIDC config.
#
# attribute_requirements:
# - attribute: family_name
# value: "Stephensson"
# - attribute: groups
# value: "admin"
#
# See https://github.com/matrix-org/synapse/blob/master/docs/openid.md
# for information on how to configure these options.
Expand Down Expand Up @@ -1905,6 +1918,9 @@ oidc_providers:
# localpart_template: "{{ user.login }}"
# display_name_template: "{{ user.name }}"
# email_template: "{{ user.email }}"
# attribute_requirements:
# - attribute: userGroup
# value: "synapseUsers"

# For use with Keycloak
#
Expand All @@ -1914,6 +1930,9 @@ oidc_providers:
# client_id: "synapse"
# client_secret: "copy secret generated in Keycloak UI"
# scopes: ["openid", "profile"]
# attribute_requirements:
# - attribute: groups
# value: "admin"

# For use with Github
#
Expand Down
33 changes: 33 additions & 0 deletions synapse/config/oidc_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import attr

from synapse.config._util import validate_config
from synapse.config.sso import SsoAttributeRequirement
from synapse.python_dependencies import DependencyException, check_requirements
from synapse.types import Collection, JsonDict
from synapse.util.module_loader import load_module
Expand Down Expand Up @@ -190,6 +191,19 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
# When rendering, the Jinja2 templates are given a 'user' variable,
# which is set to the claims returned by the UserInfo Endpoint and/or
# in the ID Token.
# It is possible to configure Synapse to only allow logins if certain attributes
HubbeKing marked this conversation as resolved.
Show resolved Hide resolved
# match particular values in the OIDC userinfo. The requirements can be listed under
# `attribute_requirements` as shown below. All of the listed attributes must
# match for the login to be permitted. Note that fields listed may need to be added
# to the userinfo return in the OIDC provider, by extending or adding a scope.
# If the attributes listed are not part of the default scopes, these scopes may
# also need to be added to the `scopes` section of the OIDC config.
HubbeKing marked this conversation as resolved.
Show resolved Hide resolved
#
# attribute_requirements:
# - attribute: family_name
# value: "Stephensson"
# - attribute: groups
# value: "admin"
HubbeKing marked this conversation as resolved.
Show resolved Hide resolved
#
# See https://github.com/matrix-org/synapse/blob/master/docs/openid.md
# for information on how to configure these options.
Expand Down Expand Up @@ -223,6 +237,9 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
# localpart_template: "{{{{ user.login }}}}"
# display_name_template: "{{{{ user.name }}}}"
# email_template: "{{{{ user.email }}}}"
# attribute_requirements:
# - attribute: userGroup
# value: "synapseUsers"

# For use with Keycloak
#
Expand All @@ -232,6 +249,9 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
# client_id: "synapse"
# client_secret: "copy secret generated in Keycloak UI"
# scopes: ["openid", "profile"]
# attribute_requirements:
# - attribute: groups
# value: "admin"

# For use with Github
#
Expand Down Expand Up @@ -324,6 +344,10 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
},
"allow_existing_users": {"type": "boolean"},
"user_mapping_provider": {"type": ["object", "null"]},
"attribute_requirements": {
"type": "array",
"items": SsoAttributeRequirement.JSON_SCHEMA,
},
},
}

Expand Down Expand Up @@ -460,6 +484,11 @@ def _parse_oidc_config_dict(
jwt_header=client_secret_jwt_key_config["jwt_header"],
jwt_payload=client_secret_jwt_key_config.get("jwt_payload", {}),
)
# parse attribute_requirements from config (list of dicts) into a list of SsoAttributeRequirement
attribute_requirements = [
SsoAttributeRequirement(**x)
for x in oidc_config.get("attribute_requirements", [])
]

return OidcProviderConfig(
idp_id=idp_id,
Expand All @@ -482,6 +511,7 @@ def _parse_oidc_config_dict(
allow_existing_users=oidc_config.get("allow_existing_users", False),
user_mapping_provider_class=user_mapping_provider_class,
user_mapping_provider_config=user_mapping_provider_config,
attribute_requirements=attribute_requirements,
)


Expand Down Expand Up @@ -568,3 +598,6 @@ class OidcProviderConfig:

# the config of the user mapping provider
user_mapping_provider_config = attr.ib()

# required attributes to require in userinfo to allow login/registration
attribute_requirements = attr.ib(type=Iterable[SsoAttributeRequirement])
HubbeKing marked this conversation as resolved.
Show resolved Hide resolved
24 changes: 23 additions & 1 deletion synapse/handlers/oidc_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,16 @@
# limitations under the License.
import inspect
import logging
from typing import TYPE_CHECKING, Dict, Generic, List, Optional, TypeVar, Union
from typing import (
TYPE_CHECKING,
Dict,
Generic,
Iterable,
List,
Optional,
TypeVar,
Union,
)
from urllib.parse import urlencode

import attr
Expand All @@ -40,6 +49,7 @@
OidcProviderClientSecretJwtKey,
OidcProviderConfig,
)
from synapse.config.sso import SsoAttributeRequirement
from synapse.handlers.sso import MappingException, UserAttributes
from synapse.http.site import SynapseRequest
from synapse.logging.context import make_deferred_yieldable
Expand Down Expand Up @@ -278,6 +288,9 @@ def __init__(
self._config = provider
self._callback_url = hs.config.oidc_callback_url # type: str

self._oidc_attribute_requirements = (
provider.attribute_requirements
) # type: Iterable[SsoAttributeRequirement]
HubbeKing marked this conversation as resolved.
Show resolved Hide resolved
self._scopes = provider.scopes
self._user_profile_method = provider.user_profile_method

Expand Down Expand Up @@ -854,6 +867,15 @@ async def handle_oidc_callback(
)

# otherwise, it's a login
logger.debug("Userinfo for OIDC login: %s", userinfo)

# Ensure that the attributes of the logged in user meet the required
# attributes
# With OIDC, we check userinfo against attribute_requirements
HubbeKing marked this conversation as resolved.
Show resolved Hide resolved
if not self._sso_handler.check_required_attributes(
request, userinfo, self._oidc_attribute_requirements
):
clokep marked this conversation as resolved.
Show resolved Hide resolved
return

# Call the mapper to register/login the user
try:
Expand Down
34 changes: 34 additions & 0 deletions tests/handlers/test_oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,40 @@ def test_null_localpart(self):
self.get_success(_make_callback_with_userinfo(self.hs, userinfo))
self.assertRenderedError("mapping_error", "localpart is invalid: ")

@override_config(
{
"oidc_config": {
**DEFAULT_CONFIG,
"attribute_requirements": [{"attribute": "test", "value": "foobar"}],
}
}
)
def test_attribute_requirements(self):
"""The required attributes must be met from the OIDC userinfo response."""
auth_handler = self.hs.get_auth_handler()
auth_handler.complete_sso_login = simple_async_mock()

# userinfo lacking "test": "foobar" attribute should fail.
userinfo = {
"sub": "tester",
"username": "tester",
}
self.get_success(_make_callback_with_userinfo(self.hs, userinfo))
auth_handler.complete_sso_login.assert_not_called()

# userinfo with "test": "foobar" attribute should succeed.
userinfo = {
"sub": "tester",
"username": "tester",
"test": "foobar",
}
self.get_success(_make_callback_with_userinfo(self.hs, userinfo))

# check that the auth handler got called as expected
auth_handler.complete_sso_login.assert_called_once_with(
"@tester:test", "oidc", ANY, ANY, None, new_user=True
)

def _generate_oidc_session_token(
self,
state: str,
Expand Down