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

Commit

Permalink
Check the sub claim when receiving a logout token
Browse files Browse the repository at this point in the history
  • Loading branch information
sandhose committed Sep 21, 2022
1 parent 3ae014c commit 0d33dd9
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 15 deletions.
7 changes: 7 additions & 0 deletions synapse/config/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ def oidc_enabled(self) -> bool:
"jwks_uri": {"type": "string"},
"skip_verification": {"type": "boolean"},
"backchannel_logout_enabled": {"type": "boolean"},
"backchannel_logout_ignore_sub": {"type": "boolean"},
"user_profile_method": {
"type": "string",
"enum": ["auto", "userinfo_endpoint"],
Expand Down Expand Up @@ -294,6 +295,9 @@ def _parse_oidc_config_dict(
userinfo_endpoint=oidc_config.get("userinfo_endpoint"),
jwks_uri=oidc_config.get("jwks_uri"),
backchannel_logout_enabled=oidc_config.get("backchannel_logout_enabled", False),
backchannel_logout_ignore_sub=oidc_config.get(
"backchannel_logout_ignore_sub", False
),
skip_verification=oidc_config.get("skip_verification", False),
user_profile_method=oidc_config.get("user_profile_method", "auto"),
allow_existing_users=oidc_config.get("allow_existing_users", False),
Expand Down Expand Up @@ -373,6 +377,9 @@ class OidcProviderConfig:
# Whether Synapse should react to backchannel logouts
backchannel_logout_enabled: bool

# Whetever Synapse should ignore the `sub` claim in backchannel logouts or not
backchannel_logout_ignore_sub: bool

# Whether to skip metadata verification
skip_verification: bool

Expand Down
59 changes: 44 additions & 15 deletions synapse/handlers/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -508,21 +508,24 @@ def _validate_metadata(self, m: OpenIDProviderMetadata) -> None:
self.issuer,
)

# If OIDC backchannel logouts are enabled, the provider mapping provider
# should use the `sub` claim. We verify that by mapping a dumb user and see
# if we get back the sub claim
user = UserInfo({"sub": "thisisasubject"})
try:
subject = self._user_mapping_provider.get_remote_user_id(user)
if subject != user["sub"]:
raise Exception()
except Exception:
logger.warning(
"OIDC Back-Channel Logout is enabled for issuer %r but it looks "
"like the configured `user_mapping_provider` does not use the "
"`sub` claim as subject, which may be resuired for logouts to work",
self.issuer,
)
if not self._config.backchannel_logout_ignore_sub:
# If OIDC backchannel logouts are enabled, the provider mapping provider
# should use the `sub` claim. We verify that by mapping a dumb user and
# see if we get back the sub claim
user = UserInfo({"sub": "thisisasubject"})
try:
subject = self._user_mapping_provider.get_remote_user_id(user)
if subject != user["sub"]:
raise Exception()
except Exception:
logger.warning(
f"OIDC Back-Channel Logout is enabled for issuer {self.issuer!r} "
f"but it looks like the configured `user_mapping_provider` "
f"does not use the `sub` claim as subject. If it is the case, "
f"and you want Synapse to ignore the `sub` claim in OIDC "
f"Back-Channel Logouts, set `backchannel_logout_ignore_sub` "
f"to `true` in the issuer config."
)

@property
def _uses_userinfo(self) -> bool:
Expand Down Expand Up @@ -1271,6 +1274,16 @@ async def handle_backchannel_logout(
# point the `sid` claim exists and is a string.
sid: str = claims.get("sid")

# If the `sub` claim was included in the logout token, we check that it matches
# that it matches the right user. We can have cases where the `sub` claim is not
# the ID saved in database, so we let admins disable this check in config.
sub: Optional[str] = claims.get("sub")
expected_user_id: Optional[str] = None
if sub is not None and not self._config.backchannel_logout_ignore_sub:
expected_user_id = await self._store.get_user_by_external_id(
self.idp_id, sub
)

# Fetch any device(s) in the store associated with the session ID.
devices = await self._store.get_devices_by_auth_provider_session_id(
auth_provider_id=self.idp_id,
Expand All @@ -1283,6 +1296,22 @@ async def handle_backchannel_logout(
for device in devices:
user_id = device["user_id"]
device_id = device["device_id"]

# If the user_id associated with that device/session is not the one we got
# out of the `sub` claim, skip that device and show log an error.
if expected_user_id is not None and user_id != expected_user_id:
logger.error(
f"Received an OIDC Back-Channel Logout request "
f"from issuer {self.issuer!r}, "
f"for the user {expected_user_id!r} (sub: {sub!r}), "
f"but with a session ID ({sid!r}) which belongs to {user_id!r}. "
f"This may happen when the OIDC user mapper uses something else "
f"than the `sub` claim as subject claim. "
f"Set `backchannel_logout_ignore_sub` to `true` "
f"in the provider config it that is the case."
)
continue

logger.info(
"Logging out %r (device %r) via OIDC backchannel logout (sid %r).",
user_id,
Expand Down

0 comments on commit 0d33dd9

Please sign in to comment.