Moves the OAuth2 configuration under OIDC.

Easier to understand setting oidc.client_id when using OIDC.

This also adds a call to Verify in unit tests and fixes the current
test config issues.

setup/config/config.go

@@ -371,6 +371,14 @@ func (errs *ConfigErrors) Add(str string) {
 	*errs = append(*errs, str)
 }
 
+// checkNotEmpty verifies the given value is empty in the configuration.
+// If it is, adds an error to the list.
+func checkEmpty(configErrs *ConfigErrors, key, value string) {
+	if value != "" {
+		configErrs.Add(fmt.Sprintf("expected empty key %q: %s", key, value))
+	}
+}
+
 // checkNotEmpty verifies the given value is not empty in the configuration.
 // If it is, adds an error to the list.
 func checkNotEmpty(configErrs *ConfigErrors, key, value string) {
@@ -415,6 +423,26 @@ func checkURL(configErrs *ConfigErrors, key, value string) {
 	}
 }
 
+// checkIconURL verifies that the parameter is a valid icon URL.
+func checkIconURL(configErrs *ConfigErrors, key, value string) {
+	if value == "" {
+		configErrs.Add(fmt.Sprintf("missing config key %q", key))
+		return
+	}
+	url, err := url.Parse(value)
+	if err != nil {
+		configErrs.Add(fmt.Sprintf("config key %q contains invalid URL (%s)", key, err.Error()))
+		return
+	}
+	switch url.Scheme {
+	case "http":
+	case "https":
+	case "mxc":
+	default:
+		configErrs.Add(fmt.Sprintf("invalid URL scheme for config key %q: %s", key, value))
+	}
+}
+
 // checkLogging verifies the parameters logging.* are valid.
 func (config *Dendrite) checkLogging(configErrs *ConfigErrors) {
 	for _, logrusHook := range config.Logging {

setup/config/config_clientapi.go

@@ -162,6 +162,12 @@ func (sso *SSO) Verify(configErrs *ConfigErrors) {
 }
 
 type IdentityProvider struct {
+	// OAuth2 contains settings for IdPs based on OAuth2 (but not OpenID Connect).
+	OAuth2 OAuth2 `yaml:"oauth2"`
+
+	// OIDC contains settings for IdPs based on OpenID Connect.
+	OIDC OIDC `yaml:"oidc"`
+
 	// ID is the unique identifier of this IdP. If empty, the brand will be used.
 	ID string `yaml:"id"`
 
@@ -178,14 +184,8 @@ type IdentityProvider struct {
 	Icon string `yaml:"icon"`
 
 	// Type describes how this IdP is implemented. If this is empty, a default is chosen
-	// based on brand.
+	// based on brand or which subkeys exist.
 	Type IdentityProviderType `yaml:"type"`
-
-	// OAuth2 contains settings for IdPs based on OpenID Connect and OAuth2.
-	OAuth2 OAuth2 `yaml:"oauth2"`
-
-	// OIDC contains settings for IdPs based on OpenID Connect.
-	OIDC OIDC `yaml:"oidc"`
 }
 
 func (idp *IdentityProvider) WithDefaults() IdentityProvider {
@@ -216,6 +216,7 @@ type OAuth2 struct {
 }
 
 type OIDC struct {
+	OAuth2       `yaml:",inline"`
 	DiscoveryURL string `yaml:"discovery_url"`
 }
 
@@ -231,18 +232,21 @@ func (idp *IdentityProvider) verifyNormalized(configErrs *ConfigErrors) {
 		configErrs.Add(fmt.Sprintf("unrecognised brand in identity provider %q for config key %q: %s", idp.ID, "client_api.sso.providers", idp.Brand))
 	}
 	if idp.Icon != "" {
-		checkURL(configErrs, "client_api.sso.providers.icon", idp.Icon)
+		checkIconURL(configErrs, "client_api.sso.providers.icon", idp.Icon)
 	}
 
 	switch idp.Type {
 	case SSOTypeOIDC:
-		checkNotEmpty(configErrs, "client_api.sso.providers.oidc.client_id", idp.OAuth2.ClientID)
+		checkNotEmpty(configErrs, "client_api.sso.providers.oidc.client_id", idp.OIDC.ClientID)
-		checkNotEmpty(configErrs, "client_api.sso.providers.oidc.client_secret", idp.OAuth2.ClientSecret)
+		checkNotEmpty(configErrs, "client_api.sso.providers.oidc.client_secret", idp.OIDC.ClientSecret)
 		checkNotEmpty(configErrs, "client_api.sso.providers.oidc.discovery_url", idp.OIDC.DiscoveryURL)
 
+		checkEmpty(configErrs, "client_api.sso.providers.oauth2.client_id", idp.OAuth2.ClientID)
+		checkEmpty(configErrs, "client_api.sso.providers.oauth2.client_secret", idp.OAuth2.ClientSecret)
+
 	case SSOTypeGitHub:
-		checkNotEmpty(configErrs, "client_api.sso.providers.oidc.client_id", idp.OAuth2.ClientID)
+		checkNotEmpty(configErrs, "client_api.sso.providers.oauth2.client_id", idp.OAuth2.ClientID)
-		checkNotEmpty(configErrs, "client_api.sso.providers.oidc.client_secret", idp.OAuth2.ClientSecret)
+		checkNotEmpty(configErrs, "client_api.sso.providers.oauth2.client_secret", idp.OAuth2.ClientSecret)
 
 	default:
 		configErrs.Add(fmt.Sprintf("unrecognised type in identity provider %q for config key %q: %s", idp.ID, "client_api.sso.providers", idp.Type))

setup/config/config_test.go

@@ -22,7 +22,7 @@ import (
 )
 
 func TestLoadConfigRelative(t *testing.T) {
-	_, err := loadConfig("/my/config/dir", []byte(testConfig),
+	c, err := loadConfig("/my/config/dir", []byte(testConfig),
 		mockReadFile{
 			"/my/config/dir/matrix_key.pem": testKey,
 			"/my/config/dir/tls_cert.pem":   testCert,
@@ -32,6 +32,13 @@ func TestLoadConfigRelative(t *testing.T) {
 	if err != nil {
 		t.Error("failed to load config:", err)
 	}
+
+	var ss ConfigErrors
+	c.Verify(&ss, true)
+
+	for _, s := range ss {
+		t.Errorf("Verify: %s", s)
+	}
 }
 
 const testConfig = `
@@ -84,11 +91,11 @@ client_api:
     listen: http://[::]:8071
   registration_disabled: false
   registration_shared_secret: ""
-  enable_registration_captcha: false
+  enable_registration_captcha: true
-  recaptcha_public_key: ""
+  recaptcha_public_key: a
-  recaptcha_private_key: ""
+  recaptcha_private_key: b
   recaptcha_bypass_secret: ""
-  recaptcha_siteverify_api: ""
+  recaptcha_siteverify_api: c
   login:
     sso:
       enabled: true
@@ -96,15 +103,17 @@ client_api:
       default_provider: github
       providers:
       - brand: github
+        oauth2:
+          client_id: aclientid
+          client_secret: aclientsecret
       - id: custom
         name: "Custom Provider"
         icon: "mxc://example.com/abc123"
         type: oidc
-        oauth2:
-          client_id: aclientid
-          client_secret: aclientsecret
         oidc:
           discovery_url: http://auth.example.com/.well-known/openid-configuration
+          client_id: aclientid
+          client_secret: aclientsecret
   turn:
     turn_user_lifetime: ""
     turn_uris: []