fix(auth): avoid revoking sessions for unchanged user policy

internal/api/handlers/admin.go

@@ -9,6 +9,7 @@ import (
 	"io"
 	"log/slog"
 	"net/http"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -463,7 +464,7 @@ func (h *AdminHandler) HandleUpdateUser(w http.ResponseWriter, r *http.Request)
 		permissions = &normalized
 	}
 
-	err = h.userRepo.Update(r.Context(), id, models.UpdateUserInput{
+	updateInput := models.UpdateUserInput{
 		Username:                 req.Username,
 		Email:                    req.Email,
 		Password:                 req.Password,
@@ -477,12 +478,23 @@ func (h *AdminHandler) HandleUpdateUser(w http.ResponseWriter, r *http.Request)
 		MaxProfiles:              req.MaxProfiles,
 		DownloadAllowed:          req.DownloadAllowed,
 		DownloadTranscodeAllowed: req.DownloadTranscodeAllowed,
-	})
+	}
+
+	var currentUser *models.User
+	if updateMayRequireSessionRevocation(updateInput) {
+		currentUser, err = h.userRepo.GetByID(r.Context(), id)
+		if err != nil {
+			writeError(w, http.StatusNotFound, "not_found", "User not found")
+			return
+		}
+	}
+
+	err = h.userRepo.Update(r.Context(), id, updateInput)
 	if err != nil {
 		writeError(w, http.StatusInternalServerError, "internal_error", "Failed to update user")
 		return
 	}
-	if updateRequiresSessionRevocation(req) {
+	if updateRequiresSessionRevocation(currentUser, updateInput) {
 		if err := h.revokeUserSessions(r.Context(), id); err != nil {
 			writeError(w, http.StatusInternalServerError, "internal_error", "Failed to revoke updated user sessions")
 			return
@@ -750,13 +762,39 @@ func (h *AdminHandler) HandleListUserProfiles(w http.ResponseWriter, r *http.Req
 	writeJSON(w, http.StatusOK, resp)
 }
 
-func updateRequiresSessionRevocation(req updateUserRequest) bool {
-	return req.Password != nil ||
-		req.Role != nil ||
-		req.Enabled != nil ||
-		req.LibraryIDs.Set ||
-		req.Permissions.Set ||
-		req.MaxPlaybackQuality != nil
+func updateMayRequireSessionRevocation(input models.UpdateUserInput) bool {
+	return input.Password != nil ||
+		input.Role != nil ||
+		input.Enabled != nil ||
+		input.LibraryIDs != nil ||
+		input.Permissions != nil ||
+		input.MaxPlaybackQuality != nil
+}
+
+func updateRequiresSessionRevocation(current *models.User, input models.UpdateUserInput) bool {
+	if input.Password != nil {
+		return true
+	}
+	if current == nil {
+		return updateMayRequireSessionRevocation(input)
+	}
+	if input.Role != nil && *input.Role != current.Role {
+		return true
+	}
+	if input.Enabled != nil && *input.Enabled != current.Enabled {
+		return true
+	}
+	if input.LibraryIDs != nil && !slices.Equal(*input.LibraryIDs, current.LibraryIDs) {
+		return true
+	}
+	if input.Permissions != nil && !slices.Equal(*input.Permissions, current.Permissions) {
+		return true
+	}
+	if input.MaxPlaybackQuality != nil &&
+		access.NormalizePlaybackQuality(*input.MaxPlaybackQuality) != access.NormalizePlaybackQuality(current.MaxPlaybackQuality) {
+		return true
+	}
+	return false
 }
 
 func (h *AdminHandler) revokeUserSessions(ctx context.Context, userID int) error {

internal/api/handlers/admin_test.go

@@ -1,71 +1,109 @@
 package handlers
 
-import "testing"
+import (
+	"testing"
+
+	"github.com/Silo-Server/silo-server/internal/models"
+)
 
 func TestUpdateRequiresSessionRevocation(t *testing.T) {
 	role := "admin"
+	sameRole := "user"
 	enabled := true
+	disabled := false
 	libraryIDs := []int{1, 2}
+	sameLibraryIDs := []int{1}
 	maxPlaybackQuality := "1080p"
+	sameMaxPlaybackQuality := "original"
 	password := "new-password"
 	username := "renamed"
 	maxStreams := 4
+	permissions := []string{"metadata_curation"}
+	samePermissions := []string{"download"}
+
+	current := &models.User{
+		Role:               "user",
+		Permissions:        []string{"download"},
+		Enabled:            false,
+		LibraryIDs:         []int{1},
+		MaxPlaybackQuality: "original",
+	}
 
 	tests := []struct {
 		name string
-		req  updateUserRequest
+		in   models.UpdateUserInput
 		want bool
 	}{
 		{
 			name: "permissions set",
-			req:  updateUserRequest{Permissions: updateStringSliceField{Set: true, Value: []string{"metadata_curation"}}},
+			in:   models.UpdateUserInput{Permissions: &permissions},
 			want: true,
 		},
 		{
-			name: "permissions unset",
-			req:  updateUserRequest{Permissions: updateStringSliceField{Set: false, Value: []string{"metadata_curation"}}},
+			name: "permissions unchanged",
+			in:   models.UpdateUserInput{Permissions: &samePermissions},
 			want: false,
 		},
 		{
 			name: "role",
-			req:  updateUserRequest{Role: &role},
+			in:   models.UpdateUserInput{Role: &role},
 			want: true,
 		},
+		{
+			name: "role unchanged",
+			in:   models.UpdateUserInput{Role: &sameRole},
+			want: false,
+		},
 		{
 			name: "enabled",
-			req:  updateUserRequest{Enabled: &enabled},
+			in:   models.UpdateUserInput{Enabled: &enabled},
 			want: true,
 		},
+		{
+			name: "enabled unchanged",
+			in:   models.UpdateUserInput{Enabled: &disabled},
+			want: false,
+		},
 		{
 			name: "library ids",
-			req:  updateUserRequest{LibraryIDs: updateLibraryIDsField{Set: true, Value: libraryIDs}},
+			in:   models.UpdateUserInput{LibraryIDs: &libraryIDs},
 			want: true,
 		},
+		{
+			name: "library ids unchanged",
+			in:   models.UpdateUserInput{LibraryIDs: &sameLibraryIDs},
+			want: false,
+		},
 		{
 			name: "max playback quality",
-			req:  updateUserRequest{MaxPlaybackQuality: &maxPlaybackQuality},
+			in:   models.UpdateUserInput{MaxPlaybackQuality: &maxPlaybackQuality},
 			want: true,
 		},
+		{
+			name: "max playback quality unchanged",
+			in:   models.UpdateUserInput{MaxPlaybackQuality: &sameMaxPlaybackQuality},
+			want: false,
+		},
 		{
 			name: "password",
-			req:  updateUserRequest{Password: &password},
+			in:   models.UpdateUserInput{Password: &password},
 			want: true,
 		},
 		{
 			name: "non access fields",
-			req:  updateUserRequest{Username: &username, MaxStreams: &maxStreams},
+			in:   models.UpdateUserInput{Username: &username, MaxStreams: &maxStreams},
 			want: false,
 		},
 		{
 			name: "empty update",
-			req:  updateUserRequest{},
+			in:   models.UpdateUserInput{},
 			want: false,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := updateRequiresSessionRevocation(tt.req); got != tt.want {
+			if got := updateRequiresSessionRevocation(current, tt.in); got != tt.want {
 				t.Fatalf("updateRequiresSessionRevocation() = %v, want %v", got, tt.want)
 			}
 		})