diff options
Diffstat (limited to 'internal/configuration')
34 files changed, 1833 insertions, 462 deletions
diff --git a/internal/configuration/config.template.yml b/internal/configuration/config.template.yml index 1058fecc1..077237358 100644 --- a/internal/configuration/config.template.yml +++ b/internal/configuration/config.template.yml @@ -1480,12 +1480,6 @@ notifier: # - email # - profile - ## Grant Types configures which grants this client can obtain. - ## It's not recommended to define this unless you know what you're doing. - # grant_types: - # - refresh_token - # - authorization_code - ## Response Types configures which responses this client can be sent. ## It's not recommended to define this unless you know what you're doing. # response_types: @@ -1495,7 +1489,14 @@ notifier: # response_modes: # - form_post # - query - # - fragment + + ## Grant Types configures which grants this client can obtain. + ## It's not recommended to define this unless you know what you're doing. + # grant_types: + # - authorization_code + + ## The permitted client authentication method for the Token Endpoint for this client. + # token_endpoint_auth_method: client_secret_basic ## The policy to require for this client; one_factor or two_factor. # authorization_policy: two_factor diff --git a/internal/configuration/schema/identity_providers.go b/internal/configuration/schema/identity_providers.go index 57376dc87..d253a4d07 100644 --- a/internal/configuration/schema/identity_providers.go +++ b/internal/configuration/schema/identity_providers.go @@ -64,6 +64,8 @@ type OpenIDConnectClientConfiguration struct { ResponseTypes []string `koanf:"response_types"` ResponseModes []string `koanf:"response_modes"` + TokenEndpointAuthMethod string `koanf:"token_endpoint_auth_method"` + Policy string `koanf:"authorization_policy"` EnforcePAR bool `koanf:"enforce_par"` @@ -91,9 +93,8 @@ var defaultOIDCClientConsentPreConfiguredDuration = time.Hour * 24 * 7 var DefaultOpenIDConnectClientConfiguration = OpenIDConnectClientConfiguration{ Policy: "two_factor", Scopes: []string{"openid", "groups", "profile", "email"}, - GrantTypes: []string{"refresh_token", "authorization_code"}, ResponseTypes: []string{"code"}, - ResponseModes: []string{"form_post", "query", "fragment"}, + ResponseModes: []string{"form_post"}, UserinfoSigningAlgorithm: "none", ConsentMode: "auto", diff --git a/internal/configuration/schema/keys.go b/internal/configuration/schema/keys.go index 526913918..85923eb1a 100644 --- a/internal/configuration/schema/keys.go +++ b/internal/configuration/schema/keys.go @@ -45,6 +45,7 @@ var Keys = []string{ "identity_providers.oidc.clients[].grant_types", "identity_providers.oidc.clients[].response_types", "identity_providers.oidc.clients[].response_modes", + "identity_providers.oidc.clients[].token_endpoint_auth_method", "identity_providers.oidc.clients[].authorization_policy", "identity_providers.oidc.clients[].enforce_par", "identity_providers.oidc.clients[].enforce_pkce", diff --git a/internal/configuration/validator/access_control.go b/internal/configuration/validator/access_control.go index 994d7559c..93f1efa4c 100644 --- a/internal/configuration/validator/access_control.go +++ b/internal/configuration/validator/access_control.go @@ -59,7 +59,7 @@ func ValidateAccessControl(config *schema.Configuration, validator *schema.Struc } if !IsPolicyValid(config.AccessControl.DefaultPolicy) { - validator.Push(fmt.Errorf(errFmtAccessControlDefaultPolicyValue, strings.Join(validACLRulePolicies, "', '"), config.AccessControl.DefaultPolicy)) + validator.Push(fmt.Errorf(errFmtAccessControlDefaultPolicyValue, strJoinOr(validACLRulePolicies), config.AccessControl.DefaultPolicy)) } if config.AccessControl.Networks != nil { @@ -92,8 +92,13 @@ func ValidateRules(config *schema.Configuration, validator *schema.StructValidat validateDomains(rulePosition, rule, validator) - if !IsPolicyValid(rule.Policy) { - validator.Push(fmt.Errorf(errFmtAccessControlRuleInvalidPolicy, ruleDescriptor(rulePosition, rule), rule.Policy)) + switch rule.Policy { + case "": + validator.Push(fmt.Errorf(errFmtAccessControlRuleNoPolicy, ruleDescriptor(rulePosition, rule))) + default: + if !IsPolicyValid(rule.Policy) { + validator.Push(fmt.Errorf(errFmtAccessControlRuleInvalidPolicy, ruleDescriptor(rulePosition, rule), strJoinOr(validACLRulePolicies), rule.Policy)) + } } validateNetworks(rulePosition, rule, config.AccessControl, validator) @@ -156,10 +161,14 @@ func validateSubjects(rulePosition int, rule schema.ACLRule, validator *schema.S } func validateMethods(rulePosition int, rule schema.ACLRule, validator *schema.StructValidator) { - for _, method := range rule.Methods { - if !utils.IsStringInSliceFold(method, validACLHTTPMethodVerbs) { - validator.Push(fmt.Errorf(errFmtAccessControlRuleMethodInvalid, ruleDescriptor(rulePosition, rule), method, strings.Join(validACLHTTPMethodVerbs, "', '"))) - } + invalid, duplicates := validateList(rule.Methods, validACLHTTPMethodVerbs, true) + + if len(invalid) != 0 { + validator.Push(fmt.Errorf(errFmtAccessControlRuleInvalidEntries, ruleDescriptor(rulePosition, rule), "methods", strJoinOr(validACLHTTPMethodVerbs), strJoinAnd(invalid))) + } + + if len(duplicates) != 0 { + validator.Push(fmt.Errorf(errFmtAccessControlRuleInvalidDuplicates, ruleDescriptor(rulePosition, rule), "methods", strJoinAnd(duplicates))) } } @@ -177,7 +186,7 @@ func validateQuery(i int, rule schema.ACLRule, config *schema.Configuration, val } } } else if !utils.IsStringInSliceFold(config.AccessControl.Rules[i].Query[j][k].Operator, validACLRuleOperators) { - validator.Push(fmt.Errorf(errFmtAccessControlRuleQueryInvalid, ruleDescriptor(i+1, rule), config.AccessControl.Rules[i].Query[j][k].Operator, strings.Join(validACLRuleOperators, "', '"))) + validator.Push(fmt.Errorf(errFmtAccessControlRuleQueryInvalid, ruleDescriptor(i+1, rule), strJoinOr(validACLRuleOperators), config.AccessControl.Rules[i].Query[j][k].Operator)) } if config.AccessControl.Rules[i].Query[j][k].Key == "" { diff --git a/internal/configuration/validator/access_control_test.go b/internal/configuration/validator/access_control_test.go index 0671455a1..1543959e0 100644 --- a/internal/configuration/validator/access_control_test.go +++ b/internal/configuration/validator/access_control_test.go @@ -58,7 +58,7 @@ func (suite *AccessControl) TestShouldValidateEitherDomainsOrDomainsRegex() { suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - assert.EqualError(suite.T(), suite.validator.Errors()[0], "access control: rule #3: rule is invalid: must have the option 'domain' or 'domain_regex' configured") + assert.EqualError(suite.T(), suite.validator.Errors()[0], "access control: rule #3: option 'domain' or 'domain_regex' must be present but are both absent") } func (suite *AccessControl) TestShouldRaiseErrorInvalidDefaultPolicy() { @@ -69,7 +69,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidDefaultPolicy() { suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "access control: option 'default_policy' must be one of 'bypass', 'one_factor', 'two_factor', 'deny' but it is configured as 'invalid'") + suite.Assert().EqualError(suite.validator.Errors()[0], "access control: option 'default_policy' must be one of 'bypass', 'one_factor', 'two_factor', or 'deny' but it's configured as 'invalid'") } func (suite *AccessControl) TestShouldRaiseErrorInvalidNetworkGroupNetwork() { @@ -141,10 +141,10 @@ func (suite *AccessControl) TestShouldRaiseErrorsWithEmptyRules() { suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 4) - suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1: rule is invalid: must have the option 'domain' or 'domain_regex' configured") - suite.Assert().EqualError(suite.validator.Errors()[1], "access control: rule #1: rule 'policy' option '' is invalid: must be one of 'deny', 'two_factor', 'one_factor' or 'bypass'") - suite.Assert().EqualError(suite.validator.Errors()[2], "access control: rule #2: rule is invalid: must have the option 'domain' or 'domain_regex' configured") - suite.Assert().EqualError(suite.validator.Errors()[3], "access control: rule #2: rule 'policy' option 'wrong' is invalid: must be one of 'deny', 'two_factor', 'one_factor' or 'bypass'") + suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1: option 'domain' or 'domain_regex' must be present but are both absent") + suite.Assert().EqualError(suite.validator.Errors()[1], "access control: rule #1: option 'policy' must be present but it's absent") + suite.Assert().EqualError(suite.validator.Errors()[2], "access control: rule #2: option 'domain' or 'domain_regex' must be present but are both absent") + suite.Assert().EqualError(suite.validator.Errors()[3], "access control: rule #2: option 'policy' must be one of 'bypass', 'one_factor', 'two_factor', or 'deny' but it's configured as 'wrong'") } func (suite *AccessControl) TestShouldRaiseErrorInvalidPolicy() { @@ -160,7 +160,7 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidPolicy() { suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1 (domain 'public.example.com'): rule 'policy' option 'invalid' is invalid: must be one of 'deny', 'two_factor', 'one_factor' or 'bypass'") + suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1 (domain 'public.example.com'): option 'policy' must be one of 'bypass', 'one_factor', 'two_factor', or 'deny' but it's configured as 'invalid'") } func (suite *AccessControl) TestShouldRaiseErrorInvalidNetwork() { @@ -194,7 +194,24 @@ func (suite *AccessControl) TestShouldRaiseErrorInvalidMethod() { suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1 (domain 'public.example.com'): 'methods' option 'HOP' is invalid: must be one of 'GET', 'HEAD', 'POST', 'PUT', 'PATCH', 'DELETE', 'TRACE', 'CONNECT', 'OPTIONS', 'COPY', 'LOCK', 'MKCOL', 'MOVE', 'PROPFIND', 'PROPPATCH', 'UNLOCK'") + suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1 (domain 'public.example.com'): option 'methods' must only have the values 'GET', 'HEAD', 'POST', 'PUT', 'PATCH', 'DELETE', 'TRACE', 'CONNECT', 'OPTIONS', 'COPY', 'LOCK', 'MKCOL', 'MOVE', 'PROPFIND', 'PROPPATCH', or 'UNLOCK' but the values 'HOP' are present") +} + +func (suite *AccessControl) TestShouldRaiseErrorDuplicateMethod() { + suite.config.AccessControl.Rules = []schema.ACLRule{ + { + Domains: []string{"public.example.com"}, + Policy: "bypass", + Methods: []string{"GET", "GET"}, + }, + } + + ValidateRules(suite.config, suite.validator) + + suite.Assert().Len(suite.validator.Warnings(), 0) + suite.Require().Len(suite.validator.Errors(), 1) + + suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1 (domain 'public.example.com'): option 'methods' must have unique values but the values 'GET' are duplicated") } func (suite *AccessControl) TestShouldRaiseErrorInvalidSubject() { @@ -367,13 +384,13 @@ func (suite *AccessControl) TestShouldErrorOnInvalidRulesQuery() { suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 7) - suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1 (domain 'public.example.com'): 'query' option 'value' is invalid: must have a value when the operator is 'equal'") - suite.Assert().EqualError(suite.validator.Errors()[1], "access control: rule #2 (domain 'public.example.com'): 'query' option 'key' is invalid: must have a value") - suite.Assert().EqualError(suite.validator.Errors()[2], "access control: rule #5 (domain 'public.example.com'): 'query' option 'key' is invalid: must have a value") - suite.Assert().EqualError(suite.validator.Errors()[3], "access control: rule #6 (domain 'public.example.com'): 'query' option 'operator' with value 'not' is invalid: must be one of 'present', 'absent', 'equal', 'not equal', 'pattern', 'not pattern'") - suite.Assert().EqualError(suite.validator.Errors()[4], "access control: rule #7 (domain 'public.example.com'): 'query' option 'value' is invalid: error parsing regexp: missing closing ): `(bad pattern`") - suite.Assert().EqualError(suite.validator.Errors()[5], "access control: rule #8 (domain 'public.example.com'): 'query' option 'value' is invalid: must not have a value when the operator is 'present'") - suite.Assert().EqualError(suite.validator.Errors()[6], "access control: rule #9 (domain 'public.example.com'): 'query' option 'value' is invalid: expected type was string but got int") + suite.Assert().EqualError(suite.validator.Errors()[0], "access control: rule #1 (domain 'public.example.com'): query: option 'value' must be present when the option 'operator' is 'equal' but it's absent") + suite.Assert().EqualError(suite.validator.Errors()[1], "access control: rule #2 (domain 'public.example.com'): query: option 'key' is required but it's absent") + suite.Assert().EqualError(suite.validator.Errors()[2], "access control: rule #5 (domain 'public.example.com'): query: option 'key' is required but it's absent") + suite.Assert().EqualError(suite.validator.Errors()[3], "access control: rule #6 (domain 'public.example.com'): query: option 'operator' must be one of 'present', 'absent', 'equal', 'not equal', 'pattern', or 'not pattern' but it's configured as 'not'") + suite.Assert().EqualError(suite.validator.Errors()[4], "access control: rule #7 (domain 'public.example.com'): query: option 'value' is invalid: error parsing regexp: missing closing ): `(bad pattern`") + suite.Assert().EqualError(suite.validator.Errors()[5], "access control: rule #8 (domain 'public.example.com'): query: option 'value' must not be present when the option 'operator' is 'present' but it's present") + suite.Assert().EqualError(suite.validator.Errors()[6], "access control: rule #9 (domain 'public.example.com'): query: option 'value' is invalid: expected type was string but got int") } func TestAccessControl(t *testing.T) { diff --git a/internal/configuration/validator/authentication.go b/internal/configuration/validator/authentication.go index bcd64fabf..fb209f179 100644 --- a/internal/configuration/validator/authentication.go +++ b/internal/configuration/validator/authentication.go @@ -71,7 +71,7 @@ func ValidatePasswordConfiguration(config *schema.Password, validator *schema.St case utils.IsStringInSlice(config.Algorithm, validHashAlgorithms): break default: - validator.Push(fmt.Errorf(errFmtFileAuthBackendPasswordUnknownAlg, config.Algorithm, strings.Join(validHashAlgorithms, "', '"))) + validator.Push(fmt.Errorf(errFmtFileAuthBackendPasswordUnknownAlg, strJoinOr(validHashAlgorithms), config.Algorithm)) } validateFileAuthenticationBackendPasswordConfigArgon2(config, validator) @@ -89,7 +89,7 @@ func validateFileAuthenticationBackendPasswordConfigArgon2(config *schema.Passwo case utils.IsStringInSlice(config.Argon2.Variant, validArgon2Variants): break default: - validator.Push(fmt.Errorf(errFmtFileAuthBackendPasswordInvalidVariant, hashArgon2, config.Argon2.Variant, strings.Join(validArgon2Variants, "', '"))) + validator.Push(fmt.Errorf(errFmtFileAuthBackendPasswordInvalidVariant, hashArgon2, strJoinOr(validArgon2Variants), config.Argon2.Variant)) } switch { @@ -147,7 +147,7 @@ func validateFileAuthenticationBackendPasswordConfigSHA2Crypt(config *schema.Pas case utils.IsStringInSlice(config.SHA2Crypt.Variant, validSHA2CryptVariants): break default: - validator.Push(fmt.Errorf(errFmtFileAuthBackendPasswordInvalidVariant, hashSHA2Crypt, config.SHA2Crypt.Variant, strings.Join(validSHA2CryptVariants, "', '"))) + validator.Push(fmt.Errorf(errFmtFileAuthBackendPasswordInvalidVariant, hashSHA2Crypt, strJoinOr(validSHA2CryptVariants), config.SHA2Crypt.Variant)) } switch { @@ -176,7 +176,7 @@ func validateFileAuthenticationBackendPasswordConfigPBKDF2(config *schema.Passwo case utils.IsStringInSlice(config.PBKDF2.Variant, validPBKDF2Variants): break default: - validator.Push(fmt.Errorf(errFmtFileAuthBackendPasswordInvalidVariant, hashPBKDF2, config.PBKDF2.Variant, strings.Join(validPBKDF2Variants, "', '"))) + validator.Push(fmt.Errorf(errFmtFileAuthBackendPasswordInvalidVariant, hashPBKDF2, strJoinOr(validPBKDF2Variants), config.PBKDF2.Variant)) } switch { @@ -205,7 +205,7 @@ func validateFileAuthenticationBackendPasswordConfigBCrypt(config *schema.Passwo case utils.IsStringInSlice(config.BCrypt.Variant, validBCryptVariants): break default: - validator.Push(fmt.Errorf(errFmtFileAuthBackendPasswordInvalidVariant, hashBCrypt, config.BCrypt.Variant, strings.Join(validBCryptVariants, "', '"))) + validator.Push(fmt.Errorf(errFmtFileAuthBackendPasswordInvalidVariant, hashBCrypt, strJoinOr(validBCryptVariants), config.BCrypt.Variant)) } switch { @@ -369,7 +369,7 @@ func validateLDAPAuthenticationBackendImplementation(config *schema.Authenticati case schema.LDAPImplementationGLAuth: implementation = &schema.DefaultLDAPAuthenticationBackendConfigurationImplementationGLAuth default: - validator.Push(fmt.Errorf(errFmtLDAPAuthBackendImplementation, config.LDAP.Implementation, strings.Join(validLDAPImplementations, "', '"))) + validator.Push(fmt.Errorf(errFmtLDAPAuthBackendImplementation, strJoinOr(validLDAPImplementations), config.LDAP.Implementation)) } tlsconfig := &schema.TLSConfig{} diff --git a/internal/configuration/validator/authentication_test.go b/internal/configuration/validator/authentication_test.go index cc540f064..9abec6e1a 100644 --- a/internal/configuration/validator/authentication_test.go +++ b/internal/configuration/validator/authentication_test.go @@ -256,7 +256,7 @@ func (suite *FileBasedAuthenticationBackend) TestShouldRaiseErrorOnInvalidArgon2 suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: file: password: argon2: option 'variant' is configured as 'invalid' but must be one of the following values: 'argon2id', 'id', 'argon2i', 'i', 'argon2d', 'd'") + suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: file: password: argon2: option 'variant' must be one of 'argon2id', 'id', 'argon2i', 'i', 'argon2d', or 'd' but it's configured as 'invalid'") } func (suite *FileBasedAuthenticationBackend) TestShouldRaiseErrorOnInvalidSHA2CryptVariant() { @@ -270,7 +270,7 @@ func (suite *FileBasedAuthenticationBackend) TestShouldRaiseErrorOnInvalidSHA2Cr suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: file: password: sha2crypt: option 'variant' is configured as 'invalid' but must be one of the following values: 'sha256', 'sha512'") + suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: file: password: sha2crypt: option 'variant' must be one of 'sha256' or 'sha512' but it's configured as 'invalid'") } func (suite *FileBasedAuthenticationBackend) TestShouldRaiseErrorOnInvalidSHA2CryptSaltLength() { @@ -298,7 +298,7 @@ func (suite *FileBasedAuthenticationBackend) TestShouldRaiseErrorOnInvalidPBKDF2 suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: file: password: pbkdf2: option 'variant' is configured as 'invalid' but must be one of the following values: 'sha1', 'sha224', 'sha256', 'sha384', 'sha512'") + suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: file: password: pbkdf2: option 'variant' must be one of 'sha1', 'sha224', 'sha256', 'sha384', or 'sha512' but it's configured as 'invalid'") } func (suite *FileBasedAuthenticationBackend) TestShouldRaiseErrorOnInvalidBCryptVariant() { @@ -312,7 +312,7 @@ func (suite *FileBasedAuthenticationBackend) TestShouldRaiseErrorOnInvalidBCrypt suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: file: password: bcrypt: option 'variant' is configured as 'invalid' but must be one of the following values: 'standard', 'sha256'") + suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: file: password: bcrypt: option 'variant' must be one of 'standard' or 'sha256' but it's configured as 'invalid'") } func (suite *FileBasedAuthenticationBackend) TestShouldRaiseErrorWhenSHA2CryptOptionsTooLow() { @@ -497,7 +497,7 @@ func (suite *FileBasedAuthenticationBackend) TestShouldRaiseErrorWhenBadAlgorith suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: file: password: option 'algorithm' is configured as 'bogus' but must be one of the following values: 'sha2crypt', 'pbkdf2', 'scrypt', 'bcrypt', 'argon2'") + suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: file: password: option 'algorithm' must be one of 'sha2crypt', 'pbkdf2', 'scrypt', 'bcrypt', or 'argon2' but it's configured as 'bogus'") } func (suite *FileBasedAuthenticationBackend) TestShouldSetDefaultValues() { @@ -609,7 +609,7 @@ func (suite *LDAPAuthenticationBackendSuite) TestShouldRaiseErrorWhenImplementat suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: ldap: option 'implementation' is configured as 'masd' but must be one of the following values: 'custom', 'activedirectory', 'rfc2307bis', 'freeipa', 'lldap', 'glauth'") + suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: ldap: option 'implementation' must be one of 'custom', 'activedirectory', 'rfc2307bis', 'freeipa', 'lldap', or 'glauth' but it's configured as 'masd'") } func (suite *LDAPAuthenticationBackendSuite) TestShouldRaiseErrorWhenURLNotProvided() { @@ -755,7 +755,7 @@ func (suite *LDAPAuthenticationBackendSuite) TestShouldRaiseErrorOnBadFilterPlac suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: ldap: option 'users_filter' has an invalid placeholder: '{0}' has been removed, please use '{input}' instead") suite.Assert().EqualError(suite.validator.Errors()[1], "authentication_backend: ldap: option 'groups_filter' has an invalid placeholder: '{0}' has been removed, please use '{input}' instead") suite.Assert().EqualError(suite.validator.Errors()[2], "authentication_backend: ldap: option 'groups_filter' has an invalid placeholder: '{1}' has been removed, please use '{username}' instead") - suite.Assert().EqualError(suite.validator.Errors()[3], "authentication_backend: ldap: option 'users_filter' must contain the placeholder '{input}' but it is required") + suite.Assert().EqualError(suite.validator.Errors()[3], "authentication_backend: ldap: option 'users_filter' must contain the placeholder '{input}' but it's absent") } func (suite *LDAPAuthenticationBackendSuite) TestShouldSetDefaultGroupNameAttribute() { @@ -823,7 +823,7 @@ func (suite *LDAPAuthenticationBackendSuite) TestShouldRaiseWhenUsersFilterDoesN suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: ldap: option 'users_filter' must contain the placeholder '{username_attribute}' but it is required") + suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: ldap: option 'users_filter' must contain the placeholder '{username_attribute}' but it's absent") } func (suite *LDAPAuthenticationBackendSuite) TestShouldHelpDetectNoInputPlaceholder() { @@ -834,7 +834,7 @@ func (suite *LDAPAuthenticationBackendSuite) TestShouldHelpDetectNoInputPlacehol suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: ldap: option 'users_filter' must contain the placeholder '{input}' but it is required") + suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: ldap: option 'users_filter' must contain the placeholder '{input}' but it's absent") } func (suite *LDAPAuthenticationBackendSuite) TestShouldSetDefaultTLSMinimumVersion() { @@ -986,7 +986,7 @@ func (suite *ActiveDirectoryAuthenticationBackendSuite) TestShouldRaiseErrorOnIn validateLDAPAuthenticationBackendURL(suite.config.LDAP, suite.validator) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: ldap: option 'url' must have either the 'ldap' or 'ldaps' scheme but it is configured as 'http'") + suite.Assert().EqualError(suite.validator.Errors()[0], "authentication_backend: ldap: option 'url' must have either the 'ldap' or 'ldaps' scheme but it's configured as 'http'") } func (suite *ActiveDirectoryAuthenticationBackendSuite) TestShouldRaiseErrorOnInvalidURLWithBadCharacters() { diff --git a/internal/configuration/validator/configuration.go b/internal/configuration/validator/configuration.go index 13045b86a..874e0809b 100644 --- a/internal/configuration/validator/configuration.go +++ b/internal/configuration/validator/configuration.go @@ -78,7 +78,7 @@ func validateDefault2FAMethod(config *schema.Configuration, validator *schema.St } if !utils.IsStringInSlice(config.Default2FAMethod, validDefault2FAMethods) { - validator.Push(fmt.Errorf(errFmtInvalidDefault2FAMethod, config.Default2FAMethod, strings.Join(validDefault2FAMethods, "', '"))) + validator.Push(fmt.Errorf(errFmtInvalidDefault2FAMethod, strJoinOr(validDefault2FAMethods), config.Default2FAMethod)) return } @@ -98,6 +98,6 @@ func validateDefault2FAMethod(config *schema.Configuration, validator *schema.St } if !utils.IsStringInSlice(config.Default2FAMethod, enabledMethods) { - validator.Push(fmt.Errorf(errFmtInvalidDefault2FAMethodDisabled, config.Default2FAMethod, strings.Join(enabledMethods, "', '"))) + validator.Push(fmt.Errorf(errFmtInvalidDefault2FAMethodDisabled, strJoinOr(enabledMethods), config.Default2FAMethod)) } } diff --git a/internal/configuration/validator/configuration_test.go b/internal/configuration/validator/configuration_test.go index 77e35c3b7..7fee1355e 100644 --- a/internal/configuration/validator/configuration_test.go +++ b/internal/configuration/validator/configuration_test.go @@ -221,7 +221,7 @@ func TestValidateDefault2FAMethod(t *testing.T) { TOTP: schema.TOTPConfiguration{Disable: true}, }, expectedErrs: []string{ - "option 'default_2fa_method' is configured as 'totp' but must be one of the following enabled method values: 'webauthn', 'mobile_push'", + "option 'default_2fa_method' must be one of the enabled options 'webauthn' or 'mobile_push' but it's configured as 'totp'", }, }, { @@ -236,7 +236,7 @@ func TestValidateDefault2FAMethod(t *testing.T) { Webauthn: schema.WebauthnConfiguration{Disable: true}, }, expectedErrs: []string{ - "option 'default_2fa_method' is configured as 'webauthn' but must be one of the following enabled method values: 'totp', 'mobile_push'", + "option 'default_2fa_method' must be one of the enabled options 'totp' or 'mobile_push' but it's configured as 'webauthn'", }, }, { @@ -246,7 +246,7 @@ func TestValidateDefault2FAMethod(t *testing.T) { DuoAPI: schema.DuoAPIConfiguration{Disable: true}, }, expectedErrs: []string{ - "option 'default_2fa_method' is configured as 'mobile_push' but must be one of the following enabled method values: 'totp', 'webauthn'", + "option 'default_2fa_method' must be one of the enabled options 'totp' or 'webauthn' but it's configured as 'mobile_push'", }, }, { @@ -255,7 +255,7 @@ func TestValidateDefault2FAMethod(t *testing.T) { Default2FAMethod: "duo", }, expectedErrs: []string{ - "option 'default_2fa_method' is configured as 'duo' but must be one of the following values: 'totp', 'webauthn', 'mobile_push'", + "option 'default_2fa_method' must be one of 'totp', 'webauthn', or 'mobile_push' but it's configured as 'duo'", }, }, } diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go index 44ac2622b..5f697f7e8 100644 --- a/internal/configuration/validator/const.go +++ b/internal/configuration/validator/const.go @@ -67,7 +67,7 @@ const ( ) const ( - errSuffixMustBeOneOf = "is configured as '%s' but must be one of the following values: '%s'" + errSuffixMustBeOneOf = "must be one of %s but it's configured as '%s'" ) // Authentication Backend Error constants. @@ -105,19 +105,19 @@ const ( errFmtLDAPAuthBackendURLNotParsable = "authentication_backend: ldap: option " + "'url' could not be parsed: %w" errFmtLDAPAuthBackendURLInvalidScheme = "authentication_backend: ldap: option " + - "'url' must have either the 'ldap' or 'ldaps' scheme but it is configured as '%s'" + "'url' must have either the 'ldap' or 'ldaps' scheme but it's configured as '%s'" errFmtLDAPAuthBackendFilterEnclosingParenthesis = "authentication_backend: ldap: option " + "'%s' must contain enclosing parenthesis: '%s' should probably be '(%s)'" errFmtLDAPAuthBackendFilterMissingPlaceholder = "authentication_backend: ldap: option " + - "'%s' must contain the placeholder '{%s}' but it is required" + "'%s' must contain the placeholder '{%s}' but it's absent" ) // TOTP Error constants. const ( - errFmtTOTPInvalidAlgorithm = "totp: option 'algorithm' must be one of '%s' but it is configured as '%s'" - errFmtTOTPInvalidPeriod = "totp: option 'period' option must be 15 or more but it is configured as '%d'" - errFmtTOTPInvalidDigits = "totp: option 'digits' must be 6 or 8 but it is configured as '%d'" - errFmtTOTPInvalidSecretSize = "totp: option 'secret_size' must be %d or higher but it is configured as '%d'" //nolint:gosec + errFmtTOTPInvalidAlgorithm = "totp: option 'algorithm' must be one of %s but it's configured as '%s'" + errFmtTOTPInvalidPeriod = "totp: option 'period' option must be 15 or more but it's configured as '%d'" + errFmtTOTPInvalidDigits = "totp: option 'digits' must be 6 or 8 but it's configured as '%d'" + errFmtTOTPInvalidSecretSize = "totp: option 'secret_size' must be %d or higher but it's configured as '%d'" //nolint:gosec ) // Storage Error constants. @@ -128,14 +128,14 @@ const ( errFmtStorageUserPassMustBeProvided = "storage: %s: option 'username' and 'password' are required" //nolint:gosec errFmtStorageOptionMustBeProvided = "storage: %s: option '%s' is required" errFmtStorageTLSConfigInvalid = "storage: %s: tls: %w" - errFmtStoragePostgreSQLInvalidSSLMode = "storage: postgres: ssl: option 'mode' must be one of '%s' but it is configured as '%s'" + errFmtStoragePostgreSQLInvalidSSLMode = "storage: postgres: ssl: option 'mode' must be one of %s but it's configured as '%s'" errFmtStoragePostgreSQLInvalidSSLAndTLSConfig = "storage: postgres: can't define both 'tls' and 'ssl' configuration options" warnFmtStoragePostgreSQLInvalidSSLDeprecated = "storage: postgres: ssl: the ssl configuration options are deprecated and we recommend the tls options instead" ) // Telemetry Error constants. const ( - errFmtTelemetryMetricsScheme = "telemetry: metrics: option 'address' must have a scheme 'tcp://' but it is configured as '%s'" + errFmtTelemetryMetricsScheme = "telemetry: metrics: option 'address' must have a scheme 'tcp://' but it's configured as '%s'" ) // OpenID Error constants. @@ -148,17 +148,16 @@ const ( errFmtOIDCCertificateMismatch = "identity_providers: oidc: option 'issuer_private_key' does not appear to be the private key the certificate provided by option 'issuer_certificate_chain'" errFmtOIDCCertificateChain = "identity_providers: oidc: option 'issuer_certificate_chain' produced an error during validation of the chain: %w" errFmtOIDCEnforcePKCEInvalidValue = "identity_providers: oidc: option 'enforce_pkce' must be 'never', " + - "'public_clients_only' or 'always', but it is configured as '%s'" + "'public_clients_only' or 'always', but it's configured as '%s'" errFmtOIDCCORSInvalidOrigin = "identity_providers: oidc: cors: option 'allowed_origins' contains an invalid value '%s' as it has a %s: origins must only be scheme, hostname, and an optional port" errFmtOIDCCORSInvalidOriginWildcard = "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" errFmtOIDCCORSInvalidOriginWildcardWithClients = "identity_providers: oidc: cors: option 'allowed_origins' contains the wildcard origin '*' cannot be specified with option 'allowed_origins_from_client_redirect_uris' enabled" - errFmtOIDCCORSInvalidEndpoint = "identity_providers: oidc: cors: option 'endpoints' contains an invalid value '%s': must be one of '%s'" + errFmtOIDCCORSInvalidEndpoint = "identity_providers: oidc: cors: option 'endpoints' contains an invalid value '%s': must be one of %s" - errFmtOIDCClientsDuplicateID = "identity_providers: oidc: one or more clients have the same id but all client" + - "id's must be unique" - errFmtOIDCClientsWithEmptyID = "identity_providers: oidc: one or more clients have been configured with " + - "an empty id" + errFmtOIDCClientsDuplicateID = "identity_providers: oidc: clients: option 'id' must be unique for every client but one or more clients share the following 'id' values %s" + errFmtOIDCClientsWithEmptyID = "identity_providers: oidc: clients: option 'id' is required but was absent on the clients in positions %s" + errFmtOIDCClientsDeprecated = "identity_providers: oidc: clients: warnings for clients above indicate deprecated functionality and it's strongly suggested these issues are checked and fixed if they're legitimate issues or reported if they are not as in a future version these warnings will become errors" errFmtOIDCClientInvalidSecret = "identity_providers: oidc: client '%s': option 'secret' is required" errFmtOIDCClientInvalidSecretPlainText = "identity_providers: oidc: client '%s': option 'secret' is plaintext but it should be a hashed value as plaintext values are deprecated and will be removed when oidc becomes stable" @@ -170,36 +169,43 @@ const ( "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 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'" - errFmtOIDCClientInvalidPKCEChallengeMethod = "identity_providers: oidc: client '%s': option 'pkce_challenge_method' must be 'plain' " + - "or 'S256' but it is configured as '%s'" + "invalid value: redirect uri '%s' must have a scheme but it's absent" errFmtOIDCClientInvalidConsentMode = "identity_providers: oidc: client '%s': consent: option 'mode' must be one of " + - "'%s' but it is configured as '%s'" - errFmtOIDCClientInvalidEntry = "identity_providers: oidc: client '%s': option '%s' must only have the values " + - "'%s' but one option is configured as '%s'" - errFmtOIDCClientInvalidUserinfoAlgorithm = "identity_providers: oidc: client '%s': option " + - "'userinfo_signing_algorithm' must be one of '%s' but it is configured as '%s'" + "%s but it's configured as '%s'" + errFmtOIDCClientInvalidEntries = "identity_providers: oidc: client '%s': option '%s' must only have the values " + + "%s but the values %s are present" + errFmtOIDCClientInvalidEntryDuplicates = "identity_providers: oidc: client '%s': option '%s' must have unique values but the values %s are duplicated" + errFmtOIDCClientInvalidValue = "identity_providers: oidc: client '%s': option " + + "'%s' must be one of %s but it's configured as '%s'" + errFmtOIDCClientInvalidTokenEndpointAuthMethod = "identity_providers: oidc: client '%s': option " + + "'token_endpoint_auth_method' must be one of %s when configured as the confidential client type unless it only includes implicit flow response types such as %s but it's configured as '%s'" + errFmtOIDCClientInvalidTokenEndpointAuthMethodPublic = "identity_providers: oidc: client '%s': option " + + "'token_endpoint_auth_method' must be 'none' when configured as the public client type but it's configured as '%s'" errFmtOIDCClientInvalidSectorIdentifier = "identity_providers: oidc: client '%s': option " + "'sector_identifier' with value '%s': must be a URL with only the host component for example '%s' but it has a %s with the value '%s'" errFmtOIDCClientInvalidSectorIdentifierWithoutValue = "identity_providers: oidc: client '%s': option " + "'sector_identifier' with value '%s': must be a URL with only the host component for example '%s' but it has a %s" errFmtOIDCClientInvalidSectorIdentifierHost = "identity_providers: oidc: client '%s': option " + "'sector_identifier' with value '%s': must be a URL with only the host component but appears to be invalid" + errFmtOIDCClientInvalidGrantTypeMatch = "identity_providers: oidc: client '%s': option " + + "'grant_types' should only have grant type values which are valid with the configured 'response_types' for the client but '%s' expects a response type %s such as %s but the response types are %s" + errFmtOIDCClientInvalidGrantTypeRefresh = "identity_providers: oidc: client '%s': option " + + "'grant_types' should only have the 'refresh_token' value if the client is also configured with the 'offline_access' scope" + errFmtOIDCClientInvalidRefreshTokenOptionWithoutCodeResponseType = "identity_providers: oidc: client '%s': option " + + "'%s' should only have the values %s if the client is also configured with a 'response_type' such as %s which respond with authorization codes" errFmtOIDCServerInsecureParameterEntropy = "openid connect provider: SECURITY ISSUE - minimum parameter entropy is " + "configured to an unsafe value, it should be above 8 but it's configured to %d" ) // Webauthn Error constants. const ( - errFmtWebauthnConveyancePreference = "webauthn: option 'attestation_conveyance_preference' must be one of '%s' but it is configured as '%s'" - errFmtWebauthnUserVerification = "webauthn: option 'user_verification' must be one of 'discouraged', 'preferred', 'required' but it is configured as '%s'" + errFmtWebauthnConveyancePreference = "webauthn: option 'attestation_conveyance_preference' must be one of %s but it's configured as '%s'" + errFmtWebauthnUserVerification = "webauthn: option 'user_verification' must be one of %s but it's configured as '%s'" ) // Access Control error constants. const ( - errFmtAccessControlDefaultPolicyValue = "access control: option 'default_policy' must be one of '%s' but it is " + + errFmtAccessControlDefaultPolicyValue = "access control: option 'default_policy' must be one of %s but it's " + "configured as '%s'" errFmtAccessControlDefaultPolicyWithoutRules = "access control: 'default_policy' option '%s' is invalid: when " + "no rules are specified it must be 'two_factor' or 'one_factor'" @@ -207,10 +213,9 @@ const ( "network '%s' is not a valid IP or CIDR notation" errFmtAccessControlWarnNoRulesDefaultPolicy = "access control: no rules have been specified so the " + "'default_policy' of '%s' is going to be applied to all requests" - errFmtAccessControlRuleNoDomains = "access control: rule %s: rule is invalid: must have the option " + - "'domain' or 'domain_regex' configured" - errFmtAccessControlRuleInvalidPolicy = "access control: rule %s: rule 'policy' option '%s' " + - "is invalid: must be one of 'deny', 'two_factor', 'one_factor' or 'bypass'" + errFmtAccessControlRuleNoDomains = "access control: rule %s: option 'domain' or 'domain_regex' must be present but are both absent" + errFmtAccessControlRuleNoPolicy = "access control: rule %s: option 'policy' must be present but it's absent" + errFmtAccessControlRuleInvalidPolicy = "access control: rule %s: option 'policy' must be one of %s but it's configured as '%s'" errAccessControlRuleBypassPolicyInvalidWithSubjects = "access control: rule %s: 'policy' option 'bypass' is " + "not supported when 'subject' option is configured: see " + "https://www.authelia.com/c/acl#bypass" @@ -221,39 +226,35 @@ const ( "valid Group Name, IP, or CIDR notation" errFmtAccessControlRuleSubjectInvalid = "access control: rule %s: 'subject' option '%s' is " + "invalid: must start with 'user:' or 'group:'" - errFmtAccessControlRuleMethodInvalid = "access control: rule %s: 'methods' option '%s' is " + - "invalid: must be one of '%s'" - errFmtAccessControlRuleQueryInvalid = "access control: rule %s: 'query' option 'operator' with value '%s' is " + - "invalid: must be one of '%s'" - errFmtAccessControlRuleQueryInvalidNoValue = "access control: rule %s: 'query' option '%s' is " + - "invalid: must have a value" - errFmtAccessControlRuleQueryInvalidNoValueOperator = "access control: rule %s: 'query' option '%s' is " + - "invalid: must have a value when the operator is '%s'" - errFmtAccessControlRuleQueryInvalidValue = "access control: rule %s: 'query' option '%s' is " + - "invalid: must not have a value when the operator is '%s'" - errFmtAccessControlRuleQueryInvalidValueParse = "access control: rule %s: 'query' option '%s' is " + + errFmtAccessControlRuleInvalidEntries = "access control: rule %s: option '%s' must only have the values %s but the values %s are present" + errFmtAccessControlRuleInvalidDuplicates = "access control: rule %s: option '%s' must have unique values but the values %s are duplicated" + errFmtAccessControlRuleQueryInvalid = "access control: rule %s: query: option 'operator' must be one of %s but it's configured as '%s'" + errFmtAccessControlRuleQueryInvalidNoValue = "access control: rule %s: query: option '%s' is required but it's absent" + errFmtAccessControlRuleQueryInvalidNoValueOperator = "access control: rule %s: query: option '%s' must be present when the option 'operator' is '%s' but it's absent" + errFmtAccessControlRuleQueryInvalidValue = "access control: rule %s: query: option '%s' must not be present when the option 'operator' is '%s' but it's present" + errFmtAccessControlRuleQueryInvalidValueParse = "access control: rule %s: query: option '%s' is " + "invalid: %w" - errFmtAccessControlRuleQueryInvalidValueType = "access control: rule %s: 'query' option 'value' is " + + errFmtAccessControlRuleQueryInvalidValueType = "access control: rule %s: query: option 'value' is " + "invalid: expected type was string but got %T" ) // Theme Error constants. const ( - errFmtThemeName = "option 'theme' must be one of '%s' but it is configured as '%s'" + errFmtThemeName = "option 'theme' must be one of %s but it's configured as '%s'" ) // NTP Error constants. const ( - errFmtNTPVersion = "ntp: option 'version' must be either 3 or 4 but it is configured as '%d'" + errFmtNTPVersion = "ntp: option 'version' must be either 3 or 4 but it's configured as '%d'" ) // Session error constants. const ( errFmtSessionOptionRequired = "session: option '%s' is required" errFmtSessionLegacyAndWarning = "session: option 'domain' and option 'cookies' can't be specified at the same time" - errFmtSessionSameSite = "session: option 'same_site' must be one of '%s' but is configured as '%s'" + errFmtSessionSameSite = "session: option 'same_site' must be one of %s but it's configured as '%s'" errFmtSessionSecretRequired = "session: option 'secret' is required when using the '%s' provider" - errFmtSessionRedisPortRange = "session: redis: option 'port' must be between 1 and 65535 but is configured as '%d'" + errFmtSessionRedisPortRange = "session: redis: option 'port' must be between 1 and 65535 but it's configured as '%d'" errFmtSessionRedisHostRequired = "session: redis: option 'host' is required" errFmtSessionRedisHostOrNodesRequired = "session: redis: option 'host' or the 'high_availability' option 'nodes' is required" errFmtSessionRedisTLSConfigInvalid = "session: redis: tls: %w" @@ -261,8 +262,8 @@ const ( errFmtSessionRedisSentinelMissingName = "session: redis: high_availability: option 'sentinel_name' is required" errFmtSessionRedisSentinelNodeHostMissing = "session: redis: high_availability: option 'nodes': option 'host' is required for each node but one or more nodes are missing this" - errFmtSessionDomainMustBeRoot = "session: domain config %s: option 'domain' must be the domain you wish to protect not a wildcard domain but it is configured as '%s'" - errFmtSessionDomainSameSite = "session: domain config %s: option 'same_site' must be one of '%s' but is configured as '%s'" + errFmtSessionDomainMustBeRoot = "session: domain config %s: option 'domain' must be the domain you wish to protect not a wildcard domain but it's configured as '%s'" + errFmtSessionDomainSameSite = "session: domain config %s: option 'same_site' must be one of %s but it's configured as '%s'" errFmtSessionDomainRequired = "session: domain config %s: option 'domain' is required" errFmtSessionDomainHasPeriodPrefix = "session: domain config %s: option 'domain' has a prefix of '.' which is not supported or intended behaviour: you can use this at your own risk but we recommend removing it" errFmtSessionDomainDuplicate = "session: domain config %s: option 'domain' is a duplicate value for another configured session domain" @@ -291,8 +292,8 @@ const ( errFmtServerPathNoForwardSlashes = "server: option 'path' must not contain any forward slashes" errFmtServerPathAlphaNum = "server: option 'path' must only contain alpha numeric characters" - errFmtServerEndpointsAuthzImplementation = "server: endpoints: authz: %s: option 'implementation' must be one of '%s' but is configured as '%s'" - errFmtServerEndpointsAuthzStrategy = "server: endpoints: authz: %s: authn_strategies: option 'name' must be one of '%s' but is configured as '%s'" + errFmtServerEndpointsAuthzImplementation = "server: endpoints: authz: %s: option 'implementation' must be one of %s but it's configured as '%s'" + errFmtServerEndpointsAuthzStrategy = "server: endpoints: authz: %s: authn_strategies: option 'name' must be one of %s but it's configured as '%s'" errFmtServerEndpointsAuthzStrategyDuplicate = "server: endpoints: authz: %s: authn_strategies: duplicate strategy name detected with name '%s'" errFmtServerEndpointsAuthzPrefixDuplicate = "server: endpoints: authz: %s: endpoint starts with the same prefix as the '%s' endpoint with the '%s' implementation which accepts prefixes as part of its implementation" errFmtServerEndpointsAuthzInvalidName = "server: endpoints: authz: %s: contains invalid characters" @@ -302,7 +303,7 @@ const ( const ( errPasswordPolicyMultipleDefined = "password_policy: only a single password policy mechanism can be specified" - errFmtPasswordPolicyStandardMinLengthNotGreaterThanZero = "password_policy: standard: option 'min_length' must be greater than 0 but is configured as %d" + errFmtPasswordPolicyStandardMinLengthNotGreaterThanZero = "password_policy: standard: option 'min_length' must be greater than 0 but it's configured as %d" errFmtPasswordPolicyZXCVBNMinScoreInvalid = "password_policy: zxcvbn: option 'min_score' is invalid: must be between 1 and 4 but it's configured as %d" ) @@ -312,19 +313,17 @@ const ( ) const ( - errFmtDuoMissingOption = "duo_api: option '%s' is required when duo is enabled but it is missing" + errFmtDuoMissingOption = "duo_api: option '%s' is required when duo is enabled but it's absent" ) // Error constants. const ( - errFmtInvalidDefault2FAMethod = "option 'default_2fa_method' is configured as '%s' but must be one of " + - "the following values: '%s'" - errFmtInvalidDefault2FAMethodDisabled = "option 'default_2fa_method' is configured as '%s' " + - "but must be one of the following enabled method values: '%s'" + errFmtInvalidDefault2FAMethod = "option 'default_2fa_method' must be one of %s but it's configured as '%s'" + errFmtInvalidDefault2FAMethodDisabled = "option 'default_2fa_method' must be one of the enabled options %s but it's configured as '%s'" errFmtReplacedConfigurationKey = "invalid configuration key '%s' was replaced by '%s'" - errFmtLoggingLevelInvalid = "log: option 'level' must be one of '%s' but it is configured as '%s'" + errFmtLoggingLevelInvalid = "log: option 'level' must be one of %s but it's configured as '%s'" errFileHashing = "config key incorrect: authentication_backend.file.hashing should be authentication_backend.file.password" errFilePHashing = "config key incorrect: authentication_backend.file.password_hashing should be authentication_backend.file.password" @@ -357,6 +356,10 @@ const ( authzImplementationExtAuthz = "ExtAuthz" ) +const ( + auto = "auto" +) + var ( validAuthzImplementations = []string{"AuthRequest", "ForwardAuth", authzImplementationExtAuthz, authzImplementationLegacy} validAuthzAuthnStrategies = []string{"CookieSession", "HeaderAuthorization", "HeaderProxyAuthorization", "HeaderAuthRequestProxyAuthorization", "HeaderLegacy"} @@ -372,7 +375,7 @@ var ( var ( validStoragePostgreSQLSSLModes = []string{"disable", "require", "verify-ca", "verify-full"} - validThemeNames = []string{"light", "dark", "grey", "auto"} + validThemeNames = []string{"light", "dark", "grey", auto} validSessionSameSiteValues = []string{"none", "lax", "strict"} validLogLevels = []string{"trace", "debug", "info", "warn", "error"} validWebauthnConveyancePreferences = []string{string(protocol.PreferNoAttestation), string(protocol.PreferIndirectAttestation), string(protocol.PreferDirectAttestation)} @@ -389,19 +392,38 @@ var ( var validDefault2FAMethods = []string{"totp", "webauthn", "mobile_push"} +const ( + attrOIDCScopes = "scopes" + attrOIDCResponseTypes = "response_types" + attrOIDCResponseModes = "response_modes" + attrOIDCGrantTypes = "grant_types" + attrOIDCRedirectURIs = "redirect_uris" + attrOIDCTokenAuthMethod = "token_endpoint_auth_method" + attrOIDCUsrSigAlg = "userinfo_signing_algorithm" + attrOIDCPKCEChallengeMethod = "pkce_challenge_method" +) + var ( - validOIDCScopes = []string{oidc.ScopeOpenID, oidc.ScopeEmail, oidc.ScopeProfile, oidc.ScopeGroups, oidc.ScopeOfflineAccess} - validOIDCGrantTypes = []string{oidc.GrantTypeImplicit, oidc.GrantTypeRefreshToken, oidc.GrantTypeAuthorizationCode, oidc.GrantTypePassword, oidc.GrantTypeClientCredentials} - validOIDCResponseModes = []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery, oidc.ResponseModeFragment} - validOIDCUserinfoAlgorithms = []string{oidc.SigningAlgorithmNone, oidc.SigningAlgorithmRSAWithSHA256} - validOIDCCORSEndpoints = []string{oidc.EndpointAuthorization, oidc.EndpointPushedAuthorizationRequest, oidc.EndpointToken, oidc.EndpointIntrospection, oidc.EndpointRevocation, oidc.EndpointUserinfo} - validOIDCClientConsentModes = []string{"auto", oidc.ClientConsentModeImplicit.String(), oidc.ClientConsentModeExplicit.String(), oidc.ClientConsentModePreConfigured.String()} + validOIDCCORSEndpoints = []string{oidc.EndpointAuthorization, oidc.EndpointPushedAuthorizationRequest, oidc.EndpointToken, oidc.EndpointIntrospection, oidc.EndpointRevocation, oidc.EndpointUserinfo} + + validOIDCClientScopes = []string{oidc.ScopeOpenID, oidc.ScopeEmail, oidc.ScopeProfile, oidc.ScopeGroups, oidc.ScopeOfflineAccess} + validOIDCClientUserinfoAlgorithms = []string{oidc.SigningAlgorithmNone, oidc.SigningAlgorithmRSAWithSHA256} + validOIDCClientConsentModes = []string{auto, oidc.ClientConsentModeImplicit.String(), oidc.ClientConsentModeExplicit.String(), oidc.ClientConsentModePreConfigured.String()} + validOIDCClientResponseModes = []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery, oidc.ResponseModeFragment} + validOIDCClientResponseTypes = []string{oidc.ResponseTypeAuthorizationCodeFlow, oidc.ResponseTypeImplicitFlowIDToken, oidc.ResponseTypeImplicitFlowToken, oidc.ResponseTypeImplicitFlowBoth, oidc.ResponseTypeHybridFlowIDToken, oidc.ResponseTypeHybridFlowToken, oidc.ResponseTypeHybridFlowBoth} + validOIDCClientResponseTypesImplicitFlow = []string{oidc.ResponseTypeImplicitFlowIDToken, oidc.ResponseTypeImplicitFlowToken, oidc.ResponseTypeImplicitFlowBoth} + validOIDCClientResponseTypesHybridFlow = []string{oidc.ResponseTypeHybridFlowIDToken, oidc.ResponseTypeHybridFlowToken, oidc.ResponseTypeHybridFlowBoth} + validOIDCClientResponseTypesRefreshToken = []string{oidc.ResponseTypeAuthorizationCodeFlow, oidc.ResponseTypeHybridFlowIDToken, oidc.ResponseTypeHybridFlowToken, oidc.ResponseTypeHybridFlowBoth} + validOIDCClientGrantTypes = []string{oidc.GrantTypeImplicit, oidc.GrantTypeRefreshToken, oidc.GrantTypeAuthorizationCode} + + validOIDCClientTokenEndpointAuthMethods = []string{oidc.ClientAuthMethodNone, oidc.ClientAuthMethodClientSecretPost, oidc.ClientAuthMethodClientSecretBasic} + validOIDCClientTokenEndpointAuthMethodsConfidential = []string{oidc.ClientAuthMethodClientSecretPost, oidc.ClientAuthMethodClientSecretBasic} ) var ( reKeyReplacer = regexp.MustCompile(`\[\d+]`) reDomainCharacters = regexp.MustCompile(`^[a-z0-9-]+(\.[a-z0-9-]+)+[a-z0-9]$`) - reAuthzEndpointName = regexp.MustCompile(`^[a-zA-Z](([a-zA-Z0-9/\._-]*)([a-zA-Z]))?$`) + reAuthzEndpointName = regexp.MustCompile(`^[a-zA-Z](([a-zA-Z0-9/._-]*)([a-zA-Z]))?$`) ) var replacedKeys = map[string]string{ diff --git a/internal/configuration/validator/duo_test.go b/internal/configuration/validator/duo_test.go index ef4856b56..21cdbf6b1 100644 --- a/internal/configuration/validator/duo_test.go +++ b/internal/configuration/validator/duo_test.go @@ -23,6 +23,11 @@ func TestValidateDuo(t *testing.T) { expected: schema.DuoAPIConfiguration{Disable: true}, }, { + desc: "ShouldDisableDuoConfigured", + have: &schema.Configuration{DuoAPI: schema.DuoAPIConfiguration{Disable: true, Hostname: "example.com"}}, + expected: schema.DuoAPIConfiguration{Disable: true, Hostname: "example.com"}, + }, + { desc: "ShouldNotDisableDuo", have: &schema.Configuration{DuoAPI: schema.DuoAPIConfiguration{ Hostname: "test", @@ -46,7 +51,7 @@ func TestValidateDuo(t *testing.T) { IntegrationKey: "test", }, errs: []string{ - "duo_api: option 'secret_key' is required when duo is enabled but it is missing", + "duo_api: option 'secret_key' is required when duo is enabled but it's absent", }, }, { @@ -60,7 +65,7 @@ func TestValidateDuo(t *testing.T) { SecretKey: "test", }, errs: []string{ - "duo_api: option 'integration_key' is required when duo is enabled but it is missing", + "duo_api: option 'integration_key' is required when duo is enabled but it's absent", }, }, { @@ -74,7 +79,7 @@ func TestValidateDuo(t *testing.T) { SecretKey: "test", }, errs: []string{ - "duo_api: option 'hostname' is required when duo is enabled but it is missing", + "duo_api: option 'hostname' is required when duo is enabled but it's absent", }, }, } diff --git a/internal/configuration/validator/identity_providers.go b/internal/configuration/validator/identity_providers.go index b9ea7e9b9..cb61e31db 100644 --- a/internal/configuration/validator/identity_providers.go +++ b/internal/configuration/validator/identity_providers.go @@ -3,6 +3,7 @@ package validator import ( "fmt" "net/url" + "strconv" "strings" "time" @@ -125,10 +126,10 @@ func validateOIDCOptionsCORSAllowedOriginsFromClientRedirectURIs(config *schema. continue } - origin := utils.OriginFromURL(*uri) + origin := utils.OriginFromURL(uri) - if !utils.IsURLInSlice(origin, config.CORS.AllowedOrigins) { - config.CORS.AllowedOrigins = append(config.CORS.AllowedOrigins, origin) + if !utils.IsURLInSlice(*origin, config.CORS.AllowedOrigins) { + config.CORS.AllowedOrigins = append(config.CORS.AllowedOrigins, *origin) } } } @@ -137,113 +138,135 @@ func validateOIDCOptionsCORSAllowedOriginsFromClientRedirectURIs(config *schema. func validateOIDCOptionsCORSEndpoints(config *schema.OpenIDConnectConfiguration, val *schema.StructValidator) { for _, endpoint := range config.CORS.Endpoints { if !utils.IsStringInSlice(endpoint, validOIDCCORSEndpoints) { - val.Push(fmt.Errorf(errFmtOIDCCORSInvalidEndpoint, endpoint, strings.Join(validOIDCCORSEndpoints, "', '"))) + val.Push(fmt.Errorf(errFmtOIDCCORSInvalidEndpoint, endpoint, strJoinOr(validOIDCCORSEndpoints))) } } } func validateOIDCClients(config *schema.OpenIDConnectConfiguration, val *schema.StructValidator) { - invalidID, duplicateIDs := false, false + var ( + errDeprecated bool - var ids []string + clientIDs, duplicateClientIDs, blankClientIDs []string + ) + + errDeprecatedFunc := func() { errDeprecated = true } for c, client := range config.Clients { if client.ID == "" { - invalidID = true + blankClientIDs = append(blankClientIDs, "#"+strconv.Itoa(c+1)) } else { if client.Description == "" { config.Clients[c].Description = client.ID } - if utils.IsStringInSliceFold(client.ID, ids) { - duplicateIDs = true + if id := strings.ToLower(client.ID); utils.IsStringInSlice(id, clientIDs) { + if !utils.IsStringInSlice(id, duplicateClientIDs) { + duplicateClientIDs = append(duplicateClientIDs, id) + } + } else { + clientIDs = append(clientIDs, id) } - ids = append(ids, client.ID) } - if client.Public { - if client.Secret != nil { - val.Push(fmt.Errorf(errFmtOIDCClientPublicInvalidSecret, client.ID)) - } - } else { - if client.Secret == nil { - val.Push(fmt.Errorf(errFmtOIDCClientInvalidSecret, client.ID)) - } else if client.Secret.IsPlainText() { - val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidSecretPlainText, client.ID)) - } - } + validateOIDCClient(c, config, val, errDeprecatedFunc) + } - if client.Policy == "" { - config.Clients[c].Policy = schema.DefaultOpenIDConnectClientConfiguration.Policy - } else if client.Policy != policyOneFactor && client.Policy != policyTwoFactor { - val.Push(fmt.Errorf(errFmtOIDCClientInvalidPolicy, client.ID, client.Policy)) - } + if errDeprecated { + val.PushWarning(fmt.Errorf(errFmtOIDCClientsDeprecated)) + } - switch client.PKCEChallengeMethod { - case "", "plain", "S256": - break - default: - val.Push(fmt.Errorf(errFmtOIDCClientInvalidPKCEChallengeMethod, client.ID, client.PKCEChallengeMethod)) - } + if len(blankClientIDs) != 0 { + val.Push(fmt.Errorf(errFmtOIDCClientsWithEmptyID, buildJoinedString(", ", "or", "", blankClientIDs))) + } - validateOIDCClientConsentMode(c, config, val) - validateOIDCClientSectorIdentifier(client, val) - validateOIDCClientScopes(c, config, val) - validateOIDCClientGrantTypes(c, config, val) - validateOIDCClientResponseTypes(c, config, val) - validateOIDCClientResponseModes(c, config, val) - validateOIDDClientUserinfoAlgorithm(c, config, val) - validateOIDCClientRedirectURIs(client, val) + if len(duplicateClientIDs) != 0 { + val.Push(fmt.Errorf(errFmtOIDCClientsDuplicateID, strJoinOr(duplicateClientIDs))) } +} - if invalidID { - val.Push(fmt.Errorf(errFmtOIDCClientsWithEmptyID)) +func validateOIDCClient(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator, errDeprecatedFunc func()) { + if config.Clients[c].Public { + if config.Clients[c].Secret != nil { + val.Push(fmt.Errorf(errFmtOIDCClientPublicInvalidSecret, config.Clients[c].ID)) + } + } else { + if config.Clients[c].Secret == nil { + val.Push(fmt.Errorf(errFmtOIDCClientInvalidSecret, config.Clients[c].ID)) + } else if config.Clients[c].Secret.IsPlainText() { + val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidSecretPlainText, config.Clients[c].ID)) + } } - if duplicateIDs { - val.Push(fmt.Errorf(errFmtOIDCClientsDuplicateID)) + switch config.Clients[c].Policy { + case "": + config.Clients[c].Policy = schema.DefaultOpenIDConnectClientConfiguration.Policy + case policyOneFactor, policyTwoFactor: + break + default: + val.Push(fmt.Errorf(errFmtOIDCClientInvalidValue, config.Clients[c].ID, "policy", strJoinOr([]string{policyOneFactor, policyTwoFactor}), config.Clients[c].Policy)) + } + + switch config.Clients[c].PKCEChallengeMethod { + case "", oidc.PKCEChallengeMethodPlain, oidc.PKCEChallengeMethodSHA256: + break + default: + val.Push(fmt.Errorf(errFmtOIDCClientInvalidValue, config.Clients[c].ID, attrOIDCPKCEChallengeMethod, strJoinOr([]string{oidc.PKCEChallengeMethodPlain, oidc.PKCEChallengeMethodSHA256}), config.Clients[c].PKCEChallengeMethod)) } + + validateOIDCClientConsentMode(c, config, val) + + validateOIDCClientScopes(c, config, val, errDeprecatedFunc) + validateOIDCClientResponseTypes(c, config, val, errDeprecatedFunc) + validateOIDCClientResponseModes(c, config, val, errDeprecatedFunc) + validateOIDCClientGrantTypes(c, config, val, errDeprecatedFunc) + validateOIDCClientRedirectURIs(c, config, val, errDeprecatedFunc) + + validateOIDCClientTokenEndpointAuthMethod(c, config, val) + validateOIDDClientUserinfoAlgorithm(c, config, val) + + validateOIDCClientSectorIdentifier(c, config, val) } -func validateOIDCClientSectorIdentifier(client schema.OpenIDConnectClientConfiguration, val *schema.StructValidator) { - if client.SectorIdentifier.String() != "" { - if utils.IsURLHostComponent(client.SectorIdentifier) || utils.IsURLHostComponentWithPort(client.SectorIdentifier) { +func validateOIDCClientSectorIdentifier(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator) { + if config.Clients[c].SectorIdentifier.String() != "" { + if utils.IsURLHostComponent(config.Clients[c].SectorIdentifier) || utils.IsURLHostComponentWithPort(config.Clients[c].SectorIdentifier) { return } - if client.SectorIdentifier.Scheme != "" { - val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, client.ID, client.SectorIdentifier.String(), client.SectorIdentifier.Host, "scheme", client.SectorIdentifier.Scheme)) + if config.Clients[c].SectorIdentifier.Scheme != "" { + val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, config.Clients[c].ID, config.Clients[c].SectorIdentifier.String(), config.Clients[c].SectorIdentifier.Host, "scheme", config.Clients[c].SectorIdentifier.Scheme)) - if client.SectorIdentifier.Path != "" { - val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, client.ID, client.SectorIdentifier.String(), client.SectorIdentifier.Host, "path", client.SectorIdentifier.Path)) + if config.Clients[c].SectorIdentifier.Path != "" { + val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, config.Clients[c].ID, config.Clients[c].SectorIdentifier.String(), config.Clients[c].SectorIdentifier.Host, "path", config.Clients[c].SectorIdentifier.Path)) } - if client.SectorIdentifier.RawQuery != "" { - val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, client.ID, client.SectorIdentifier.String(), client.SectorIdentifier.Host, "query", client.SectorIdentifier.RawQuery)) + if config.Clients[c].SectorIdentifier.RawQuery != "" { + val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, config.Clients[c].ID, config.Clients[c].SectorIdentifier.String(), config.Clients[c].SectorIdentifier.Host, "query", config.Clients[c].SectorIdentifier.RawQuery)) } - if client.SectorIdentifier.Fragment != "" { - val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, client.ID, client.SectorIdentifier.String(), client.SectorIdentifier.Host, "fragment", client.SectorIdentifier.Fragment)) + if config.Clients[c].SectorIdentifier.Fragment != "" { + val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, config.Clients[c].ID, config.Clients[c].SectorIdentifier.String(), config.Clients[c].SectorIdentifier.Host, "fragment", config.Clients[c].SectorIdentifier.Fragment)) } - if client.SectorIdentifier.User != nil { - if client.SectorIdentifier.User.Username() != "" { - val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, client.ID, client.SectorIdentifier.String(), client.SectorIdentifier.Host, "username", client.SectorIdentifier.User.Username())) + if config.Clients[c].SectorIdentifier.User != nil { + if config.Clients[c].SectorIdentifier.User.Username() != "" { + val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifier, config.Clients[c].ID, config.Clients[c].SectorIdentifier.String(), config.Clients[c].SectorIdentifier.Host, "username", config.Clients[c].SectorIdentifier.User.Username())) } - if _, set := client.SectorIdentifier.User.Password(); set { - val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifierWithoutValue, client.ID, client.SectorIdentifier.String(), client.SectorIdentifier.Host, "password")) + if _, set := config.Clients[c].SectorIdentifier.User.Password(); set { + val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifierWithoutValue, config.Clients[c].ID, config.Clients[c].SectorIdentifier.String(), config.Clients[c].SectorIdentifier.Host, "password")) } } - } else if client.SectorIdentifier.Host == "" { - val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifierHost, client.ID, client.SectorIdentifier.String())) + } else if config.Clients[c].SectorIdentifier.Host == "" { + val.Push(fmt.Errorf(errFmtOIDCClientInvalidSectorIdentifierHost, config.Clients[c].ID, config.Clients[c].SectorIdentifier.String())) } } } func validateOIDCClientConsentMode(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator) { switch { - case utils.IsStringInSlice(config.Clients[c].ConsentMode, []string{"", "auto"}): + case utils.IsStringInSlice(config.Clients[c].ConsentMode, []string{"", auto}): if config.Clients[c].ConsentPreConfiguredDuration != nil { config.Clients[c].ConsentMode = oidc.ClientConsentModePreConfigured.String() } else { @@ -252,7 +275,7 @@ func validateOIDCClientConsentMode(c int, config *schema.OpenIDConnectConfigurat case utils.IsStringInSlice(config.Clients[c].ConsentMode, validOIDCClientConsentModes): break default: - val.Push(fmt.Errorf(errFmtOIDCClientInvalidConsentMode, config.Clients[c].ID, strings.Join(append(validOIDCClientConsentModes, "auto"), "', '"), config.Clients[c].ConsentMode)) + val.Push(fmt.Errorf(errFmtOIDCClientInvalidConsentMode, config.Clients[c].ID, strJoinOr(append(validOIDCClientConsentModes, auto)), config.Clients[c].ConsentMode)) } if config.Clients[c].ConsentMode == oidc.ClientConsentModePreConfigured.String() && config.Clients[c].ConsentPreConfiguredDuration == nil { @@ -260,92 +283,233 @@ func validateOIDCClientConsentMode(c int, config *schema.OpenIDConnectConfigurat } } -func validateOIDCClientScopes(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator) { +func validateOIDCClientScopes(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator, errDeprecatedFunc func()) { if len(config.Clients[c].Scopes) == 0 { config.Clients[c].Scopes = schema.DefaultOpenIDConnectClientConfiguration.Scopes - return } if !utils.IsStringInSlice(oidc.ScopeOpenID, config.Clients[c].Scopes) { - config.Clients[c].Scopes = append(config.Clients[c].Scopes, oidc.ScopeOpenID) + config.Clients[c].Scopes = append([]string{oidc.ScopeOpenID}, config.Clients[c].Scopes...) } - for _, scope := range config.Clients[c].Scopes { - if !utils.IsStringInSlice(scope, validOIDCScopes) { - val.Push(fmt.Errorf( - errFmtOIDCClientInvalidEntry, - config.Clients[c].ID, "scopes", strings.Join(validOIDCScopes, "', '"), scope)) - } + invalid, duplicates := validateList(config.Clients[c].Scopes, validOIDCClientScopes, true) + + if len(invalid) != 0 { + val.Push(fmt.Errorf(errFmtOIDCClientInvalidEntries, config.Clients[c].ID, attrOIDCScopes, strJoinOr(validOIDCClientScopes), strJoinAnd(invalid))) } -} -func validateOIDCClientGrantTypes(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator) { - if len(config.Clients[c].GrantTypes) == 0 { - config.Clients[c].GrantTypes = schema.DefaultOpenIDConnectClientConfiguration.GrantTypes - return + if len(duplicates) != 0 { + errDeprecatedFunc() + + val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidEntryDuplicates, config.Clients[c].ID, attrOIDCScopes, strJoinAnd(duplicates))) } - for _, grantType := range config.Clients[c].GrantTypes { - if !utils.IsStringInSlice(grantType, validOIDCGrantTypes) { - val.Push(fmt.Errorf( - errFmtOIDCClientInvalidEntry, - config.Clients[c].ID, "grant_types", strings.Join(validOIDCGrantTypes, "', '"), grantType)) - } + if utils.IsStringSliceContainsAny([]string{oidc.ScopeOfflineAccess, oidc.ScopeOffline}, config.Clients[c].Scopes) && + !utils.IsStringSliceContainsAny(validOIDCClientResponseTypesRefreshToken, config.Clients[c].ResponseTypes) { + errDeprecatedFunc() + + val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidRefreshTokenOptionWithoutCodeResponseType, + config.Clients[c].ID, attrOIDCScopes, + strJoinOr([]string{oidc.ScopeOfflineAccess, oidc.ScopeOffline}), + strJoinOr(validOIDCClientResponseTypesRefreshToken)), + ) } } -func validateOIDCClientResponseTypes(c int, config *schema.OpenIDConnectConfiguration, _ *schema.StructValidator) { +func validateOIDCClientResponseTypes(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator, errDeprecatedFunc func()) { if len(config.Clients[c].ResponseTypes) == 0 { config.Clients[c].ResponseTypes = schema.DefaultOpenIDConnectClientConfiguration.ResponseTypes - return + } + + invalid, duplicates := validateList(config.Clients[c].ResponseTypes, validOIDCClientResponseTypes, true) + + if len(invalid) != 0 { + val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidEntries, config.Clients[c].ID, attrOIDCResponseTypes, strJoinOr(validOIDCClientResponseTypes), strJoinAnd(invalid))) + } + + if len(duplicates) != 0 { + errDeprecatedFunc() + + val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidEntryDuplicates, config.Clients[c].ID, attrOIDCResponseTypes, strJoinAnd(duplicates))) } } -func validateOIDCClientResponseModes(c int, config *schema.OpenIDConnectConfiguration, validator *schema.StructValidator) { +func validateOIDCClientResponseModes(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator, errDeprecatedFunc func()) { if len(config.Clients[c].ResponseModes) == 0 { config.Clients[c].ResponseModes = schema.DefaultOpenIDConnectClientConfiguration.ResponseModes - return + + for _, responseType := range config.Clients[c].ResponseTypes { + switch responseType { + case oidc.ResponseTypeAuthorizationCodeFlow: + if !utils.IsStringInSlice(oidc.ResponseModeQuery, config.Clients[c].ResponseModes) { + config.Clients[c].ResponseModes = append(config.Clients[c].ResponseModes, oidc.ResponseModeQuery) + } + case oidc.ResponseTypeImplicitFlowIDToken, oidc.ResponseTypeImplicitFlowToken, oidc.ResponseTypeImplicitFlowBoth, + oidc.ResponseTypeHybridFlowIDToken, oidc.ResponseTypeHybridFlowToken, oidc.ResponseTypeHybridFlowBoth: + if !utils.IsStringInSlice(oidc.ResponseModeFragment, config.Clients[c].ResponseModes) { + config.Clients[c].ResponseModes = append(config.Clients[c].ResponseModes, oidc.ResponseModeFragment) + } + } + } + } + + invalid, duplicates := validateList(config.Clients[c].ResponseModes, validOIDCClientResponseModes, true) + + if len(invalid) != 0 { + val.Push(fmt.Errorf(errFmtOIDCClientInvalidEntries, config.Clients[c].ID, attrOIDCResponseModes, strJoinOr(validOIDCClientResponseModes), strJoinAnd(invalid))) + } + + if len(duplicates) != 0 { + errDeprecatedFunc() + + val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidEntryDuplicates, config.Clients[c].ID, attrOIDCResponseModes, strJoinAnd(duplicates))) + } +} + +func validateOIDCClientGrantTypes(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator, errDeprecatedFunc func()) { + if len(config.Clients[c].GrantTypes) == 0 { + validateOIDCClientGrantTypesSetDefaults(c, config) + } + + validateOIDCClientGrantTypesCheckRelated(c, config, val, errDeprecatedFunc) + + invalid, duplicates := validateList(config.Clients[c].GrantTypes, validOIDCClientGrantTypes, true) + + if len(invalid) != 0 { + val.Push(fmt.Errorf(errFmtOIDCClientInvalidEntries, config.Clients[c].ID, attrOIDCGrantTypes, strJoinOr(validOIDCClientGrantTypes), strJoinAnd(invalid))) } - for _, responseMode := range config.Clients[c].ResponseModes { - if !utils.IsStringInSlice(responseMode, validOIDCResponseModes) { - validator.Push(fmt.Errorf( - errFmtOIDCClientInvalidEntry, - config.Clients[c].ID, "response_modes", strings.Join(validOIDCResponseModes, "', '"), responseMode)) + if len(duplicates) != 0 { + errDeprecatedFunc() + + val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidEntryDuplicates, config.Clients[c].ID, attrOIDCGrantTypes, strJoinAnd(duplicates))) + } +} + +func validateOIDCClientGrantTypesSetDefaults(c int, config *schema.OpenIDConnectConfiguration) { + for _, responseType := range config.Clients[c].ResponseTypes { + switch responseType { + case oidc.ResponseTypeAuthorizationCodeFlow: + if !utils.IsStringInSlice(oidc.GrantTypeAuthorizationCode, config.Clients[c].GrantTypes) { + config.Clients[c].GrantTypes = append(config.Clients[c].GrantTypes, oidc.GrantTypeAuthorizationCode) + } + case oidc.ResponseTypeImplicitFlowIDToken, oidc.ResponseTypeImplicitFlowToken, oidc.ResponseTypeImplicitFlowBoth: + if !utils.IsStringInSlice(oidc.GrantTypeImplicit, config.Clients[c].GrantTypes) { + config.Clients[c].GrantTypes = append(config.Clients[c].GrantTypes, oidc.GrantTypeImplicit) + } + case oidc.ResponseTypeHybridFlowIDToken, oidc.ResponseTypeHybridFlowToken, oidc.ResponseTypeHybridFlowBoth: + if !utils.IsStringInSlice(oidc.GrantTypeAuthorizationCode, config.Clients[c].GrantTypes) { + config.Clients[c].GrantTypes = append(config.Clients[c].GrantTypes, oidc.GrantTypeAuthorizationCode) + } + + if !utils.IsStringInSlice(oidc.GrantTypeImplicit, config.Clients[c].GrantTypes) { + config.Clients[c].GrantTypes = append(config.Clients[c].GrantTypes, oidc.GrantTypeImplicit) + } } } } -func validateOIDDClientUserinfoAlgorithm(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator) { - if config.Clients[c].UserinfoSigningAlgorithm == "" { - config.Clients[c].UserinfoSigningAlgorithm = schema.DefaultOpenIDConnectClientConfiguration.UserinfoSigningAlgorithm - } else if !utils.IsStringInSlice(config.Clients[c].UserinfoSigningAlgorithm, validOIDCUserinfoAlgorithms) { - val.Push(fmt.Errorf(errFmtOIDCClientInvalidUserinfoAlgorithm, - config.Clients[c].ID, strings.Join(validOIDCUserinfoAlgorithms, ", "), config.Clients[c].UserinfoSigningAlgorithm)) +func validateOIDCClientGrantTypesCheckRelated(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator, errDeprecatedFunc func()) { + for _, grantType := range config.Clients[c].GrantTypes { + switch grantType { + case oidc.GrantTypeImplicit: + if !utils.IsStringSliceContainsAny(validOIDCClientResponseTypesImplicitFlow, config.Clients[c].ResponseTypes) && !utils.IsStringSliceContainsAny(validOIDCClientResponseTypesHybridFlow, config.Clients[c].ResponseTypes) { + errDeprecatedFunc() + + val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidGrantTypeMatch, config.Clients[c].ID, grantType, "for either the implicit or hybrid flow", strJoinOr(append(append([]string{}, validOIDCClientResponseTypesImplicitFlow...), validOIDCClientResponseTypesHybridFlow...)), strJoinAnd(config.Clients[c].ResponseTypes))) + } + case oidc.GrantTypeAuthorizationCode: + if !utils.IsStringInSlice(oidc.ResponseTypeAuthorizationCodeFlow, config.Clients[c].ResponseTypes) && !utils.IsStringSliceContainsAny(validOIDCClientResponseTypesHybridFlow, config.Clients[c].ResponseTypes) { + errDeprecatedFunc() + + val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidGrantTypeMatch, config.Clients[c].ID, grantType, "for either the authorization code or hybrid flow", strJoinOr(append([]string{oidc.ResponseTypeAuthorizationCodeFlow}, validOIDCClientResponseTypesHybridFlow...)), strJoinAnd(config.Clients[c].ResponseTypes))) + } + case oidc.GrantTypeRefreshToken: + if !utils.IsStringSliceContainsAny([]string{oidc.ScopeOfflineAccess, oidc.ScopeOffline}, config.Clients[c].Scopes) { + errDeprecatedFunc() + + val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidGrantTypeRefresh, config.Clients[c].ID)) + } + + if !utils.IsStringSliceContainsAny(validOIDCClientResponseTypesRefreshToken, config.Clients[c].ResponseTypes) { + errDeprecatedFunc() + + val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidRefreshTokenOptionWithoutCodeResponseType, + config.Clients[c].ID, attrOIDCGrantTypes, + strJoinOr([]string{oidc.GrantTypeRefreshToken}), + strJoinOr(validOIDCClientResponseTypesRefreshToken)), + ) + } + } } } -func validateOIDCClientRedirectURIs(client schema.OpenIDConnectClientConfiguration, val *schema.StructValidator) { - for _, redirectURI := range client.RedirectURIs { +func validateOIDCClientRedirectURIs(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator, errDeprecatedFunc func()) { + var ( + parsedRedirectURI *url.URL + err error + ) + + for _, redirectURI := range config.Clients[c].RedirectURIs { if redirectURI == oauth2InstalledApp { - if client.Public { + if config.Clients[c].Public { continue } - val.Push(fmt.Errorf(errFmtOIDCClientRedirectURIPublic, client.ID, oauth2InstalledApp)) + val.Push(fmt.Errorf(errFmtOIDCClientRedirectURIPublic, config.Clients[c].ID, oauth2InstalledApp)) continue } - parsedURL, err := url.Parse(redirectURI) - if err != nil { - val.Push(fmt.Errorf(errFmtOIDCClientRedirectURICantBeParsed, client.ID, redirectURI, err)) + if parsedRedirectURI, err = url.Parse(redirectURI); err != nil { + val.Push(fmt.Errorf(errFmtOIDCClientRedirectURICantBeParsed, config.Clients[c].ID, redirectURI, err)) continue } - if !parsedURL.IsAbs() || (!client.Public && parsedURL.Scheme == "") { - val.Push(fmt.Errorf(errFmtOIDCClientRedirectURIAbsolute, client.ID, redirectURI)) + if !parsedRedirectURI.IsAbs() || (!config.Clients[c].Public && parsedRedirectURI.Scheme == "") { + val.Push(fmt.Errorf(errFmtOIDCClientRedirectURIAbsolute, config.Clients[c].ID, redirectURI)) return } } + + _, duplicates := validateList(config.Clients[c].RedirectURIs, nil, true) + + if len(duplicates) != 0 { + errDeprecatedFunc() + + val.PushWarning(fmt.Errorf(errFmtOIDCClientInvalidEntryDuplicates, config.Clients[c].ID, attrOIDCRedirectURIs, strJoinAnd(duplicates))) + } +} + +func validateOIDCClientTokenEndpointAuthMethod(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator) { + implcit := len(config.Clients[c].ResponseTypes) != 0 && utils.IsStringSliceContainsAll(config.Clients[c].ResponseTypes, validOIDCClientResponseTypesImplicitFlow) + + if config.Clients[c].TokenEndpointAuthMethod == "" && (config.Clients[c].Public || implcit) { + config.Clients[c].TokenEndpointAuthMethod = oidc.ClientAuthMethodNone + } + + switch { + case config.Clients[c].TokenEndpointAuthMethod == "": + break + case !utils.IsStringInSlice(config.Clients[c].TokenEndpointAuthMethod, validOIDCClientTokenEndpointAuthMethods): + val.Push(fmt.Errorf(errFmtOIDCClientInvalidValue, + config.Clients[c].ID, attrOIDCTokenAuthMethod, strJoinOr(validOIDCClientTokenEndpointAuthMethods), config.Clients[c].TokenEndpointAuthMethod)) + case config.Clients[c].TokenEndpointAuthMethod == oidc.ClientAuthMethodNone && !config.Clients[c].Public && !implcit: + val.Push(fmt.Errorf(errFmtOIDCClientInvalidTokenEndpointAuthMethod, + config.Clients[c].ID, strJoinOr(validOIDCClientTokenEndpointAuthMethodsConfidential), strJoinAnd(validOIDCClientResponseTypesImplicitFlow), config.Clients[c].TokenEndpointAuthMethod)) + case config.Clients[c].TokenEndpointAuthMethod != oidc.ClientAuthMethodNone && config.Clients[c].Public: + val.Push(fmt.Errorf(errFmtOIDCClientInvalidTokenEndpointAuthMethodPublic, + config.Clients[c].ID, config.Clients[c].TokenEndpointAuthMethod)) + } +} + +func validateOIDDClientUserinfoAlgorithm(c int, config *schema.OpenIDConnectConfiguration, val *schema.StructValidator) { + if config.Clients[c].UserinfoSigningAlgorithm == "" { + config.Clients[c].UserinfoSigningAlgorithm = schema.DefaultOpenIDConnectClientConfiguration.UserinfoSigningAlgorithm + } + + if !utils.IsStringInSlice(config.Clients[c].UserinfoSigningAlgorithm, validOIDCClientUserinfoAlgorithms) { + val.Push(fmt.Errorf(errFmtOIDCClientInvalidValue, + config.Clients[c].ID, attrOIDCUsrSigAlg, strJoinOr(validOIDCClientUserinfoAlgorithms), config.Clients[c].UserinfoSigningAlgorithm)) + } } diff --git a/internal/configuration/validator/identity_providers_test.go b/internal/configuration/validator/identity_providers_test.go index 8957c61f8..bdb11f3d0 100644 --- a/internal/configuration/validator/identity_providers_test.go +++ b/internal/configuration/validator/identity_providers_test.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "net/url" - "strings" "testing" "time" @@ -31,8 +30,8 @@ func TestShouldRaiseErrorWhenInvalidOIDCServerConfiguration(t *testing.T) { require.Len(t, validator.Errors(), 2) - assert.EqualError(t, validator.Errors()[0], errFmtOIDCNoPrivateKey) - assert.EqualError(t, validator.Errors()[1], errFmtOIDCNoClientsConfigured) + assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: option 'issuer_private_key' is required") + assert.EqualError(t, validator.Errors()[1], "identity_providers: oidc: option 'clients' must have one or more clients configured") } func TestShouldNotRaiseErrorWhenCORSEndpointsValid(t *testing.T) { @@ -80,7 +79,7 @@ func TestShouldRaiseErrorWhenCORSEndpointsNotValid(t *testing.T) { require.Len(t, validator.Errors(), 1) - assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: cors: option 'endpoints' contains an invalid value 'invalid_endpoint': must be one of 'authorization', 'pushed-authorization-request', 'token', 'introspection', 'revocation', 'userinfo'") + assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: cors: option 'endpoints' contains an invalid value 'invalid_endpoint': must be one of 'authorization', 'pushed-authorization-request', 'token', 'introspection', 'revocation', or 'userinfo'") } func TestShouldRaiseErrorWhenOIDCPKCEEnforceValueInvalid(t *testing.T) { @@ -97,8 +96,8 @@ func TestShouldRaiseErrorWhenOIDCPKCEEnforceValueInvalid(t *testing.T) { require.Len(t, validator.Errors(), 2) - assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: option 'enforce_pkce' must be 'never', 'public_clients_only' or 'always', but it is configured as 'invalid'") - assert.EqualError(t, validator.Errors()[1], errFmtOIDCNoClientsConfigured) + assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: option 'enforce_pkce' must be 'never', 'public_clients_only' or 'always', but it's configured as 'invalid'") + assert.EqualError(t, validator.Errors()[1], "identity_providers: oidc: option 'clients' must have one or more clients configured") } func TestShouldRaiseErrorWhenOIDCCORSOriginsHasInvalidValues(t *testing.T) { @@ -150,7 +149,7 @@ func TestShouldRaiseErrorWhenOIDCServerNoClients(t *testing.T) { require.Len(t, validator.Errors(), 1) - assert.EqualError(t, validator.Errors()[0], errFmtOIDCNoClientsConfigured) + assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: option 'clients' must have one or more clients configured") } func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { @@ -180,7 +179,7 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { }, Errors: []string{ "identity_providers: oidc: client '': option 'secret' is required", - "identity_providers: oidc: one or more clients have been configured with an empty id", + "identity_providers: oidc: clients: option 'id' is required but was absent on the clients in positions #1", }, }, { @@ -195,7 +194,9 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { }, }, }, - Errors: []string{"identity_providers: oidc: client 'client-1': option 'policy' must be 'one_factor' or 'two_factor' but it is configured as 'a-policy'"}, + Errors: []string{ + "identity_providers: oidc: client 'client-1': option 'policy' must be one of 'one_factor' or 'two_factor' but it's configured as 'a-policy'", + }, }, { Name: "ClientIDDuplicated", @@ -213,7 +214,9 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { RedirectURIs: []string{}, }, }, - Errors: []string{errFmtOIDCClientsDuplicateID}, + Errors: []string{ + "identity_providers: oidc: clients: option 'id' must be unique for every client but one or more clients share the following 'id' values 'client-x'", + }, }, { Name: "RedirectURIInvalid", @@ -228,7 +231,7 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { }, }, Errors: []string{ - fmt.Sprintf(errFmtOIDCClientRedirectURICantBeParsed, "client-check-uri-parse", "http://abc@%two", errors.New("parse \"http://abc@%two\": invalid URL escape \"%tw\"")), + "identity_providers: oidc: client 'client-check-uri-parse': option 'redirect_uris' has an invalid value: redirect uri 'http://abc@%two' could not be parsed: parse \"http://abc@%two\": invalid URL escape \"%tw\"", }, }, { @@ -244,7 +247,7 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { }, }, Errors: []string{ - fmt.Sprintf(errFmtOIDCClientRedirectURIAbsolute, "client-check-uri-abs", "google.com"), + "identity_providers: oidc: client 'client-check-uri-abs': option 'redirect_uris' has an invalid value: redirect uri 'google.com' must have a scheme but it's absent", }, }, { @@ -289,12 +292,12 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { }, }, Errors: []string{ - fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifier, "client-invalid-sector", "https://user:pass@example.com/path?query=abc#fragment", exampleDotCom, "scheme", "https"), - fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifier, "client-invalid-sector", "https://user:pass@example.com/path?query=abc#fragment", exampleDotCom, "path", "/path"), - fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifier, "client-invalid-sector", "https://user:pass@example.com/path?query=abc#fragment", exampleDotCom, "query", "query=abc"), - fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifier, "client-invalid-sector", "https://user:pass@example.com/path?query=abc#fragment", exampleDotCom, "fragment", "fragment"), - fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifier, "client-invalid-sector", "https://user:pass@example.com/path?query=abc#fragment", exampleDotCom, "username", "user"), - fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifierWithoutValue, "client-invalid-sector", "https://user:pass@example.com/path?query=abc#fragment", exampleDotCom, "password"), + "identity_providers: oidc: client 'client-invalid-sector': option 'sector_identifier' with value 'https://user:pass@example.com/path?query=abc#fragment': must be a URL with only the host component for example 'example.com' but it has a scheme with the value 'https'", + "identity_providers: oidc: client 'client-invalid-sector': option 'sector_identifier' with value 'https://user:pass@example.com/path?query=abc#fragment': must be a URL with only the host component for example 'example.com' but it has a path with the value '/path'", + "identity_providers: oidc: client 'client-invalid-sector': option 'sector_identifier' with value 'https://user:pass@example.com/path?query=abc#fragment': must be a URL with only the host component for example 'example.com' but it has a query with the value 'query=abc'", + "identity_providers: oidc: client 'client-invalid-sector': option 'sector_identifier' with value 'https://user:pass@example.com/path?query=abc#fragment': must be a URL with only the host component for example 'example.com' but it has a fragment with the value 'fragment'", + "identity_providers: oidc: client 'client-invalid-sector': option 'sector_identifier' with value 'https://user:pass@example.com/path?query=abc#fragment': must be a URL with only the host component for example 'example.com' but it has a username with the value 'user'", + "identity_providers: oidc: client 'client-invalid-sector': option 'sector_identifier' with value 'https://user:pass@example.com/path?query=abc#fragment': must be a URL with only the host component for example 'example.com' but it has a password", }, }, { @@ -311,7 +314,7 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { }, }, Errors: []string{ - fmt.Sprintf(errFmtOIDCClientInvalidSectorIdentifierHost, "client-invalid-sector", "example.com/path?query=abc#fragment"), + "identity_providers: oidc: client 'client-invalid-sector': option 'sector_identifier' with value 'example.com/path?query=abc#fragment': must be a URL with only the host component but appears to be invalid", }, }, { @@ -328,7 +331,7 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { }, }, Errors: []string{ - fmt.Sprintf(errFmtOIDCClientInvalidConsentMode, "client-bad-consent-mode", strings.Join(append(validOIDCClientConsentModes, "auto"), "', '"), "cap"), + "identity_providers: oidc: client 'client-bad-consent-mode': consent: option 'mode' must be one of 'auto', 'implicit', 'explicit', 'pre-configured', or 'auto' but it's configured as 'cap'", }, }, { @@ -345,7 +348,7 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { }, }, Errors: []string{ - fmt.Sprintf(errFmtOIDCClientInvalidPKCEChallengeMethod, "client-bad-pkce-mode", "abc"), + "identity_providers: oidc: client 'client-bad-pkce-mode': option 'pkce_challenge_method' must be one of 'plain' or 'S256' but it's configured as 'abc'", }, }, { @@ -362,7 +365,7 @@ func TestShouldRaiseErrorWhenOIDCServerClientBadValues(t *testing.T) { }, }, Errors: []string{ - fmt.Sprintf(errFmtOIDCClientInvalidPKCEChallengeMethod, "client-bad-pkce-mode-s256", "s256"), + "identity_providers: oidc: client 'client-bad-pkce-mode-s256': option 'pkce_challenge_method' must be one of 'plain' or 'S256' but it's configured as 's256'", }, }, } @@ -415,7 +418,7 @@ func TestShouldRaiseErrorWhenOIDCClientConfiguredWithBadScopes(t *testing.T) { ValidateIdentityProviders(config, validator) require.Len(t, validator.Errors(), 1) - assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: client 'good_id': option 'scopes' must only have the values 'openid', 'email', 'profile', 'groups', 'offline_access' but one option is configured as 'bad_scope'") + assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: client 'good_id': option 'scopes' must only have the values 'openid', 'email', 'profile', 'groups', or 'offline_access' but the values 'bad_scope' are present") } func TestShouldRaiseErrorWhenOIDCClientConfiguredWithBadGrantTypes(t *testing.T) { @@ -441,7 +444,7 @@ func TestShouldRaiseErrorWhenOIDCClientConfiguredWithBadGrantTypes(t *testing.T) ValidateIdentityProviders(config, validator) require.Len(t, validator.Errors(), 1) - assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: client 'good_id': option 'grant_types' must only have the values 'implicit', 'refresh_token', 'authorization_code', 'password', 'client_credentials' but one option is configured as 'bad_grant_type'") + assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: client 'good_id': option 'grant_types' must only have the values 'implicit', 'refresh_token', or 'authorization_code' but the values 'bad_grant_type' are present") } func TestShouldNotErrorOnCertificateValid(t *testing.T) { @@ -577,7 +580,7 @@ func TestShouldRaiseErrorWhenOIDCClientConfiguredWithBadResponseModes(t *testing ValidateIdentityProviders(config, validator) require.Len(t, validator.Errors(), 1) - assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: client 'good_id': option 'response_modes' must only have the values 'form_post', 'query', 'fragment' but one option is configured as 'bad_responsemode'") + assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: client 'good_id': option 'response_modes' must only have the values 'form_post', 'query', or 'fragment' but the values 'bad_responsemode' are present") } func TestShouldRaiseErrorWhenOIDCClientConfiguredWithBadUserinfoAlg(t *testing.T) { @@ -603,7 +606,7 @@ func TestShouldRaiseErrorWhenOIDCClientConfiguredWithBadUserinfoAlg(t *testing.T ValidateIdentityProviders(config, validator) require.Len(t, validator.Errors(), 1) - assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: client 'good_id': option 'userinfo_signing_algorithm' must be one of 'none, RS256' but it is configured as 'rs256'") + assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: client 'good_id': option 'userinfo_signing_algorithm' must be one of 'none' or 'RS256' but it's configured as 'rs256'") } func TestValidateIdentityProvidersShouldRaiseWarningOnSecurityIssue(t *testing.T) { @@ -668,8 +671,8 @@ func TestValidateIdentityProvidersShouldRaiseErrorsOnInvalidClientTypes(t *testi require.Len(t, validator.Errors(), 2) assert.Len(t, validator.Warnings(), 0) - assert.EqualError(t, validator.Errors()[0], fmt.Sprintf(errFmtOIDCClientPublicInvalidSecret, "client-with-invalid-secret")) - assert.EqualError(t, validator.Errors()[1], fmt.Sprintf(errFmtOIDCClientRedirectURIPublic, "client-with-bad-redirect-uri", oauth2InstalledApp)) + assert.EqualError(t, validator.Errors()[0], "identity_providers: oidc: client 'client-with-invalid-secret': option 'secret' is required to be empty when option 'public' is true") + assert.EqualError(t, validator.Errors()[1], "identity_providers: oidc: client 'client-with-bad-redirect-uri': 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") } func TestValidateIdentityProvidersShouldNotRaiseErrorsOnValidClientOptions(t *testing.T) { @@ -758,175 +761,28 @@ func TestValidateIdentityProvidersShouldRaiseWarningOnPlainTextClients(t *testin assert.EqualError(t, validator.Warnings()[0], "identity_providers: oidc: client 'client-with-invalid-secret_standard': option 'secret' is plaintext but it should be a hashed value as plaintext values are deprecated and will be removed when oidc becomes stable") } -func TestValidateIdentityProvidersShouldSetDefaultValues(t *testing.T) { - timeDay := time.Hour * 24 - - validator := schema.NewStructValidator() - config := &schema.IdentityProvidersConfiguration{ - OIDC: &schema.OpenIDConnectConfiguration{ - HMACSecret: "rLABDrx87et5KvRHVUgTm3pezWWd8LMN", - IssuerPrivateKey: MustParseRSAPrivateKey(testKey1), - Clients: []schema.OpenIDConnectClientConfiguration{ - { - ID: "a-client", - Secret: MustDecodeSecret(goodOpenIDConnectClientSecret), - RedirectURIs: []string{ - "https://google.com", - }, - ConsentPreConfiguredDuration: &timeDay, - }, - { - ID: "b-client", - Description: "Normal Description", - Secret: MustDecodeSecret(goodOpenIDConnectClientSecret), - Policy: policyOneFactor, - UserinfoSigningAlgorithm: "RS256", - RedirectURIs: []string{ - "https://google.com", - }, - Scopes: []string{ - "groups", - }, - GrantTypes: []string{ - "refresh_token", - }, - ResponseTypes: []string{ - "token", - "code", - }, - ResponseModes: []string{ - "form_post", - "fragment", - }, - }, - { - ID: "c-client", - Secret: MustDecodeSecret(goodOpenIDConnectClientSecret), - RedirectURIs: []string{ - "https://google.com", - }, - ConsentMode: "implicit", - }, - { - ID: "d-client", - Secret: MustDecodeSecret(goodOpenIDConnectClientSecret), - RedirectURIs: []string{ - "https://google.com", - }, - ConsentMode: "explicit", - }, - { - ID: "e-client", - Secret: MustDecodeSecret(goodOpenIDConnectClientSecret), - RedirectURIs: []string{ - "https://google.com", - }, - ConsentMode: "pre-configured", - }, - }, - }, - } - - ValidateIdentityProviders(config, validator) - - assert.Len(t, validator.Warnings(), 0) - assert.Len(t, validator.Errors(), 0) - - // Assert Clients[0] Policy is set to the default, and the default doesn't override Clients[1]'s Policy. - assert.Equal(t, policyTwoFactor, config.OIDC.Clients[0].Policy) - assert.Equal(t, policyOneFactor, config.OIDC.Clients[1].Policy) - - assert.Equal(t, "none", config.OIDC.Clients[0].UserinfoSigningAlgorithm) - assert.Equal(t, "RS256", config.OIDC.Clients[1].UserinfoSigningAlgorithm) - - // Assert Clients[0] Description is set to the Clients[0] ID, and Clients[1]'s Description is not overridden. - assert.Equal(t, config.OIDC.Clients[0].ID, config.OIDC.Clients[0].Description) - assert.Equal(t, "Normal Description", config.OIDC.Clients[1].Description) - - // Assert Clients[0] ends up configured with the default Scopes. - require.Len(t, config.OIDC.Clients[0].Scopes, 4) - assert.Equal(t, "openid", config.OIDC.Clients[0].Scopes[0]) - assert.Equal(t, "groups", config.OIDC.Clients[0].Scopes[1]) - assert.Equal(t, "profile", config.OIDC.Clients[0].Scopes[2]) - assert.Equal(t, "email", config.OIDC.Clients[0].Scopes[3]) - - // Assert Clients[1] ends up configured with the configured Scopes and the openid Scope. - require.Len(t, config.OIDC.Clients[1].Scopes, 2) - assert.Equal(t, "groups", config.OIDC.Clients[1].Scopes[0]) - assert.Equal(t, "openid", config.OIDC.Clients[1].Scopes[1]) - - // Assert Clients[0] ends up configured with the correct consent mode. - require.NotNil(t, config.OIDC.Clients[0].ConsentPreConfiguredDuration) - assert.Equal(t, time.Hour*24, *config.OIDC.Clients[0].ConsentPreConfiguredDuration) - assert.Equal(t, "pre-configured", config.OIDC.Clients[0].ConsentMode) - - // Assert Clients[1] ends up configured with the correct consent mode. - assert.Nil(t, config.OIDC.Clients[1].ConsentPreConfiguredDuration) - assert.Equal(t, "explicit", config.OIDC.Clients[1].ConsentMode) - - // Assert Clients[0] ends up configured with the default GrantTypes. - require.Len(t, config.OIDC.Clients[0].GrantTypes, 2) - assert.Equal(t, "refresh_token", config.OIDC.Clients[0].GrantTypes[0]) - assert.Equal(t, "authorization_code", config.OIDC.Clients[0].GrantTypes[1]) - - // Assert Clients[1] ends up configured with only the configured GrantTypes. - require.Len(t, config.OIDC.Clients[1].GrantTypes, 1) - assert.Equal(t, "refresh_token", config.OIDC.Clients[1].GrantTypes[0]) - - // Assert Clients[0] ends up configured with the default ResponseTypes. - require.Len(t, config.OIDC.Clients[0].ResponseTypes, 1) - assert.Equal(t, "code", config.OIDC.Clients[0].ResponseTypes[0]) - - // Assert Clients[1] ends up configured only with the configured ResponseTypes. - require.Len(t, config.OIDC.Clients[1].ResponseTypes, 2) - assert.Equal(t, "token", config.OIDC.Clients[1].ResponseTypes[0]) - assert.Equal(t, "code", config.OIDC.Clients[1].ResponseTypes[1]) - - // Assert Clients[0] ends up configured with the default ResponseModes. - require.Len(t, config.OIDC.Clients[0].ResponseModes, 3) - assert.Equal(t, "form_post", config.OIDC.Clients[0].ResponseModes[0]) - assert.Equal(t, "query", config.OIDC.Clients[0].ResponseModes[1]) - assert.Equal(t, "fragment", config.OIDC.Clients[0].ResponseModes[2]) - - // Assert Clients[1] ends up configured only with the configured ResponseModes. - require.Len(t, config.OIDC.Clients[1].ResponseModes, 2) - assert.Equal(t, "form_post", config.OIDC.Clients[1].ResponseModes[0]) - assert.Equal(t, "fragment", config.OIDC.Clients[1].ResponseModes[1]) - - assert.Equal(t, false, config.OIDC.EnableClientDebugMessages) - assert.Equal(t, time.Hour, config.OIDC.AccessTokenLifespan) - assert.Equal(t, time.Minute, config.OIDC.AuthorizeCodeLifespan) - assert.Equal(t, time.Hour, config.OIDC.IDTokenLifespan) - assert.Equal(t, time.Minute*90, config.OIDC.RefreshTokenLifespan) - - assert.Equal(t, "implicit", config.OIDC.Clients[2].ConsentMode) - assert.Nil(t, config.OIDC.Clients[2].ConsentPreConfiguredDuration) - - assert.Equal(t, "explicit", config.OIDC.Clients[3].ConsentMode) - assert.Nil(t, config.OIDC.Clients[3].ConsentPreConfiguredDuration) - - assert.Equal(t, "pre-configured", config.OIDC.Clients[4].ConsentMode) - assert.Equal(t, schema.DefaultOpenIDConnectClientConfiguration.ConsentPreConfiguredDuration, config.OIDC.Clients[4].ConsentPreConfiguredDuration) -} - // All valid schemes are supported as defined in https://datatracker.ietf.org/doc/html/rfc8252#section-7.1 func TestValidateOIDCClientRedirectURIsSupportingPrivateUseURISchemes(t *testing.T) { - conf := schema.OpenIDConnectClientConfiguration{ - ID: "owncloud", - RedirectURIs: []string{ - "https://www.mywebsite.com", - "http://www.mywebsite.com", - "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, + have := &schema.OpenIDConnectConfiguration{ + Clients: []schema.OpenIDConnectClientConfiguration{ + { + ID: "owncloud", + RedirectURIs: []string{ + "https://www.mywebsite.com", + "http://www.mywebsite.com", + "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, + }, + }, }, } t.Run("public", func(t *testing.T) { validator := schema.NewStructValidator() - conf.Public = true - validateOIDCClientRedirectURIs(conf, validator) + have.Clients[0].Public = true + validateOIDCClientRedirectURIs(0, have, validator, nil) assert.Len(t, validator.Warnings(), 0) assert.Len(t, validator.Errors(), 0) @@ -934,8 +790,8 @@ func TestValidateOIDCClientRedirectURIsSupportingPrivateUseURISchemes(t *testing t.Run("not public", func(t *testing.T) { validator := schema.NewStructValidator() - conf.Public = false - validateOIDCClientRedirectURIs(conf, validator) + have.Clients[0].Public = false + validateOIDCClientRedirectURIs(0, have, validator, nil) assert.Len(t, validator.Warnings(), 0) assert.Len(t, validator.Errors(), 1) @@ -945,6 +801,1143 @@ func TestValidateOIDCClientRedirectURIsSupportingPrivateUseURISchemes(t *testing }) } +func TestValidateOIDCClients(t *testing.T) { + type tcv struct { + Scopes []string + ResponseTypes []string + ResponseModes []string + GrantTypes []string + } + + testCasses := []struct { + name string + setup func(have *schema.OpenIDConnectConfiguration) + validate func(t *testing.T, have *schema.OpenIDConnectConfiguration) + have tcv + expected tcv + serrs []string // Soft errors which will be warnings before GA. + errs []string + }{ + { + "ShouldSetDefaultResponseTypeAndResponseModes", + nil, + nil, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldIncludeMinimalScope", + nil, + nil, + tcv{ + []string{oidc.ScopeEmail}, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldSetDefaultResponseModesFlowAuthorizeCode", + nil, + nil, + tcv{ + nil, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldSetDefaultResponseModesFlowImplicit", + nil, + nil, + tcv{ + nil, + []string{oidc.ResponseTypeImplicitFlowBoth}, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeImplicitFlowBoth}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeFragment}, + []string{oidc.GrantTypeImplicit}, + }, + nil, + nil, + }, + { + "ShouldSetDefaultResponseModesFlowHybrid", + nil, + nil, + tcv{ + nil, + []string{oidc.ResponseTypeHybridFlowBoth}, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeHybridFlowBoth}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeFragment}, + []string{oidc.GrantTypeAuthorizationCode, oidc.GrantTypeImplicit}, + }, + nil, + nil, + }, + { + "ShouldSetDefaultResponseModesFlowMixedAuthorizeCodeHybrid", + nil, + nil, + tcv{ + nil, + []string{oidc.ResponseTypeAuthorizationCodeFlow, oidc.ResponseTypeHybridFlowBoth}, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow, oidc.ResponseTypeHybridFlowBoth}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery, oidc.ResponseModeFragment}, + []string{oidc.GrantTypeAuthorizationCode, oidc.GrantTypeImplicit}, + }, + nil, + nil, + }, + { + "ShouldSetDefaultResponseModesFlowMixedAuthorizeCodeImplicit", + nil, + nil, + tcv{ + nil, + []string{oidc.ResponseTypeAuthorizationCodeFlow, oidc.ResponseTypeImplicitFlowBoth}, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow, oidc.ResponseTypeImplicitFlowBoth}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery, oidc.ResponseModeFragment}, + []string{oidc.GrantTypeAuthorizationCode, oidc.GrantTypeImplicit}, + }, + nil, + nil, + }, + { + "ShouldSetDefaultResponseModesFlowMixedAll", + nil, + nil, + tcv{ + nil, + []string{oidc.ResponseTypeAuthorizationCodeFlow, oidc.ResponseTypeImplicitFlowBoth, oidc.ResponseTypeHybridFlowBoth}, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow, oidc.ResponseTypeImplicitFlowBoth, oidc.ResponseTypeHybridFlowBoth}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery, oidc.ResponseModeFragment}, + []string{oidc.GrantTypeAuthorizationCode, oidc.GrantTypeImplicit}, + }, + nil, + nil, + }, + { + "ShouldNotOverrideValues", + nil, + nil, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups}, + []string{oidc.ResponseTypeAuthorizationCodeFlow, oidc.ResponseTypeImplicitFlowBoth, oidc.ResponseTypeHybridFlowBoth}, + []string{oidc.ResponseModeFormPost}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups}, + []string{oidc.ResponseTypeAuthorizationCodeFlow, oidc.ResponseTypeImplicitFlowBoth, oidc.ResponseTypeHybridFlowBoth}, + []string{oidc.ResponseModeFormPost}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldRaiseErrorOnDuplicateScopes", + nil, + nil, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeProfile, oidc.ScopeOpenID}, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeProfile, oidc.ScopeOpenID}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + []string{ + "identity_providers: oidc: client 'test': option 'scopes' must have unique values but the values 'openid' are duplicated", + }, + nil, + }, + { + "ShouldRaiseErrorOnInvalidScopes", + nil, + nil, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeProfile, "group"}, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeProfile, "group"}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + []string{ + "identity_providers: oidc: client 'test': option 'scopes' must only have the values 'openid', 'email', 'profile', 'groups', or 'offline_access' but the values 'group' are present", + }, + }, + { + "ShouldRaiseErrorOnMissingAuthorizationCodeFlowResponseTypeWithRefreshTokenValues", + nil, + nil, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeProfile, oidc.ScopeOfflineAccess}, + []string{oidc.ResponseTypeImplicitFlowBoth}, + nil, + []string{oidc.GrantTypeImplicit, oidc.GrantTypeRefreshToken}, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeProfile, oidc.ScopeOfflineAccess}, + []string{oidc.ResponseTypeImplicitFlowBoth}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeFragment}, + []string{oidc.GrantTypeImplicit, oidc.GrantTypeRefreshToken}, + }, + []string{ + "identity_providers: oidc: client 'test': option 'scopes' should only have the values 'offline_access' or 'offline' if the client is also configured with a 'response_type' such as 'code', 'code id_token', 'code token', or 'code id_token token' which respond with authorization codes", + "identity_providers: oidc: client 'test': option 'grant_types' should only have the values 'refresh_token' if the client is also configured with a 'response_type' such as 'code', 'code id_token', 'code token', or 'code id_token token' which respond with authorization codes", + }, + nil, + }, + { + "ShouldRaiseErrorOnDuplicateResponseTypes", + nil, + nil, + tcv{ + nil, + []string{oidc.ResponseTypeAuthorizationCodeFlow, oidc.ResponseTypeImplicitFlowBoth, oidc.ResponseTypeAuthorizationCodeFlow}, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow, oidc.ResponseTypeImplicitFlowBoth, oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery, oidc.ResponseModeFragment}, + []string{oidc.GrantTypeAuthorizationCode, oidc.GrantTypeImplicit}, + }, + []string{ + "identity_providers: oidc: client 'test': option 'response_types' must have unique values but the values 'code' are duplicated", + }, + nil, + }, + { + "ShouldRaiseErrorOnInvalidResponseTypesOrder", + nil, + nil, + tcv{ + nil, + []string{oidc.ResponseTypeImplicitFlowBoth, "token id_token"}, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeImplicitFlowBoth, "token id_token"}, + []string{"form_post", "fragment"}, + []string{"implicit"}, + }, + []string{ + "identity_providers: oidc: client 'test': option 'response_types' must only have the values 'code', 'id_token', 'token', 'id_token token', 'code id_token', 'code token', or 'code id_token token' but the values 'token id_token' are present", + }, + nil, + }, + { + "ShouldRaiseErrorOnInvalidResponseTypes", + nil, + nil, + tcv{ + nil, + []string{"not_valid"}, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{"not_valid"}, + []string{oidc.ResponseModeFormPost}, + nil, + }, + []string{ + "identity_providers: oidc: client 'test': option 'response_types' must only have the values 'code', 'id_token', 'token', 'id_token token', 'code id_token', 'code token', or 'code id_token token' but the values 'not_valid' are present", + }, + nil, + }, + { + "ShouldRaiseErrorOnInvalidResponseModes", + nil, + nil, + tcv{ + nil, + nil, + []string{"not_valid"}, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{"not_valid"}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + []string{ + "identity_providers: oidc: client 'test': option 'response_modes' must only have the values 'form_post', 'query', or 'fragment' but the values 'not_valid' are present", + }, + }, + { + "ShouldRaiseErrorOnDuplicateResponseModes", + nil, + nil, + tcv{ + nil, + nil, + []string{oidc.ResponseModeQuery, oidc.ResponseModeQuery}, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeQuery, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + []string{ + "identity_providers: oidc: client 'test': option 'response_modes' must have unique values but the values 'query' are duplicated", + }, + nil, + }, + { + "ShouldRaiseErrorOnInvalidGrantTypes", + nil, + nil, + tcv{ + nil, + nil, + nil, + []string{"invalid"}, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{"invalid"}, + }, + nil, + []string{ + "identity_providers: oidc: client 'test': option 'grant_types' must only have the values 'implicit', 'refresh_token', or 'authorization_code' but the values 'invalid' are present", + }, + }, + { + "ShouldRaiseErrorOnDuplicateGrantTypes", + nil, + nil, + tcv{ + nil, + nil, + nil, + []string{oidc.GrantTypeAuthorizationCode, oidc.GrantTypeAuthorizationCode}, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode, oidc.GrantTypeAuthorizationCode}, + }, + []string{ + "identity_providers: oidc: client 'test': option 'grant_types' must have unique values but the values 'authorization_code' are duplicated", + }, + nil, + }, + { + "ShouldRaiseErrorOnGrantTypeRefreshTokenWithoutScopeOfflineAccess", + nil, + nil, + tcv{ + nil, + nil, + nil, + []string{oidc.GrantTypeAuthorizationCode, oidc.GrantTypeRefreshToken}, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode, oidc.GrantTypeRefreshToken}, + }, + []string{ + "identity_providers: oidc: client 'test': option 'grant_types' should only have the 'refresh_token' value if the client is also configured with the 'offline_access' scope", + }, + nil, + }, + { + "ShouldRaiseErrorOnGrantTypeAuthorizationCodeWithoutAuthorizationCodeOrHybridFlow", + nil, + nil, + tcv{ + nil, + []string{oidc.ResponseTypeImplicitFlowBoth}, + nil, + []string{oidc.GrantTypeAuthorizationCode}, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeImplicitFlowBoth}, + []string{"form_post", "fragment"}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + []string{ + "identity_providers: oidc: client 'test': option 'grant_types' should only have grant type values which are valid with the configured 'response_types' for the client but 'authorization_code' expects a response type for either the authorization code or hybrid flow such as 'code', 'code id_token', 'code token', or 'code id_token token' but the response types are 'id_token token'", + }, + nil, + }, + { + "ShouldRaiseErrorOnGrantTypeImplicitWithoutImplicitOrHybridFlow", + nil, + nil, + tcv{ + nil, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + nil, + []string{oidc.GrantTypeImplicit}, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeImplicit}, + }, + []string{ + "identity_providers: oidc: client 'test': option 'grant_types' should only have grant type values which are valid with the configured 'response_types' for the client but 'implicit' expects a response type for either the implicit or hybrid flow such as 'id_token', 'token', 'id_token token', 'code id_token', 'code token', or 'code id_token token' but the response types are 'code'", + }, + nil, + }, + { + "ShouldValidateCorrectRedirectURIsConfidentialClientType", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].RedirectURIs = []string{ + "https://google.com", + } + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, []string{"https://google.com"}, have.Clients[0].RedirectURIs) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldValidateCorrectRedirectURIsPublicClientType", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].Public = true + have.Clients[0].Secret = nil + have.Clients[0].RedirectURIs = []string{ + oauth2InstalledApp, + } + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, []string{oauth2InstalledApp}, have.Clients[0].RedirectURIs) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldRaiseErrorOnInvalidRedirectURIsPublicOnly", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].RedirectURIs = []string{ + "urn:ietf:wg:oauth:2.0:oob", + } + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, []string{oauth2InstalledApp}, have.Clients[0].RedirectURIs) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + []string{ + "identity_providers: oidc: client 'test': 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", + }, + }, + { + "ShouldRaiseErrorOnInvalidRedirectURIsMalformedURI", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].RedirectURIs = []string{ + "http://abc@%two", + } + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, []string{"http://abc@%two"}, have.Clients[0].RedirectURIs) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + []string{ + "identity_providers: oidc: client 'test': option 'redirect_uris' has an invalid value: redirect uri 'http://abc@%two' could not be parsed: parse \"http://abc@%two\": invalid URL escape \"%tw\"", + }, + }, + { + "ShouldRaiseErrorOnInvalidRedirectURIsNotAbsolute", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].RedirectURIs = []string{ + "google.com", + } + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, []string{"google.com"}, have.Clients[0].RedirectURIs) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + []string{ + "identity_providers: oidc: client 'test': option 'redirect_uris' has an invalid value: redirect uri 'google.com' must have a scheme but it's absent", + }, + }, + { + "ShouldRaiseErrorOnDuplicateRedirectURI", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].RedirectURIs = []string{ + "https://google.com", + "https://google.com", + } + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, []string{"https://google.com", "https://google.com"}, have.Clients[0].RedirectURIs) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + []string{ + "identity_providers: oidc: client 'test': option 'redirect_uris' must have unique values but the values 'https://google.com' are duplicated", + }, + nil, + }, + { + "ShouldNotSetDefaultTokenEndpointClientAuthMethodConfidentialClientType", + nil, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, "", have.Clients[0].TokenEndpointAuthMethod) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldSetDefaultTokenEndpointClientAuthMethodPublicClientType", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].Public = true + have.Clients[0].Secret = nil + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, oidc.ClientAuthMethodNone, have.Clients[0].TokenEndpointAuthMethod) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldSetDefaultTokenEndpointClientAuthMethodConfidentialClientTypeImplicitFlow", + nil, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, oidc.ClientAuthMethodNone, have.Clients[0].TokenEndpointAuthMethod) + }, + tcv{ + nil, + []string{oidc.ResponseTypeImplicitFlowIDToken, oidc.ResponseTypeImplicitFlowToken, oidc.ResponseTypeImplicitFlowBoth}, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeImplicitFlowIDToken, oidc.ResponseTypeImplicitFlowToken, oidc.ResponseTypeImplicitFlowBoth}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeFragment}, + []string{oidc.GrantTypeImplicit}, + }, + nil, + nil, + }, + { + "ShouldNotOverrideValidClientAuthMethod", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].TokenEndpointAuthMethod = oidc.ClientAuthMethodClientSecretPost + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, oidc.ClientAuthMethodClientSecretPost, have.Clients[0].TokenEndpointAuthMethod) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldRaiseErrorOnInvalidClientAuthMethod", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].TokenEndpointAuthMethod = "client_credentials" + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, "client_credentials", have.Clients[0].TokenEndpointAuthMethod) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + []string{ + "identity_providers: oidc: client 'test': option 'token_endpoint_auth_method' must be one of 'none', 'client_secret_post', or 'client_secret_basic' but it's configured as 'client_credentials'", + }, + }, + { + "ShouldRaiseErrorOnInvalidClientAuthMethodForPublicClientType", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].TokenEndpointAuthMethod = oidc.ClientAuthMethodClientSecretBasic + have.Clients[0].Public = true + have.Clients[0].Secret = nil + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, oidc.ClientAuthMethodClientSecretBasic, have.Clients[0].TokenEndpointAuthMethod) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + []string{ + "identity_providers: oidc: client 'test': option 'token_endpoint_auth_method' must be 'none' when configured as the public client type but it's configured as 'client_secret_basic'", + }, + }, + { + "ShouldRaiseErrorOnInvalidClientAuthMethodForConfidentialClientTypeAuthorizationCodeFlow", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].TokenEndpointAuthMethod = oidc.ClientAuthMethodNone + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, oidc.ClientAuthMethodNone, have.Clients[0].TokenEndpointAuthMethod) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + []string{ + "identity_providers: oidc: client 'test': option 'token_endpoint_auth_method' must be one of 'client_secret_post' or 'client_secret_basic' when configured as the confidential client type unless it only includes implicit flow response types such as 'id_token', 'token', and 'id_token token' but it's configured as 'none'", + }, + }, + { + "ShouldRaiseErrorOnInvalidClientAuthMethodForConfidentialClientTypeHybridFlow", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].TokenEndpointAuthMethod = oidc.ClientAuthMethodNone + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, oidc.ClientAuthMethodNone, have.Clients[0].TokenEndpointAuthMethod) + }, + tcv{ + nil, + []string{oidc.ResponseTypeHybridFlowToken}, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeHybridFlowToken}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeFragment}, + []string{oidc.GrantTypeAuthorizationCode, oidc.GrantTypeImplicit}, + }, + nil, + []string{ + "identity_providers: oidc: client 'test': option 'token_endpoint_auth_method' must be one of 'client_secret_post' or 'client_secret_basic' when configured as the confidential client type unless it only includes implicit flow response types such as 'id_token', 'token', and 'id_token token' but it's configured as 'none'", + }, + }, + { + "ShouldSetDefaultUserInfoAlg", + nil, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, oidc.SigningAlgorithmNone, have.Clients[0].UserinfoSigningAlgorithm) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldNotOverrideUserInfoAlg", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].UserinfoSigningAlgorithm = oidc.SigningAlgorithmRSAWithSHA256 + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, oidc.SigningAlgorithmRSAWithSHA256, have.Clients[0].UserinfoSigningAlgorithm) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldRaiseErrorOnInvalidUserInfoAlg", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].UserinfoSigningAlgorithm = "rs256" + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, "rs256", have.Clients[0].UserinfoSigningAlgorithm) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + []string{ + "identity_providers: oidc: client 'test': option 'userinfo_signing_algorithm' must be one of 'none' or 'RS256' but it's configured as 'rs256'", + }, + }, + { + "ShouldSetDefaultConsentMode", + nil, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, "explicit", have.Clients[0].ConsentMode) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldSetDefaultConsentModeAuto", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].ConsentMode = auto + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, "explicit", have.Clients[0].ConsentMode) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldSetDefaultConsentModePreConfigured", + func(have *schema.OpenIDConnectConfiguration) { + d := time.Minute + + have.Clients[0].ConsentMode = "" + have.Clients[0].ConsentPreConfiguredDuration = &d + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, "pre-configured", have.Clients[0].ConsentMode) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldSetDefaultConsentModeAutoPreConfigured", + func(have *schema.OpenIDConnectConfiguration) { + d := time.Minute + + have.Clients[0].ConsentMode = auto + have.Clients[0].ConsentPreConfiguredDuration = &d + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, "pre-configured", have.Clients[0].ConsentMode) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldNotOverrideConsentMode", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].ConsentMode = "implicit" + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, "implicit", have.Clients[0].ConsentMode) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + { + "ShouldSentConsentPreConfiguredDefaultDuration", + func(have *schema.OpenIDConnectConfiguration) { + have.Clients[0].ConsentMode = "pre-configured" + }, + func(t *testing.T, have *schema.OpenIDConnectConfiguration) { + assert.Equal(t, "pre-configured", have.Clients[0].ConsentMode) + assert.Equal(t, schema.DefaultOpenIDConnectClientConfiguration.ConsentPreConfiguredDuration, have.Clients[0].ConsentPreConfiguredDuration) + }, + tcv{ + nil, + nil, + nil, + nil, + }, + tcv{ + []string{oidc.ScopeOpenID, oidc.ScopeGroups, oidc.ScopeProfile, oidc.ScopeEmail}, + []string{oidc.ResponseTypeAuthorizationCodeFlow}, + []string{oidc.ResponseModeFormPost, oidc.ResponseModeQuery}, + []string{oidc.GrantTypeAuthorizationCode}, + }, + nil, + nil, + }, + } + + errDeprecatedFunc := func() {} + + for _, tc := range testCasses { + t.Run(tc.name, func(t *testing.T) { + have := &schema.OpenIDConnectConfiguration{ + Clients: []schema.OpenIDConnectClientConfiguration{ + { + ID: "test", + Secret: MustDecodeSecret("$pbkdf2-sha512$310000$c8p78n7pUMln0jzvd4aK4Q$JNRBzwAo0ek5qKn50cFzzvE9RXV88h1wJn5KGiHrD0YKtZaR/nCb2CJPOsKaPK0hjf.9yHxzQGZziziccp6Yng"), + Scopes: tc.have.Scopes, + ResponseModes: tc.have.ResponseModes, + ResponseTypes: tc.have.ResponseTypes, + GrantTypes: tc.have.GrantTypes, + }, + }, + } + + if tc.setup != nil { + tc.setup(have) + } + + val := schema.NewStructValidator() + + validateOIDCClient(0, have, val, errDeprecatedFunc) + + t.Run("General", func(t *testing.T) { + assert.Equal(t, tc.expected.Scopes, have.Clients[0].Scopes) + assert.Equal(t, tc.expected.ResponseTypes, have.Clients[0].ResponseTypes) + assert.Equal(t, tc.expected.ResponseModes, have.Clients[0].ResponseModes) + assert.Equal(t, tc.expected.GrantTypes, have.Clients[0].GrantTypes) + + if tc.validate != nil { + tc.validate(t, have) + } + }) + + t.Run("Warnings", func(t *testing.T) { + require.Len(t, val.Warnings(), len(tc.serrs)) + for i, err := range tc.serrs { + assert.EqualError(t, val.Warnings()[i], err) + } + }) + + t.Run("Errors", func(t *testing.T) { + require.Len(t, val.Errors(), len(tc.errs)) + for i, err := range tc.errs { + assert.EqualError(t, val.Errors()[i], err) + } + }) + }) + } +} + +func TestValidateOIDCClientTokenEndpointAuthMethod(t *testing.T) { + testCasses := []struct { + name string + have string + public bool + expected string + errs []string + }{ + {"ShouldSetDefaultValueConfidential", "", false, "", nil}, + {"ShouldSetDefaultValuePublic", "", true, oidc.ClientAuthMethodNone, nil}, + {"ShouldErrorOnInvalidValue", "abc", false, "abc", + []string{ + "identity_providers: oidc: client 'test': option 'token_endpoint_auth_method' must be one of 'none', 'client_secret_post', or 'client_secret_basic' but it's configured as 'abc'", + }, + }, + {"ShouldErrorOnInvalidValueForPublicClient", "client_secret_post", true, "client_secret_post", + []string{ + "identity_providers: oidc: client 'test': option 'token_endpoint_auth_method' must be 'none' when configured as the public client type but it's configured as 'client_secret_post'", + }, + }, + {"ShouldErrorOnInvalidValueForConfidentialClient", "none", false, "none", + []string{ + "identity_providers: oidc: client 'test': option 'token_endpoint_auth_method' must be one of 'client_secret_post' or 'client_secret_basic' when configured as the confidential client type unless it only includes implicit flow response types such as 'id_token', 'token', and 'id_token token' but it's configured as 'none'", + }, + }, + } + + for _, tc := range testCasses { + t.Run(tc.name, func(t *testing.T) { + have := &schema.OpenIDConnectConfiguration{ + Clients: []schema.OpenIDConnectClientConfiguration{ + { + ID: "test", + Public: tc.public, + TokenEndpointAuthMethod: tc.have, + }, + }, + } + + val := schema.NewStructValidator() + + validateOIDCClientTokenEndpointAuthMethod(0, have, val) + + assert.Equal(t, tc.expected, have.Clients[0].TokenEndpointAuthMethod) + assert.Len(t, val.Warnings(), 0) + require.Len(t, val.Errors(), len(tc.errs)) + + if tc.errs != nil { + for i, err := range tc.errs { + assert.EqualError(t, val.Errors()[i], err) + } + } + }) + } +} + func MustDecodeSecret(value string) *schema.PasswordDigest { if secret, err := schema.DecodePasswordDigest(value); err != nil { panic(err) diff --git a/internal/configuration/validator/keys.go b/internal/configuration/validator/keys.go index 67b9d964d..863d2030e 100644 --- a/internal/configuration/validator/keys.go +++ b/internal/configuration/validator/keys.go @@ -100,7 +100,7 @@ func NewKeyMapPattern(key string) (pattern *regexp.Regexp, err error) { } if i < n { - buf.WriteString("\\.[a-z0-9]([a-z0-9-_]+)?[a-z0-9]") + buf.WriteString("\\.[a-z0-9](([a-z0-9-_]+)?[a-z0-9])?") } } diff --git a/internal/configuration/validator/keys_test.go b/internal/configuration/validator/keys_test.go index 989b4b815..a5123ef8c 100644 --- a/internal/configuration/validator/keys_test.go +++ b/internal/configuration/validator/keys_test.go @@ -101,6 +101,20 @@ func TestSpecificErrorKeys(t *testing.T) { assert.EqualError(t, errs[4], specificErrorKeys["authentication_backend.file.hashing.algorithm"]) } +func TestPatternKeys(t *testing.T) { + configKeys := []string{ + "server.endpoints.authz.xx.implementation", + "server.endpoints.authz.x.implementation", + } + + val := schema.NewStructValidator() + ValidateKeys(configKeys, "AUTHELIA_", val) + + errs := val.Errors() + + require.Len(t, errs, 0) +} + func TestReplacedErrors(t *testing.T) { configKeys := []string{ "authentication_backend.ldap.skip_verify", diff --git a/internal/configuration/validator/log.go b/internal/configuration/validator/log.go index 5c7a0761b..7b8c7f6ea 100644 --- a/internal/configuration/validator/log.go +++ b/internal/configuration/validator/log.go @@ -2,7 +2,6 @@ package validator import ( "fmt" - "strings" "github.com/authelia/authelia/v4/internal/configuration/schema" "github.com/authelia/authelia/v4/internal/utils" @@ -19,6 +18,6 @@ func ValidateLog(config *schema.Configuration, validator *schema.StructValidator } if !utils.IsStringInSlice(config.Log.Level, validLogLevels) { - validator.Push(fmt.Errorf(errFmtLoggingLevelInvalid, strings.Join(validLogLevels, "', '"), config.Log.Level)) + validator.Push(fmt.Errorf(errFmtLoggingLevelInvalid, strJoinOr(validLogLevels), config.Log.Level)) } } diff --git a/internal/configuration/validator/log_test.go b/internal/configuration/validator/log_test.go index 56cff19de..cf3de2736 100644 --- a/internal/configuration/validator/log_test.go +++ b/internal/configuration/validator/log_test.go @@ -40,5 +40,5 @@ func TestShouldRaiseErrorOnInvalidLoggingLevel(t *testing.T) { assert.Len(t, validator.Warnings(), 0) require.Len(t, validator.Errors(), 1) - assert.EqualError(t, validator.Errors()[0], "log: option 'level' must be one of 'trace', 'debug', 'info', 'warn', 'error' but it is configured as 'TRACE'") + assert.EqualError(t, validator.Errors()[0], "log: option 'level' must be one of 'trace', 'debug', 'info', 'warn', or 'error' but it's configured as 'TRACE'") } diff --git a/internal/configuration/validator/notifier_test.go b/internal/configuration/validator/notifier_test.go index c41776a29..17819993f 100644 --- a/internal/configuration/validator/notifier_test.go +++ b/internal/configuration/validator/notifier_test.go @@ -4,8 +4,10 @@ import ( "crypto/tls" "fmt" "net/mail" + "path/filepath" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "github.com/authelia/authelia/v4/internal/configuration/schema" @@ -187,6 +189,32 @@ func (suite *NotifierSuite) TestSMTPShouldEnsureSenderIsProvided() { suite.Assert().EqualError(suite.validator.Errors()[0], fmt.Sprintf(errFmtNotifierSMTPNotConfigured, "sender")) } +func (suite *NotifierSuite) TestTemplatesEmptyDir() { + dir := suite.T().TempDir() + + suite.config.TemplatePath = dir + + ValidateNotifier(&suite.config, suite.validator) + + suite.Assert().Len(suite.validator.Warnings(), 0) + suite.Assert().Len(suite.validator.Errors(), 0) +} + +func (suite *NotifierSuite) TestTemplatesEmptyDirNoExist() { + dir := suite.T().TempDir() + + p := filepath.Join(dir, "notexist") + + suite.config.TemplatePath = p + + ValidateNotifier(&suite.config, suite.validator) + + suite.Assert().Len(suite.validator.Warnings(), 0) + suite.Assert().Len(suite.validator.Errors(), 1) + + assert.EqualError(suite.T(), suite.validator.Errors()[0], fmt.Sprintf("notifier: option 'template_path' refers to location '%s' which does not exist", p)) +} + /* File Tests. */ diff --git a/internal/configuration/validator/ntp_test.go b/internal/configuration/validator/ntp_test.go index 9780245f9..0bae5b830 100644 --- a/internal/configuration/validator/ntp_test.go +++ b/internal/configuration/validator/ntp_test.go @@ -49,5 +49,5 @@ func TestShouldRaiseErrorOnInvalidNTPVersion(t *testing.T) { require.Len(t, validator.Errors(), 1) - assert.EqualError(t, validator.Errors()[0], "ntp: option 'version' must be either 3 or 4 but it is configured as '1'") + assert.EqualError(t, validator.Errors()[0], "ntp: option 'version' must be either 3 or 4 but it's configured as '1'") } diff --git a/internal/configuration/validator/password_policy_test.go b/internal/configuration/validator/password_policy_test.go index 3d27f08d6..5ce417c01 100644 --- a/internal/configuration/validator/password_policy_test.go +++ b/internal/configuration/validator/password_policy_test.go @@ -39,7 +39,7 @@ func TestValidatePasswordPolicy(t *testing.T) { }, expectedErrs: []string{ "password_policy: only a single password policy mechanism can be specified", - "password_policy: standard: option 'min_length' must be greater than 0 but is configured as -1", + "password_policy: standard: option 'min_length' must be greater than 0 but it's configured as -1", }, }, { diff --git a/internal/configuration/validator/server.go b/internal/configuration/validator/server.go index 66a12d150..c38850634 100644 --- a/internal/configuration/validator/server.go +++ b/internal/configuration/validator/server.go @@ -155,13 +155,13 @@ func validateServerEndpointsAuthzEndpoint(config *schema.Configuration, name str config.Server.Endpoints.Authz[name] = endpoint default: if !utils.IsStringInSlice(endpoint.Implementation, validAuthzImplementations) { - validator.Push(fmt.Errorf(errFmtServerEndpointsAuthzImplementation, name, strings.Join(validAuthzImplementations, "', '"), endpoint.Implementation)) + validator.Push(fmt.Errorf(errFmtServerEndpointsAuthzImplementation, name, strJoinOr(validAuthzImplementations), endpoint.Implementation)) } else { validator.Push(fmt.Errorf(errFmtServerEndpointsAuthzLegacyInvalidImplementation, name)) } } } else if !utils.IsStringInSlice(endpoint.Implementation, validAuthzImplementations) { - validator.Push(fmt.Errorf(errFmtServerEndpointsAuthzImplementation, name, strings.Join(validAuthzImplementations, "', '"), endpoint.Implementation)) + validator.Push(fmt.Errorf(errFmtServerEndpointsAuthzImplementation, name, strJoinOr(validAuthzImplementations), endpoint.Implementation)) } if !reAuthzEndpointName.MatchString(name) { @@ -180,7 +180,7 @@ func validateServerEndpointsAuthzStrategies(name string, strategies []schema.Ser names = append(names, strategy.Name) if !utils.IsStringInSlice(strategy.Name, validAuthzAuthnStrategies) { - validator.Push(fmt.Errorf(errFmtServerEndpointsAuthzStrategy, name, strings.Join(validAuthzAuthnStrategies, "', '"), strategy.Name)) + validator.Push(fmt.Errorf(errFmtServerEndpointsAuthzStrategy, name, strJoinOr(validAuthzAuthnStrategies), strategy.Name)) } } } diff --git a/internal/configuration/validator/server_test.go b/internal/configuration/validator/server_test.go index c70e7124e..cf330d393 100644 --- a/internal/configuration/validator/server_test.go +++ b/internal/configuration/validator/server_test.go @@ -314,14 +314,18 @@ func TestServerAuthzEndpointErrors(t *testing.T) { map[string]schema.ServerAuthzEndpoint{ "example": {Implementation: "zero"}, }, - []string{"server: endpoints: authz: example: option 'implementation' must be one of 'AuthRequest', 'ForwardAuth', 'ExtAuthz', 'Legacy' but is configured as 'zero'"}, + []string{ + "server: endpoints: authz: example: option 'implementation' must be one of 'AuthRequest', 'ForwardAuth', 'ExtAuthz', or 'Legacy' but it's configured as 'zero'", + }, }, { "ShouldErrorOnInvalidEndpointImplementationLegacy", map[string]schema.ServerAuthzEndpoint{ "legacy": {Implementation: "zero"}, }, - []string{"server: endpoints: authz: legacy: option 'implementation' must be one of 'AuthRequest', 'ForwardAuth', 'ExtAuthz', 'Legacy' but is configured as 'zero'"}, + []string{ + "server: endpoints: authz: legacy: option 'implementation' must be one of 'AuthRequest', 'ForwardAuth', 'ExtAuthz', or 'Legacy' but it's configured as 'zero'", + }, }, { "ShouldErrorOnInvalidEndpointLegacyImplementation", @@ -335,7 +339,9 @@ func TestServerAuthzEndpointErrors(t *testing.T) { map[string]schema.ServerAuthzEndpoint{ "example": {Implementation: "ExtAuthz", AuthnStrategies: []schema.ServerAuthzEndpointAuthnStrategy{{Name: "bad-name"}}}, }, - []string{"server: endpoints: authz: example: authn_strategies: option 'name' must be one of 'CookieSession', 'HeaderAuthorization', 'HeaderProxyAuthorization', 'HeaderAuthRequestProxyAuthorization', 'HeaderLegacy' but is configured as 'bad-name'"}, + []string{ + "server: endpoints: authz: example: authn_strategies: option 'name' must be one of 'CookieSession', 'HeaderAuthorization', 'HeaderProxyAuthorization', 'HeaderAuthRequestProxyAuthorization', or 'HeaderLegacy' but it's configured as 'bad-name'", + }, }, { "ShouldErrorOnDuplicateName", diff --git a/internal/configuration/validator/session.go b/internal/configuration/validator/session.go index f63d24ded..1de078ef8 100644 --- a/internal/configuration/validator/session.go +++ b/internal/configuration/validator/session.go @@ -45,7 +45,7 @@ func validateSession(config *schema.SessionConfiguration, validator *schema.Stru if config.SameSite == "" { config.SameSite = schema.DefaultSessionConfiguration.SameSite } else if !utils.IsStringInSlice(config.SameSite, validSessionSameSiteValues) { - validator.Push(fmt.Errorf(errFmtSessionSameSite, strings.Join(validSessionSameSiteValues, "', '"), config.SameSite)) + validator.Push(fmt.Errorf(errFmtSessionSameSite, strJoinOr(validSessionSameSiteValues), config.SameSite)) } cookies := len(config.Cookies) @@ -73,7 +73,7 @@ func validateSession(config *schema.SessionConfiguration, validator *schema.Stru func validateSessionCookieDomains(config *schema.SessionConfiguration, validator *schema.StructValidator) { if len(config.Cookies) == 0 { - validator.Push(fmt.Errorf(errFmtSessionOptionRequired, "domain")) + validator.Push(fmt.Errorf(errFmtSessionOptionRequired, "cookies")) } domains := make([]string, 0) @@ -182,7 +182,7 @@ func validateSessionSameSite(i int, config *schema.SessionConfiguration, validat config.Cookies[i].SameSite = schema.DefaultSessionConfiguration.SameSite } } else if !utils.IsStringInSlice(config.Cookies[i].SameSite, validSessionSameSiteValues) { - validator.Push(fmt.Errorf(errFmtSessionDomainSameSite, sessionDomainDescriptor(i, config.Cookies[i]), strings.Join(validSessionSameSiteValues, "', '"), config.Cookies[i].SameSite)) + validator.Push(fmt.Errorf(errFmtSessionDomainSameSite, sessionDomainDescriptor(i, config.Cookies[i]), strJoinOr(validSessionSameSiteValues), config.Cookies[i].SameSite)) } } diff --git a/internal/configuration/validator/session_test.go b/internal/configuration/validator/session_test.go index f8db62b5b..1f18eaea4 100644 --- a/internal/configuration/validator/session_test.go +++ b/internal/configuration/validator/session_test.go @@ -95,7 +95,7 @@ func TestShouldSetDefaultSessionDomainsValues(t *testing.T) { }, }, []string{ - "session: option 'same_site' must be one of 'none', 'lax', 'strict' but is configured as 'BAD VALUE'", + "session: option 'same_site' must be one of 'none', 'lax', or 'strict' but it's configured as 'BAD VALUE'", }, }, { @@ -140,6 +140,24 @@ func TestShouldSetDefaultSessionDomainsValues(t *testing.T) { }, nil, }, + { + "ShouldErrorOnEmptyConfig", + schema.SessionConfiguration{ + SessionCookieCommonConfiguration: schema.SessionCookieCommonConfiguration{ + Name: "", SameSite: "", Domain: "", + }, + Cookies: []schema.SessionCookieConfiguration{}, + }, + schema.SessionConfiguration{ + SessionCookieCommonConfiguration: schema.SessionCookieCommonConfiguration{ + Name: "authelia_session", SameSite: "lax", Expiration: time.Hour, Inactivity: time.Minute * 5, RememberMe: time.Hour * 24 * 30, + }, + Cookies: []schema.SessionCookieConfiguration{}, + }, + []string{ + "session: option 'cookies' is required", + }, + }, } validator := schema.NewStructValidator() @@ -302,7 +320,7 @@ func TestShouldRaiseErrorWhenRedisHasHostnameButNoPort(t *testing.T) { assert.False(t, validator.HasWarnings()) assert.Len(t, validator.Errors(), 1) - assert.EqualError(t, validator.Errors()[0], "session: redis: option 'port' must be between 1 and 65535 but is configured as '0'") + assert.EqualError(t, validator.Errors()[0], "session: redis: option 'port' must be between 1 and 65535 but it's configured as '0'") } func TestShouldRaiseOneErrorWhenRedisHighAvailabilityHasNodesWithNoHost(t *testing.T) { @@ -646,7 +664,7 @@ func TestShouldRaiseErrorWhenDomainIsInvalid(t *testing.T) { {"ShouldRaiseErrorOnPublicDomainDuckDNS", "duckdns.org", nil, []string{"session: domain config #1 (domain 'duckdns.org'): option 'domain' is not a valid cookie domain: the domain is part of the special public suffix list"}}, {"ShouldNotRaiseErrorOnSuffixOfPublicDomainDuckDNS", "example.duckdns.org", nil, nil}, {"ShouldRaiseWarningOnDomainWithLeadingDot", ".example.com", []string{"session: domain config #1 (domain '.example.com'): option 'domain' has a prefix of '.' which is not supported or intended behaviour: you can use this at your own risk but we recommend removing it"}, nil}, - {"ShouldRaiseErrorOnDomainWithLeadingStarDot", "*.example.com", nil, []string{"session: domain config #1 (domain '*.example.com'): option 'domain' must be the domain you wish to protect not a wildcard domain but it is configured as '*.example.com'"}}, + {"ShouldRaiseErrorOnDomainWithLeadingStarDot", "*.example.com", nil, []string{"session: domain config #1 (domain '*.example.com'): option 'domain' must be the domain you wish to protect not a wildcard domain but it's configured as '*.example.com'"}}, {"ShouldRaiseErrorOnDomainNotSet", "", nil, []string{"session: domain config #1 (domain ''): option 'domain' is required"}}, } @@ -726,8 +744,8 @@ func TestShouldRaiseErrorWhenSameSiteSetIncorrectly(t *testing.T) { assert.False(t, validator.HasWarnings()) require.Len(t, validator.Errors(), 2) - assert.EqualError(t, validator.Errors()[0], "session: option 'same_site' must be one of 'none', 'lax', 'strict' but is configured as 'NOne'") - assert.EqualError(t, validator.Errors()[1], "session: domain config #1 (domain 'example.com'): option 'same_site' must be one of 'none', 'lax', 'strict' but is configured as 'NOne'") + assert.EqualError(t, validator.Errors()[0], "session: option 'same_site' must be one of 'none', 'lax', or 'strict' but it's configured as 'NOne'") + assert.EqualError(t, validator.Errors()[1], "session: domain config #1 (domain 'example.com'): option 'same_site' must be one of 'none', 'lax', or 'strict' but it's configured as 'NOne'") } func TestShouldNotRaiseErrorWhenSameSiteSetCorrectly(t *testing.T) { diff --git a/internal/configuration/validator/storage.go b/internal/configuration/validator/storage.go index 7172383a7..035ac62e8 100644 --- a/internal/configuration/validator/storage.go +++ b/internal/configuration/validator/storage.go @@ -3,7 +3,6 @@ package validator import ( "errors" "fmt" - "strings" "github.com/authelia/authelia/v4/internal/configuration/schema" "github.com/authelia/authelia/v4/internal/utils" @@ -92,7 +91,7 @@ func validatePostgreSQLConfiguration(config *schema.PostgreSQLStorageConfigurati case config.SSL.Mode == "": config.SSL.Mode = schema.DefaultPostgreSQLStorageConfiguration.SSL.Mode case !utils.IsStringInSlice(config.SSL.Mode, validStoragePostgreSQLSSLModes): - validator.Push(fmt.Errorf(errFmtStoragePostgreSQLInvalidSSLMode, strings.Join(validStoragePostgreSQLSSLModes, "', '"), config.SSL.Mode)) + validator.Push(fmt.Errorf(errFmtStoragePostgreSQLInvalidSSLMode, strJoinOr(validStoragePostgreSQLSSLModes), config.SSL.Mode)) } } } diff --git a/internal/configuration/validator/storage_test.go b/internal/configuration/validator/storage_test.go index 8ac7d9dbb..f69bffab1 100644 --- a/internal/configuration/validator/storage_test.go +++ b/internal/configuration/validator/storage_test.go @@ -360,7 +360,7 @@ func (suite *StorageSuite) TestShouldValidatePostgresSSLModeMustBeValid() { suite.Assert().Len(suite.validator.Warnings(), 1) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "storage: postgres: ssl: option 'mode' must be one of 'disable', 'require', 'verify-ca', 'verify-full' but it is configured as 'unknown'") + suite.Assert().EqualError(suite.validator.Errors()[0], "storage: postgres: ssl: option 'mode' must be one of 'disable', 'require', 'verify-ca', or 'verify-full' but it's configured as 'unknown'") } func (suite *StorageSuite) TestShouldRaiseErrorOnNoEncryptionKey() { diff --git a/internal/configuration/validator/telemetry_test.go b/internal/configuration/validator/telemetry_test.go index aa3e59683..1643a5c40 100644 --- a/internal/configuration/validator/telemetry_test.go +++ b/internal/configuration/validator/telemetry_test.go @@ -58,7 +58,7 @@ func TestValidateTelemetry(t *testing.T) { &schema.Configuration{Telemetry: schema.TelemetryConfig{Metrics: schema.TelemetryMetricsConfig{Address: mustParseAddress("udp://0.0.0.0")}}}, &schema.Configuration{Telemetry: schema.TelemetryConfig{Metrics: schema.TelemetryMetricsConfig{Address: mustParseAddress("udp://0.0.0.0:9959")}}}, nil, - []string{"telemetry: metrics: option 'address' must have a scheme 'tcp://' but it is configured as 'udp'"}, + []string{"telemetry: metrics: option 'address' must have a scheme 'tcp://' but it's configured as 'udp'"}, }, } diff --git a/internal/configuration/validator/theme.go b/internal/configuration/validator/theme.go index f6c1a68d7..ccb8b7f52 100644 --- a/internal/configuration/validator/theme.go +++ b/internal/configuration/validator/theme.go @@ -2,7 +2,6 @@ package validator import ( "fmt" - "strings" "github.com/authelia/authelia/v4/internal/configuration/schema" "github.com/authelia/authelia/v4/internal/utils" @@ -15,6 +14,6 @@ func ValidateTheme(config *schema.Configuration, validator *schema.StructValidat } if !utils.IsStringInSlice(config.Theme, validThemeNames) { - validator.Push(fmt.Errorf(errFmtThemeName, strings.Join(validThemeNames, "', '"), config.Theme)) + validator.Push(fmt.Errorf(errFmtThemeName, strJoinOr(validThemeNames), config.Theme)) } } diff --git a/internal/configuration/validator/theme_test.go b/internal/configuration/validator/theme_test.go index abe796611..b1c8b89bc 100644 --- a/internal/configuration/validator/theme_test.go +++ b/internal/configuration/validator/theme_test.go @@ -36,7 +36,7 @@ func (suite *Theme) TestShouldRaiseErrorWhenInvalidThemeProvided() { suite.Assert().Len(suite.validator.Warnings(), 0) suite.Require().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "option 'theme' must be one of 'light', 'dark', 'grey', 'auto' but it is configured as 'invalid'") + suite.Assert().EqualError(suite.validator.Errors()[0], "option 'theme' must be one of 'light', 'dark', 'grey', or 'auto' but it's configured as 'invalid'") } func TestThemes(t *testing.T) { diff --git a/internal/configuration/validator/totp.go b/internal/configuration/validator/totp.go index 0a379a067..01a763f88 100644 --- a/internal/configuration/validator/totp.go +++ b/internal/configuration/validator/totp.go @@ -24,7 +24,7 @@ func ValidateTOTP(config *schema.Configuration, validator *schema.StructValidato config.TOTP.Algorithm = strings.ToUpper(config.TOTP.Algorithm) if !utils.IsStringInSlice(config.TOTP.Algorithm, schema.TOTPPossibleAlgorithms) { - validator.Push(fmt.Errorf(errFmtTOTPInvalidAlgorithm, strings.Join(schema.TOTPPossibleAlgorithms, "', '"), config.TOTP.Algorithm)) + validator.Push(fmt.Errorf(errFmtTOTPInvalidAlgorithm, strJoinOr(schema.TOTPPossibleAlgorithms), config.TOTP.Algorithm)) } } diff --git a/internal/configuration/validator/totp_test.go b/internal/configuration/validator/totp_test.go index 956f074c4..b94f7f00b 100644 --- a/internal/configuration/validator/totp_test.go +++ b/internal/configuration/validator/totp_test.go @@ -56,7 +56,9 @@ func TestValidateTOTP(t *testing.T) { Skew: schema.DefaultTOTPConfiguration.Skew, Issuer: "abc", }, - errs: []string{"totp: option 'algorithm' must be one of 'SHA1', 'SHA256', 'SHA512' but it is configured as 'SHA3'"}, + errs: []string{ + "totp: option 'algorithm' must be one of 'SHA1', 'SHA256', or 'SHA512' but it's configured as 'SHA3'", + }, }, { desc: "ShouldRaiseErrorWhenInvalidTOTPValue", @@ -69,10 +71,10 @@ func TestValidateTOTP(t *testing.T) { Issuer: "abc", }, errs: []string{ - "totp: option 'algorithm' must be one of 'SHA1', 'SHA256', 'SHA512' but it is configured as 'SHA3'", - "totp: option 'period' option must be 15 or more but it is configured as '5'", - "totp: option 'digits' must be 6 or 8 but it is configured as '20'", - "totp: option 'secret_size' must be 20 or higher but it is configured as '10'", + "totp: option 'algorithm' must be one of 'SHA1', 'SHA256', or 'SHA512' but it's configured as 'SHA3'", + "totp: option 'period' option must be 15 or more but it's configured as '5'", + "totp: option 'digits' must be 6 or 8 but it's configured as '20'", + "totp: option 'secret_size' must be 20 or higher but it's configured as '10'", }, }, } diff --git a/internal/configuration/validator/util.go b/internal/configuration/validator/util.go index b68bfee7a..59b9411b1 100644 --- a/internal/configuration/validator/util.go +++ b/internal/configuration/validator/util.go @@ -4,6 +4,8 @@ import ( "strings" "golang.org/x/net/publicsuffix" + + "github.com/authelia/authelia/v4/internal/utils" ) func isCookieDomainAPublicSuffix(domain string) (valid bool) { @@ -13,3 +15,95 @@ func isCookieDomainAPublicSuffix(domain string) (valid bool) { return len(strings.TrimLeft(domain, ".")) == len(suffix) } + +func strJoinOr(items []string) string { + return strJoinComma("or", items) +} + +func strJoinAnd(items []string) string { + return strJoinComma("and", items) +} + +func strJoinComma(word string, items []string) string { + if word == "" { + return buildJoinedString(",", "", "'", items) + } + + return buildJoinedString(",", word, "'", items) +} + +func buildJoinedString(sep, sepFinal, quote string, items []string) string { + n := len(items) + + if n == 0 { + return "" + } + + b := &strings.Builder{} + + for i := 0; i < n; i++ { + if quote != "" { + b.WriteString(quote) + } + + b.WriteString(items[i]) + + if quote != "" { + b.WriteString(quote) + } + + if i == (n - 1) { + continue + } + + if sep != "" { + if sepFinal == "" || n != 2 { + b.WriteString(sep) + } + + b.WriteString(" ") + } + + if sepFinal != "" && i == (n-2) { + b.WriteString(strings.Trim(sepFinal, " ")) + b.WriteString(" ") + } + } + + return b.String() +} + +func validateList(values, valid []string, chkDuplicate bool) (invalid, duplicates []string) { //nolint:unparam + chkValid := len(valid) != 0 + + for i, value := range values { + if chkValid { + if !utils.IsStringInSlice(value, valid) { + invalid = append(invalid, value) + + // Skip checking duplicates for invalid values. + continue + } + } + + if chkDuplicate { + for j, valueAlt := range values { + if i == j { + continue + } + + if value != valueAlt { + continue + } + + if utils.IsStringInSlice(value, duplicates) { + continue + } + + duplicates = append(duplicates, value) + } + } + } + + return +} diff --git a/internal/configuration/validator/webauthn.go b/internal/configuration/validator/webauthn.go index 47aaa2704..406ccc7e6 100644 --- a/internal/configuration/validator/webauthn.go +++ b/internal/configuration/validator/webauthn.go @@ -2,7 +2,6 @@ package validator import ( "fmt" - "strings" "github.com/authelia/authelia/v4/internal/configuration/schema" "github.com/authelia/authelia/v4/internal/utils" @@ -22,13 +21,13 @@ func ValidateWebauthn(config *schema.Configuration, validator *schema.StructVali case config.Webauthn.ConveyancePreference == "": config.Webauthn.ConveyancePreference = schema.DefaultWebauthnConfiguration.ConveyancePreference case !utils.IsStringInSlice(string(config.Webauthn.ConveyancePreference), validWebauthnConveyancePreferences): - validator.Push(fmt.Errorf(errFmtWebauthnConveyancePreference, strings.Join(validWebauthnConveyancePreferences, "', '"), config.Webauthn.ConveyancePreference)) + validator.Push(fmt.Errorf(errFmtWebauthnConveyancePreference, strJoinOr(validWebauthnConveyancePreferences), config.Webauthn.ConveyancePreference)) } switch { case config.Webauthn.UserVerification == "": config.Webauthn.UserVerification = schema.DefaultWebauthnConfiguration.UserVerification case !utils.IsStringInSlice(string(config.Webauthn.UserVerification), validWebauthnUserVerificationRequirement): - validator.Push(fmt.Errorf(errFmtWebauthnUserVerification, config.Webauthn.UserVerification)) + validator.Push(fmt.Errorf(errFmtWebauthnUserVerification, strJoinOr(validWebauthnConveyancePreferences), config.Webauthn.UserVerification)) } } diff --git a/internal/configuration/validator/webauthn_test.go b/internal/configuration/validator/webauthn_test.go index bfa746a32..9aaaa0aac 100644 --- a/internal/configuration/validator/webauthn_test.go +++ b/internal/configuration/validator/webauthn_test.go @@ -93,6 +93,6 @@ func TestWebauthnShouldRaiseErrorsOnInvalidOptions(t *testing.T) { require.Len(t, validator.Errors(), 2) - assert.EqualError(t, validator.Errors()[0], "webauthn: option 'attestation_conveyance_preference' must be one of 'none', 'indirect', 'direct' but it is configured as 'no'") - assert.EqualError(t, validator.Errors()[1], "webauthn: option 'user_verification' must be one of 'discouraged', 'preferred', 'required' but it is configured as 'yes'") + assert.EqualError(t, validator.Errors()[0], "webauthn: option 'attestation_conveyance_preference' must be one of 'none', 'indirect', or 'direct' but it's configured as 'no'") + assert.EqualError(t, validator.Errors()[1], "webauthn: option 'user_verification' must be one of 'none', 'indirect', or 'direct' but it's configured as 'yes'") } |