Check access on the document you write
What you'll see
You are writing a save, delete or upload endpoint for an edit form. The form already holds the document's path, and also its metadata: the folder it lives in, its Guid and its modification time. So the endpoint receives several references to the same document, and it feels natural to use whichever one is convenient at each step. The folder from the metadata goes to the access check, the Guid to the lookup of the stored version, and the path to docly.patchFile().
Every step works, and every test passes, because the form always sends references to the same document. Nothing forces them to agree, though. A caller who edits the request can make each reference point at a different document.
What's actually happening
Everything in the request comes from the caller: query, form and JSON body alike. So a request that carries two references to "the document" really carries two claims about which document it means. An access check proves something only about the document it looked at.
Server-side scripts run with the site's own rights. docly.patchFile(), docly.saveFile() and docly.deleteFile() write wherever they are told, whatever the signed-in user may do. Your access check is the only thing between the caller and the data. If it looks at one document while the write lands on another, the check guards the wrong door:
// DON'T: the check and the write each take the caller's word for a different document
export default (item, data, metadata) => {
let folder = docly.getFolder(metadata.folderpath); // caller's claim #1
let file = docly.getFiles(metadata.folderpath)
.find(f => f.Guid == metadata.Guid); // caller's claim #2
if (!canEdit(folder, file)) return docly.denyAccess();
return docly.patchFile(item, data); // caller's claim #3
}A user who may edit any one document can send metadata that describes that document, so the check passes, and set item to any path, so the write goes there. That includes their own user record, where they can set Admin: true, the site configuration, or another customer's data. Guards written against the metadata, such as "you cannot change your own role", look at the wrong document too, so they never fire.
The same shape turns up wherever an endpoint accepts an id for the lookup and a path for the action, or a parent folder for the check and a child path for the write.
What to do
Accept one reference to the target, and derive everything else on the server from the document itself:
// DO: one reference, looked up on the server; folder and owner come from the document
export default (item, data, metadata) => {
let file = docly.getFile(item);
if (!file) throw new Error("File not found.");
// Optional, but cheap: the form must still describe the document it opened
if (!metadata || file.Guid != metadata.Guid) return docly.denyAccess();
let folder = docly.getFolder(file.folderpath); // from the document, not from the caller
if (!canEdit(folder, file)) return docly.denyAccess(); // checks the document we write
return docly.patchFile(item, data); // the same reference as the lookup
}- Check the stored document, not the incoming data. Rules such as "may only edit rows for their own customer" must read the owner from
file, the stored version. Reject a change to the owner field outright; do not trustdata.Customerto say whose document it is. - Use the document's own name in every later rule. A self-protection guard ("you cannot change your own rights"), sending invitations, and deciding which share to remove must all use
file.filename.metadata.filenameis still only a claim. - Treat metadata from the form as a consistency check, never as a key. Comparing the Guid or modification time against the stored document catches stale forms. Using those values to find the document puts the caller back in charge.
- Create is the same rule applied to the folder. Resolve the target folder from the one path you received, check access on that folder, and save into that exact path. Do not check one folder and then save into a path built from other input.
- A delete endpoint that already does it right is the template. Delete handlers tend to take only a path, so they are usually correct by construction. If your save handler takes more arguments than your delete handler, find out why.
How to test it. Sign in as the lowest role that may edit anything. Open a document you are allowed to edit, and replay the save request with item pointing at a document you are not allowed to edit, such as your own user record with a raised role. The request must be refused, and the target must be unchanged afterwards.