Add 'brand' field to MSC2858 response (#9242)

We've decided to add a 'brand' field to help clients decide how to style the
buttons.

Also, fix up the allowed characters for idp_id, while I'm in the area.

10 files changed, 55 insertions, 32 deletions

diff --git a/changelog.d/9183.feature b/changelog.d/9183.feature
index 2d5c735042..3bcd9f15d1 100644
--- a/changelog.d/9183.feature
+++ b/changelog.d/9183.feature
@@ -1 +1 @@
-Add experimental support for allowing clients to pick an SSO Identity Provider ([MSC2858](https://github.com/matrix-org/matrix-doc/pull/2858).
+Add experimental support for allowing clients to pick an SSO Identity Provider ([MSC2858](https://github.com/matrix-org/matrix-doc/pull/2858)).
diff --git a/changelog.d/9242.feature b/changelog.d/9242.feature
new file mode 100644
index 0000000000..3bcd9f15d1
--- /dev/null
+++ b/changelog.d/9242.feature
@@ -0,0 +1 @@
+Add experimental support for allowing clients to pick an SSO Identity Provider ([MSC2858](https://github.com/matrix-org/matrix-doc/pull/2858)).
diff --git a/docs/openid.md b/docs/openid.md
index b86ae89768..f01f46d326 100644
--- a/docs/openid.md
+++ b/docs/openid.md
@@ -225,6 +225,7 @@ Synapse config:
 oidc_providers:
   - idp_id: github
     idp_name: Github
+    idp_brand: "org.matrix.github"  # optional: styling hint for clients
     discover: false
     issuer: "https://github.com/"
     client_id: "your-client-id" # TO BE FILLED
@@ -250,6 +251,7 @@ oidc_providers:
    oidc_providers:
      - idp_id: google
        idp_name: Google
+       idp_brand: "org.matrix.google"  # optional: styling hint for clients
        issuer: "https://accounts.google.com/"
        client_id: "your-client-id" # TO BE FILLED
        client_secret: "your-client-secret" # TO BE FILLED
@@ -296,6 +298,7 @@ Synapse config:
 oidc_providers:
   - idp_id: gitlab
     idp_name: Gitlab
+    idp_brand: "org.matrix.gitlab"  # optional: styling hint for clients
     issuer: "https://gitlab.com/"
     client_id: "your-client-id" # TO BE FILLED
     client_secret: "your-client-secret" # TO BE FILLED
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index 1c90156db9..8777e3254d 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -1727,10 +1727,14 @@ saml2_config:
 #       offer the user a choice of login mechanisms.
 #
 #   idp_icon: An optional icon for this identity provider, which is presented
-#       by identity picker pages. If given, must be an MXC URI of the format
-#       mxc://<server-name>/<media-id>. (An easy way to obtain such an MXC URI
-#       is to upload an image to an (unencrypted) room and then copy the "url"
-#       from the source of the event.)
+#       by clients and Synapse's own IdP picker page. If given, must be an
+#       MXC URI of the format mxc://<server-name>/<media-id>. (An easy way to
+#       obtain such an MXC URI is to upload an image to an (unencrypted) room
+#       and then copy the "url" from the source of the event.)
+#
+#   idp_brand: An optional brand for this identity provider, allowing clients
+#       to style the login flow according to the identity provider in question.
+#       See the spec for possible options here.
 #
 #   discover: set to 'false' to disable the use of the OIDC discovery mechanism
 #       to discover endpoints. Defaults to true.
@@ -1860,6 +1864,7 @@ oidc_providers:
   #
   #- idp_id: github
   #  idp_name: Github
+  #  idp_brand: org.matrix.github
   #  discover: false
   #  issuer: "https://github.com/"
   #  client_id: "your-client-id" # TO BE FILLED
diff --git a/synapse/config/oidc_config.py b/synapse/config/oidc_config.py
index 8237b2e797..f31511e039 100644
--- a/synapse/config/oidc_config.py
+++ b/synapse/config/oidc_config.py
@@ -14,7 +14,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import string
 from collections import Counter
 from typing import Iterable, Optional, Tuple, Type
 
@@ -79,10 +78,14 @@ class OIDCConfig(Config):
         #       offer the user a choice of login mechanisms.
         #
         #   idp_icon: An optional icon for this identity provider, which is presented
-        #       by identity picker pages. If given, must be an MXC URI of the format
-        #       mxc://<server-name>/<media-id>. (An easy way to obtain such an MXC URI
-        #       is to upload an image to an (unencrypted) room and then copy the "url"
-        #       from the source of the event.)
+        #       by clients and Synapse's own IdP picker page. If given, must be an
+        #       MXC URI of the format mxc://<server-name>/<media-id>. (An easy way to
+        #       obtain such an MXC URI is to upload an image to an (unencrypted) room
+        #       and then copy the "url" from the source of the event.)
+        #
+        #   idp_brand: An optional brand for this identity provider, allowing clients
+        #       to style the login flow according to the identity provider in question.
+        #       See the spec for possible options here.
         #
         #   discover: set to 'false' to disable the use of the OIDC discovery mechanism
         #       to discover endpoints. Defaults to true.
@@ -212,6 +215,7 @@ class OIDCConfig(Config):
           #
           #- idp_id: github
           #  idp_name: Github
+          #  idp_brand: org.matrix.github
           #  discover: false
           #  issuer: "https://github.com/"
           #  client_id: "your-client-id" # TO BE FILLED
@@ -235,11 +239,22 @@ OIDC_PROVIDER_CONFIG_SCHEMA = {
     "type": "object",
     "required": ["issuer", "client_id", "client_secret"],
     "properties": {
-        # TODO: fix the maxLength here depending on what MSC2528 decides
-        #   remember that we prefix the ID given here with `oidc-`
-        "idp_id": {"type": "string", "minLength": 1, "maxLength": 128},
+        "idp_id": {
+            "type": "string",
+            "minLength": 1,
+            # MSC2858 allows a maxlen of 255, but we prefix with "oidc-"
+            "maxLength": 250,
+            "pattern": "^[A-Za-z0-9._~-]+$",
+        },
         "idp_name": {"type": "string"},
         "idp_icon": {"type": "string"},
+        "idp_brand": {
+            "type": "string",
+            # MSC2758-style namespaced identifier
+            "minLength": 1,
+            "maxLength": 255,
+            "pattern": "^[a-z][a-z0-9_.-]*$",
+        },
         "discover": {"type": "boolean"},
         "issuer": {"type": "string"},
         "client_id": {"type": "string"},
@@ -358,25 +373,8 @@ def _parse_oidc_config_dict(
             config_path + ("user_mapping_provider", "module"),
         )
 
-    # MSC2858 will apply certain limits in what can be used as an IdP id, so let's
-    # enforce those limits now.
-    # TODO: factor out this stuff to a generic function
     idp_id = oidc_config.get("idp_id", "oidc")
 
-    # TODO: update this validity check based on what MSC2858 decides.
-    valid_idp_chars = set(string.ascii_lowercase + string.digits + "-._")
-
-    if any(c not in valid_idp_chars for c in idp_id):
-        raise ConfigError(
-            'idp_id may only contain a-z, 0-9, "-", ".", "_"',
-            config_path + ("idp_id",),
-        )
-
-    if idp_id[0] not in string.ascii_lowercase:
-        raise ConfigError(
-            "idp_id must start with a-z", config_path + ("idp_id",),
-        )
-
     # prefix the given IDP with a prefix specific to the SSO mechanism, to avoid
     # clashes with other mechs (such as SAML, CAS).
     #
@@ -402,6 +400,7 @@ def _parse_oidc_config_dict(
         idp_id=idp_id,
         idp_name=oidc_config.get("idp_name", "OIDC"),
         idp_icon=idp_icon,
+        idp_brand=oidc_config.get("idp_brand"),
         discover=oidc_config.get("discover", True),
         issuer=oidc_config["issuer"],
         client_id=oidc_config["client_id"],
@@ -432,6 +431,9 @@ class OidcProviderConfig:
     # Optional MXC URI for icon for this IdP.
     idp_icon = attr.ib(type=Optional[str])
 
+    # Optional brand identifier for this IdP.
+    idp_brand = attr.ib(type=Optional[str])
+
     # whether the OIDC discovery mechanism is used to discover endpoints
     discover = attr.ib(type=bool)
 
diff --git a/synapse/handlers/cas_handler.py b/synapse/handlers/cas_handler.py
index 0f342c607b..048523ec94 100644
--- a/synapse/handlers/cas_handler.py
+++ b/synapse/handlers/cas_handler.py
@@ -80,9 +80,10 @@ class CasHandler:
         # user-facing name of this auth provider
         self.idp_name = "CAS"
 
-        # we do not currently support icons for CAS auth, but this is required by
+        # we do not currently support brands/icons for CAS auth, but this is required by
         # the SsoIdentityProvider protocol type.
         self.idp_icon = None
+        self.idp_brand = None
 
         self._sso_handler = hs.get_sso_handler()
 
diff --git a/synapse/handlers/oidc_handler.py b/synapse/handlers/oidc_handler.py
index 324ddb798c..ca647fa78f 100644
--- a/synapse/handlers/oidc_handler.py
+++ b/synapse/handlers/oidc_handler.py
@@ -274,6 +274,9 @@ class OidcProvider:
         # MXC URI for icon for this auth provider
         self.idp_icon = provider.idp_icon
 
+        # optional brand identifier for this auth provider
+        self.idp_brand = provider.idp_brand
+
         self._sso_handler = hs.get_sso_handler()
 
         self._sso_handler.register_identity_provider(self)
diff --git a/synapse/handlers/saml_handler.py b/synapse/handlers/saml_handler.py
index 38461cf79d..5946919c33 100644
--- a/synapse/handlers/saml_handler.py
+++ b/synapse/handlers/saml_handler.py
@@ -78,9 +78,10 @@ class SamlHandler(BaseHandler):
         # user-facing name of this auth provider
         self.idp_name = "SAML"
 
-        # we do not currently support icons for SAML auth, but this is required by
+        # we do not currently support icons/brands for SAML auth, but this is required by
         # the SsoIdentityProvider protocol type.
         self.idp_icon = None
+        self.idp_brand = None
 
         # a map from saml session id to Saml2SessionData object
         self._outstanding_requests_dict = {}  # type: Dict[str, Saml2SessionData]
diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py
index afc1341d09..3308b037d2 100644
--- a/synapse/handlers/sso.py
+++ b/synapse/handlers/sso.py
@@ -80,6 +80,11 @@ class SsoIdentityProvider(Protocol):
         """Optional MXC URI for user-facing icon"""
         return None
 
+    @property
+    def idp_brand(self) -> Optional[str]:
+        """Optional branding identifier"""
+        return None
+
     @abc.abstractmethod
     async def handle_redirect_request(
         self,
diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py
index 0a561eea60..0fb9419e58 100644
--- a/synapse/rest/client/v1/login.py
+++ b/synapse/rest/client/v1/login.py
@@ -333,6 +333,8 @@ def _get_auth_flow_dict_for_idp(idp: SsoIdentityProvider) -> JsonDict:
     e = {"id": idp.idp_id, "name": idp.idp_name}  # type: JsonDict
     if idp.idp_icon:
         e["icon"] = idp.idp_icon
+    if idp.idp_brand:
+        e["brand"] = idp.idp_brand
     return e