diff --git a/internal/api/handlers/admin.go b/internal/api/handlers/admin.go index 10e27250..dffffc2a 100644 --- a/internal/api/handlers/admin.go +++ b/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 { diff --git a/internal/api/handlers/admin_test.go b/internal/api/handlers/admin_test.go index d434a029..d3bd8d64 100644 --- a/internal/api/handlers/admin_test.go +++ b/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) } })