Merge pull request #308 from vangel76/feat/review-allow-edit-ranges

Review: Allow Edit Ranges with Excel-compatible password hashes (#307)

# Conflicts:
#	ROADMAP.md
#	crates/xlsx/src/sheet_write.rs
#	crates/xlsx/src/tests/malformed.rs
#	docs/file-format-parity.md
#	docs/gaps.md
#	docs/parity-checklist.md
#	docs/target-app-parity.md

crates/engine/src/cmd/edit.rs

@@ -199,10 +199,13 @@ fn cell_set(s: &mut Session, p: &Json) -> Result<Json> {
     let input =
         str_param(p, "input").or_else(|| str_param(p, "value")).map(str::to_string).or_else(|| p.get("value").map(json_to_input)).unwrap_or_default();
     let array = bool_param(p, "array").unwrap_or(false);
-    let protected =
-        s.doc()?.wb.sheet(sheet).is_some_and(|sh| sh.is_protected() && s.doc().is_ok_and(|d| d.wb.styles.get(sh.style_id(at)).protection.locked));
-    if protected {
-        return Err(EngineError::Other("The cell or chart you're trying to change is on a protected sheet.".into()));
+    let d = s.doc()?;
+    if let Some(sh) = d.wb.sheet(sheet)
+        && sh.is_protected()
+        && d.wb.styles.get(sh.style_id(at)).protection.locked
+        && let Some(m) = locked_cell_error(sh, at, &d.unlocked_ranges)
+    {
+        return Err(EngineError::Other(m));
     }
     edit(s, |cx| {
         if array {

crates/engine/src/cmd/insert.rs

@@ -782,7 +782,7 @@ fn insert_picture(s: &mut Session, p: &Json) -> Result<Json> {
         };
         let at = sh.merge_at(at).map(|r| r.start).unwrap_or(at);
         let style = sh.style_id(at);
-        if sh.is_protected() && d.wb.styles.get(style).protection.locked {
+        if sh.is_protected() && d.wb.styles.get(style).protection.locked && locked_cell_error(sh, at, &d.unlocked_ranges).is_some() {
             return Err(EngineError::Other("The cell you're trying to change is on a protected sheet.".into()));
         }
         let picture = CellPicture { data, mime: mime.into(), alt: str_param(p, "alt").unwrap_or("").into() };

crates/engine/src/cmd/mod.rs

@@ -272,7 +272,7 @@ pub(crate) fn commit<R>(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result<R>)
     let r = f(&mut ctx)?;
     let Ctx { mut wb, changed, structural, mut fit_rows, protection_checked, .. } = ctx;
     if !protection_checked {
-        check_protection(&d.wb, &wb)?;
+        check_protection(&d.wb, &wb, &d.unlocked_ranges)?;
     }
     if changed.len() <= 200_000 {
         fit_rows.extend(changed.iter().map(|(s, c)| (*s, c.row)));
@@ -307,7 +307,7 @@ pub(crate) const PROTECTED: &str = "The cell or chart you're trying to change is
 /// Refuses an edit that changed what a locked cell on a protected sheet holds (its value or
 /// formula), whichever command made it. Sheets are matched by name, so moving, renaming or
 /// deleting a sheet isn't an edit of its cells.
-fn check_protection(old: &Workbook, new: &Workbook) -> Result<()> {
+fn check_protection(old: &Workbook, new: &Workbook, unlocked: &[(String, String)]) -> Result<()> {
     for sh in &old.sheets {
         if !sh.is_protected() {
             continue;
@@ -317,14 +317,33 @@ fn check_protection(old: &Workbook, new: &Workbook) -> Result<()> {
             continue;
         }
         for c in sh.cells.diff(&after.cells) {
-            if !same_content(sh.cell(c), after.cell(c)) && old.styles.get(sh.style_id(c)).protection.locked {
-                return Err(EngineError::Other(PROTECTED.into()));
+            if !same_content(sh.cell(c), after.cell(c))
+                && old.styles.get(sh.style_id(c)).protection.locked
+                && let Some(m) = locked_cell_error(sh, c, unlocked)
+            {
+                return Err(EngineError::Other(m));
             }
         }
     }
     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> {
+    let mut needs_password = None;
+    for r in sh.edit_ranges.iter().filter(|r| r.contains(c)) {
+        if r.password.is_none() || unlocked.iter().any(|(s, t)| *s == sh.name && *t == r.title) {
+            return None;
+        }
+        needs_password.get_or_insert(r.title.as_str());
+    }
+    Some(match needs_password {
+        Some(t) => format!("The range \"{t}\" needs its password before it can be changed (Review › Allow Edit Ranges)."),
+        None => PROTECTED.into(),
+    })
+}
+
 /// Two cells hold the same value or formula (formats aside).
 fn same_content(a: Option<&gridcraft_model::Cell>, b: Option<&gridcraft_model::Cell>) -> bool {
     let empty = |c: Option<&gridcraft_model::Cell>| c.is_none_or(|c| c.formula.is_none() && c.value.is_empty());

crates/engine/src/cmd/review.rs

@@ -1,6 +1,6 @@
 //! Review tab: protection, spelling (basic), workbook statistics.
 
-use gridcraft_model::{SheetProtection, password_hash};
+use gridcraft_model::{EditRange, PasswordHash, SheetProtection, password_hash};
 use serde_json::{Value as Json, json};
 
 use super::*;
@@ -19,6 +19,18 @@ pub fn specs() -> Vec<CommandSpec> {
         cmd!("review.unprotectSheet", "Unprotect Sheet", ["Review", "Protect"], None, "{password?}", has_doc, unprotect_sheet),
         cmd!("review.protectWorkbook", "Protect Workbook", ["Review", "Protect"], None, "{on?: bool}", has_doc, protect_workbook),
         cmd!(query "review.workbookStatistics", "Workbook Statistics", ["Review", "Proofing"], None, "{}", has_doc, stats),
+        // Not journaled: the params can hold a password.
+        CommandSpec {
+            id: "review.allowEditRanges",
+            label: "Allow Edit Ranges…",
+            menu: &["Review", "Protect"],
+            shortcut: None,
+            params: "{action?: list (default) | add | modify | delete | unlock, title?, range?, password?, newTitle?, sheet?} → ranges",
+            enabled: has_doc,
+            run: allow_edit_ranges,
+            journal: false,
+            undoable: true,
+        },
         cmd!(query "review.checkAccessibility", "Check Accessibility", ["Review", "Accessibility"], None, "{} → issues", has_doc, accessibility),
     ]
 }
@@ -60,6 +72,117 @@ fn unprotect_sheet(s: &mut Session, p: &Json) -> Result<Json> {
     })
 }
 
+fn range_text(r: &EditRange) -> String {
+    r.ranges.iter().map(|x| x.a1()).collect::<Vec<_>>().join(",")
+}
+
+/// Review › Allow Edit Ranges: list, add, modify or delete the ranges that stay editable on a
+/// protected sheet, or unlock a password range for this session.
+fn allow_edit_ranges(s: &mut Session, p: &Json) -> Result<Json> {
+    static SALT_COUNTER: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
+    let sheet = target_sheet(s, p)?;
+    let action = str_param(p, "action").unwrap_or("list");
+    let title = str_param(p, "title").map(str::trim).filter(|t| !t.is_empty());
+    let d = s.doc()?;
+    let sh = d.wb.sheet(sheet).ok_or(EngineError::NoDocument)?;
+    let sheet_name = sh.name.clone();
+    let find = |t: Option<&str>| -> Result<usize> {
+        let t = t.ok_or_else(|| bad("review.allowEditRanges", "missing `title`"))?;
+        sh.edit_ranges
+            .iter()
+            .position(|r| r.title.eq_ignore_ascii_case(t))
+            .ok_or_else(|| bad("review.allowEditRanges", format!("no range titled `{t}`")))
+    };
+    let hash = |pw: &str, title: &str| {
+        let n = SALT_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+        let seed = format!("{sheet_name}\u{0}{title}\u{0}{n}\u{0}{}\u{0}{}", d.uid, gridcraft_calc::now_serial().to_bits());
+        PasswordHash::new_sha512(pw, &gridcraft_model::password::salt_from(seed.as_bytes()))
+    };
+    if matches!(action, "add" | "modify" | "delete") && sh.is_protected() {
+        return Err(EngineError::Other("Unprotect the sheet to change its edit ranges.".into()));
+    }
+    match action {
+        "list" => {}
+        "unlock" => {
+            let i = find(title)?;
+            let Some(r) = sh.edit_ranges.get(i) else { return Err(bad("review.allowEditRanges", "no such range")) };
+            if let Some(h) = &r.password {
+                let pw = str_param(p, "password").unwrap_or("");
+                if !h.verify(pw).map_err(EngineError::Other)? {
+                    return Err(EngineError::Other("The password you supplied is not correct.".into()));
+                }
+                let key = (sheet_name.clone(), r.title.clone());
+                let d = s.doc_mut()?;
+                if !d.unlocked_ranges.contains(&key) {
+                    d.unlocked_ranges.push(key);
+                }
+            }
+        }
+        "add" => {
+            let t = match title {
+                Some(t) => t.to_string(),
+                None => {
+                    (1..).map(|n| format!("Range{n}")).find(|t| !sh.edit_ranges.iter().any(|r| r.title.eq_ignore_ascii_case(t))).unwrap_or_default()
+                }
+            };
+            if sh.edit_ranges.iter().any(|r| r.title.eq_ignore_ascii_case(&t)) {
+                return Err(bad("review.allowEditRanges", format!("a range titled `{t}` already exists")));
+            }
+            let ranges = target_ranges(s, p)?;
+            let password = str_param(p, "password").filter(|pw| !pw.is_empty()).map(|pw| hash(pw, &t));
+            let r = EditRange { title: t, ranges, password, security_descriptor: None };
+            edit(s, |cx| {
+                cx.sheet_mut(sheet)?.edit_ranges.push(r.clone());
+                Ok(())
+            })?;
+        }
+        "modify" => {
+            let i = find(title)?;
+            let mut r = sh.edit_ranges.get(i).cloned().ok_or_else(|| bad("review.allowEditRanges", "no such range"))?;
+            if let Some(n) = str_param(p, "newTitle").map(str::trim).filter(|n| !n.is_empty()) {
+                if sh.edit_ranges.iter().enumerate().any(|(j, x)| j != i && x.title.eq_ignore_ascii_case(n)) {
+                    return Err(bad("review.allowEditRanges", format!("a range titled `{n}` already exists")));
+                }
+                r.title = n.to_string();
+            }
+            if let Some(pw) = str_param(p, "password") {
+                r.password = (!pw.is_empty()).then(|| hash(pw, &r.title));
+            }
+            if p.get("range").is_some() {
+                r.ranges = target_ranges(s, p)?;
+            }
+            edit(s, |cx| {
+                if let Some(x) = cx.sheet_mut(sheet)?.edit_ranges.get_mut(i) {
+                    *x = r.clone();
+                }
+                Ok(())
+            })?;
+        }
+        "delete" => {
+            let i = find(title)?;
+            edit(s, |cx| {
+                let list = &mut cx.sheet_mut(sheet)?.edit_ranges;
+                if i < list.len() {
+                    list.remove(i);
+                }
+                Ok(())
+            })?;
+        }
+        other => return Err(bad("review.allowEditRanges", format!("unknown action `{other}`"))),
+    }
+    let d = s.doc()?;
+    let sh = d.wb.sheet(sheet).ok_or(EngineError::NoDocument)?;
+    Ok(Json::Array(
+        sh.edit_ranges
+            .iter()
+            .map(|r| {
+                let unlocked = r.password.is_none() || d.unlocked_ranges.iter().any(|(s, t)| *s == sh.name && *t == r.title);
+                json!({"title": r.title, "range": range_text(r), "password": r.password.is_some(), "unlocked": unlocked})
+            })
+            .collect(),
+    ))
+}
+
 fn protect_workbook(s: &mut Session, p: &Json) -> Result<Json> {
     let cur = s.doc()?.wb.protected_structure;
     let on = bool_param(p, "on").unwrap_or(!cur);
@@ -97,3 +220,61 @@ fn accessibility(s: &mut Session, _: &Json) -> Result<Json> {
     }
     Ok(Json::Array(issues))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn protected_book() -> Session {
+        let mut s = Session::new();
+        s.new_workbook();
+        s.execute("review.allowEditRanges", json!({"action": "add", "title": "Open", "range": "A1:A3"})).unwrap();
+        s.execute("review.allowEditRanges", json!({"action": "add", "title": "Budget", "range": "B1:B3,D1", "password": "pa55"})).unwrap();
+        s.execute("review.protectSheet", json!({})).unwrap();
+        s
+    }
+
+    #[test]
+    fn edit_ranges_open_locked_cells_and_passwords_really_verify() {
+        let mut s = protected_book();
+        s.execute("cell.set", json!({"cell": "A2", "input": "1"})).unwrap();
+        assert!(s.execute("cell.set", json!({"cell": "C1", "input": "1"})).is_err());
+        let err = s.execute("cell.set", json!({"cell": "D1", "input": "1"})).unwrap_err().to_string();
+        assert!(err.contains("Budget"), "{err}");
+        // A paste or fill is checked the same way.
+        assert!(s.execute("range.setValues", json!({"range": "B2", "values": [[5]]})).is_err());
+        assert!(s.execute("review.allowEditRanges", json!({"action": "unlock", "title": "Budget", "password": "pass"})).is_err());
+        assert!(s.execute("review.allowEditRanges", json!({"action": "unlock", "title": "Budget"})).is_err());
+        let list = s.execute("review.allowEditRanges", json!({"action": "unlock", "title": "budget", "password": "pa55"})).unwrap();
+        assert_eq!(list[1]["unlocked"], true);
+        s.execute("range.setValues", json!({"range": "B2", "values": [[5]]})).unwrap();
+        s.execute("cell.set", json!({"cell": "D1", "input": "2"})).unwrap();
+        // Ranges can't change while the sheet is protected.
+        assert!(s.execute("review.allowEditRanges", json!({"action": "delete", "title": "Open"})).is_err());
+    }
+
+    #[test]
+    fn edit_ranges_add_modify_delete_list_and_undo() {
+        let mut s = Session::new();
+        s.new_workbook();
+        // Empty params list; nothing to list yet.
+        assert_eq!(s.execute("review.allowEditRanges", json!({})).unwrap(), json!([]));
+        s.execute("selection.set", json!({"range": "C3:D4"})).unwrap();
+        let list = s.execute("review.allowEditRanges", json!({"action": "add"})).unwrap();
+        assert_eq!(list, json!([{"title": "Range1", "range": "C3:D4", "password": false, "unlocked": true}]));
+        assert!(s.execute("review.allowEditRanges", json!({"action": "add", "title": "range1", "range": "A1"})).is_err());
+        let list = s
+            .execute("review.allowEditRanges", json!({"action": "modify", "title": "Range1", "newTitle": "Mine", "range": "E5", "password": "x"}))
+            .unwrap();
+        assert_eq!((list[0]["title"].as_str(), list[0]["range"].as_str(), list[0]["password"].as_bool()), (Some("Mine"), Some("E5"), Some(true)));
+        // Inserting a row above moves the range with its cells.
+        s.execute("home.insertRows", json!({"rows": "1:1"})).unwrap();
+        assert_eq!(s.execute("review.allowEditRanges", json!({})).unwrap()[0]["range"], "E6");
+        s.execute("review.allowEditRanges", json!({"action": "delete", "title": "Mine"})).unwrap();
+        assert_eq!(s.execute("review.allowEditRanges", json!({})).unwrap(), json!([]));
+        s.execute("edit.undo", json!({})).unwrap();
+        assert_eq!(s.execute("review.allowEditRanges", json!({})).unwrap()[0]["title"], "Mine");
+        assert!(s.execute("review.allowEditRanges", json!({"action": "nope"})).is_err());
+        assert!(s.execute("review.allowEditRanges", json!({"action": "delete"})).is_err());
+    }
+}

crates/engine/src/cmd/sheet.rs

@@ -146,6 +146,10 @@ fn shift_sheet_features(sh: &mut Sheet, axis: Axis, at: u32, count: u32, insert:
     for cf in sh.cond_formats.iter_mut() {
         cf.ranges = cf.ranges.iter().filter_map(|r| map_range(*r)).collect();
     }
+    for er in sh.edit_ranges.iter_mut() {
+        er.ranges = er.ranges.iter().filter_map(|r| map_range(*r)).collect();
+    }
+    sh.edit_ranges.retain(|er| !er.ranges.is_empty());
     sh.cond_formats.retain(|cf| !cf.ranges.is_empty());
     for dv in sh.validations.iter_mut() {
         dv.ranges = dv.ranges.iter().filter_map(|r| map_range(*r)).collect();
@@ -693,6 +697,10 @@ fn shift_cell_features(sh: &mut Sheet, e: &Edit) {
     for cf in sh.cond_formats.iter_mut() {
         cf.ranges = cf.ranges.iter().filter_map(|r| map_range(*r, e)).collect();
     }
+    for er in sh.edit_ranges.iter_mut() {
+        er.ranges = er.ranges.iter().filter_map(|r| map_range(*r, e)).collect();
+    }
+    sh.edit_ranges.retain(|er| !er.ranges.is_empty());
     sh.cond_formats.retain(|cf| !cf.ranges.is_empty());
     for dv in sh.validations.iter_mut() {
         dv.ranges = dv.ranges.iter().filter_map(|r| map_range(*r, e)).collect();