Add a serializable "permission root locator" to a FileSystemHandle

[a-sully]

Until now, access checks were tied to a "file system entry". As of #96, a "file system entry" corresponds to an actual file or directory on disk. We need to support access checks on FileSystemHandles which don't correspond to an entry (e.g. the entry has been removed), so the access checks must be tied to something else.

This PR gives each FileSystemHandle a "permission root locator". A newly created child FileSystemHandle will inherit this attribute from its parent (as opposed to its access algorithms), such that if subDir was created from rootDir, subDir.requestPermission() is equivalent to rootDir.requestPermission(). This "permission root locator" is serialized, such that storing a handle in IDB now retains information about where the handle originated from

The new "query access" and one "request access" algorithms are expected to be overridden in https://wicg.github.io/file-system-access/#permissions to handle non-BucketFileSystem use cases.

Fixes #101

---

Preview | Diff

[a-sully]

Q: is use of global correct here? (and throughout this PR)

[annevk]

No, the algorithm sits on a FileSystemHandle, right? So I'm not sure how this can work. It's already a bit odd to access a FileSystemHandle in parallel. Maybe it would be better if they were associated with the locator? Or does that end up doing the wrong thing?

[a-sully]

The only issue with using the locator is that it doesn't contain the context from which the handle was acquired. For example, consider this scenario:

• The user selects ~/Documents/foo/bar.txt from the file picker as file
• The user selects ~/Documents/foo/ from the directory picker, then the site calls dir.getFileHandle('bar.txt') to get fileFromDir

Note that in this case, file.isSameEntry(fileFromDir) is true and the respective file system locators are the same

However, if file.requestPermission() is called, the "request access" algorithm corresponds to ~/Documents/foo/bar.txt

Meanwhile, if fileFromDir.requestPermission() is called, the "request access" algorithm (at least in Chromium browsers) corresponds to ~/Documents/foo/. This is because the getFileHandle() method passes down its access check algorithms to its children (see step 6). When serializing a FileSystemHandle, our implementation actually includes the paths of both the parent (which was selected from the picker) and the child in the serialization.

Given all this, I see three paths forward:

1. Argue that this is all just an implementation detail (e.g. it's up to the user agent whether fileFromDir.requestPermission() should request permission to ~/Documents/foo/ or ~/Documents/foo/bar.txt), and for the purposes of the spec we should just tie access checks to a "fie system locator" and call it a day
2. Give each FileSystemHandle two locators - one for itself and one for the root it was created from. Access checks for any handle use the root's locator. Serializing a handle requires serializing both of these locators
3. Give each FileSystemHandle some attribute that encapsulates its access check algorithms (e.g. "file system permission state"). This is passed down to all created children similar to what is currently specified for the access check algorithms (see step 6 of getFileHandle()). Access checks are performed based only on this attribute, and the attribute would need to be serialized (though I'm not sure exactly how that would look)

Thoughts?

Edit: numbered options so we can more easily refer to them in future comments

[a-sully]

Looking at the WICG spec more closely... (3) could look like giving a FileSystemHandle a FileSystemPermissionDescriptor member (though I'm not sure if it's a problem that a descriptor is currently specified to have a handle)

I guess there's still:

4. Tie access check algorithms to the FileSystemHandle itself, if there's a way to access a FileSystemHandle in parallel (as the WICG spec currently does by incorrectly using this directly). As @mkruisselbrink pointed out, though, it's unclear how this permission state could be serialized

FWIW this "inherit permissions from the parent" behavior is also specified in step 3 of the permission state constraints algorithm for a FileSystemPermissionDescriptor. That text's use of "parent" was made outdated by #96, though (i.e. it can no longer be null). It seems that we should either introduce the concept of a "permission root" (as options 2 and 3 do explicitly and 4 does implicitly) or say that the context from which the handle was acquired is not relevant and the locator is all that matters (i.e. that file.requestPermission() should always request permission to ~/Documents/foo/)

On another note... given that that constraint exists, I wonder if updating serialization steps might be unnecessary, since it's just a mechanism used to enforce this constraint?

Lots of options here. Eager to hear thoughts from others :)

[annevk]

Given that what constraint exists?

I think you're correct that locator alone doesn't work. Presumably if you do subDir.requestPermission() it should still impact rootDir? I think that argues for either storing a reference to the root handle or the root locator that a potential access algorithm could then use.

[a-sully]

In the latest patch I've given each FileSystemHandle a "permission root locator" (let me know if you have a better name for this). As mentioned above, this matches how Chrome serializes a handle today.

In this model, creating a handle through one of the API's entry points (which in this spec is only navigator.storage.getDirectory(), but also includes e.g. the picker methods in https://wicg.github.io/file-system-access/) will set the "permission root locator" to the "locator" of the handle itself. Meanwhile, any children which are created from this handle will be passed the permission root, such that subDir.requestPermission() is the same as rootDir.requestPermission()

A key advantage of this approach (as opposed to passing the access algorithm itself from parent to child) is that it's serializable. The old approach tied "query access" and "request access" algorithms to an entry (and an version of this PR tied these access algorithms to the FileSystemHandle itself). But these algorithms are (presumably? Please let me know if that's incorrect) not serializable, so postMessage() or storing a handle in IDB would lose its "where did I originate from" information

As such, there is now just one "query access" and one "request access" algorithm - which return "granted" for locators in a bucket file system; otherwise "denied" - that I intend to override in https://wicg.github.io/file-system-access/#permissions to handle the non-BucketFileSystem use cases. Does that seem like a reasonable approach?

[a-sully]

FYI: I've re-ordered the "locating an entry" statements in several algorithms, since the |entry| is no longer needed for the access checks

[a-sully]

cc @mkruisselbrink but he's OOO for a bit so sending over to @annevk for review. Thanks!

[annevk]

No, the algorithm sits on a FileSystemHandle, right? So I'm not sure how this can work. It's already a bit odd to access a FileSystemHandle in parallel. Maybe it would be better if they were associated with the locator? Or does that end up doing the wrong thing?

[a-sully]

I don't have much experience with abstract algorithms, so please let me know whether it makes sense to use in this case

[annevk]

I think abstract-op is primarily for JavaScript and some legacy cases. For new cases we can just define an algorithm as we already do elsewhere.

To query file system access, given ...

[annevk]

Might be good to get feedback from someone else on the substance of this PR. Just gave some feedback on whether to use an abstract operation.

[annevk]

I think abstract-op is primarily for JavaScript and some legacy cases. For new cases we can just define an algorithm as we already do elsewhere.

To query file system access, given ...