[MISC] Refactor Authentication (#987)

* only do salt validation in validate salt
* fix tests
* remove panic(err.Error())
* use file mode const
* do hash cleanup on file read instead of check password
* design ConfigAlgoToCryptoAlgo and implement it
* split HashPassword func into functional chunks that could theoretically be reused

1 files changed, 10 insertions, 9 deletions

diff --git a/internal/authentication/file_user_provider_test.go b/internal/authentication/file_user_provider_test.go
index 7e7dda16c..2c9dbe1ff 100644
--- a/internal/authentication/file_user_provider_test.go
+++ b/internal/authentication/file_user_provider_test.go
@@ -86,8 +86,9 @@ func TestShouldCheckUserPasswordOfUserThatDoesNotExist(t *testing.T) {
 		config.Path = path
 		provider := NewFileUserProvider(&config)
 		ok, err := provider.CheckUserPassword("fake", "password")
-		assert.NoError(t, err)
+		assert.Error(t, err)
 		assert.Equal(t, false, ok)
+		assert.EqualError(t, err, "user not found")
 	})
 }
 
@@ -126,7 +127,7 @@ func TestShouldUpdatePasswordHashingAlgorithmToArgon2id(t *testing.T) {
 		config := DefaultFileAuthenticationBackendConfiguration
 		config.Path = path
 		provider := NewFileUserProvider(&config)
-		assert.True(t, strings.HasPrefix(provider.database.Users["harry"].HashedPassword, "{CRYPT}$6$"))
+		assert.True(t, strings.HasPrefix(provider.database.Users["harry"].HashedPassword, "$6$"))
 		err := provider.UpdatePassword("harry", "newpassword")
 		assert.NoError(t, err)
 
@@ -147,7 +148,7 @@ func TestShouldUpdatePasswordHashingAlgorithmToSHA512(t *testing.T) {
 		config.Password.Iterations = 50000
 
 		provider := NewFileUserProvider(&config)
-		assert.True(t, strings.HasPrefix(provider.database.Users["john"].HashedPassword, "{CRYPT}$argon2id$"))
+		assert.True(t, strings.HasPrefix(provider.database.Users["john"].HashedPassword, "$argon2id$"))
 		err := provider.UpdatePassword("john", "newpassword")
 		assert.NoError(t, err)
 
@@ -164,7 +165,7 @@ func TestShouldRaiseWhenLoadingMalformedDatabaseForFirstTime(t *testing.T) {
 	WithDatabase(MalformedUserDatabaseContent, func(path string) {
 		config := DefaultFileAuthenticationBackendConfiguration
 		config.Path = path
-		assert.PanicsWithValue(t, "Unable to parse database: yaml: line 4: mapping values are not allowed in this context", func() {
+		assert.PanicsWithError(t, "Unable to parse database: yaml: line 4: mapping values are not allowed in this context", func() {
 			NewFileUserProvider(&config)
 		})
 	})
@@ -174,7 +175,7 @@ func TestShouldRaiseWhenLoadingDatabaseWithBadSchemaForFirstTime(t *testing.T) {
 	WithDatabase(BadSchemaUserDatabaseContent, func(path string) {
 		config := DefaultFileAuthenticationBackendConfiguration
 		config.Path = path
-		assert.PanicsWithValue(t, "Invalid schema of database: Users: non zero value required", func() {
+		assert.PanicsWithError(t, "Invalid schema of database: Users: non zero value required", func() {
 			NewFileUserProvider(&config)
 		})
 	})
@@ -184,7 +185,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.PanicsWithError(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)
 		})
 	})
@@ -194,7 +195,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.PanicsWithError(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)
 		})
 	})
@@ -204,7 +205,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.PanicsWithError(t, "Unable to parse hash of user john: Hash key contains invalid base64 characters", func() {
 			NewFileUserProvider(&config)
 		})
 	})
@@ -214,7 +215,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.PanicsWithError(t, "Unable to parse hash of user john: Salt contains invalid base64 characters", func() {
 			NewFileUserProvider(&config)
 		})
 	})