fix(configuration): valid oidc redirect uris not accepted (#4410)
This fixes an issue where redirect URIs which may be valid are rejected by the configuration validator. This will instead allow the OpenID Connect 1.0 flows to validate them individually.pull/4411/head
parent
ef28345f05
commit
c481ac86bb
|
@ -161,15 +161,13 @@ const (
|
||||||
errFmtOIDCClientInvalidSecret = "identity_providers: oidc: client '%s': option 'secret' is required"
|
errFmtOIDCClientInvalidSecret = "identity_providers: oidc: client '%s': option 'secret' is required"
|
||||||
errFmtOIDCClientPublicInvalidSecret = "identity_providers: oidc: client '%s': option 'secret' is " +
|
errFmtOIDCClientPublicInvalidSecret = "identity_providers: oidc: client '%s': option 'secret' is " +
|
||||||
"required to be empty when option 'public' is true"
|
"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 " +
|
errFmtOIDCClientRedirectURICantBeParsed = "identity_providers: oidc: client '%s': option 'redirect_uris' has an " +
|
||||||
"invalid value: redirect uri '%s' could not be parsed: %v"
|
"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 " +
|
"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"
|
"for the openid connect confidential client type"
|
||||||
errFmtOIDCClientRedirectURIAbsolute = "identity_providers: oidc: client '%s': option 'redirect_uris' has an " +
|
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' " +
|
errFmtOIDCClientInvalidPolicy = "identity_providers: oidc: client '%s': option 'policy' must be 'one_factor' " +
|
||||||
"or 'two_factor' but it is configured as '%s'"
|
"or 'two_factor' but it is configured as '%s'"
|
||||||
errFmtOIDCClientInvalidConsentMode = "identity_providers: oidc: client '%s': consent: option 'mode' must be one of " +
|
errFmtOIDCClientInvalidConsentMode = "identity_providers: oidc: client '%s': consent: option 'mode' must be one of " +
|
||||||
|
|
|
@ -330,13 +330,9 @@ func validateOIDCClientRedirectURIs(client schema.OpenIDConnectClientConfigurati
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
if !parsedURL.IsAbs() {
|
if !parsedURL.IsAbs() || (!client.Public && parsedURL.Scheme == "") {
|
||||||
validator.Push(fmt.Errorf(errFmtOIDCClientRedirectURIAbsolute, client.ID, redirectURI))
|
validator.Push(fmt.Errorf(errFmtOIDCClientRedirectURIAbsolute, client.ID, redirectURI))
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if !client.Public && parsedURL.Scheme != schemeHTTPS && parsedURL.Scheme != schemeHTTP {
|
|
||||||
validator.Push(fmt.Errorf(errFmtOIDCClientRedirectURI, client.ID, redirectURI, parsedURL.Scheme))
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -124,13 +124,12 @@ func TestShouldRaiseErrorWhenOIDCCORSOriginsHasInvalidValues(t *testing.T) {
|
||||||
|
|
||||||
ValidateIdentityProviders(config, validator)
|
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()[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()[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()[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()[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()[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)
|
require.Len(t, config.OIDC.CORS.AllowedOrigins, 6)
|
||||||
assert.Equal(t, "*", config.OIDC.CORS.AllowedOrigins[3].String())
|
assert.Equal(t, "*", config.OIDC.CORS.AllowedOrigins[3].String())
|
||||||
|
@ -749,6 +748,7 @@ func TestValidateOIDCClientRedirectURIsSupportingPrivateUseURISchemes(t *testing
|
||||||
"oc://ios.owncloud.com",
|
"oc://ios.owncloud.com",
|
||||||
// example given in the RFC https://datatracker.ietf.org/doc/html/rfc8252#section-7.1
|
// example given in the RFC https://datatracker.ietf.org/doc/html/rfc8252#section-7.1
|
||||||
"com.example.app:/oauth2redirect/example-provider",
|
"com.example.app:/oauth2redirect/example-provider",
|
||||||
|
oauth2InstalledApp,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -767,10 +767,9 @@ func TestValidateOIDCClientRedirectURIsSupportingPrivateUseURISchemes(t *testing
|
||||||
validateOIDCClientRedirectURIs(conf, validator)
|
validateOIDCClientRedirectURIs(conf, validator)
|
||||||
|
|
||||||
assert.Len(t, validator.Warnings(), 0)
|
assert.Len(t, validator.Warnings(), 0)
|
||||||
assert.Len(t, validator.Errors(), 2)
|
assert.Len(t, validator.Errors(), 1)
|
||||||
assert.ElementsMatch(t, validator.Errors(), []error{
|
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 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"),
|
||||||
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"),
|
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue