Skip to content

Commit

Permalink
Refactor user validation
Browse files Browse the repository at this point in the history 
Validate each user field for creation/modification via API and web UI
  • Loading branch information
@fguillot
fguillot committed Jan 4, 2021
1 parent 291bf96 commit e45cc2d
Show file tree
Hide file tree
Showing 40 changed files with 564 additions and 397 deletions.
107 changes: 0 additions & 107 deletions api/payload.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,113 +167,6 @@ func decodeFeedModificationRequest(r io.ReadCloser) (*feedModificationRequest, e
return &feed, nil
}

type userCreationRequest struct {
Username string `json:"username"`
Password string `json:"password"`
IsAdmin bool `json:"is_admin"`
GoogleID string `json:"google_id"`
OpenIDConnectID string `json:"openid_connect_id"`
}

func decodeUserCreationRequest(r io.ReadCloser) (*userCreationRequest, error) {
defer r.Close()

var request userCreationRequest
decoder := json.NewDecoder(r)
if err := decoder.Decode(&request); err != nil {
return nil, fmt.Errorf("Unable to decode user creation JSON object: %v", err)
}

return &request, nil
}

type userModificationRequest struct {
Username *string `json:"username"`
Password *string `json:"password"`
IsAdmin *bool `json:"is_admin"`
Theme *string `json:"theme"`
Language *string `json:"language"`
Timezone *string `json:"timezone"`
EntryDirection *string `json:"entry_sorting_direction"`
Stylesheet *string `json:"stylesheet"`
GoogleID *string `json:"google_id"`
OpenIDConnectID *string `json:"openid_connect_id"`
EntriesPerPage *int `json:"entries_per_page"`
KeyboardShortcuts *bool `json:"keyboard_shortcuts"`
ShowReadingTime *bool `json:"show_reading_time"`
EntrySwipe *bool `json:"entry_swipe"`
}

func (u *userModificationRequest) Update(user *model.User) {
if u.Username != nil {
user.Username = *u.Username
}

if u.Password != nil {
user.Password = *u.Password
}

if u.IsAdmin != nil {
user.IsAdmin = *u.IsAdmin
}

if u.Theme != nil {
user.Theme = *u.Theme
}

if u.Language != nil {
user.Language = *u.Language
}

if u.Timezone != nil {
user.Timezone = *u.Timezone
}

if u.EntryDirection != nil {
user.EntryDirection = *u.EntryDirection
}

if u.Stylesheet != nil {
user.Stylesheet = *u.Stylesheet
}

if u.GoogleID != nil {
user.GoogleID = *u.GoogleID
}

if u.OpenIDConnectID != nil {
user.OpenIDConnectID = *u.OpenIDConnectID
}

if u.EntriesPerPage != nil {
user.EntriesPerPage = *u.EntriesPerPage
}

if u.KeyboardShortcuts != nil {
user.KeyboardShortcuts = *u.KeyboardShortcuts
}

if u.ShowReadingTime != nil {
user.ShowReadingTime = *u.ShowReadingTime
}

if u.EntrySwipe != nil {
user.EntrySwipe = *u.EntrySwipe
}
}

func decodeUserModificationRequest(r io.ReadCloser) (*userModificationRequest, error) {
defer r.Close()

var request userModificationRequest
decoder := json.NewDecoder(r)
if err := decoder.Decode(&request); err != nil {
return nil, fmt.Errorf("Unable to decode user modification JSON object: %v", err)
}

return &request, nil
}

func decodeEntryStatusRequest(r io.ReadCloser) ([]int64, string, error) {
type payload struct {
EntryIDs []int64 `json:"entry_ids"`
Expand Down
21 changes: 0 additions & 21 deletions api/payload_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,24 +218,3 @@ func TestUpdateFeedToFetchViaProxy(t *testing.T) {
t.Errorf(`The field FetchViaProxy should be %v`, value)
}
}

func TestUpdateUserTheme(t *testing.T) {
theme := "Example 2"
changes := &userModificationRequest{Theme: &theme}
user := &model.User{Theme: "Example"}
changes.Update(user)

if user.Theme != theme {
t.Errorf(`Unexpected value, got %q instead of %q`, user.Theme, theme)
}
}

func TestUserThemeWhenNotSet(t *testing.T) {
changes := &userModificationRequest{}
user := &model.User{Theme: "Example"}
changes.Update(user)

if user.Theme != "Example" {
t.Error(`The user Theme should not be modified`)
}
}
40 changes: 15 additions & 25 deletions api/user.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@
package api // import "miniflux.app/api"

import (
json_parser "encoding/json"
"errors"
"net/http"

"miniflux.app/http/request"
"miniflux.app/http/response/json"
"miniflux.app/model"
"miniflux.app/validator"
)

func (h *handler) currentUser(w http.ResponseWriter, r *http.Request) {
Expand All @@ -29,50 +31,38 @@ func (h *handler) createUser(w http.ResponseWriter, r *http.Request) {
return
}

userCreationRequest, err := decodeUserCreationRequest(r.Body)
if err != nil {
json.BadRequest(w, r, err)
return
}

user := model.NewUser()
user.Username = userCreationRequest.Username
user.Password = userCreationRequest.Password
user.IsAdmin = userCreationRequest.IsAdmin
user.GoogleID = userCreationRequest.GoogleID
user.OpenIDConnectID = userCreationRequest.OpenIDConnectID

if err := user.ValidateUserCreation(); err != nil {
var userCreationRequest model.UserCreationRequest
if err := json_parser.NewDecoder(r.Body).Decode(&userCreationRequest); err != nil {
json.BadRequest(w, r, err)
return
}

if h.store.UserExists(user.Username) {
json.BadRequest(w, r, errors.New("This user already exists"))
if validationErr := validator.ValidateUserCreationWithPassword(h.store, &userCreationRequest); validationErr != nil {
json.BadRequest(w, r, validationErr.Error())
return
}

err = h.store.CreateUser(user)
user, err := h.store.CreateUser(&userCreationRequest)
if err != nil {
json.ServerError(w, r, err)
return
}

user.Password = ""
json.Created(w, r, user)
}

func (h *handler) updateUser(w http.ResponseWriter, r *http.Request) {
userID := request.RouteInt64Param(r, "userID")
userChanges, err := decodeUserModificationRequest(r.Body)
if err != nil {

var userModificationRequest model.UserModificationRequest
if err := json_parser.NewDecoder(r.Body).Decode(&userModificationRequest); err != nil {
json.BadRequest(w, r, err)
return
}

originalUser, err := h.store.UserByID(userID)
if err != nil {
json.BadRequest(w, r, errors.New("Unable to fetch this user from the database"))
json.ServerError(w, r, err)
return
}

Expand All @@ -87,18 +77,18 @@ func (h *handler) updateUser(w http.ResponseWriter, r *http.Request) {
return
}

if userChanges.IsAdmin != nil && *userChanges.IsAdmin {
if userModificationRequest.IsAdmin != nil && *userModificationRequest.IsAdmin {
json.BadRequest(w, r, errors.New("Only administrators can change permissions of standard users"))
return
}
}

userChanges.Update(originalUser)
if err := originalUser.ValidateUserModification(); err != nil {
json.BadRequest(w, r, err)
if validationErr := validator.ValidateUserModification(h.store, originalUser.ID, &userModificationRequest); validationErr != nil {
json.BadRequest(w, r, validationErr.Error())
return
}

userModificationRequest.Patch(originalUser)
if err = h.store.UpdateUser(originalUser); err != nil {
json.ServerError(w, r, err)
return
Expand Down
28 changes: 15 additions & 13 deletions cli/create_admin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,31 @@ import (
"miniflux.app/logger"
"miniflux.app/model"
"miniflux.app/storage"
"miniflux.app/validator"
)

func createAdmin(store *storage.Storage) {
user := model.NewUser()
user.Username = config.Opts.AdminUsername()
user.Password = config.Opts.AdminPassword()
user.IsAdmin = true

if user.Username == "" || user.Password == "" {
user.Username, user.Password = askCredentials()
userCreationRequest := &model.UserCreationRequest{
Username: config.Opts.AdminUsername(),
Password: config.Opts.AdminPassword(),
IsAdmin: true,
}

if err := user.ValidateUserCreation(); err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
os.Exit(1)
if userCreationRequest.Username == "" || userCreationRequest.Password == "" {
userCreationRequest.Username, userCreationRequest.Password = askCredentials()
}

if store.UserExists(user.Username) {
logger.Info(`User %q already exists, skipping creation`, user.Username)
if store.UserExists(userCreationRequest.Username) {
logger.Info(`User %q already exists, skipping creation`, userCreationRequest.Username)
return
}

if err := store.CreateUser(user); err != nil {
if validationErr := validator.ValidateUserCreationWithPassword(store, userCreationRequest); validationErr != nil {
fmt.Fprintf(os.Stderr, "%s\n", validationErr)
os.Exit(1)
}

if _, err := store.CreateUser(userCreationRequest); err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
os.Exit(1)
}
Expand Down
10 changes: 7 additions & 3 deletions cli/reset_password.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import (
"fmt"
"os"

"miniflux.app/model"
"miniflux.app/storage"
"miniflux.app/validator"
)

func resetPassword(store *storage.Storage) {
Expand All @@ -24,9 +26,11 @@ func resetPassword(store *storage.Storage) {
os.Exit(1)
}

user.Password = password
if err := user.ValidatePassword(); err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
userModificationRequest := &model.UserModificationRequest{
Password: &password,
}
if validationErr := validator.ValidateUserModification(store, user.ID, userModificationRequest); validationErr != nil {
fmt.Fprintf(os.Stderr, "%s\n", validationErr)
os.Exit(1)
}

Expand Down
Loading

0 comments on commit e45cc2d

Please sign in to comment.