Allow regular users to change settings via API

api/user.go

@@ -63,11 +63,6 @@ func (h *handler) createUser(w http.ResponseWriter, r *http.Request) {
 }
 
 func (h *handler) updateUser(w http.ResponseWriter, r *http.Request) {
-	if !request.IsAdminUser(r) {
-		json.Forbidden(w, r)
-		return
-	}
-
 	userID := request.RouteInt64Param(r, "userID")
 	userChanges, err := decodeUserModificationRequest(r.Body)
 	if err != nil {
@@ -86,6 +81,18 @@ func (h *handler) updateUser(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if !request.IsAdminUser(r) {
+		if originalUser.ID != request.UserID(r) {
+			json.Forbidden(w, r)
+			return
+		}
+
+		if userChanges.IsAdmin != nil && *userChanges.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)

tests/user_test.go

@@ -372,22 +372,56 @@ func TestCannotGetUserAsNonAdmin(t *testing.T) {
 }
 
 func TestCannotUpdateUserAsNonAdmin(t *testing.T) {
-	username := getRandomUsername()
-	client := miniflux.New(testBaseURL, testAdminUsername, testAdminPassword)
-	user, err := client.CreateUser(username, testStandardPassword, false)
+	adminClient := miniflux.New(testBaseURL, testAdminUsername, testAdminPassword)
+
+	usernameA := getRandomUsername()
+	userA, err := adminClient.CreateUser(usernameA, testStandardPassword, false)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	client = miniflux.New(testBaseURL, username, testStandardPassword)
-	_, err = client.UpdateUser(user.ID, &miniflux.UserModification{})
+	usernameB := getRandomUsername()
+	_, err = adminClient.CreateUser(usernameB, testStandardPassword, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	entriesPerPage := 10
+	userAClient := miniflux.New(testBaseURL, usernameA, testStandardPassword)
+	userAAfterUpdate, err := userAClient.UpdateUser(userA.ID, &miniflux.UserModification{EntriesPerPage: &entriesPerPage})
+	if err != nil {
+		t.Fatal(`Standard users should be able to update themselves`)
+	}
+
+	if userAAfterUpdate.EntriesPerPage != entriesPerPage {
+		t.Fatalf(`The EntriesPerPage field of this user should be updated`)
+	}
+
+	isAdmin := true
+	_, err = userAClient.UpdateUser(userA.ID, &miniflux.UserModification{IsAdmin: &isAdmin})
 	if err == nil {
-		t.Fatal(`Standard users should not be able to update any users`)
+		t.Fatal(`Standard users should not be able to become admin`)
+	}
+
+	userBClient := miniflux.New(testBaseURL, usernameB, testStandardPassword)
+	_, err = userBClient.UpdateUser(userA.ID, &miniflux.UserModification{})
+	if err == nil {
+		t.Fatal(`Standard users should not be able to update other users`)
 	}
 
 	if err != miniflux.ErrForbidden {
 		t.Fatal(`A "Forbidden" error should be raised`)
 	}
+
+	stylesheet := "test"
+	userC, err := adminClient.UpdateUser(userA.ID, &miniflux.UserModification{Stylesheet: &stylesheet})
+	if err != nil {
+		t.Fatal(`Admin users should be able to update any users`)
+	}
+
+	if userC.Stylesheet != stylesheet {
+		t.Fatalf(`The Stylesheet field of this user should be updated`)
+	}
 }
 
 func TestCannotCreateUserAsNonAdmin(t *testing.T) {

ui/settings_update.go

@@ -22,7 +22,7 @@ func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) {
 	sess := session.New(h.store, request.SessionID(r))
 	view := view.New(h.tpl, r, sess)
 
-	user, err := h.store.UserByID(request.UserID(r))
+	loggedUser, err := h.store.UserByID(request.UserID(r))
 	if err != nil {
 		html.ServerError(w, r, err)
 		return
@@ -41,9 +41,9 @@ func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) {
 	view.Set("languages", locale.AvailableLanguages())
 	view.Set("timezones", timezones)
 	view.Set("menu", "settings")
-	view.Set("user", user)
-	view.Set("countUnread", h.store.CountUnreadEntries(user.ID))
-	view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(user.ID))
+	view.Set("user", loggedUser)
+	view.Set("countUnread", h.store.CountUnreadEntries(loggedUser.ID))
+	view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(loggedUser.ID))
 
 	if err := settingsForm.Validate(); err != nil {
 		view.Set("errorMessage", err.Error())
@@ -51,13 +51,13 @@ func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if h.store.AnotherUserExists(user.ID, settingsForm.Username) {
+	if h.store.AnotherUserExists(loggedUser.ID, settingsForm.Username) {
 		view.Set("errorMessage", "error.user_already_exists")
 		html.OK(w, r, view.Render("settings"))
 		return
 	}
 
-	err = h.store.UpdateUser(settingsForm.Merge(user))
+	err = h.store.UpdateUser(settingsForm.Merge(loggedUser))
 	if err != nil {
 		logger.Error("[UI:UpdateSettings] %v", err)
 		view.Set("errorMessage", "error.unable_to_update_user")
@@ -65,8 +65,8 @@ func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	sess.SetLanguage(user.Language)
-	sess.SetTheme(user.Theme)
+	sess.SetLanguage(loggedUser.Language)
+	sess.SetTheme(loggedUser.Theme)
 	sess.NewFlashMessage(locale.NewPrinter(request.UserLanguage(r)).Printf("alert.prefs_saved"))
 	html.Redirect(w, r, route.Path(h.router, "settings"))
 }