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 +18 -21
1 changed files with 18 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 8f5ce1d9fc - Show all commits

39
handlers.go
View File

@ -246,22 +246,11 @@ func (rl *rushlink) viewActionSuccess(w http.ResponseWriter, r *http.Request, p
func (rl *rushlink) newPasteHandler(w http.ResponseWriter, r *http.Request) { func (rl *rushlink) newPasteHandler(w http.ResponseWriter, r *http.Request) {
// Check if the user is authenticated // Check if the user is authenticated
username, password, ok := r.BasicAuth() user := rl.authenticateUser(w, r, false, nil)
if !ok { if user == nil {
// User is not authenticated, return a 401 Unauthorized response
w.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`)
w.WriteHeader(http.StatusUnauthorized)
return return
} }
// Authenticate the user
user, err := db.Authenticate(rl.db, username, password)
if err != nil {
// Authentication failed, return a 401 Unauthorized response
w.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`)
w.WriteHeader(http.StatusUnauthorized)
return
}
if err := r.ParseMultipartForm(formParseMaxMemory); err != nil { if err := r.ParseMultipartForm(formParseMaxMemory); err != nil {
msg := fmt.Sprintf("could not parse form: %v\n", err) msg := fmt.Sprintf("could not parse form: %v\n", err)
rl.renderError(w, r, http.StatusBadRequest, msg) rl.renderError(w, r, http.StatusBadRequest, msg)
@ -322,25 +311,33 @@ func (rl *rushlink) createUserHandler(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusCreated) w.WriteHeader(http.StatusCreated)
} }
mrngm commented 2023-04-30 19:40:34 +02:00
Review
Copy Link

Missing err check

Missing `err` check
func (rl *rushlink) setWWWAuthenticate(w http.ResponseWriter, r *http.Request) {
// Set authentication headers for Basic Authentication
w.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`)
w.WriteHeader(http.StatusUnauthorized)
}
func (rl *rushlink) authenticateUser(w http.ResponseWriter, r *http.Request, shouldBeAdmin bool, canAlsoBe *string) *db.User { func (rl *rushlink) authenticateUser(w http.ResponseWriter, r *http.Request, shouldBeAdmin bool, canAlsoBe *string) *db.User {
// Check if the user is authenticated // Check if the user is authenticated
username, password, ok := r.BasicAuth() username, password, ok := r.BasicAuth()
if !ok { if !ok {
// User is not authenticated, return a 401 Unauthorized response // User is not authenticated, return a 401 Unauthorized response
w.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`) rl.setWWWAuthenticate(w, r)
w.WriteHeader(http.StatusUnauthorized)
return nil return nil
} }
// Authenticate the user // Authenticate the user
user, err := db.Authenticate(rl.db, username, password) user, err := db.Authenticate(rl.db, username, password)
if err != nil || (shouldBeAdmin && !user.Admin && (canAlsoBe == nil || *canAlsoBe != user.User)) { if err != nil {
rl.setWWWAuthenticate(w, r)
log.Printf("authentication failure: %s", err)
return nil
}
if (shouldBeAdmin && !user.Admin && (canAlsoBe == nil || *canAlsoBe != user.User)) {
yorick marked this conversation as resolved
mrngm commented 2023-04-30 19:55:38 +02:00
Review
Copy Link

This could be correct, but it's not exactly readable. It's not clear what canAlsoBe means in this context, and what kind of values it should represent.

It's perhaps better to add a function like:

func (rl *rushlink) setWWWAuthenticate(w http.ResponseWriter, r *http.Request) {
	// Set authentication headers for Basic Authentication
	w.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`)
	w.WriteHeader(http.StatusUnauthorized)
}

and then implement the logic for authenticating the user

user, err := db.Authenticate(rl.db, username, password)
if err != nil {
	log.Printf("authentication failure: %v", err)
	rl.setWWWAuthenticate(w, r)
	return nil
}

if shouldBeAdmin && !user.Admin {
	log.Printf("user should be admin, but isn't: %v", err)
	rl.setWWWAuthenticate(w, r)
	return nil
}

// TODO: canAlsoBe?

// User has been authenticated

return user
This could be correct, but it's not exactly readable. It's not clear what `canAlsoBe` means in this context, and what kind of values it should represent. It's perhaps better to add a function like: ```go func (rl *rushlink) setWWWAuthenticate(w http.ResponseWriter, r *http.Request) { // Set authentication headers for Basic Authentication w.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`) w.WriteHeader(http.StatusUnauthorized) } ``` and then implement the logic for authenticating the user ```go user, err := db.Authenticate(rl.db, username, password) if err != nil { log.Printf("authentication failure: %v", err) rl.setWWWAuthenticate(w, r) return nil } if shouldBeAdmin && !user.Admin { log.Printf("user should be admin, but isn't: %v", err) rl.setWWWAuthenticate(w, r) return nil } // TODO: canAlsoBe? // User has been authenticated return user ```
// Authentication failed, return a 401 Unauthorized response // Authentication failed, return a 401 Unauthorized response
w.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`) rl.setWWWAuthenticate(w, r)
w.WriteHeader(http.StatusUnauthorized) log.Printf("user '%s' should be admin (or '%s'), but isn't", username, canAlsoBe)
if err != nil {
log.Printf("authentication failure: %s", err)
}
return nil return nil
} }
return user return user