Allow only requiring a field be present in an SSO response, rather than specifying a required value (#18454)

Author: anoadragon453

changelog.d/18454.misc

@@ -0,0 +1 @@
+Allow checking only for the existence of a field in an SSO provider's response, rather than requiring the value(s) to check.

docs/usage/configuration/config_documentation.md

@@ -3782,17 +3782,23 @@ match particular values in the OIDC userinfo. The requirements can be listed und
 ```yaml
 attribute_requirements:
      - attribute: family_name
-       value: "Stephensson"
+       one_of: ["Stephensson", "Smith"]
      - attribute: groups
        value: "admin"
+     # If `value` or `one_of` are not specified, the attribute only needs
+     # to exist, regardless of value.
+     - attribute: picture
 ```
+
+`attribute` is a required field, while `value` and `one_of` are optional.
+
 All of the listed attributes must match for the login to be permitted. Additional attributes can be added to
 userinfo by expanding the `scopes` section of the OIDC config to retrieve
 additional information from the OIDC provider.
 
 If the OIDC claim is a list, then the attribute must match any value in the list.
 Otherwise, it must exactly match the value of the claim. Using the example
-above, the `family_name` claim MUST be "Stephensson", but the `groups`
+above, the `family_name` claim MUST be either "Stephensson" or "Smith", but the `groups`
 claim MUST contain "admin".
 
 Example configuration:

synapse/config/sso.py

@@ -43,8 +43,7 @@ class SsoAttributeRequirement:
     """Object describing a single requirement for SSO attributes."""
 
     attribute: str
-    # If neither value nor one_of is given, the attribute must simply exist. This is
-    # only true for CAS configs which use a different JSON schema than the one below.
+    # If neither `value` nor `one_of` is given, the attribute must simply exist.
     value: Optional[str] = None
     one_of: Optional[List[str]] = None
 
@@ -56,10 +55,6 @@ class SsoAttributeRequirement:
             "one_of": {"type": "array", "items": {"type": "string"}},
         },
         "required": ["attribute"],
-        "oneOf": [
-            {"required": ["value"]},
-            {"required": ["one_of"]},
-        ],
     }
 
 

tests/handlers/test_oidc.py

@@ -1453,7 +1453,7 @@ def test_attribute_requirements_contains(self) -> None:
             }
         }
     )
-    def test_attribute_requirements_one_of(self) -> None:
+    def test_attribute_requirements_one_of_succeeds(self) -> None:
         """Test that auth succeeds if userinfo attribute has multiple values and CONTAINS required value"""
         # userinfo with "test": ["bar"] attribute should succeed.
         userinfo = {
@@ -1475,6 +1475,81 @@ def test_attribute_requirements_one_of(self) -> None:
             auth_provider_session_id=None,
         )
 
+    @override_config(
+        {
+            "oidc_config": {
+                **DEFAULT_CONFIG,
+                "attribute_requirements": [
+                    {"attribute": "test", "one_of": ["foo", "bar"]}
+                ],
+            }
+        }
+    )
+    def test_attribute_requirements_one_of_fails(self) -> None:
+        """Test that auth fails if userinfo attribute has multiple values yet
+        DOES NOT CONTAIN a required value
+        """
+        # userinfo with "test": ["something else"] attribute should fail.
+        userinfo = {
+            "sub": "tester",
+            "username": "tester",
+            "test": ["something else"],
+        }
+        request, _ = self.start_authorization(userinfo)
+        self.get_success(self.handler.handle_oidc_callback(request))
+        self.complete_sso_login.assert_not_called()
+
+    @override_config(
+        {
+            "oidc_config": {
+                **DEFAULT_CONFIG,
+                "attribute_requirements": [{"attribute": "test"}],
+            }
+        }
+    )
+    def test_attribute_requirements_does_not_exist(self) -> None:
+        """OIDC login fails if the required attribute does not exist in the OIDC userinfo response."""
+        # userinfo lacking "test" attribute should fail.
+        userinfo = {
+            "sub": "tester",
+            "username": "tester",
+        }
+        request, _ = self.start_authorization(userinfo)
+        self.get_success(self.handler.handle_oidc_callback(request))
+        self.complete_sso_login.assert_not_called()
+
+    @override_config(
+        {
+            "oidc_config": {
+                **DEFAULT_CONFIG,
+                "attribute_requirements": [{"attribute": "test"}],
+            }
+        }
+    )
+    def test_attribute_requirements_exist(self) -> None:
+        """OIDC login succeeds if the required attribute exist (regardless of value)
+        in the OIDC userinfo response.
+        """
+        # userinfo with "test" attribute and random value should succeed.
+        userinfo = {
+            "sub": "tester",
+            "username": "tester",
+            "test": random_string(5),  # value does not matter
+        }
+        request, _ = self.start_authorization(userinfo)
+        self.get_success(self.handler.handle_oidc_callback(request))
+
+        # check that the auth handler got called as expected
+        self.complete_sso_login.assert_called_once_with(
+            "@tester:test",
+            self.provider.idp_id,
+            request,
+            ANY,
+            None,
+            new_user=True,
+            auth_provider_session_id=None,
+        )
+
     @override_config(
         {
             "oidc_config": {