diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go index a8b102ac7..63e618bc0 100644 --- a/internal/configuration/validator/const.go +++ b/internal/configuration/validator/const.go @@ -161,15 +161,13 @@ const ( errFmtOIDCClientInvalidSecret = "identity_providers: oidc: client '%s': option 'secret' is required" errFmtOIDCClientPublicInvalidSecret = "identity_providers: oidc: client '%s': option 'secret' is " + "required to be empty when option 'public' is true" - errFmtOIDCClientRedirectURI = "identity_providers: oidc: client '%s': option 'redirect_uris' has an " + - "invalid value: redirect uri '%s' must have a scheme of 'http' or 'https' but '%s' is configured" errFmtOIDCClientRedirectURICantBeParsed = "identity_providers: oidc: client '%s': option 'redirect_uris' has an " + "invalid value: redirect uri '%s' could not be parsed: %v" - errFmtOIDCClientRedirectURIPublic = "identity_providers: oidc: client '%s': option 'redirect_uris' has the" + + errFmtOIDCClientRedirectURIPublic = "identity_providers: oidc: client '%s': option 'redirect_uris' has the " + "redirect uri '%s' when option 'public' is false but this is invalid as this uri is not valid " + "for the openid connect confidential client type" errFmtOIDCClientRedirectURIAbsolute = "identity_providers: oidc: client '%s': option 'redirect_uris' has an " + - "invalid value: redirect uri '%s' must have the scheme 'http' or 'https' but it has no scheme" + "invalid value: redirect uri '%s' must have the scheme but it is absent" errFmtOIDCClientInvalidPolicy = "identity_providers: oidc: client '%s': option 'policy' must be 'one_factor' " + "or 'two_factor' but it is configured as '%s'" errFmtOIDCClientInvalidConsentMode = "identity_providers: oidc: client '%s': consent: option 'mode' must be one of " + diff --git a/internal/configuration/validator/identity_providers.go b/internal/configuration/validator/identity_providers.go index a2ee5a549..3bea1c63b 100644 --- a/internal/configuration/validator/identity_providers.go +++ b/internal/configuration/validator/identity_providers.go @@ -330,13 +330,9 @@ func validateOIDCClientRedirectURIs(client schema.OpenIDConnectClientConfigurati continue } - if !parsedURL.IsAbs() { + if !parsedURL.IsAbs() || (!client.Public && parsedURL.Scheme == "") { validator.Push(fmt.Errorf(errFmtOIDCClientRedirectURIAbsolute, client.ID, redirectURI)) return } - - if !client.Public && parsedURL.Scheme != schemeHTTPS && parsedURL.Scheme != schemeHTTP { - validator.Push(fmt.Errorf(errFmtOIDCClientRedirectURI, client.ID, redirectURI, parsedURL.Scheme)) - } } } diff --git a/internal/configuration/validator/identity_providers_test.go b/internal/configuration/validator/identity_providers_test.go index 727676bb4..b07f9f29f 100644 --- a/internal/configuration/validator/identity_providers_test.go +++ b/internal/configuration/validator/identity_providers_test.go @@ -124,13 +124,12 @@ func TestShouldRaiseErrorWhenOIDCCORSOriginsHasInvalidValues(t *testing.T) { ValidateIdentityProviders(config, validator) - require.Len(t, validator.Errors(), 6) + require.Len(t, validator.Errors(), 5) assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: cors: option 'allowed_origins' contains an invalid value 'https://example.com/' as it has a path: origins must only be scheme, hostname, and an optional port") assert.EqualError(t, validator.Errors()[1], "identity_providers: oidc: cors: option 'allowed_origins' contains an invalid value 'https://site.example.com/subpath' as it has a path: origins must only be scheme, hostname, and an optional port") assert.EqualError(t, validator.Errors()[2], "identity_providers: oidc: cors: option 'allowed_origins' contains an invalid value 'https://site.example.com?example=true' as it has a query string: origins must only be scheme, hostname, and an optional port") assert.EqualError(t, validator.Errors()[3], "identity_providers: oidc: cors: option 'allowed_origins' contains the wildcard origin '*' with more than one origin but the wildcard origin must be defined by itself") assert.EqualError(t, validator.Errors()[4], "identity_providers: oidc: cors: option 'allowed_origins' contains the wildcard origin '*' cannot be specified with option 'allowed_origins_from_client_redirect_uris' enabled") - assert.EqualError(t, validator.Errors()[5], "identity_providers: oidc: client 'myclient': option 'redirect_uris' has an invalid value: redirect uri 'file://a/file' must have a scheme of 'http' or 'https' but 'file' is configured") require.Len(t, config.OIDC.CORS.AllowedOrigins, 6) assert.Equal(t, "*", config.OIDC.CORS.AllowedOrigins[3].String()) @@ -749,6 +748,7 @@ func TestValidateOIDCClientRedirectURIsSupportingPrivateUseURISchemes(t *testing "oc://ios.owncloud.com", // example given in the RFC https://datatracker.ietf.org/doc/html/rfc8252#section-7.1 "com.example.app:/oauth2redirect/example-provider", + oauth2InstalledApp, }, } @@ -767,10 +767,9 @@ func TestValidateOIDCClientRedirectURIsSupportingPrivateUseURISchemes(t *testing validateOIDCClientRedirectURIs(conf, validator) assert.Len(t, validator.Warnings(), 0) - assert.Len(t, validator.Errors(), 2) + assert.Len(t, validator.Errors(), 1) assert.ElementsMatch(t, validator.Errors(), []error{ - errors.New("identity_providers: oidc: client 'owncloud': option 'redirect_uris' has an invalid value: redirect uri 'oc://ios.owncloud.com' must have a scheme of 'http' or 'https' but 'oc' is configured"), - errors.New("identity_providers: oidc: client 'owncloud': option 'redirect_uris' has an invalid value: redirect uri 'com.example.app:/oauth2redirect/example-provider' must have a scheme of 'http' or 'https' but 'com.example.app' is configured"), + errors.New("identity_providers: oidc: client 'owncloud': option 'redirect_uris' has the redirect uri 'urn:ietf:wg:oauth:2.0:oob' when option 'public' is false but this is invalid as this uri is not valid for the openid connect confidential client type"), }) }) }