From de2c5836fd8a2b99260b555173fcd3023292da0f Mon Sep 17 00:00:00 2001 From: Amir Zarrinkafsh Date: Thu, 9 Apr 2020 11:05:17 +1000 Subject: [PATCH] [Buildkite] Introduce CI linting with golangci-lint and reviewdog (#832) * [Buildkite] Introduce CI linting with golangci-lint and reviewdog * Initial pass of golangci-lint * Add gosimple (megacheck) recommendations * Add golint recommendations * [BUGFIX] Migrate authentication traces from v3 mongodb * Add deadcode recommendations * [BUGFIX] Fix ShortTimeouts suite when run in dev workflow * Add unused recommendations * Add unparam recommendations * Disable linting on unfixable errors instead of skipping files * Adjust nolint notation for unparam * Fix ineffectual assignment to err raised by linter. * Export environment variable in agent hook * Add ineffassign recommendations * Add staticcheck recommendations * Add gocyclo recommendations * Adjust ineffassign recommendations Co-authored-by: Clement Michaud --- .buildkite/hooks/post-command | 7 +++ .golangci.yml | 25 ++++++++ .reviewdog.yml | 8 +++ cmd/authelia-scripts/cmd_bootstrap.go | 46 ++++++--------- cmd/authelia-scripts/cmd_ci.go | 10 ---- cmd/authelia-scripts/cmd_suites.go | 4 +- cmd/authelia-scripts/main.go | 16 ++--- cmd/authelia/main.go | 2 +- internal/authentication/const.go | 12 ++-- .../authentication/file_user_provider_test.go | 8 +-- internal/authentication/ldap_user_provider.go | 2 +- .../authentication/ldap_user_provider_test.go | 18 +++--- internal/authentication/password_hash.go | 50 ++++++++-------- internal/authentication/password_hash_test.go | 50 ++++++++-------- internal/authorization/authorizer_test.go | 6 -- internal/commands/certificates.go | 1 + internal/commands/hash.go | 4 +- internal/commands/migration_local.go | 2 +- internal/commands/migration_mongo.go | 3 +- internal/configuration/reader.go | 6 -- .../configuration/schema/authentication.go | 10 ++-- internal/configuration/schema/validator.go | 2 +- .../configuration/validator/authentication.go | 6 +- .../configuration/validator/regulation.go | 7 +-- internal/configuration/validator/session.go | 6 +- .../configuration/validator/session_test.go | 6 +- internal/configuration/validator/totp_test.go | 1 - internal/handlers/const.go | 1 - .../handler_extended_configuration_test.go | 18 +++--- internal/handlers/handler_firstfactor_test.go | 14 +---- .../handler_register_u2f_step1_test.go | 7 --- .../handlers/handler_register_u2f_step2.go | 4 ++ internal/handlers/handler_user_info_test.go | 9 ++- internal/handlers/handler_verify.go | 40 ++++++------- internal/handlers/handler_verify_test.go | 58 +++++++++---------- internal/handlers/types.go | 6 -- internal/mocks/mock_authelia_ctx.go | 8 +-- internal/notification/smtp_notifier.go | 54 ++++++++--------- internal/regulation/regulator_test.go | 46 +++++++-------- internal/session/provider_config_test.go | 2 +- internal/storage/sql_provider.go | 15 ++--- .../suites/ShortTimeouts/configuration.yml | 2 +- internal/suites/action_login.go | 2 +- internal/suites/environment.go | 8 ++- .../suites/example/compose/squid/squid.conf | 2 +- internal/suites/scenario_regulation_test.go | 1 - internal/suites/suite_bypass_all.go | 2 +- internal/suites/suite_docker.go | 2 +- internal/suites/suite_duo_push.go | 2 +- internal/suites/suite_haproxy.go | 2 +- internal/suites/suite_high_availability.go | 2 +- .../suites/suite_high_availability_test.go | 6 +- internal/suites/suite_ldap.go | 2 +- internal/suites/suite_mariadb.go | 2 +- internal/suites/suite_mysql.go | 2 +- internal/suites/suite_network_acl.go | 2 +- internal/suites/suite_one_factor_only.go | 2 +- internal/suites/suite_postgres.go | 2 +- internal/suites/suite_short_timeouts.go | 2 +- internal/suites/suite_standalone.go | 2 +- internal/suites/suite_traefik.go | 2 +- internal/suites/suite_traefik2.go | 2 +- internal/utils/safe_redirection_test.go | 2 +- internal/utils/time.go | 17 +++--- internal/utils/time_test.go | 8 +-- 65 files changed, 326 insertions(+), 352 deletions(-) create mode 100644 .golangci.yml create mode 100644 .reviewdog.yml diff --git a/.buildkite/hooks/post-command b/.buildkite/hooks/post-command index b9a30d822..edcdb7141 100755 --- a/.buildkite/hooks/post-command +++ b/.buildkite/hooks/post-command @@ -2,6 +2,13 @@ set +u +if [[ $BUILDKITE_PULL_REQUEST != "false" ]]; then + if [[ $BUILDKITE_LABEL == ":hammer_and_wrench: Unit Test" ]]; then + echo "--- :go::service_dog: Linting pull request" + reviewdog -reporter=github-pr-review + fi +fi + if [[ $BUILDKITE_LABEL =~ ":selenium:" ]] || [[ $BUILDKITE_LABEL =~ ":docker: Build Image" ]]; then CONTAINERS=$(docker ps -a -q) if [[ ${CONTAINERS} != "" ]]; then diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..6bb377f81 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,25 @@ +run: + timeout: 3m + +linters-settings: + gocyclo: + min-complexity: 15 + goimports: + local-prefixes: github.com/authelia/authelia + +linters: + enable: + - gocyclo + - gofmt + - goimports + - golint + - interfacer + - maligned + - misspell + - prealloc + - unparam + - whitespace + +issues: + max-issues-per-linter: 0 + max-same-issues: 0 \ No newline at end of file diff --git a/.reviewdog.yml b/.reviewdog.yml new file mode 100644 index 000000000..c058d7086 --- /dev/null +++ b/.reviewdog.yml @@ -0,0 +1,8 @@ +runner: + golangci: + cmd: golangci-lint run + errorformat: + - '%E%f:%l:%c: %m' + - '%E%f:%l: %m' + - '%C%.%#' + level: warning \ No newline at end of file diff --git a/cmd/authelia-scripts/cmd_bootstrap.go b/cmd/authelia-scripts/cmd_bootstrap.go index 11013d43b..435736b0b 100644 --- a/cmd/authelia-scripts/cmd_bootstrap.go +++ b/cmd/authelia-scripts/cmd_bootstrap.go @@ -21,33 +21,33 @@ type HostEntry struct { var hostEntries = []HostEntry{ // For authelia backend - HostEntry{Domain: "authelia.example.com", IP: "192.168.240.50"}, + {Domain: "authelia.example.com", IP: "192.168.240.50"}, // For common tests - HostEntry{Domain: "login.example.com", IP: "192.168.240.100"}, - HostEntry{Domain: "admin.example.com", IP: "192.168.240.100"}, - HostEntry{Domain: "singlefactor.example.com", IP: "192.168.240.100"}, - HostEntry{Domain: "dev.example.com", IP: "192.168.240.100"}, - HostEntry{Domain: "home.example.com", IP: "192.168.240.100"}, - HostEntry{Domain: "mx1.mail.example.com", IP: "192.168.240.100"}, - HostEntry{Domain: "mx2.mail.example.com", IP: "192.168.240.100"}, - HostEntry{Domain: "public.example.com", IP: "192.168.240.100"}, - HostEntry{Domain: "secure.example.com", IP: "192.168.240.100"}, - HostEntry{Domain: "mail.example.com", IP: "192.168.240.100"}, - HostEntry{Domain: "duo.example.com", IP: "192.168.240.100"}, + {Domain: "login.example.com", IP: "192.168.240.100"}, + {Domain: "admin.example.com", IP: "192.168.240.100"}, + {Domain: "singlefactor.example.com", IP: "192.168.240.100"}, + {Domain: "dev.example.com", IP: "192.168.240.100"}, + {Domain: "home.example.com", IP: "192.168.240.100"}, + {Domain: "mx1.mail.example.com", IP: "192.168.240.100"}, + {Domain: "mx2.mail.example.com", IP: "192.168.240.100"}, + {Domain: "public.example.com", IP: "192.168.240.100"}, + {Domain: "secure.example.com", IP: "192.168.240.100"}, + {Domain: "mail.example.com", IP: "192.168.240.100"}, + {Domain: "duo.example.com", IP: "192.168.240.100"}, // For Traefik suite - HostEntry{Domain: "traefik.example.com", IP: "192.168.240.100"}, + {Domain: "traefik.example.com", IP: "192.168.240.100"}, // For HAProxy suite - HostEntry{Domain: "haproxy.example.com", IP: "192.168.240.100"}, + {Domain: "haproxy.example.com", IP: "192.168.240.100"}, // For testing network ACLs - HostEntry{Domain: "proxy-client1.example.com", IP: "192.168.240.201"}, - HostEntry{Domain: "proxy-client2.example.com", IP: "192.168.240.202"}, - HostEntry{Domain: "proxy-client3.example.com", IP: "192.168.240.203"}, + {Domain: "proxy-client1.example.com", IP: "192.168.240.201"}, + {Domain: "proxy-client2.example.com", IP: "192.168.240.202"}, + {Domain: "proxy-client3.example.com", IP: "192.168.240.203"}, // Kubernetes dashboard - HostEntry{Domain: "kubernetes.example.com", IP: "192.168.240.110"}, + {Domain: "kubernetes.example.com", IP: "192.168.240.110"}, } func runCommand(cmd string, args ...string) { @@ -71,16 +71,6 @@ func checkCommandExist(cmd string) { fmt.Println(" OK") } -func installClientNpmPackages() { - command := utils.CommandWithStdout("yarn", "install") - command.Dir = "client" - err := command.Run() - - if err != nil { - panic(err) - } -} - func createTemporaryDirectory() { err := os.MkdirAll("/tmp/authelia", 0755) diff --git a/cmd/authelia-scripts/cmd_ci.go b/cmd/authelia-scripts/cmd_ci.go index b409f9b8a..08c0cbe44 100644 --- a/cmd/authelia-scripts/cmd_ci.go +++ b/cmd/authelia-scripts/cmd_ci.go @@ -7,16 +7,6 @@ import ( "github.com/authelia/authelia/internal/utils" ) -const dockerPullCommandLine = "docker-compose -p authelia -f internal/suites/docker-compose.yml " + - "-f internal/suites/example/compose/mariadb/docker-compose.yml " + - "-f internal/suites/example/compose/redis/docker-compose.yml " + - "-f internal/suites/example/compose/nginx/portal/docker-compose.yml " + - "-f internal/suites/example/compose/smtp/docker-compose.yml " + - "-f internal/suites/example/compose/httpbin/docker-compose.yml " + - "-f internal/suites/example/compose/ldap/docker-compose.admin.yml " + - "-f internal/suites/example/compose/ldap/docker-compose.yml " + - "pull" - // RunCI run the CI scripts func RunCI(cmd *cobra.Command, args []string) { log.Info("=====> Build stage <=====") diff --git a/cmd/authelia-scripts/cmd_suites.go b/cmd/authelia-scripts/cmd_suites.go index eb79b1651..e73fefd04 100644 --- a/cmd/authelia-scripts/cmd_suites.go +++ b/cmd/authelia-scripts/cmd_suites.go @@ -106,9 +106,7 @@ var SuitesTestCmd = &cobra.Command{ func listSuites() []string { suiteNames := make([]string, 0) - for _, k := range suites.GlobalRegistry.Suites() { - suiteNames = append(suiteNames, k) - } + suiteNames = append(suiteNames, suites.GlobalRegistry.Suites()...) sort.Strings(suiteNames) return suiteNames } diff --git a/cmd/authelia-scripts/main.go b/cmd/authelia-scripts/main.go index d4a537f89..19171fc31 100755 --- a/cmd/authelia-scripts/main.go +++ b/cmd/authelia-scripts/main.go @@ -28,35 +28,35 @@ type CobraCommands = []*cobra.Command // Commands is the list of commands of authelia-scripts var Commands = []AutheliaCommandDefinition{ - AutheliaCommandDefinition{ + { Name: "bootstrap", Short: "Prepare environment for development and testing.", Long: `Prepare environment for development and testing. This command prepares docker images and download tools like Kind for Kubernetes testing.`, Func: Bootstrap, }, - AutheliaCommandDefinition{ + { Name: "build", Short: "Build Authelia binary and static assets", Func: Build, }, - AutheliaCommandDefinition{ + { Name: "clean", Short: "Clean build artifacts", Func: Clean, }, - AutheliaCommandDefinition{ + { Name: "docker", Short: "Commands related to building and publishing docker image", SubCommands: CobraCommands{DockerBuildCmd, DockerPushCmd, DockerManifestCmd}, }, - AutheliaCommandDefinition{ + { Name: "serve [config]", Short: "Serve compiled version of Authelia", Func: ServeCmd, Args: cobra.MinimumNArgs(1), }, - AutheliaCommandDefinition{ + { Name: "suites", Short: "Compute hash of a password for creating a file-based users database", SubCommands: CobraCommands{ @@ -66,12 +66,12 @@ var Commands = []AutheliaCommandDefinition{ SuitesTeardownCmd, }, }, - AutheliaCommandDefinition{ + { Name: "ci", Short: "Run continuous integration script", Func: RunCI, }, - AutheliaCommandDefinition{ + { Name: "unittest", Short: "Run unit tests", Func: RunUnitTest, diff --git a/cmd/authelia/main.go b/cmd/authelia/main.go index 8a0cb0ea7..364f69d70 100644 --- a/cmd/authelia/main.go +++ b/cmd/authelia/main.go @@ -25,6 +25,7 @@ import ( var configPathFlag string +//nolint:gocyclo // TODO: Consider refactoring/simplifying, time permitting func startServer() { if configPathFlag == "" { log.Fatal(errors.New("No config file path provided")) @@ -47,7 +48,6 @@ func startServer() { case "info": logging.Logger().Info("Logging severity set to info") logging.SetLevel(logrus.InfoLevel) - break case "debug": logging.Logger().Info("Logging severity set to debug") logging.SetLevel(logrus.DebugLevel) diff --git a/internal/authentication/const.go b/internal/authentication/const.go index 1931b4342..a63c678c6 100644 --- a/internal/authentication/const.go +++ b/internal/authentication/const.go @@ -4,11 +4,11 @@ package authentication type Level int const ( - // NotAuthenticated if the user is not authenticated yet. + // NotAuthenticated if the user is not authenticated yet NotAuthenticated Level = iota - // OneFactor if the user has passed first factor only. + // OneFactor if the user has passed first factor only OneFactor Level = iota - // TwoFactor if the user has passed two factors. + // TwoFactor if the user has passed two factors TwoFactor Level = iota ) @@ -17,11 +17,11 @@ const ( TOTP = "totp" // U2F Method using U2F devices like Yubikeys U2F = "u2f" - // Push Method using Duo application to receive push notifications. + // Push Method using Duo application to receive push notifications Push = "mobile_push" ) -// PossibleMethods is the set of all possible 2FA methods. +// PossibleMethods is the set of all possible 2FA methods var PossibleMethods = []string{TOTP, U2F, Push} const ( @@ -40,5 +40,5 @@ const ( HashingDefaultSHA512Iterations = 5000 ) -// Valid Hashing runes +// HashingPossibleSaltCharacters represents valid hashing runes var HashingPossibleSaltCharacters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789+/") diff --git a/internal/authentication/file_user_provider_test.go b/internal/authentication/file_user_provider_test.go index c5f3e7ce4..b55196b19 100644 --- a/internal/authentication/file_user_provider_test.go +++ b/internal/authentication/file_user_provider_test.go @@ -172,7 +172,7 @@ func TestShouldRaiseWhenLoadingDatabaseWithBadSHA512HashesForTheFirstTime(t *tes WithDatabase(BadSHA512HashContent, func(path string) { config := DefaultFileAuthenticationBackendConfiguration config.Path = path - assert.PanicsWithValue(t, "Unable to parse hash of user john: Hash key is not the last parameter, the hash is likely malformed ($6$rounds00000$jgiCMRyGXzoqpxS3$w2pJeZnnH8bwW3zzvoMWtTRfQYsHbWbD/hquuQ5vUeIyl9gdwBIt6RWk2S6afBA0DPakbeWgD/4SZPiS0hYtU/).", func() { + assert.PanicsWithValue(t, "Unable to parse hash of user john: Hash key is not the last parameter, the hash is likely malformed ($6$rounds00000$jgiCMRyGXzoqpxS3$w2pJeZnnH8bwW3zzvoMWtTRfQYsHbWbD/hquuQ5vUeIyl9gdwBIt6RWk2S6afBA0DPakbeWgD/4SZPiS0hYtU/)", func() { NewFileUserProvider(&config) }) }) @@ -182,7 +182,7 @@ func TestShouldRaiseWhenLoadingDatabaseWithBadArgon2idHashSettingsForTheFirstTim WithDatabase(BadArgon2idHashSettingsContent, func(path string) { config := DefaultFileAuthenticationBackendConfiguration config.Path = path - assert.PanicsWithValue(t, "Unable to parse hash of user john: Hash key is not the last parameter, the hash is likely malformed ($argon2id$v=19$m65536,t3,p2$BpLnfgDsc2WD8F2q$o/vzA4myCqZZ36bUGsDY//8mKUYNZZaR0t4MFFSs+iM).", func() { + assert.PanicsWithValue(t, "Unable to parse hash of user john: Hash key is not the last parameter, the hash is likely malformed ($argon2id$v=19$m65536,t3,p2$BpLnfgDsc2WD8F2q$o/vzA4myCqZZ36bUGsDY//8mKUYNZZaR0t4MFFSs+iM)", func() { NewFileUserProvider(&config) }) }) @@ -192,7 +192,7 @@ func TestShouldRaiseWhenLoadingDatabaseWithBadArgon2idHashKeyForTheFirstTime(t * WithDatabase(BadArgon2idHashKeyContent, func(path string) { config := DefaultFileAuthenticationBackendConfiguration config.Path = path - assert.PanicsWithValue(t, "Unable to parse hash of user john: Hash key contains invalid base64 characters.", func() { + assert.PanicsWithValue(t, "Unable to parse hash of user john: Hash key contains invalid base64 characters", func() { NewFileUserProvider(&config) }) }) @@ -202,7 +202,7 @@ func TestShouldRaiseWhenLoadingDatabaseWithBadArgon2idHashSaltForTheFirstTime(t WithDatabase(BadArgon2idHashSaltContent, func(path string) { config := DefaultFileAuthenticationBackendConfiguration config.Path = path - assert.PanicsWithValue(t, "Unable to parse hash of user john: Salt contains invalid base64 characters.", func() { + assert.PanicsWithValue(t, "Unable to parse hash of user john: Salt contains invalid base64 characters", func() { NewFileUserProvider(&config) }) }) diff --git a/internal/authentication/ldap_user_provider.go b/internal/authentication/ldap_user_provider.go index 1ece76562..a9a26760e 100644 --- a/internal/authentication/ldap_user_provider.go +++ b/internal/authentication/ldap_user_provider.go @@ -178,7 +178,7 @@ func (p *LDAPUserProvider) getUserProfile(conn LDAPConnection, inputUsername str return &userProfile, nil } -func (p *LDAPUserProvider) resolveGroupsFilter(inputUsername string, profile *ldapUserProfile) (string, error) { +func (p *LDAPUserProvider) resolveGroupsFilter(inputUsername string, profile *ldapUserProfile) (string, error) { //nolint:unparam inputUsername = p.ldapEscape(inputUsername) // We temporarily keep placeholder {0} for backward compatibility diff --git a/internal/authentication/ldap_user_provider_test.go b/internal/authentication/ldap_user_provider_test.go index b07e811fa..fb5841d99 100644 --- a/internal/authentication/ldap_user_provider_test.go +++ b/internal/authentication/ldap_user_provider_test.go @@ -180,7 +180,7 @@ func TestShouldCombineUsernameFilterAndUsersFilter(t *testing.T) { func createSearchResultWithAttributes(attributes ...*ldap.EntryAttribute) *ldap.SearchResult { return &ldap.SearchResult{ Entries: []*ldap.Entry{ - &ldap.Entry{Attributes: attributes}, + {Attributes: attributes}, }, } } @@ -227,14 +227,14 @@ func TestShouldNotCrashWhenGroupsAreNotRetrievedFromLDAP(t *testing.T) { Search(gomock.Any()). Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ - &ldap.Entry{ + { DN: "uid=test,dc=example,dc=com", Attributes: []*ldap.EntryAttribute{ - &ldap.EntryAttribute{ + { Name: "mail", Values: []string{"test@example.com"}, }, - &ldap.EntryAttribute{ + { Name: "uid", Values: []string{"john"}, }, @@ -288,10 +288,10 @@ func TestShouldNotCrashWhenEmailsAreNotRetrievedFromLDAP(t *testing.T) { Search(gomock.Any()). Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ - &ldap.Entry{ + { DN: "uid=test,dc=example,dc=com", Attributes: []*ldap.EntryAttribute{ - &ldap.EntryAttribute{ + { Name: "uid", Values: []string{"john"}, }, @@ -346,14 +346,14 @@ func TestShouldReturnUsernameFromLDAP(t *testing.T) { Search(gomock.Any()). Return(&ldap.SearchResult{ Entries: []*ldap.Entry{ - &ldap.Entry{ + { DN: "uid=test,dc=example,dc=com", Attributes: []*ldap.EntryAttribute{ - &ldap.EntryAttribute{ + { Name: "mail", Values: []string{"test@example.com"}, }, - &ldap.EntryAttribute{ + { Name: "uid", Values: []string{"John"}, }, diff --git a/internal/authentication/password_hash.go b/internal/authentication/password_hash.go index f7977bab5..1ab17e557 100644 --- a/internal/authentication/password_hash.go +++ b/internal/authentication/password_hash.go @@ -12,7 +12,7 @@ import ( ) // PasswordHash represents all characteristics of a password hash. -// Authelia only supports salted SHA512 or salted argon2id method, i.e., $6$ mode or $argon2id$ mode. +// Authelia only supports salted SHA512 or salted argon2id method, i.e., $6$ mode or $argon2id$ mode type PasswordHash struct { Algorithm string Iterations int @@ -23,7 +23,7 @@ type PasswordHash struct { Parallelism int } -// ParseHash extracts all characteristics of a hash given its string representation. +// ParseHash extracts all characteristics of a hash given its string representation func ParseHash(hash string) (passwordHash *PasswordHash, err error) { parts := strings.Split(hash, "$") @@ -35,7 +35,7 @@ func ParseHash(hash string) (passwordHash *PasswordHash, err error) { h.Key = key if h.Key != parts[len(parts)-1] { - return nil, fmt.Errorf("Hash key is not the last parameter, the hash is likely malformed (%s).", hash) + return nil, fmt.Errorf("Hash key is not the last parameter, the hash is likely malformed (%s)", hash) } if h.Key == "" { return nil, fmt.Errorf("Hash key contains no characters or the field length is invalid (%s)", hash) @@ -43,14 +43,14 @@ func ParseHash(hash string) (passwordHash *PasswordHash, err error) { _, err = crypt.Base64Encoding.DecodeString(h.Salt) if err != nil { - return nil, errors.New("Salt contains invalid base64 characters.") + return nil, errors.New("Salt contains invalid base64 characters") } if code == HashingAlgorithmSHA512 { h.Iterations = parameters.GetInt("rounds", HashingDefaultSHA512Iterations) h.Algorithm = HashingAlgorithmSHA512 if parameters["rounds"] != "" && parameters["rounds"] != strconv.Itoa(h.Iterations) { - return nil, fmt.Errorf("SHA512 iterations is not numeric (%s).", parameters["rounds"]) + return nil, fmt.Errorf("SHA512 iterations is not numeric (%s)", parameters["rounds"]) } } else if code == HashingAlgorithmArgon2id { version := parameters.GetInt("v", 0) @@ -58,9 +58,9 @@ func ParseHash(hash string) (passwordHash *PasswordHash, err error) { if version == 0 { return nil, fmt.Errorf("Argon2id version parameter not found (%s)", hash) } - return nil, fmt.Errorf("Argon2id versions less than v19 are not supported (hash is version %d).", version) + return nil, fmt.Errorf("Argon2id versions less than v19 are not supported (hash is version %d)", version) } else if version > 19 { - return nil, fmt.Errorf("Argon2id versions greater than v19 are not supported (hash is version %d).", version) + return nil, fmt.Errorf("Argon2id versions greater than v19 are not supported (hash is version %d)", version) } h.Algorithm = HashingAlgorithmArgon2id h.Memory = parameters.GetInt("m", HashingDefaultArgon2idMemory) @@ -70,10 +70,10 @@ func ParseHash(hash string) (passwordHash *PasswordHash, err error) { decodedKey, err := crypt.Base64Encoding.DecodeString(h.Key) if err != nil { - return nil, errors.New("Hash key contains invalid base64 characters.") + return nil, errors.New("Hash key contains invalid base64 characters") } if len(decodedKey) != h.KeyLength { - return nil, fmt.Errorf("Argon2id key length parameter (%d) does not match the actual key length (%d).", h.KeyLength, len(decodedKey)) + return nil, fmt.Errorf("Argon2id key length parameter (%d) does not match the actual key length (%d)", h.KeyLength, len(decodedKey)) } } else { return nil, fmt.Errorf("Authelia only supports salted SHA512 hashing ($6$) and salted argon2id ($argon2id$), not $%s$", code) @@ -81,47 +81,47 @@ func ParseHash(hash string) (passwordHash *PasswordHash, err error) { return h, nil } -// HashPassword generate a salt and hash the password with the salt and a constant -// number of rounds. +// HashPassword generate a salt and hash the password with the salt and a constant number of rounds +//nolint:gocyclo // TODO: Consider refactoring/simplifying, time permitting func HashPassword(password, salt, algorithm string, iterations, memory, parallelism, keyLength, saltLength int) (hash string, err error) { var settings string if algorithm != HashingAlgorithmArgon2id && algorithm != HashingAlgorithmSHA512 { - return "", fmt.Errorf("Hashing algorithm input of '%s' is invalid, only values of %s and %s are supported.", algorithm, HashingAlgorithmArgon2id, HashingAlgorithmSHA512) + return "", fmt.Errorf("Hashing algorithm input of '%s' is invalid, only values of %s and %s are supported", algorithm, HashingAlgorithmArgon2id, HashingAlgorithmSHA512) } if salt == "" { if saltLength < 2 { - return "", fmt.Errorf("Salt length input of %d is invalid, it must be 2 or higher.", saltLength) + return "", fmt.Errorf("Salt length input of %d is invalid, it must be 2 or higher", saltLength) } else if saltLength > 16 { - return "", fmt.Errorf("Salt length input of %d is invalid, it must be 16 or lower.", saltLength) + return "", fmt.Errorf("Salt length input of %d is invalid, it must be 16 or lower", saltLength) } } else if len(salt) > 16 { - return "", fmt.Errorf("Salt input of %s is invalid (%d characters), it must be 16 or fewer characters.", salt, len(salt)) + return "", fmt.Errorf("Salt input of %s is invalid (%d characters), it must be 16 or fewer characters", salt, len(salt)) } else if len(salt) < 2 { - return "", fmt.Errorf("Salt input of %s is invalid (%d characters), it must be 2 or more characters.", salt, len(salt)) + return "", fmt.Errorf("Salt input of %s is invalid (%d characters), it must be 2 or more characters", salt, len(salt)) } else if _, err = crypt.Base64Encoding.DecodeString(salt); err != nil { - return "", fmt.Errorf("Salt input of %s is invalid, only characters [a-zA-Z0-9+/] are valid for input.", salt) + return "", fmt.Errorf("Salt input of %s is invalid, only characters [a-zA-Z0-9+/] are valid for input", salt) } if algorithm == HashingAlgorithmArgon2id { - // Caution: Increasing any of the values in the below block has a high chance in old passwords that cannot be verified. + // Caution: Increasing any of the values in the below block has a high chance in old passwords that cannot be verified if memory < 8 { - return "", fmt.Errorf("Memory (argon2id) input of %d is invalid, it must be 8 or higher.", memory) + return "", fmt.Errorf("Memory (argon2id) input of %d is invalid, it must be 8 or higher", memory) } if parallelism < 1 { - return "", fmt.Errorf("Parallelism (argon2id) input of %d is invalid, it must be 1 or higher.", parallelism) + return "", fmt.Errorf("Parallelism (argon2id) input of %d is invalid, it must be 1 or higher", parallelism) } if memory < parallelism*8 { - return "", fmt.Errorf("Memory (argon2id) input of %d is invalid with a paraellelism input of %d, it must be %d (parallelism * 8) or higher.", memory, parallelism, parallelism*8) + return "", fmt.Errorf("Memory (argon2id) input of %d is invalid with a parallelism input of %d, it must be %d (parallelism * 8) or higher", memory, parallelism, parallelism*8) } if keyLength < 16 { - return "", fmt.Errorf("Key length (argon2id) input of %d is invalid, it must be 16 or higher.", keyLength) + return "", fmt.Errorf("Key length (argon2id) input of %d is invalid, it must be 16 or higher", keyLength) } if iterations < 1 { - return "", fmt.Errorf("Iterations (argon2id) input of %d is invalid, it must be 1 or more.", iterations) + return "", fmt.Errorf("Iterations (argon2id) input of %d is invalid, it must be 1 or more", iterations) } - // Caution: Increasing any of the values in the above block has a high chance in old passwords that cannot be verified. + // Caution: Increasing any of the values in the above block has a high chance in old passwords that cannot be verified } if salt == "" { @@ -138,7 +138,7 @@ func HashPassword(password, salt, algorithm string, iterations, memory, parallel return hash, nil } -// CheckPassword check a password against a hash. +// CheckPassword check a password against a hash func CheckPassword(password, hash string) (ok bool, err error) { passwordHash, err := ParseHash(hash) if err != nil { diff --git a/internal/authentication/password_hash_test.go b/internal/authentication/password_hash_test.go index 58434a73f..39b2e5522 100644 --- a/internal/authentication/password_hash_test.go +++ b/internal/authentication/password_hash_test.go @@ -17,7 +17,7 @@ func TestShouldHashSHA512Password(t *testing.T) { assert.NoError(t, err) - code, parameters, salt, hash, err := crypt.DecodeSettings(hash) + code, parameters, salt, hash, _ := crypt.DecodeSettings(hash) assert.Equal(t, "6", code) assert.Equal(t, "aFr56HjK3DrB8t3S", salt) @@ -78,7 +78,7 @@ func TestShouldNotHashPasswordWithNonExistentAlgorithm(t *testing.T) { schema.DefaultCIPasswordOptionsConfiguration.SaltLength) assert.Equal(t, "", hash) - assert.EqualError(t, err, "Hashing algorithm input of 'bogus' is invalid, only values of argon2id and 6 are supported.") + assert.EqualError(t, err, "Hashing algorithm input of 'bogus' is invalid, only values of argon2id and 6 are supported") } func TestShouldNotHashArgon2idPasswordDueToMemoryParallelismMismatch(t *testing.T) { @@ -87,7 +87,7 @@ func TestShouldNotHashArgon2idPasswordDueToMemoryParallelismMismatch(t *testing. schema.DefaultCIPasswordOptionsConfiguration.KeyLength, schema.DefaultCIPasswordOptionsConfiguration.SaltLength) assert.Equal(t, "", hash) - assert.EqualError(t, err, "Memory (argon2id) input of 8 is invalid with a paraellelism input of 2, it must be 16 (parallelism * 8) or higher.") + assert.EqualError(t, err, "Memory (argon2id) input of 8 is invalid with a parallelism input of 2, it must be 16 (parallelism * 8) or higher") } func TestShouldNotHashArgon2idPasswordDueToMemoryLessThanEight(t *testing.T) { @@ -96,7 +96,7 @@ func TestShouldNotHashArgon2idPasswordDueToMemoryLessThanEight(t *testing.T) { schema.DefaultCIPasswordOptionsConfiguration.KeyLength, schema.DefaultCIPasswordOptionsConfiguration.SaltLength) assert.Equal(t, "", hash) - assert.EqualError(t, err, "Memory (argon2id) input of 1 is invalid, it must be 8 or higher.") + assert.EqualError(t, err, "Memory (argon2id) input of 1 is invalid, it must be 8 or higher") } func TestShouldNotHashArgon2idPasswordDueToKeyLengthLessThanSixteen(t *testing.T) { @@ -105,7 +105,7 @@ func TestShouldNotHashArgon2idPasswordDueToKeyLengthLessThanSixteen(t *testing.T schema.DefaultCIPasswordOptionsConfiguration.Parallelism, 5, schema.DefaultCIPasswordOptionsConfiguration.SaltLength) assert.Equal(t, "", hash) - assert.EqualError(t, err, "Key length (argon2id) input of 5 is invalid, it must be 16 or higher.") + assert.EqualError(t, err, "Key length (argon2id) input of 5 is invalid, it must be 16 or higher") } func TestShouldNotHashArgon2idPasswordDueParallelismLessThanOne(t *testing.T) { @@ -114,7 +114,7 @@ func TestShouldNotHashArgon2idPasswordDueParallelismLessThanOne(t *testing.T) { schema.DefaultCIPasswordOptionsConfiguration.KeyLength, schema.DefaultCIPasswordOptionsConfiguration.SaltLength) assert.Equal(t, "", hash) - assert.EqualError(t, err, "Parallelism (argon2id) input of -1 is invalid, it must be 1 or higher.") + assert.EqualError(t, err, "Parallelism (argon2id) input of -1 is invalid, it must be 1 or higher") } func TestShouldNotHashArgon2idPasswordDueIterationsLessThanOne(t *testing.T) { @@ -123,7 +123,7 @@ func TestShouldNotHashArgon2idPasswordDueIterationsLessThanOne(t *testing.T) { schema.DefaultCIPasswordOptionsConfiguration.KeyLength, schema.DefaultCIPasswordOptionsConfiguration.SaltLength) assert.Equal(t, "", hash) - assert.EqualError(t, err, "Iterations (argon2id) input of 0 is invalid, it must be 1 or more.") + assert.EqualError(t, err, "Iterations (argon2id) input of 0 is invalid, it must be 1 or more") } func TestShouldNotHashPasswordDueToSaltLength(t *testing.T) { @@ -132,14 +132,14 @@ func TestShouldNotHashPasswordDueToSaltLength(t *testing.T) { schema.DefaultCIPasswordOptionsConfiguration.Parallelism, schema.DefaultCIPasswordOptionsConfiguration.KeyLength, 0) assert.Equal(t, "", hash) - assert.EqualError(t, err, "Salt length input of 0 is invalid, it must be 2 or higher.") + assert.EqualError(t, err, "Salt length input of 0 is invalid, it must be 2 or higher") hash, err = HashPassword("password", "", HashingAlgorithmArgon2id, schema.DefaultCIPasswordOptionsConfiguration.Iterations, schema.DefaultCIPasswordOptionsConfiguration.Memory*1024, schema.DefaultCIPasswordOptionsConfiguration.Parallelism, schema.DefaultCIPasswordOptionsConfiguration.KeyLength, 20) assert.Equal(t, "", hash) - assert.EqualError(t, err, "Salt length input of 20 is invalid, it must be 16 or lower.") + assert.EqualError(t, err, "Salt length input of 20 is invalid, it must be 16 or lower") } func TestShouldNotHashPasswordDueToSaltCharLengthTooLong(t *testing.T) { @@ -148,7 +148,7 @@ func TestShouldNotHashPasswordDueToSaltCharLengthTooLong(t *testing.T) { schema.DefaultCIPasswordOptionsConfiguration.Parallelism, schema.DefaultCIPasswordOptionsConfiguration.KeyLength, schema.DefaultCIPasswordOptionsConfiguration.SaltLength) assert.Equal(t, "", hash) - assert.EqualError(t, err, "Salt input of abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 is invalid (62 characters), it must be 16 or fewer characters.") + assert.EqualError(t, err, "Salt input of abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 is invalid (62 characters), it must be 16 or fewer characters") } func TestShouldNotHashPasswordDueToSaltCharLengthTooShort(t *testing.T) { @@ -157,7 +157,7 @@ func TestShouldNotHashPasswordDueToSaltCharLengthTooShort(t *testing.T) { schema.DefaultCIPasswordOptionsConfiguration.Parallelism, schema.DefaultCIPasswordOptionsConfiguration.KeyLength, schema.DefaultCIPasswordOptionsConfiguration.SaltLength) assert.Equal(t, "", hash) - assert.EqualError(t, err, "Salt input of a is invalid (1 characters), it must be 2 or more characters.") + assert.EqualError(t, err, "Salt input of a is invalid (1 characters), it must be 2 or more characters") } func TestShouldNotHashPasswordWithNonBase64CharsInSalt(t *testing.T) { @@ -166,18 +166,18 @@ func TestShouldNotHashPasswordWithNonBase64CharsInSalt(t *testing.T) { schema.DefaultCIPasswordOptionsConfiguration.Parallelism, schema.DefaultCIPasswordOptionsConfiguration.KeyLength, schema.DefaultCIPasswordOptionsConfiguration.SaltLength) assert.Equal(t, "", hash) - assert.EqualError(t, err, "Salt input of abc&123 is invalid, only characters [a-zA-Z0-9+/] are valid for input.") + assert.EqualError(t, err, "Salt input of abc&123 is invalid, only characters [a-zA-Z0-9+/] are valid for input") } func TestShouldNotParseHashWithNoneBase64CharsInKey(t *testing.T) { passwordHash, err := ParseHash("$argon2id$v=19$m=65536,t=3,p=2$BpLnfgDsc2WD8F2q$^^vzA4myCqZZ36bUGsDY//8mKUYNZZaR0t4MFFSs+iM") - assert.EqualError(t, err, "Hash key contains invalid base64 characters.") + assert.EqualError(t, err, "Hash key contains invalid base64 characters") assert.Nil(t, passwordHash) } func TestShouldNotParseHashWithNoneBase64CharsInSalt(t *testing.T) { passwordHash, err := ParseHash("$argon2id$v=19$m=65536$^^wTFoFjITudo57a$Z4NH/EKkdv6PJ01Ye1twJ61fsmRJujZZn1IXdUOyrJY") - assert.EqualError(t, err, "Salt contains invalid base64 characters.") + assert.EqualError(t, err, "Salt contains invalid base64 characters") assert.Nil(t, passwordHash) } @@ -187,15 +187,15 @@ func TestShouldNotParseWithMalformedHash(t *testing.T) { hashMissingSalt := "$argon2id$v=1$m=65536,t=3,p=2$2t9X8nNCN2n3/kFYJ3xWNBg5k/rO782Qr7JJoJIK7G4" passwordHash, err := ParseHash(hashExtraField) - assert.EqualError(t, err, fmt.Sprintf("Hash key is not the last parameter, the hash is likely malformed (%s).", hashExtraField)) + assert.EqualError(t, err, fmt.Sprintf("Hash key is not the last parameter, the hash is likely malformed (%s)", hashExtraField)) assert.Nil(t, passwordHash) passwordHash, err = ParseHash(hashMissingSaltAndParams) - assert.EqualError(t, err, fmt.Sprintf("Hash key is not the last parameter, the hash is likely malformed (%s).", hashMissingSaltAndParams)) + assert.EqualError(t, err, fmt.Sprintf("Hash key is not the last parameter, the hash is likely malformed (%s)", hashMissingSaltAndParams)) assert.Nil(t, passwordHash) passwordHash, err = ParseHash(hashMissingSalt) - assert.EqualError(t, err, fmt.Sprintf("Hash key is not the last parameter, the hash is likely malformed (%s).", hashMissingSalt)) + assert.EqualError(t, err, fmt.Sprintf("Hash key is not the last parameter, the hash is likely malformed (%s)", hashMissingSalt)) assert.Nil(t, passwordHash) } @@ -216,7 +216,7 @@ func TestShouldNotParseArgon2idHashWithEmptyVersion(t *testing.T) { func TestShouldNotParseArgon2idHashWithWrongKeyLength(t *testing.T) { hash := "$argon2id$v=19$m=65536,k=50$fvwTFoFjITudo57a$Z4NH/EKkdv6PJ01Ye1twJ61fsmRJujZZn1IXdUOyrJY" passwordHash, err := ParseHash(hash) - assert.EqualError(t, err, "Argon2id key length parameter (50) does not match the actual key length (32).") + assert.EqualError(t, err, "Argon2id key length parameter (50) does not match the actual key length (32)") assert.Nil(t, passwordHash) } @@ -244,14 +244,14 @@ func TestShouldCheckArgon2idPassword(t *testing.T) { func TestCannotParseSHA512Hash(t *testing.T) { ok, err := CheckPassword("password", "$6$roSnSL3fEVkK0yHFQ.oFFAd8D4OhPAy18K5U61Z2eBhxQXExGU/eknXlY1") - assert.EqualError(t, err, "Hash key is not the last parameter, the hash is likely malformed ($6$roSnSL3fEVkK0yHFQ.oFFAd8D4OhPAy18K5U61Z2eBhxQXExGU/eknXlY1).") + assert.EqualError(t, err, "Hash key is not the last parameter, the hash is likely malformed ($6$roSnSL3fEVkK0yHFQ.oFFAd8D4OhPAy18K5U61Z2eBhxQXExGU/eknXlY1)") assert.False(t, ok) } func TestCannotParseArgon2idHash(t *testing.T) { ok, err := CheckPassword("password", "$argon2id$o/vzA4myCqZZ36bUGsDY//8mKUYNZZaR0t4MFFSs+iM") - assert.EqualError(t, err, "Hash key is not the last parameter, the hash is likely malformed ($argon2id$o/vzA4myCqZZ36bUGsDY//8mKUYNZZaR0t4MFFSs+iM).") + assert.EqualError(t, err, "Hash key is not the last parameter, the hash is likely malformed ($argon2id$o/vzA4myCqZZ36bUGsDY//8mKUYNZZaR0t4MFFSs+iM)") assert.False(t, ok) } @@ -266,35 +266,35 @@ func TestCannotFindNumberOfRounds(t *testing.T) { hash := "$6$rounds50000$aFr56HjK3DrB8t3S$zhPQiS85cgBlNhUKKE6n/AHMlpqrvYSnSL3fEVkK0yHFQ.oFFAd8D4OhPAy18K5U61Z2eBhxQXExGU/eknXlY1" ok, err := CheckPassword("password", hash) - assert.EqualError(t, err, fmt.Sprintf("Hash key is not the last parameter, the hash is likely malformed (%s).", hash)) + assert.EqualError(t, err, fmt.Sprintf("Hash key is not the last parameter, the hash is likely malformed (%s)", hash)) assert.False(t, ok) } func TestCannotMatchArgon2idParamPattern(t *testing.T) { ok, err := CheckPassword("password", "$argon2id$v=19$m65536,t3,p2$BpLnfgDsc2WD8F2q$o/vzA4myCqZZ36bUGsDY//8mKUYNZZaR0t4MFFSs+iM") - assert.EqualError(t, err, "Hash key is not the last parameter, the hash is likely malformed ($argon2id$v=19$m65536,t3,p2$BpLnfgDsc2WD8F2q$o/vzA4myCqZZ36bUGsDY//8mKUYNZZaR0t4MFFSs+iM).") + assert.EqualError(t, err, "Hash key is not the last parameter, the hash is likely malformed ($argon2id$v=19$m65536,t3,p2$BpLnfgDsc2WD8F2q$o/vzA4myCqZZ36bUGsDY//8mKUYNZZaR0t4MFFSs+iM)") assert.False(t, ok) } func TestArgon2idVersionLessThanSupported(t *testing.T) { ok, err := CheckPassword("password", "$argon2id$v=18$m=65536,t=3,p=2$BpLnfgDsc2WD8F2q$o/vzA4myCqZZ36bUGsDY//8mKUYNZZaR0t4MFFSs+iM") - assert.EqualError(t, err, "Argon2id versions less than v19 are not supported (hash is version 18).") + assert.EqualError(t, err, "Argon2id versions less than v19 are not supported (hash is version 18)") assert.False(t, ok) } func TestArgon2idVersionGreaterThanSupported(t *testing.T) { ok, err := CheckPassword("password", "$argon2id$v=20$m=65536,t=3,p=2$BpLnfgDsc2WD8F2q$o/vzA4myCqZZ36bUGsDY//8mKUYNZZaR0t4MFFSs+iM") - assert.EqualError(t, err, "Argon2id versions greater than v19 are not supported (hash is version 20).") + assert.EqualError(t, err, "Argon2id versions greater than v19 are not supported (hash is version 20)") assert.False(t, ok) } func TestNumberOfRoundsNotInt(t *testing.T) { ok, err := CheckPassword("password", "$6$rounds=abc$aFr56HjK3DrB8t3S$zhPQiS85cgBlNhUKKE6n/AHMlpqrvYSnSL3fEVkK0yHFQ.oFFAd8D4OhPAy18K5U61Z2eBhxQXExGU/eknXlY1") - assert.EqualError(t, err, "SHA512 iterations is not numeric (abc).") + assert.EqualError(t, err, "SHA512 iterations is not numeric (abc)") assert.False(t, ok) } diff --git a/internal/authorization/authorizer_test.go b/internal/authorization/authorizer_test.go index 3e9354211..8ea0d0165 100644 --- a/internal/authorization/authorizer_test.go +++ b/internal/authorization/authorizer_test.go @@ -12,12 +12,6 @@ import ( "github.com/stretchr/testify/assert" ) -var NoNet = []string{} -var LocalNet = []string{"127.0.0.1"} -var PrivateNet = []string{"192.168.1.0/24"} -var MultipleNet = []string{"192.168.1.0/24", "10.0.0.0/8"} -var MixedNetIP = []string{"192.168.1.0/24", "192.168.2.4"} - type AuthorizerSuite struct { suite.Suite } diff --git a/internal/commands/certificates.go b/internal/commands/certificates.go index aa1ea678c..5da3c120d 100644 --- a/internal/commands/certificates.go +++ b/internal/commands/certificates.go @@ -62,6 +62,7 @@ func publicKey(priv interface{}) interface{} { } } +//nolint:gocyclo // TODO: Consider refactoring/simplifying, time permitting func generateSelfSignedCertificate(cmd *cobra.Command, args []string) { // implementation retrieved from https://golang.org/src/crypto/tls/generate_cert.go var priv interface{} diff --git a/internal/commands/hash.go b/internal/commands/hash.go index e4a357a53..381e7d2eb 100644 --- a/internal/commands/hash.go +++ b/internal/commands/hash.go @@ -46,9 +46,9 @@ var HashPasswordCmd = &cobra.Command{ hash, err = authentication.HashPassword(args[0], salt, algorithm, iterations, memory*1024, parallelism, keyLength, saltLength) if err != nil { - fmt.Println(fmt.Sprintf("Error occured during hashing: %s", err)) + fmt.Printf("Error occurred during hashing: %s", err) } else { - fmt.Println(fmt.Sprintf("Password hash: %s", hash)) + fmt.Printf("Password hash: %s", hash) } }, Args: cobra.MinimumNArgs(1), diff --git a/internal/commands/migration_local.go b/internal/commands/migration_local.go index cb355a00c..a344eb456 100644 --- a/internal/commands/migration_local.go +++ b/internal/commands/migration_local.go @@ -103,7 +103,7 @@ func migrateLocalU2FSecret(dbProvider storage.Provider) { } func migrateLocalPreferences(dbProvider storage.Provider) { - file, err := os.Open(path.Join(localDatabasePath, "prefered_2fa_method")) + file, err := os.Open(path.Join(localDatabasePath, "prefered_2fa_method")) //nolint:misspell if err != nil { log.Fatal(err) } diff --git a/internal/commands/migration_mongo.go b/internal/commands/migration_mongo.go index f9805e9aa..6d5ca3e76 100644 --- a/internal/commands/migration_mongo.go +++ b/internal/commands/migration_mongo.go @@ -54,6 +54,7 @@ func migrateMongo(cmd *cobra.Command, args []string) { migrateMongoU2FDevices(db, dbProvider) migrateMongoTOTPDevices(db, dbProvider) migrateMongoPreferences(db, dbProvider) + migrateMongoAuthenticationTraces(db, dbProvider) log.Println("Migration done!") } @@ -125,7 +126,7 @@ func migrateMongoTOTPDevices(db *mongo.Database, dbProvider storage.Provider) { } func migrateMongoPreferences(db *mongo.Database, dbProvider storage.Provider) { - u2fCollection := db.Collection("prefered_2fa_method") + u2fCollection := db.Collection("prefered_2fa_method") //nolint:misspell cur, err := u2fCollection.Find(context.Background(), bson.D{}) if err != nil { diff --git a/internal/configuration/reader.go b/internal/configuration/reader.go index 176757b2e..94b5493cd 100644 --- a/internal/configuration/reader.go +++ b/internal/configuration/reader.go @@ -10,12 +10,6 @@ import ( "github.com/authelia/authelia/internal/configuration/validator" ) -func check(e error) { - if e != nil { - panic(e) - } -} - // Read a YAML configuration and create a Configuration object out of it. func Read(configPath string) (*schema.Configuration, []error) { viper.SetEnvPrefix("AUTHELIA") diff --git a/internal/configuration/schema/authentication.go b/internal/configuration/schema/authentication.go index 42ebda589..65e39d34c 100644 --- a/internal/configuration/schema/authentication.go +++ b/internal/configuration/schema/authentication.go @@ -1,6 +1,6 @@ package schema -// LDAPAuthenticationBackendConfiguration represents the configuration related to LDAP server. +// LDAPAuthenticationBackendConfiguration represents the configuration related to LDAP server type LDAPAuthenticationBackendConfiguration struct { URL string `mapstructure:"url"` SkipVerify bool `mapstructure:"skip_verify"` @@ -31,7 +31,7 @@ type PasswordHashingConfiguration struct { Parallelism int `mapstructure:"parallelism"` } -// Default Argon2id Configuration +// DefaultPasswordOptionsConfiguration represents the default configuration related to Argon2id hashing var DefaultPasswordOptionsConfiguration = PasswordHashingConfiguration{ Iterations: 1, KeyLength: 32, @@ -41,7 +41,7 @@ var DefaultPasswordOptionsConfiguration = PasswordHashingConfiguration{ Parallelism: 8, } -// Default Argon2id Configuration for CI testing when calling HashPassword() +// DefaultCIPasswordOptionsConfiguration represents the default configuration related to Argon2id hashing for CI var DefaultCIPasswordOptionsConfiguration = PasswordHashingConfiguration{ Iterations: 1, KeyLength: 32, @@ -51,14 +51,14 @@ var DefaultCIPasswordOptionsConfiguration = PasswordHashingConfiguration{ Parallelism: 8, } -// Default SHA512 Cofniguration +// DefaultPasswordOptionsSHA512Configuration represents the default configuration related to SHA512 hashing var DefaultPasswordOptionsSHA512Configuration = PasswordHashingConfiguration{ Iterations: 50000, SaltLength: 16, Algorithm: "sha512", } -// AuthenticationBackendConfiguration represents the configuration related to the authentication backend. +// AuthenticationBackendConfiguration represents the configuration related to the authentication backend type AuthenticationBackendConfiguration struct { DisableResetPassword bool `mapstructure:"disable_reset_password"` Ldap *LDAPAuthenticationBackendConfiguration `mapstructure:"ldap"` diff --git a/internal/configuration/schema/validator.go b/internal/configuration/schema/validator.go index 2e52a3d5d..441630488 100644 --- a/internal/configuration/schema/validator.go +++ b/internal/configuration/schema/validator.go @@ -32,7 +32,7 @@ type QueueItem struct { path string } -func (v *Validator) validateOne(item QueueItem, q *queue.Queue) error { +func (v *Validator) validateOne(item QueueItem, q *queue.Queue) error { //nolint:unparam if item.value.Type().Kind() == reflect.Ptr { if item.value.IsNil() { return nil diff --git a/internal/configuration/validator/authentication.go b/internal/configuration/validator/authentication.go index 7eeba813e..d3f35d11c 100644 --- a/internal/configuration/validator/authentication.go +++ b/internal/configuration/validator/authentication.go @@ -9,9 +9,7 @@ import ( "github.com/authelia/authelia/internal/configuration/schema" ) -var ldapProtocolPrefix = "ldap://" -var ldapsProtocolPrefix = "ldaps://" - +//nolint:gocyclo // TODO: Consider refactoring/simplifying, time permitting func validateFileAuthenticationBackend(configuration *schema.FileAuthenticationBackendConfiguration, validator *schema.StructValidator) { if configuration.Path == "" { validator.Push(errors.New("Please provide a `path` for the users database in `authentication_backend`")) @@ -50,7 +48,6 @@ func validateFileAuthenticationBackend(configuration *schema.FileAuthenticationB } if configuration.PasswordHashing.Algorithm == "argon2id" { - // Parallelism if configuration.PasswordHashing.Parallelism == 0 { configuration.PasswordHashing.Parallelism = schema.DefaultPasswordOptionsConfiguration.Parallelism @@ -101,6 +98,7 @@ func validateLdapURL(ldapURL string, validator *schema.StructValidator) string { return u.String() } +//nolint:gocyclo // TODO: Consider refactoring/simplifying, time permitting func validateLdapAuthenticationBackend(configuration *schema.LDAPAuthenticationBackendConfiguration, validator *schema.StructValidator) { if configuration.URL == "" { validator.Push(errors.New("Please provide a URL to the LDAP server")) diff --git a/internal/configuration/validator/regulation.go b/internal/configuration/validator/regulation.go index 54f91de30..8bdff6e8f 100644 --- a/internal/configuration/validator/regulation.go +++ b/internal/configuration/validator/regulation.go @@ -1,7 +1,6 @@ package validator import ( - "errors" "fmt" "github.com/authelia/authelia/internal/configuration/schema" @@ -18,13 +17,13 @@ func ValidateRegulation(configuration *schema.RegulationConfiguration, validator } findTime, err := utils.ParseDurationString(configuration.FindTime) if err != nil { - validator.Push(errors.New(fmt.Sprintf("Error occurred parsing regulation find_time string: %s", err))) + validator.Push(fmt.Errorf("Error occurred parsing regulation find_time string: %s", err)) } banTime, err := utils.ParseDurationString(configuration.BanTime) if err != nil { - validator.Push(errors.New(fmt.Sprintf("Error occurred parsing regulation ban_time string: %s", err))) + validator.Push(fmt.Errorf("Error occurred parsing regulation ban_time string: %s", err)) } if findTime > banTime { - validator.Push(errors.New(fmt.Sprintf("find_time cannot be greater than ban_time"))) + validator.Push(fmt.Errorf("find_time cannot be greater than ban_time")) } } diff --git a/internal/configuration/validator/session.go b/internal/configuration/validator/session.go index 89fc6df9c..072da55d8 100644 --- a/internal/configuration/validator/session.go +++ b/internal/configuration/validator/session.go @@ -21,19 +21,19 @@ func ValidateSession(configuration *schema.SessionConfiguration, validator *sche if configuration.Expiration == "" { configuration.Expiration = schema.DefaultSessionConfiguration.Expiration // 1 hour } else if _, err := utils.ParseDurationString(configuration.Expiration); err != nil { - validator.Push(errors.New(fmt.Sprintf("Error occurred parsing session expiration string: %s", err))) + validator.Push(fmt.Errorf("Error occurred parsing session expiration string: %s", err)) } if configuration.Inactivity == "" { configuration.Inactivity = schema.DefaultSessionConfiguration.Inactivity // 5 min } else if _, err := utils.ParseDurationString(configuration.Inactivity); err != nil { - validator.Push(errors.New(fmt.Sprintf("Error occurred parsing session inactivity string: %s", err))) + validator.Push(fmt.Errorf("Error occurred parsing session inactivity string: %s", err)) } if configuration.RememberMeDuration == "" { configuration.RememberMeDuration = schema.DefaultSessionConfiguration.RememberMeDuration // 1 month } else if _, err := utils.ParseDurationString(configuration.RememberMeDuration); err != nil { - validator.Push(errors.New(fmt.Sprintf("Error occurred parsing session remember_me_duration string: %s", err))) + validator.Push(fmt.Errorf("Error occurred parsing session remember_me_duration string: %s", err)) } if configuration.Domain == "" { diff --git a/internal/configuration/validator/session_test.go b/internal/configuration/validator/session_test.go index 34829169a..786bfbefc 100644 --- a/internal/configuration/validator/session_test.go +++ b/internal/configuration/validator/session_test.go @@ -84,8 +84,8 @@ func TestShouldRaiseErrorWhenBadInactivityAndExpirationSet(t *testing.T) { ValidateSession(&config, validator) assert.Len(t, validator.Errors(), 2) - assert.EqualError(t, validator.Errors()[0], "Error occurred parsing session expiration string: could not convert the input string of -1 into a duration") - assert.EqualError(t, validator.Errors()[1], "Error occurred parsing session inactivity string: could not convert the input string of -1 into a duration") + assert.EqualError(t, validator.Errors()[0], "Error occurred parsing session expiration string: Could not convert the input string of -1 into a duration") + assert.EqualError(t, validator.Errors()[1], "Error occurred parsing session inactivity string: Could not convert the input string of -1 into a duration") } func TestShouldRaiseErrorWhenBadRememberMeDurationSet(t *testing.T) { @@ -96,7 +96,7 @@ func TestShouldRaiseErrorWhenBadRememberMeDurationSet(t *testing.T) { ValidateSession(&config, validator) assert.Len(t, validator.Errors(), 1) - assert.EqualError(t, validator.Errors()[0], "Error occurred parsing session remember_me_duration string: could not convert the input string of 1 year into a duration") + assert.EqualError(t, validator.Errors()[0], "Error occurred parsing session remember_me_duration string: Could not convert the input string of 1 year into a duration") } func TestShouldSetDefaultRememberMeDuration(t *testing.T) { diff --git a/internal/configuration/validator/totp_test.go b/internal/configuration/validator/totp_test.go index 26d015577..3357fa639 100644 --- a/internal/configuration/validator/totp_test.go +++ b/internal/configuration/validator/totp_test.go @@ -32,5 +32,4 @@ func TestShouldRaiseErrorWhenInvalidTOTPMinimumValues(t *testing.T) { assert.Len(t, validator.Errors(), 2) assert.EqualError(t, validator.Errors()[0], "TOTP Period must be 1 or more") assert.EqualError(t, validator.Errors()[1], "TOTP Skew must be 0 or more") - } diff --git a/internal/handlers/const.go b/internal/handlers/const.go index 5d57ff1db..b2afb2855 100644 --- a/internal/handlers/const.go +++ b/internal/handlers/const.go @@ -33,4 +33,3 @@ const unableToRegisterOneTimePasswordMessage = "Unable to set up one-time passwo const unableToRegisterSecurityKeyMessage = "Unable to register your security key." const unableToResetPasswordMessage = "Unable to reset your password." const mfaValidationFailedMessage = "Authentication failed, please retry later." -const badBasicAuthFormatMessage = "Content of Proxy-Authorization header is wrong." diff --git a/internal/handlers/handler_extended_configuration_test.go b/internal/handlers/handler_extended_configuration_test.go index 9f41b3ecf..1065899b8 100644 --- a/internal/handlers/handler_extended_configuration_test.go +++ b/internal/handlers/handler_extended_configuration_test.go @@ -67,15 +67,15 @@ func (s *SecondFactorAvailableMethodsFixture) TestShouldCheckSecondFactorIsDisab s.mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(schema.AccessControlConfiguration{ DefaultPolicy: "bypass", Rules: []schema.ACLRule{ - schema.ACLRule{ + { Domain: "example.com", Policy: "deny", }, - schema.ACLRule{ + { Domain: "abc.example.com", Policy: "single_factor", }, - schema.ACLRule{ + { Domain: "def.example.com", Policy: "bypass", }, @@ -98,15 +98,15 @@ func (s *SecondFactorAvailableMethodsFixture) TestShouldCheckSecondFactorIsEnabl s.mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(schema.AccessControlConfiguration{ DefaultPolicy: "two_factor", Rules: []schema.ACLRule{ - schema.ACLRule{ + { Domain: "example.com", Policy: "deny", }, - schema.ACLRule{ + { Domain: "abc.example.com", Policy: "single_factor", }, - schema.ACLRule{ + { Domain: "def.example.com", Policy: "bypass", }, @@ -129,15 +129,15 @@ func (s *SecondFactorAvailableMethodsFixture) TestShouldCheckSecondFactorIsEnabl s.mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(schema.AccessControlConfiguration{ DefaultPolicy: "bypass", Rules: []schema.ACLRule{ - schema.ACLRule{ + { Domain: "example.com", Policy: "deny", }, - schema.ACLRule{ + { Domain: "abc.example.com", Policy: "two_factor", }, - schema.ACLRule{ + { Domain: "def.example.com", Policy: "bypass", }, diff --git a/internal/handlers/handler_firstfactor_test.go b/internal/handlers/handler_firstfactor_test.go index e15bf13b3..7d8901716 100644 --- a/internal/handlers/handler_firstfactor_test.go +++ b/internal/handlers/handler_firstfactor_test.go @@ -10,7 +10,6 @@ import ( "github.com/authelia/authelia/internal/models" "github.com/golang/mock/gomock" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" @@ -31,13 +30,6 @@ func (s *FirstFactorSuite) TearDownTest() { s.mock.Close() } -func (s *FirstFactorSuite) assertError500(err string) { - assert.Equal(s.T(), 500, s.mock.Ctx.Response.StatusCode()) - assert.Equal(s.T(), []byte(InternalError), s.mock.Ctx.Response.Body()) - assert.Equal(s.T(), err, s.mock.Hook.LastEntry().Message) - assert.Equal(s.T(), logrus.ErrorLevel, s.mock.Hook.LastEntry().Level) -} - func (s *FirstFactorSuite) TestShouldFailIfBodyIsNil() { FirstFactorPost(s.mock.Ctx) @@ -288,7 +280,7 @@ func (s *FirstFactorRedirectionSuite) SetupTest() { s.mock.Ctx.Configuration.DefaultRedirectionURL = "https://default.local" s.mock.Ctx.Configuration.AccessControl.DefaultPolicy = "bypass" s.mock.Ctx.Configuration.AccessControl.Rules = []schema.ACLRule{ - schema.ACLRule{ + { Domain: "default.local", Policy: "one_factor", }, @@ -384,11 +376,11 @@ func (s *FirstFactorRedirectionSuite) TestShouldReply200WhenUnsafeTargetURLProvi s.mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(schema.AccessControlConfiguration{ DefaultPolicy: "one_factor", Rules: []schema.ACLRule{ - schema.ACLRule{ + { Domain: "test.example.com", Policy: "one_factor", }, - schema.ACLRule{ + { Domain: "example.com", Policy: "two_factor", }, diff --git a/internal/handlers/handler_register_u2f_step1_test.go b/internal/handlers/handler_register_u2f_step1_test.go index d806518d5..b8cf61fce 100644 --- a/internal/handlers/handler_register_u2f_step1_test.go +++ b/internal/handlers/handler_register_u2f_step1_test.go @@ -46,13 +46,6 @@ func createToken(secret string, username string, action string, expiresAt time.T return ss } -func newFinishArgs() middlewares.IdentityVerificationFinishArgs { - return middlewares.IdentityVerificationFinishArgs{ - ActionClaim: U2FRegistrationAction, - IsTokenUserValidFunc: func(ctx *middlewares.AutheliaCtx, username string) bool { return true }, - } -} - func (s *HandlerRegisterU2FStep1Suite) TestShouldRaiseWhenXForwardedProtoIsMissing() { token := createToken(s.mock.Ctx.Configuration.JWTSecret, "john", U2FRegistrationAction, time.Now().Add(1*time.Minute)) diff --git a/internal/handlers/handler_register_u2f_step2.go b/internal/handlers/handler_register_u2f_step2.go index 6d3856fdd..0c7e63ca1 100644 --- a/internal/handlers/handler_register_u2f_step2.go +++ b/internal/handlers/handler_register_u2f_step2.go @@ -15,6 +15,10 @@ func SecondFactorU2FRegister(ctx *middlewares.AutheliaCtx) { responseBody := u2f.RegisterResponse{} err := ctx.ParseBody(&responseBody) + if err != nil { + ctx.Error(fmt.Errorf("Unable to parse response body: %v", err), unableToRegisterSecurityKeyMessage) + } + userSession := ctx.GetSession() if userSession.U2FChallenge == nil { diff --git a/internal/handlers/handler_user_info_test.go b/internal/handlers/handler_user_info_test.go index 102a62845..1177c45e3 100644 --- a/internal/handlers/handler_user_info_test.go +++ b/internal/handlers/handler_user_info_test.go @@ -62,25 +62,24 @@ func setPreferencesExpectations(preferences UserPreferences, provider *storage.M LoadTOTPSecret(gomock.Eq("john")). Return("", storage.ErrNoTOTPSecret) } - } func TestMethodSetToU2F(t *testing.T) { table := []UserPreferences{ - UserPreferences{ + { Method: "totp", }, - UserPreferences{ + { Method: "u2f", HasU2F: true, HasTOTP: true, }, - UserPreferences{ + { Method: "u2f", HasU2F: true, HasTOTP: false, }, - UserPreferences{ + { Method: "mobile_push", HasU2F: false, HasTOTP: false, diff --git a/internal/handlers/handler_verify.go b/internal/handlers/handler_verify.go index acf54c511..84753ba0c 100644 --- a/internal/handlers/handler_verify.go +++ b/internal/handlers/handler_verify.go @@ -27,7 +27,7 @@ func isSchemeWSS(url *url.URL) bool { return url.Scheme == "wss" } -// getOriginalURL extract the URL from the request headers (X-Original-URI or X-Forwarded-* headers). +// getOriginalURL extract the URL from the request headers (X-Original-URI or X-Forwarded-* headers) func getOriginalURL(ctx *middlewares.AutheliaCtx) (*url.URL, error) { originalURL := ctx.XOriginalURL() if originalURL != nil { @@ -65,8 +65,8 @@ func getOriginalURL(ctx *middlewares.AutheliaCtx) (*url.URL, error) { return url, nil } -// parseBasicAuth parses an HTTP Basic Authentication string. -// "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==" returns ("Aladdin", "open sesame", true). +// parseBasicAuth parses an HTTP Basic Authentication string +// "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==" returns ("Aladdin", "open sesame", true) func parseBasicAuth(auth string) (username, password string, err error) { if !strings.HasPrefix(auth, authPrefix) { return "", "", fmt.Errorf("%s prefix not found in %s header", strings.Trim(authPrefix, " "), AuthorizationHeader) @@ -83,10 +83,9 @@ func parseBasicAuth(auth string) (username, password string, err error) { return cs[:s], cs[s+1:], nil } -// isTargetURLAuthorized check whether the given user is authorized to access the resource. +// isTargetURLAuthorized check whether the given user is authorized to access the resource func isTargetURLAuthorized(authorizer *authorization.Authorizer, targetURL url.URL, username string, userGroups []string, clientIP net.IP, authLevel authentication.Level) authorizationMatching { - level := authorizer.GetRequiredLevel(authorization.Subject{ Username: username, Groups: userGroups, @@ -98,10 +97,10 @@ func isTargetURLAuthorized(authorizer *authorization.Authorizer, targetURL url.U } else if username != "" && level == authorization.Denied { // If the user is not anonymous, it means that we went through // all the rules related to that user and knowing who he is we can - // deduce the access is forbidden. + // deduce the access is forbidden // For anonymous users though, we cannot be sure that she // could not be granted the rights to access the resource. Consequently - // for anonymous users we send Unauthorized instead of Forbidden. + // for anonymous users we send Unauthorized instead of Forbidden return Forbidden } else { if level == authorization.OneFactor && @@ -116,8 +115,8 @@ func isTargetURLAuthorized(authorizer *authorization.Authorizer, targetURL url.U } // verifyBasicAuth verify that the provided username and password are correct and -// that the user is authorized to target the resource. -func verifyBasicAuth(auth []byte, targetURL url.URL, ctx *middlewares.AutheliaCtx) (username string, groups []string, authLevel authentication.Level, err error) { +// that the user is authorized to target the resource +func verifyBasicAuth(auth []byte, targetURL url.URL, ctx *middlewares.AutheliaCtx) (username string, groups []string, authLevel authentication.Level, err error) { //nolint:unparam username, password, err := parseBasicAuth(string(auth)) if err != nil { @@ -130,7 +129,7 @@ func verifyBasicAuth(auth []byte, targetURL url.URL, ctx *middlewares.AutheliaCt return "", nil, authentication.NotAuthenticated, fmt.Errorf("Unable to check credentials extracted from %s header: %s", AuthorizationHeader, err) } - // If the user is not correctly authenticated, send a 401. + // If the user is not correctly authenticated, send a 401 if !authenticated { // Request Basic Authentication otherwise return "", nil, authentication.NotAuthenticated, fmt.Errorf("User %s is not authenticated", username) @@ -145,7 +144,7 @@ func verifyBasicAuth(auth []byte, targetURL url.URL, ctx *middlewares.AutheliaCt return username, details.Groups, authentication.OneFactor, nil } -// setForwardedHeaders set the forwarded User and Groups headers. +// setForwardedHeaders set the forwarded User and Groups headers func setForwardedHeaders(headers *fasthttp.ResponseHeader, username string, groups []string) { if username != "" { headers.Set(remoteUserHeader, username) @@ -153,9 +152,8 @@ func setForwardedHeaders(headers *fasthttp.ResponseHeader, username string, grou } } -// hasUserBeenInactiveLongEnough check whether the user has been inactive for too long. -func hasUserBeenInactiveLongEnough(ctx *middlewares.AutheliaCtx) (bool, error) { - +// hasUserBeenInactiveLongEnough check whether the user has been inactive for too long +func hasUserBeenInactiveLongEnough(ctx *middlewares.AutheliaCtx) (bool, error) { //nolint:unparam maxInactivityPeriod := int64(ctx.Providers.SessionProvider.Inactivity.Seconds()) if maxInactivityPeriod == 0 { return false, nil @@ -174,10 +172,10 @@ func hasUserBeenInactiveLongEnough(ctx *middlewares.AutheliaCtx) (bool, error) { return false, nil } -// verifyFromSessionCookie verify if a user identified by a cookie is allowed to access target URL. -func verifyFromSessionCookie(targetURL url.URL, ctx *middlewares.AutheliaCtx) (username string, groups []string, authLevel authentication.Level, err error) { +// verifyFromSessionCookie verify if a user identified by a cookie is allowed to access target URL +func verifyFromSessionCookie(targetURL url.URL, ctx *middlewares.AutheliaCtx) (username string, groups []string, authLevel authentication.Level, err error) { //nolint:unparam userSession := ctx.GetSession() - // No username in the session means the user is anonymous. + // No username in the session means the user is anonymous isUserAnonymous := userSession.Username == "" if isUserAnonymous && userSession.AuthenticationLevel != authentication.NotAuthenticated { @@ -191,7 +189,7 @@ func verifyFromSessionCookie(targetURL url.URL, ctx *middlewares.AutheliaCtx) (u } if inactiveLongEnough { - // Destroy the session a new one will be regenerated on next request. + // Destroy the session a new one will be regenerated on next request err := ctx.Providers.SessionProvider.DestroySession(ctx.RequestCtx) if err != nil { return "", nil, authentication.NotAuthenticated, fmt.Errorf("Unable to destroy user session after long inactivity: %s", err) @@ -203,7 +201,7 @@ func verifyFromSessionCookie(targetURL url.URL, ctx *middlewares.AutheliaCtx) (u return userSession.Username, userSession.Groups, userSession.AuthenticationLevel, nil } -// VerifyGet is the handler verifying if a request is allowed to go through. +// VerifyGet is the handler verifying if a request is allowed to go through func VerifyGet(ctx *middlewares.AutheliaCtx) { ctx.Logger.Tracef("Headers=%s", ctx.Request.Header.String()) targetURL, err := getOriginalURL(ctx) @@ -256,7 +254,7 @@ func VerifyGet(ctx *middlewares.AutheliaCtx) { } else if authorization == NotAuthorized { // Kubernetes ingress controller and Traefik use the rd parameter of the verify // endpoint to provide the URL of the login portal. The target URL of the user - // is computed from X-Fowarded-* headers or X-Original-URL. + // is computed from X-Fowarded-* headers or X-Original-URL rd := string(ctx.QueryArgs().Peek("rd")) if rd != "" { redirectionURL := fmt.Sprintf("%s?rd=%s", rd, url.QueryEscape(targetURL.String())) @@ -273,7 +271,7 @@ func VerifyGet(ctx *middlewares.AutheliaCtx) { setForwardedHeaders(&ctx.Response.Header, username, groups) } - // We mark activity of the current user if he comes with a session cookie. + // We mark activity of the current user if he comes with a session cookie if !hasBasicAuth && username != "" { // Mark current activity userSession := ctx.GetSession() diff --git a/internal/handlers/handler_verify_test.go b/internal/handlers/handler_verify_test.go index 6bd8e23b5..1fed911a7 100644 --- a/internal/handlers/handler_verify_test.go +++ b/internal/handlers/handler_verify_test.go @@ -147,21 +147,21 @@ func TestShouldCheckAuthorizationMatching(t *testing.T) { ExpectedMatching authorizationMatching } rules := []Rule{ - Rule{"bypass", authentication.NotAuthenticated, Authorized}, - Rule{"bypass", authentication.OneFactor, Authorized}, - Rule{"bypass", authentication.TwoFactor, Authorized}, + {"bypass", authentication.NotAuthenticated, Authorized}, + {"bypass", authentication.OneFactor, Authorized}, + {"bypass", authentication.TwoFactor, Authorized}, - Rule{"one_factor", authentication.NotAuthenticated, NotAuthorized}, - Rule{"one_factor", authentication.OneFactor, Authorized}, - Rule{"one_factor", authentication.TwoFactor, Authorized}, + {"one_factor", authentication.NotAuthenticated, NotAuthorized}, + {"one_factor", authentication.OneFactor, Authorized}, + {"one_factor", authentication.TwoFactor, Authorized}, - Rule{"two_factor", authentication.NotAuthenticated, NotAuthorized}, - Rule{"two_factor", authentication.OneFactor, NotAuthorized}, - Rule{"two_factor", authentication.TwoFactor, Authorized}, + {"two_factor", authentication.NotAuthenticated, NotAuthorized}, + {"two_factor", authentication.OneFactor, NotAuthorized}, + {"two_factor", authentication.TwoFactor, Authorized}, - Rule{"deny", authentication.NotAuthenticated, NotAuthorized}, - Rule{"deny", authentication.OneFactor, Forbidden}, - Rule{"deny", authentication.TwoFactor, Forbidden}, + {"deny", authentication.NotAuthenticated, NotAuthorized}, + {"deny", authentication.OneFactor, Forbidden}, + {"deny", authentication.TwoFactor, Forbidden}, } url, _ := url.ParseRequestURI("https://test.example.com") @@ -169,7 +169,7 @@ func TestShouldCheckAuthorizationMatching(t *testing.T) { for _, rule := range rules { authorizer := authorization.NewAuthorizer(schema.AccessControlConfiguration{ DefaultPolicy: "deny", - Rules: []schema.ACLRule{schema.ACLRule{ + Rules: []schema.ACLRule{{ Domain: "test.example.com", Policy: rule.Policy, }}, @@ -419,23 +419,23 @@ func (p Pair) String() string { func TestShouldVerifyAuthorizationsUsingSessionCookie(t *testing.T) { testCases := []Pair{ - Pair{"https://test.example.com", "", authentication.NotAuthenticated, 401}, - Pair{"https://bypass.example.com", "", authentication.NotAuthenticated, 200}, - Pair{"https://one-factor.example.com", "", authentication.NotAuthenticated, 401}, - Pair{"https://two-factor.example.com", "", authentication.NotAuthenticated, 401}, - Pair{"https://deny.example.com", "", authentication.NotAuthenticated, 401}, + {"https://test.example.com", "", authentication.NotAuthenticated, 401}, + {"https://bypass.example.com", "", authentication.NotAuthenticated, 200}, + {"https://one-factor.example.com", "", authentication.NotAuthenticated, 401}, + {"https://two-factor.example.com", "", authentication.NotAuthenticated, 401}, + {"https://deny.example.com", "", authentication.NotAuthenticated, 401}, - Pair{"https://test.example.com", "john", authentication.OneFactor, 403}, - Pair{"https://bypass.example.com", "john", authentication.OneFactor, 200}, - Pair{"https://one-factor.example.com", "john", authentication.OneFactor, 200}, - Pair{"https://two-factor.example.com", "john", authentication.OneFactor, 401}, - Pair{"https://deny.example.com", "john", authentication.OneFactor, 403}, + {"https://test.example.com", "john", authentication.OneFactor, 403}, + {"https://bypass.example.com", "john", authentication.OneFactor, 200}, + {"https://one-factor.example.com", "john", authentication.OneFactor, 200}, + {"https://two-factor.example.com", "john", authentication.OneFactor, 401}, + {"https://deny.example.com", "john", authentication.OneFactor, 403}, - Pair{"https://test.example.com", "john", authentication.TwoFactor, 403}, - Pair{"https://bypass.example.com", "john", authentication.TwoFactor, 200}, - Pair{"https://one-factor.example.com", "john", authentication.TwoFactor, 200}, - Pair{"https://two-factor.example.com", "john", authentication.TwoFactor, 200}, - Pair{"https://deny.example.com", "john", authentication.TwoFactor, 403}, + {"https://test.example.com", "john", authentication.TwoFactor, 403}, + {"https://bypass.example.com", "john", authentication.TwoFactor, 200}, + {"https://one-factor.example.com", "john", authentication.TwoFactor, 200}, + {"https://two-factor.example.com", "john", authentication.TwoFactor, 200}, + {"https://deny.example.com", "john", authentication.TwoFactor, 403}, } for _, testCase := range testCases { @@ -628,7 +628,6 @@ func TestSchemeIsHTTPS(t *testing.T) { GetURL("wss://mytest.example.com/abc/?query=abc"))) assert.True(t, isSchemeHTTPS( GetURL("https://mytest.example.com/abc/?query=abc"))) - } func TestSchemeIsWSS(t *testing.T) { @@ -646,5 +645,4 @@ func TestSchemeIsWSS(t *testing.T) { GetURL("https://mytest.example.com/abc/?query=abc"))) assert.True(t, isSchemeWSS( GetURL("wss://mytest.example.com/abc/?query=abc"))) - } diff --git a/internal/handlers/types.go b/internal/handlers/types.go index f6b65b1c8..f6c4baf3b 100644 --- a/internal/handlers/types.go +++ b/internal/handlers/types.go @@ -49,12 +49,6 @@ type firstFactorRequestBody struct { KeepMeLoggedIn *bool `json:"keepMeLoggedIn"` } -// FirstFactorMessageResponse represents the response sent by the first factor endpoint -// when no redirection URL has been provided by the user. -type firstFactorMessageResponse struct { - Message string `json:"message"` -} - // redirectResponse represent the response sent by the first factor endpoint // when a redirection URL has been provided. type redirectResponse struct { diff --git a/internal/mocks/mock_authelia_ctx.go b/internal/mocks/mock_authelia_ctx.go index 19f375591..f481ff430 100644 --- a/internal/mocks/mock_authelia_ctx.go +++ b/internal/mocks/mock_authelia_ctx.go @@ -72,16 +72,16 @@ func NewMockAutheliaCtx(t *testing.T) *MockAutheliaCtx { configuration.Session.RememberMeDuration = schema.DefaultSessionConfiguration.RememberMeDuration configuration.Session.Name = "authelia_session" configuration.AccessControl.DefaultPolicy = "deny" - configuration.AccessControl.Rules = []schema.ACLRule{schema.ACLRule{ + configuration.AccessControl.Rules = []schema.ACLRule{{ Domain: "bypass.example.com", Policy: "bypass", - }, schema.ACLRule{ + }, { Domain: "one-factor.example.com", Policy: "one_factor", - }, schema.ACLRule{ + }, { Domain: "two-factor.example.com", Policy: "two_factor", - }, schema.ACLRule{ + }, { Domain: "deny.example.com", Policy: "deny", }} diff --git a/internal/notification/smtp_notifier.go b/internal/notification/smtp_notifier.go index 65ee2acba..28a40abe3 100644 --- a/internal/notification/smtp_notifier.go +++ b/internal/notification/smtp_notifier.go @@ -15,7 +15,7 @@ import ( "github.com/authelia/authelia/internal/utils" ) -// SMTPNotifier a notifier to send emails to SMTP servers. +// SMTPNotifier a notifier to send emails to SMTP servers type SMTPNotifier struct { username string password string @@ -31,7 +31,7 @@ type SMTPNotifier struct { tlsConfig *tls.Config } -// NewSMTPNotifier create an SMTPNotifier targeting a given address. +// NewSMTPNotifier create an SMTPNotifier targeting a given address func NewSMTPNotifier(configuration schema.SMTPNotifierConfiguration) *SMTPNotifier { notifier := &SMTPNotifier{ username: configuration.Username, @@ -50,15 +50,15 @@ func NewSMTPNotifier(configuration schema.SMTPNotifierConfiguration) *SMTPNotifi } func (n *SMTPNotifier) initializeTLSConfig() { - // Do not allow users to disable verification of certs if they have also set a trusted cert that was loaded. - // The second part of this check happens in the Configure Cert Pool code block. + // Do not allow users to disable verification of certs if they have also set a trusted cert that was loaded + // The second part of this check happens in the Configure Cert Pool code block log.Debug("Notifier SMTP client initializing TLS configuration") insecureSkipVerify := false if n.disableVerifyCert { insecureSkipVerify = true } - //Configure Cert Pool. + //Configure Cert Pool certPool, err := x509.SystemCertPool() if err != nil || certPool == nil { certPool = x509.NewCertPool() @@ -92,9 +92,9 @@ func (n *SMTPNotifier) initializeTLSConfig() { } } -// Do startTLS if available (some servers only provide the auth extension after, and encryption is preferred). +// Do startTLS if available (some servers only provide the auth extension after, and encryption is preferred) func (n *SMTPNotifier) startTLS() (bool, error) { - // Only start if not already encrypted. + // Only start if not already encrypted if _, ok := n.client.TLSConnectionState(); ok { log.Debugf("Notifier SMTP connection is already encrypted, skipping STARTTLS") return ok, nil @@ -107,9 +107,8 @@ func (n *SMTPNotifier) startTLS() (bool, error) { err := n.client.StartTLS(n.tlsConfig) if err != nil { return ok, err - } else { - log.Debug("Notifier SMTP STARTTLS completed without error") } + log.Debug("Notifier SMTP STARTTLS completed without error") } else if n.disableRequireTLS { log.Warn("Notifier SMTP server does not support STARTTLS and SMTP configuration is set to disable the TLS requirement (only useful for unauthenticated emails over plain text)") } else { @@ -118,16 +117,16 @@ func (n *SMTPNotifier) startTLS() (bool, error) { return ok, nil } -// Attempt Authentication. +// Attempt Authentication func (n *SMTPNotifier) auth() (bool, error) { - // Attempt AUTH if password is specified only. + // Attempt AUTH if password is specified only if n.password != "" { _, ok := n.client.TLSConnectionState() if !ok { return false, errors.New("Notifier SMTP client does not support authentication over plain text and the connection is currently plain text") } - // Check the server supports AUTH, and get the mechanisms. + // Check the server supports AUTH, and get the mechanisms ok, m := n.client.Extension("AUTH") if ok { log.Debugf("Notifier SMTP server supports authentication with the following mechanisms: %s", m) @@ -143,26 +142,23 @@ func (n *SMTPNotifier) auth() (bool, error) { log.Debug("Notifier SMTP client attempting AUTH LOGIN with server") } - // Throw error since AUTH extension is not supported. + // Throw error since AUTH extension is not supported if auth == nil { return false, fmt.Errorf("notifier SMTP server does not advertise a AUTH mechanism that are supported by Authelia (PLAIN or LOGIN are supported, but server advertised %s mechanisms)", m) } - // Authenticate. + // Authenticate err := n.client.Auth(auth) if err != nil { return false, err - } else { - log.Debug("Notifier SMTP client authenticated successfully with the server") - return true, nil } - } else { - return false, errors.New("Notifier SMTP server does not advertise the AUTH extension but config requires AUTH (password specified), either disable AUTH, or use an SMTP host that supports AUTH PLAIN or AUTH LOGIN") + log.Debug("Notifier SMTP client authenticated successfully with the server") + return true, nil } - } else { - log.Debug("Notifier SMTP config has no password specified so authentication is being skipped") - return false, nil + return false, errors.New("Notifier SMTP server does not advertise the AUTH extension but config requires AUTH (password specified), either disable AUTH, or use an SMTP host that supports AUTH PLAIN or AUTH LOGIN") } + log.Debug("Notifier SMTP config has no password specified so authentication is being skipped") + return false, nil } func (n *SMTPNotifier) compose(recipient, subject, body string) error { @@ -185,7 +181,7 @@ func (n *SMTPNotifier) compose(recipient, subject, body string) error { "MIME-version: 1.0;\nContent-Type: text/html; charset=\"UTF-8\";\n\n" + body - _, err = fmt.Fprintf(wc, msg) + _, err = fmt.Fprint(wc, msg) if err != nil { log.Debugf("Notifier SMTP client error while sending email body over WriteCloser: %s", err) return err @@ -199,7 +195,7 @@ func (n *SMTPNotifier) compose(recipient, subject, body string) error { return nil } -// Dial the SMTP server with the SMTPNotifier config. +// Dial the SMTP server with the SMTPNotifier config func (n *SMTPNotifier) dial() error { log.Debugf("Notifier SMTP client attempting connection to %s", n.address) if n.port == 465 { @@ -224,7 +220,7 @@ func (n *SMTPNotifier) dial() error { return nil } -// Closes the connection properly. +// Closes the connection properly func (n *SMTPNotifier) cleanup() { err := n.client.Quit() if err != nil { @@ -239,10 +235,10 @@ func (n *SMTPNotifier) Send(recipient, title, body string) error { return err } - // Always execute QUIT at the end once we're connected. + // Always execute QUIT at the end once we're connected defer n.cleanup() - // Start TLS and then Authenticate. + // Start TLS and then Authenticate if _, err := n.startTLS(); err != nil { return err } @@ -250,7 +246,7 @@ func (n *SMTPNotifier) Send(recipient, title, body string) error { return err } - // Set the sender and recipient first. + // Set the sender and recipient first if err := n.client.Mail(n.sender); err != nil { log.Debugf("Notifier SMTP failed while sending MAIL FROM (using sender) with error: %s", err) return err @@ -260,7 +256,7 @@ func (n *SMTPNotifier) Send(recipient, title, body string) error { return err } - // Compose and send the email body to the server. + // Compose and send the email body to the server if err := n.compose(recipient, subject, body); err != nil { return err } diff --git a/internal/regulation/regulator_test.go b/internal/regulation/regulator_test.go index b9b4e9030..bf92e605e 100644 --- a/internal/regulation/regulator_test.go +++ b/internal/regulation/regulator_test.go @@ -42,7 +42,7 @@ func (s *RegulatorSuite) TearDownTest() { func (s *RegulatorSuite) TestShouldNotThrowWhenUserIsLegitimate() { attemptsInDB := []models.AuthenticationAttempt{ - models.AuthenticationAttempt{ + { Username: "john", Successful: true, Time: s.clock.Now().Add(-4 * time.Minute), @@ -63,17 +63,17 @@ func (s *RegulatorSuite) TestShouldNotThrowWhenUserIsLegitimate() { // with a certain amount of time larger than FindTime. Meaning the user should not be banned. func (s *RegulatorSuite) TestShouldNotThrowWhenFailedAuthenticationNotInFindTime() { attemptsInDB := []models.AuthenticationAttempt{ - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-1 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-90 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-180 * time.Second), @@ -94,22 +94,22 @@ func (s *RegulatorSuite) TestShouldNotThrowWhenFailedAuthenticationNotInFindTime // seconds ago (meaning we are checking from now back to now-FindTime). func (s *RegulatorSuite) TestShouldBanUserIfLatestAttemptsAreWithinFinTime() { attemptsInDB := []models.AuthenticationAttempt{ - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-1 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-4 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-6 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-180 * time.Second), @@ -132,17 +132,17 @@ func (s *RegulatorSuite) TestShouldBanUserIfLatestAttemptsAreWithinFinTime() { // banned right now. func (s *RegulatorSuite) TestShouldCheckUserIsStillBanned() { attemptsInDB := []models.AuthenticationAttempt{ - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-31 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-34 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-36 * time.Second), @@ -161,12 +161,12 @@ func (s *RegulatorSuite) TestShouldCheckUserIsStillBanned() { func (s *RegulatorSuite) TestShouldCheckUserIsNotYetBanned() { attemptsInDB := []models.AuthenticationAttempt{ - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-34 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-36 * time.Second), @@ -185,7 +185,7 @@ func (s *RegulatorSuite) TestShouldCheckUserIsNotYetBanned() { func (s *RegulatorSuite) TestShouldCheckUserWasAboutToBeBanned() { attemptsInDB := []models.AuthenticationAttempt{ - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-14 * time.Second), @@ -193,12 +193,12 @@ func (s *RegulatorSuite) TestShouldCheckUserWasAboutToBeBanned() { // more than 30 seconds elapsed between this auth and the preceding one. // In that case we don't need to regulate the user even though the number // of retrieved attempts is 3. - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-94 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-96 * time.Second), @@ -217,24 +217,24 @@ func (s *RegulatorSuite) TestShouldCheckUserWasAboutToBeBanned() { func (s *RegulatorSuite) TestShouldCheckRegulationHasBeenResetOnSuccessfulAttempt() { attemptsInDB := []models.AuthenticationAttempt{ - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-90 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: true, Time: s.clock.Now().Add(-93 * time.Second), }, // The user was almost banned but he did a successful attempt. Therefore, even if the next // failure happens within FindTime, he should not be banned. - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-94 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-96 * time.Second), @@ -259,17 +259,17 @@ func TestRunRegulatorSuite(t *testing.T) { // This test checks that the regulator is disabled when configuration is set to 0. func (s *RegulatorSuite) TestShouldHaveRegulatorDisabled() { attemptsInDB := []models.AuthenticationAttempt{ - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-31 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-34 * time.Second), }, - models.AuthenticationAttempt{ + { Username: "john", Successful: false, Time: s.clock.Now().Add(-36 * time.Second), diff --git a/internal/session/provider_config_test.go b/internal/session/provider_config_test.go index dea94cac0..b720aa419 100644 --- a/internal/session/provider_config_test.go +++ b/internal/session/provider_config_test.go @@ -106,6 +106,6 @@ func TestShouldUseEncryptingSerializerWithRedis(t *testing.T) { require.NoError(t, err) decoded := session.Dict{} - _, err = decoded.UnmarshalMsg(decrypted) + _, _ = decoded.UnmarshalMsg(decrypted) assert.Equal(t, "value", decoded.Get("key")) } diff --git a/internal/storage/sql_provider.go b/internal/storage/sql_provider.go index 3815fa57f..993895c8a 100644 --- a/internal/storage/sql_provider.go +++ b/internal/storage/sql_provider.go @@ -81,19 +81,16 @@ func (p *SQLProvider) initialize(db *sql.DB) error { // LoadPreferred2FAMethod load the preferred method for 2FA from sqlite db. func (p *SQLProvider) LoadPreferred2FAMethod(username string) (string, error) { rows, err := p.db.Query(p.sqlGetPreferencesByUsername, username) - defer rows.Close() if err != nil { return "", err } - if rows.Next() { - var method string - err = rows.Scan(&method) - if err != nil { - return "", err - } - return method, nil + defer rows.Close() + if !rows.Next() { + return "", nil } - return "", nil + var method string + err = rows.Scan(&method) + return method, err } // SavePreferred2FAMethod save the preferred method for 2FA in sqlite db. diff --git a/internal/suites/ShortTimeouts/configuration.yml b/internal/suites/ShortTimeouts/configuration.yml index 6446ebf9b..158b7c6a8 100644 --- a/internal/suites/ShortTimeouts/configuration.yml +++ b/internal/suites/ShortTimeouts/configuration.yml @@ -64,7 +64,7 @@ access_control: regulation: max_retries: 3 find_time: 3 - ban_time: 5 + ban_time: 10 notifier: smtp: diff --git a/internal/suites/action_login.go b/internal/suites/action_login.go index 632eca772..52df94acc 100644 --- a/internal/suites/action_login.go +++ b/internal/suites/action_login.go @@ -50,7 +50,7 @@ func (wds *WebDriverSession) doLoginAndRegisterTOTP(ctx context.Context, t *test } // Register a user with TOTP, logout and then authenticate until TOTP-2FA. -func (wds *WebDriverSession) doRegisterAndLogin2FA(ctx context.Context, t *testing.T, username, password string, keepMeLoggedIn bool, targetURL string) string { +func (wds *WebDriverSession) doRegisterAndLogin2FA(ctx context.Context, t *testing.T, username, password string, keepMeLoggedIn bool, targetURL string) string { //nolint:unparam // Register TOTP secret and logout. secret := wds.doRegisterThenLogout(ctx, t, username, password) wds.doLoginTwoFactor(ctx, t, username, password, false, secret, targetURL) diff --git a/internal/suites/environment.go b/internal/suites/environment.go index 343a27688..627b49b07 100644 --- a/internal/suites/environment.go +++ b/internal/suites/environment.go @@ -2,6 +2,7 @@ package suites import ( "fmt" + "os" "strings" "time" @@ -10,6 +11,7 @@ import ( "github.com/authelia/authelia/internal/utils" ) +//nolint:unparam func waitUntilServiceLogDetected( interval time.Duration, timeout time.Duration, @@ -61,8 +63,10 @@ func waitUntilAutheliaIsReady(dockerEnvironment *DockerEnvironment) error { return err } - if err := waitUntilAutheliaFrontendIsReady(dockerEnvironment); err != nil { - return err + if os.Getenv("CI") != "true" { + if err := waitUntilAutheliaFrontendIsReady(dockerEnvironment); err != nil { + return err + } } log.Info("Authelia is now ready!") return nil diff --git a/internal/suites/example/compose/squid/squid.conf b/internal/suites/example/compose/squid/squid.conf index b824b242f..6f96d2300 100644 --- a/internal/suites/example/compose/squid/squid.conf +++ b/internal/suites/example/compose/squid/squid.conf @@ -4579,7 +4579,7 @@ coredump_dir /var/spool/squid # The default is to use HTTP request URL as the store ID. # # BH -# An internal error occured in the helper, preventing +# An internal error occurred in the helper, preventing # a result being identified. # # In addition to the above kv-pairs Squid also understands the following diff --git a/internal/suites/scenario_regulation_test.go b/internal/suites/scenario_regulation_test.go index c78006693..f752cf9a8 100644 --- a/internal/suites/scenario_regulation_test.go +++ b/internal/suites/scenario_regulation_test.go @@ -67,7 +67,6 @@ func (s *RegulationScenario) TestShouldBanUserAfterTooManyAttempt() { time.Sleep(1 * time.Second) s.verifyIsFirstFactorPage(ctx, s.T()) - time.Sleep(9 * time.Second) // Enter the correct password and test a successful login diff --git a/internal/suites/suite_bypass_all.go b/internal/suites/suite_bypass_all.go index 6c365a4ea..d31bf6a12 100644 --- a/internal/suites/suite_bypass_all.go +++ b/internal/suites/suite_bypass_all.go @@ -25,7 +25,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } onSetupTimeout := func() error { diff --git a/internal/suites/suite_docker.go b/internal/suites/suite_docker.go index cd8034ffa..ef8f5a93c 100644 --- a/internal/suites/suite_docker.go +++ b/internal/suites/suite_docker.go @@ -21,7 +21,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } onSetupTimeout := func() error { diff --git a/internal/suites/suite_duo_push.go b/internal/suites/suite_duo_push.go index 997f41d48..1b2ae9f51 100644 --- a/internal/suites/suite_duo_push.go +++ b/internal/suites/suite_duo_push.go @@ -23,7 +23,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } onSetupTimeout := func() error { diff --git a/internal/suites/suite_haproxy.go b/internal/suites/suite_haproxy.go index a71a3c59a..240112172 100644 --- a/internal/suites/suite_haproxy.go +++ b/internal/suites/suite_haproxy.go @@ -25,7 +25,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } onSetupTimeout := func() error { diff --git a/internal/suites/suite_high_availability.go b/internal/suites/suite_high_availability.go index e77842790..66d22aa63 100644 --- a/internal/suites/suite_high_availability.go +++ b/internal/suites/suite_high_availability.go @@ -28,7 +28,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(haDockerEnvironment) + return waitUntilAutheliaIsReady(haDockerEnvironment) } displayAutheliaLogs := func() error { diff --git a/internal/suites/suite_high_availability_test.go b/internal/suites/suite_high_availability_test.go index ab1b979cf..683f48f10 100644 --- a/internal/suites/suite_high_availability_test.go +++ b/internal/suites/suite_high_availability_test.go @@ -134,7 +134,7 @@ var expectedAuthorizations = map[string](map[string]bool){ } func (s *HighAvailabilityWebDriverSuite) TestShouldVerifyAccessControl() { - verifyUserIsAuthorized := func(ctx context.Context, t *testing.T, username, targetURL string, authorized bool) { + verifyUserIsAuthorized := func(ctx context.Context, t *testing.T, username, targetURL string, authorized bool) { //nolint:unparam s.doVisit(t, targetURL) s.verifyURLIs(ctx, t, targetURL) if authorized { @@ -161,8 +161,8 @@ func (s *HighAvailabilityWebDriverSuite) TestShouldVerifyAccessControl() { } } - for _, user := range []string{UserJohn, UserBob, UserHarry} { - s.T().Run(fmt.Sprintf("%s", user), verifyAuthorization(user)) + for _, user := range Users { + s.T().Run(user, verifyAuthorization(user)) } } diff --git a/internal/suites/suite_ldap.go b/internal/suites/suite_ldap.go index 8ecd04728..736ff0e4e 100644 --- a/internal/suites/suite_ldap.go +++ b/internal/suites/suite_ldap.go @@ -27,7 +27,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } displayAutheliaLogs := func() error { diff --git a/internal/suites/suite_mariadb.go b/internal/suites/suite_mariadb.go index 81928534c..b10e542d3 100644 --- a/internal/suites/suite_mariadb.go +++ b/internal/suites/suite_mariadb.go @@ -25,7 +25,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } onSetupTimeout := func() error { diff --git a/internal/suites/suite_mysql.go b/internal/suites/suite_mysql.go index 0ec5a2038..538bb39a0 100644 --- a/internal/suites/suite_mysql.go +++ b/internal/suites/suite_mysql.go @@ -25,7 +25,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } onSetupTimeout := func() error { diff --git a/internal/suites/suite_network_acl.go b/internal/suites/suite_network_acl.go index 4050b07a2..69c563fd0 100644 --- a/internal/suites/suite_network_acl.go +++ b/internal/suites/suite_network_acl.go @@ -26,7 +26,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } onSetupTimeout := func() error { diff --git a/internal/suites/suite_one_factor_only.go b/internal/suites/suite_one_factor_only.go index 4ed0386aa..e05f74d64 100644 --- a/internal/suites/suite_one_factor_only.go +++ b/internal/suites/suite_one_factor_only.go @@ -22,7 +22,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } onSetupTimeout := func() error { diff --git a/internal/suites/suite_postgres.go b/internal/suites/suite_postgres.go index 024f1ac71..5938a9cc8 100644 --- a/internal/suites/suite_postgres.go +++ b/internal/suites/suite_postgres.go @@ -25,7 +25,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } onSetupTimeout := func() error { diff --git a/internal/suites/suite_short_timeouts.go b/internal/suites/suite_short_timeouts.go index 228692535..36cbaf1b6 100644 --- a/internal/suites/suite_short_timeouts.go +++ b/internal/suites/suite_short_timeouts.go @@ -23,7 +23,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } onSetupTimeout := func() error { diff --git a/internal/suites/suite_standalone.go b/internal/suites/suite_standalone.go index c0b97ddfa..04e2363ec 100644 --- a/internal/suites/suite_standalone.go +++ b/internal/suites/suite_standalone.go @@ -25,7 +25,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } displayAutheliaLogs := func() error { diff --git a/internal/suites/suite_traefik.go b/internal/suites/suite_traefik.go index fbe4e9a86..0debc9ab6 100644 --- a/internal/suites/suite_traefik.go +++ b/internal/suites/suite_traefik.go @@ -25,7 +25,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } onSetupTimeout := func() error { diff --git a/internal/suites/suite_traefik2.go b/internal/suites/suite_traefik2.go index bbc489640..edf18a784 100644 --- a/internal/suites/suite_traefik2.go +++ b/internal/suites/suite_traefik2.go @@ -25,7 +25,7 @@ func init() { return err } - return waitUntilAutheliaBackendIsReady(dockerEnvironment) + return waitUntilAutheliaIsReady(dockerEnvironment) } onSetupTimeout := func() error { diff --git a/internal/utils/safe_redirection_test.go b/internal/utils/safe_redirection_test.go index 5429c0fcc..642e11925 100644 --- a/internal/utils/safe_redirection_test.go +++ b/internal/utils/safe_redirection_test.go @@ -7,7 +7,7 @@ import ( "github.com/stretchr/testify/assert" ) -func isURLSafe(requestURI string, domain string) bool { +func isURLSafe(requestURI string, domain string) bool { //nolint:unparam url, _ := url.ParseRequestURI(requestURI) return IsRedirectionSafe(*url, domain) } diff --git a/internal/utils/time.go b/internal/utils/time.go index bb513c2e0..e70c4a5d1 100644 --- a/internal/utils/time.go +++ b/internal/utils/time.go @@ -1,19 +1,17 @@ package utils import ( - "errors" "fmt" "strconv" "time" ) -// Parses a string to a duration +// ParseDurationString parses a string to a duration // Duration notations are an integer followed by a unit // Units are s = second, m = minute, d = day, w = week, M = month, y = year // Example 1y is the same as 1 year -func ParseDurationString(input string) (duration time.Duration, err error) { - duration = 0 - err = nil +func ParseDurationString(input string) (time.Duration, error) { + var duration time.Duration matches := parseDurationRegexp.FindStringSubmatch(input) if len(matches) == 3 && matches[2] != "" { d, _ := strconv.Atoi(matches[1]) @@ -36,13 +34,12 @@ func ParseDurationString(input string) (duration time.Duration, err error) { } else if input == "0" || len(matches) == 3 { seconds, err := strconv.Atoi(input) if err != nil { - err = errors.New(fmt.Sprintf("could not convert the input string of %s into a duration: %s", input, err)) - } else { - duration = time.Duration(seconds) * time.Second + return 0, fmt.Errorf("Could not convert the input string of %s into a duration: %s", input, err) } + duration = time.Duration(seconds) * time.Second } else if input != "" { // Throw this error if input is anything other than a blank string, blank string will default to a duration of nothing - err = errors.New(fmt.Sprintf("could not convert the input string of %s into a duration", input)) + return 0, fmt.Errorf("Could not convert the input string of %s into a duration", input) } - return + return duration, nil } diff --git a/internal/utils/time_test.go b/internal/utils/time_test.go index a4104720b..438a0c234 100644 --- a/internal/utils/time_test.go +++ b/internal/utils/time_test.go @@ -47,25 +47,25 @@ func TestShouldParseSecondsString(t *testing.T) { func TestShouldNotParseDurationStringWithOutOfOrderQuantitiesAndUnits(t *testing.T) { duration, err := ParseDurationString("h1") - assert.EqualError(t, err, "could not convert the input string of h1 into a duration") + assert.EqualError(t, err, "Could not convert the input string of h1 into a duration") assert.Equal(t, time.Duration(0), duration) } func TestShouldNotParseBadDurationString(t *testing.T) { duration, err := ParseDurationString("10x") - assert.EqualError(t, err, "could not convert the input string of 10x into a duration") + assert.EqualError(t, err, "Could not convert the input string of 10x into a duration") assert.Equal(t, time.Duration(0), duration) } func TestShouldNotParseDurationStringWithMultiValueUnits(t *testing.T) { duration, err := ParseDurationString("10ms") - assert.EqualError(t, err, "could not convert the input string of 10ms into a duration") + assert.EqualError(t, err, "Could not convert the input string of 10ms into a duration") assert.Equal(t, time.Duration(0), duration) } func TestShouldNotParseDurationStringWithLeadingZero(t *testing.T) { duration, err := ParseDurationString("005h") - assert.EqualError(t, err, "could not convert the input string of 005h into a duration") + assert.EqualError(t, err, "Could not convert the input string of 005h into a duration") assert.Equal(t, time.Duration(0), duration) }