htpasswd: refactor and add Passwd methods #3

Merged
amery merged 1 commits from pr-karasz-refactor into master 2023-09-28 20:18:36 +02:00
Owner

Signed-off-by: Nagy Károly Gábriel k@jpi.io

Signed-off-by: Nagy Károly Gábriel <k@jpi.io>
karasz added 1 commit 2023-09-27 17:25:04 +02:00
Signed-off-by: Nagy Károly Gábriel <k@jpi.io>
karasz requested review from amery 2023-09-27 17:25:08 +02:00
amery requested changes 2023-09-27 20:27:18 +02:00
amery left a comment
Owner

please see comments

please see comments
@@ -104,15 +86,25 @@ func CreateUser(file, user, passwd string, algo HashAlgorithm) error {
if err != nil {
return err
}
newpp, err := pp.CreateUser(user, passwd, algo)
Owner

as it's a map, newpp and pp are actually the same map and both contain the new user.

if we really want a new map this should be via a newpp, err := pp.Clone().AddUser(...)

as it's a map, newpp and pp are actually the same map and both contain the new user. if we really want a new map this should be via a `newpp, err := pp.Clone().AddUser(...)`
karasz marked this conversation as resolved
@@ -173,0 +188,4 @@
}
// Byte will return the Passwd as a byte slice
func (pp Passwds) Byte() []byte {
Owner

standard is Bytes() as the returned value contains many.
WriteTo() is also a nice interface

standard is `Bytes()` as the returned value contains many. `WriteTo()` is also a nice interface
karasz marked this conversation as resolved
@@ -188,10 +243,55 @@ func verifyPass(pass, hash string, alg HashAlgorithm) error {
return fmt.Errorf("unsupported hash algorithm %v", alg)
}
func identifyHash(h string) (HashAlgorithm, error) {
Owner

wouldn't it be better to return a split? (algo, hash, bool)?

wouldn't it be better to return a split? `(algo, hash, bool)`?
karasz marked this conversation as resolved
@@ -192,3 +271,4 @@
return apr1_crypt.New().Verify(hashedPassword, []byte(password))
}
func hashBcrypt(passwd string) (string, error) {
Owner

method of the algo sharing an interface?

method of the algo sharing an interface?
karasz marked this conversation as resolved
@@ -269,6 +399,16 @@ func verifySHA256(password, hashedPassword string) error {
return nil
}
func hashSha512(passwd string) (string, error) {
Owner

begging for the Hasher interface

begging for the Hasher interface
karasz marked this conversation as resolved
amery reviewed 2023-09-27 20:31:15 +02:00
@@ -107,2 +98,3 @@
func (pp Passwds) CreateUser(user, passwd string, algo HashAlgorithm) (Passwds, error) {
if _, exists := pp[user]; exists {
return fmt.Errorf("user %s already exists", user)
return nil, fmt.Errorf("user %s already exists", user)
Owner

we may want errors that can be tested without parsing the content

we may want errors that can be tested without parsing the content
Author
Owner

Not sure what you mean

Not sure what you mean
Owner

usage or wrapping predefined errors so one can use errors.Is() and similar

usage or wrapping predefined errors so one can use errors.Is() and similar
karasz force-pushed pr-karasz-refactor from 56008698b2 to 5f5cbeab4f 2023-09-28 16:13:40 +02:00 Compare
karasz requested review from amery 2023-09-28 16:16:04 +02:00
amery approved these changes 2023-09-28 20:15:40 +02:00
amery merged commit afac3a836f into master 2023-09-28 20:18:36 +02:00
Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: asciigoat/httools#3