electricdusk/rushlink
3
3
Fork 3
Code Issues 21 Pull Requests 2 Releases Activity

WIP: Add users system, required for uploading new pastes #77

Draft
yorick wants to merge 8 commits from yorick/rushlink:users into master
pull from: yorick/rushlink:users
merge into: electricdusk:master
Conversation 0 Commits 8 Files Changed 8 +5 -4
1 changed files with 5 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit f5e107e3a0 - Show all commits

9
internal/db/user.go
View File

@ -89,13 +89,14 @@ func HashPassword(password string) (string, error) {
return fmt.Sprintf("$%s$%s$%s$%s", pwdAlgo, pwdParams, encodedSalt, encodedHash), nil return fmt.Sprintf("$%s$%s$%s$%s", pwdAlgo, pwdParams, encodedSalt, encodedHash), nil
yorick marked this conversation as resolved
mrngm commented 2023-04-30 20:18:04 +02:00
Review
Copy Link

Is there a specific reason you omitted the (optional) field version? The argon2 package defines one, so it seems a good idea to include that as well.

Is there a specific reason you omitted the (optional) field `version`? The `argon2` package [defines one](https://pkg.go.dev/golang.org/x/crypto/argon2#Version), so it seems a good idea to include that as well.
} }
var errInvalidDBPasswordFormat = errors.New("invalid password format in db")
func comparePassword(hashedPassword string, password string) (bool, error) { func comparePassword(hashedPassword string, password string) (bool, error) {
// Extract the salt and hash from the hashed password string // Extract the salt and hash from the hashed password string
yorick marked this conversation as resolved
mrngm commented 2023-04-30 20:15:34 +02:00
Review
Copy Link

It's not safe to do a slice expression that requires at least two returned elements on a function that may return an empty slice, or a slice of length one.

It's not safe to do a slice expression that requires at least two returned elements on a function that may return an empty slice, or a slice of length one.
fields := strings.Split(hashedPassword, "$")[1:] fields := strings.Split(hashedPassword, "$")
if len(fields) != 4 || fields[0] != pwdAlgo || fields[1] != pwdParams { if len(fields) != 5 || fields[1] != pwdAlgo || fields[2] != pwdParams {
yorick marked this conversation as resolved
mrngm commented 2023-04-30 20:31:09 +02:00
Review
Copy Link

(another option: add var errInvalidDBPasswordFormat = errors.New("invalid password format in db") in the global scope, and return that here. Works nice with `errors.Is(err, errInvalidDBPasswordFormat), as I suppose we want to log such occurrences)

(another option: add `var errInvalidDBPasswordFormat = errors.New("invalid password format in db")` in the global scope, and return that here. Works nice with `errors.Is(err, errInvalidDBPasswordFormat), as I suppose we want to log such occurrences)
return false, errors.New("invalid password format in db") return false, errInvalidDBPasswordFormat
} }
encodedSalt, encodedHash := fields[2], fields[3] encodedSalt, encodedHash := fields[3], fields[4]
// Decode the salt and hash from base64 // Decode the salt and hash from base64
yorick marked this conversation as resolved
mrngm commented 2023-04-30 20:23:49 +02:00
Review
Copy Link

(See comment above on RawURLEncoding)

(See comment above on `RawURLEncoding`)
salt, err := base64.RawStdEncoding.DecodeString(encodedSalt) salt, err := base64.RawStdEncoding.DecodeString(encodedSalt)