Sheet protection: obey the Sort and Use AutoFilter boxes

`SheetProtection` carries eleven permissions. `format_cells` is checked in
`apply_style`, and an edit to a locked cell is caught after the fact by
`check_protection`, but `sort` and `autofilter` had no reader anywhere outside
`sheet_read.rs`, `sheet_write.rs` and a round-trip test: they only ever
travelled through the file. So a protected sheet could still be sorted, and
Ctrl+Shift+L still put an AutoFilter on it.

`refuse_unless_allowed` sits beside `locked_cell_error`, with three call sites:
`do_sort` (which both Sort… and the A-to-Z buttons come through), `toggle_filter`
and `filter_by`.

Where the box decides is worth knowing. Sorting a range of locked cells was
already refused, by the locked-cell guard rather than the permission, because a
sort moves values between locked cells — which is Excel's rule too ("users
cannot sort ranges that contain locked cells on a protected worksheet,
regardless of this setting"). So the Sort box decides for a range that has been
unlocked, while an AutoFilter changes no cell's contents and the box is the only
thing that ever decided it. The test asserts both, and that an unprotected sheet
still sorts and filters.

Not fixed here: Delete Rows and Delete Columns are refused even when their boxes
are ticked, because deleting removes locked cells and trips the same post-hoc
guard, while Insert Rows and Insert Columns work. Letting the guard know which
operation ran is a change to code every command passes through and wants its own
PR.
This commit is contained in:
Hamza
2026-10-19 22:30:36 +05:00
parent 49fe41afbf
commit e5aeea7a94
3 changed files with 102 additions and 0 deletions
+5
View File
@@ -232,6 +232,9 @@ fn sort(s: &mut Session, p: &Json) -> Result<Json> {
}
fn do_sort(s: &mut Session, r: RangeRef, header: bool, keys: Vec<SortKey>, by_cols: bool, case: bool) -> Result<Json> {
// Protect Sheet ▸ Sort. Both Sort… and the A-to-Z buttons come through here.
let sheet = s.doc()?.wb.active_sheet;
super::refuse_unless_allowed(s, sheet, "Sorting", |p| p.sort)?;
let body = if by_cols {
if header && r.width() > 1 { RangeRef::new(CellRef::new(r.start.row, r.start.col + 1), r.end) } else { r }
} else if header && r.height() > 1 {
@@ -438,6 +441,7 @@ fn sorted_anchor(sh: &Sheet, a: Anchor, bottom: (u32, u32, f32, f32), by_cols: b
fn toggle_filter(s: &mut Session, p: &Json) -> Result<Json> {
let sheet = s.doc()?.wb.active_sheet;
super::refuse_unless_allowed(s, sheet, "Filtering", |p| p.autofilter)?;
let has = s.doc()?.wb.sheet(sheet).is_some_and(|sh| sh.autofilter.is_some());
let in_table =
s.doc()?.wb.sheet(sheet).and_then(|sh| sh.table_at(s.doc().map(|d| d.selection.active).unwrap_or_default()).map(|t| t.name.clone()));
@@ -492,6 +496,7 @@ fn install_filter(s: &mut Session, p: &Json) -> Result<Json> {
fn filter_by(s: &mut Session, p: &Json) -> Result<Json> {
let sheet = s.doc()?.wb.active_sheet;
super::refuse_unless_allowed(s, sheet, "Filtering", |p| p.autofilter)?;
let Some(sh) = s.doc()?.wb.sheet(sheet) else { return Err(EngineError::NoDocument) };
let af = match &sh.autofilter {
Some(a) => a.clone(),
+12
View File
@@ -394,6 +394,18 @@ fn check_protection(old: &Workbook, new: &Workbook, unlocked: &[(String, String)
Ok(())
}
/// Refuses an action a protected sheet does not allow, by the tick box for it in Review ▸ Protect
/// Sheet. `what` names the action in the message. An unprotected sheet allows everything.
///
/// Cell edits are guarded separately, by whether the cell is locked (`locked_cell_error`): these
/// permissions are the ones that are not about one cell's contents.
pub(crate) fn refuse_unless_allowed(s: &Session, sheet: usize, what: &str, pick: fn(&gridcraft_model::SheetProtection) -> bool) -> Result<()> {
if s.doc()?.wb.sheet(sheet).is_some_and(|sh| sh.protection.as_ref().is_some_and(|pr| !pick(pr))) {
return Err(EngineError::Other(format!("{what} isn't allowed on this protected sheet (Review › Unprotect Sheet).")));
}
Ok(())
}
/// Whether a locked cell on a protected sheet stays locked: `None` when it's in an Allow Edit
/// Range without a password or one unlocked in this session, else the message to show.
pub(crate) fn locked_cell_error(sh: &gridcraft_model::Sheet, c: CellRef, unlocked: &[(String, String)]) -> Option<String> {
+85
View File
@@ -3344,3 +3344,88 @@ fn csv_as_text_keeps_identifier_columns_as_written() {
let saved = s.execute("file.saveBytes", json!({"format": "csv"})).unwrap();
assert_eq!(crate::io::base64_decode(saved["base64"].as_str().unwrap()).unwrap(), csv.replace('\n', "\r\n").as_bytes());
}
/// Protect Sheet's Sort and Use AutoFilter tick boxes are obeyed. They were read from the file and
/// written back but never consulted, so a protected sheet could still be sorted and filtered.
///
/// Sorting locked cells stays refused whichever way the box is set, which is Excel's rule too
/// ("users cannot sort ranges that contain locked cells on a protected worksheet"), so the box
/// decides for a range that has been unlocked. An AutoFilter changes no cell's contents, so there
/// the box is the only thing that decides.
#[test]
fn a_protected_sheet_allows_sorting_and_filtering_only_when_it_says_so() {
let allow = |s: &mut Session, f: fn(&mut gridcraft_model::SheetProtection)| {
let d = s.doc_mut().unwrap();
let mut wb = (*d.wb).clone();
if let Some(p) = wb.sheet_mut(0).and_then(|sh| sh.protection.as_mut()) {
f(p);
}
d.wb = std::sync::Arc::new(wb);
};
// Three rows under a header, and the cells unlocked so that the tick box is what decides.
let book = || {
let mut s = s();
for (c, v) in [("A1", "Head"), ("A2", "2"), ("A3", "1")] {
s.execute("cell.set", json!({"cell": c, "input": v})).unwrap();
}
s.execute("home.lockCell", json!({"range": "A1:A3", "on": false})).unwrap();
s
};
let order = |s: &Session| (v(s, "A2"), v(s, "A3"));
// Unprotected: both work, so the guard has not made the ordinary case stricter.
let mut s = book();
s.execute("data.sortAscending", json!({})).unwrap();
assert_eq!(order(&s), (Value::Number(1.0), Value::Number(2.0)));
s.execute("data.filter", json!({})).unwrap();
// Protected with nothing ticked, which is what Excel's dialog opens with: both refused.
let mut s = book();
s.execute("review.protectSheet", json!({})).unwrap();
for (cmd, args) in [
("data.sortAscending", json!({})),
("data.sortDescending", json!({})),
// Sort… needs a key to be a sort at all: with none it does nothing either way.
("data.sort", json!({"keys": [{"column": "A", "order": "asc"}]})),
("data.filter", json!({})),
("data.filterBy", json!({})),
] {
let e = s.execute(cmd, args).expect_err(cmd).to_string();
assert!(e.contains("protected sheet"), "{cmd}: {e}");
}
// Refused, not half-applied: the rows are where they were.
assert_eq!(order(&s), (Value::Number(2.0), Value::Number(1.0)));
assert!(s.doc().unwrap().wb.sheet(0).unwrap().autofilter.is_none());
// Sort ticked: sorting works on the unlocked range; filtering is still refused.
let mut s = book();
s.execute("review.protectSheet", json!({})).unwrap();
allow(&mut s, |p| p.sort = true);
s.execute("data.sortAscending", json!({})).unwrap();
assert_eq!(order(&s), (Value::Number(1.0), Value::Number(2.0)), "sorted under the header");
assert!(s.execute("data.filter", json!({})).is_err(), "Use AutoFilter is still off");
// Use AutoFilter ticked: the filter goes on, and sorting is refused.
let mut s = book();
s.execute("review.protectSheet", json!({})).unwrap();
allow(&mut s, |p| p.autofilter = true);
s.execute("data.filter", json!({})).unwrap();
assert!(s.doc().unwrap().wb.sheet(0).unwrap().autofilter.is_some(), "the AutoFilter is on");
assert!(s.execute("data.sortAscending", json!({})).is_err(), "Sort is still off");
// Unprotecting gives sorting back.
s.execute("review.unprotectSheet", json!({})).unwrap();
s.execute("data.sortAscending", json!({})).unwrap();
// Locked cells: ticking Sort does not let them be reordered, as in Excel.
let mut s = {
let mut t = super::tests::s();
for (c, v) in [("A1", "Head"), ("A2", "2"), ("A3", "1")] {
t.execute("cell.set", json!({"cell": c, "input": v})).unwrap();
}
t
};
s.execute("review.protectSheet", json!({})).unwrap();
allow(&mut s, |p| p.sort = true);
assert!(s.execute("data.sortAscending", json!({})).is_err(), "locked cells cannot be sorted");
assert_eq!(order(&s), (Value::Number(2.0), Value::Number(1.0)));
}