htpasswd: refactor and add Passwd methods #3

Merged
amery merged 1 commits from pr-karasz-refactor into master 1 year ago
karasz commented 1 year ago
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 1 year ago
56008698b2
htpasswd: refactor and add Passwd methods
karasz requested review from amery 1 year ago
amery requested changes 1 year ago
amery left a comment
Owner

please see comments

please see comments
if err != nil {
return err
}
newpp, err := pp.CreateUser(user, passwd, algo)
amery commented 1 year ago
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
}
// Byte will return the Passwd as a byte slice
func (pp Passwds) Byte() []byte {
amery commented 1 year ago
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
return fmt.Errorf("unsupported hash algorithm %v", alg)
}
func identifyHash(h string) (HashAlgorithm, error) {
amery commented 1 year ago
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
return apr1_crypt.New().Verify(hashedPassword, []byte(password))
}
func hashBcrypt(passwd string) (string, error) {
amery commented 1 year ago
Owner

method of the algo sharing an interface?

method of the algo sharing an interface?
karasz marked this conversation as resolved
return nil
}
func hashSha512(passwd string) (string, error) {
amery commented 1 year ago
Owner

begging for the Hasher interface

begging for the Hasher interface
karasz marked this conversation as resolved
amery reviewed 1 year ago
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)
amery commented 1 year ago
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
karasz commented 1 year ago
Poster
Owner

Not sure what you mean

Not sure what you mean
amery commented 1 year ago
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 1 year ago
karasz requested review from amery 1 year ago
amery approved these changes 1 year ago
amery merged commit afac3a836f into master 1 year ago

Reviewers

amery approved these changes 1 year ago
The pull request has been merged as afac3a836f.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.