ts-odd icon indicating copy to clipboard operation
ts-odd copied to clipboard

Default to secret paths

Open expede opened this issue 2 years ago • 5 comments

NB: Feature requests will only be considered if they solve a pain

Summary

Problem

SDK users need to always explicitly specify the access control level for a path. It can be confusing that this is spelled out directly in the path.

People sometimes expect the following to work:

fs.exists(wn.path.directory("Documents", "Todos"))

Which does not work currently, and it probably should. Currently you must specify the branch (aka. access control level):

fs.exists(wn.path.directory(wn.path.Branch.Private, "Documents", "Todos")) // or public branch

Impact

It's confusing!

Solution

Default to private paths, and fall back to public paths if not found, and log each step. This probably needs some more DX thinking about the interface, too.

We already have this behaviour in the UI of Fission Drive, so it seems to make sense to bring it to the SDK.

expede avatar Mar 23 '22 22:03 expede

What do you think about an explicit separation of the baked-in dichotomy we've established? Eg


let hasPrivateFile = await fs.exists(wn.path.private.file('somedir', 'somefile.json'));

let hasPublicFile = await fs.exists(wn.path.public.file('somedir', 'somefile.json'));

Aside: I'm not sure what @icidasset thinks but this underscores a wtf moment in Drive where it "appears" as if the Public directory is a sub-directory of the private root, whereas in reality these are separate partitions, akin to separate nfs mounts in your "home" directory on a Linux system.

jeffgca avatar Mar 24 '22 22:03 jeffgca

@expede ~~That wasn't the issue.~~ Updated issue.

    // Ask the user permission to additional filesystem paths
    fs: {
      private: [wn.path.directory("Documents", "Todos")],
   // ^^^^^^^^                    ^
   //   This               Also needs to be here
   //
   // ...which feels redundant if nothing else ;)
    }

The this pointing to private you're talking about here does not need to be there in the path.directory fn call, that code is correct as is. The issue that was described was that

fs.exists(wn.path.directory("Documents", "Todos"))

was called without the private branch. So that needed to be:

fs.exists(wn.path.directory(wn.path.Branch.Private, "Documents", "Todos"))
// or just "private"

I do agree that defaulting to private paths is a good idea 👍

icidasset avatar Mar 25 '22 10:03 icidasset

@therealjeffg I think we've discussed this as a group when we first designed Drive. I think the current design makes sense, maybe not for everyone though. If I recall correctly we've chosen this because it matches the MacOS home directory design, ie. everything in there is private by default, except for the 'Public' folder. That said, I don't doubt this could be explained better, or have a better UI, in Drive.

I like your API suggestion, but I agree with Brooklyn that we should make it private by default, like Drive does. So perhaps we could do:

wn.path.directory("Documents")
wn.path.public.directory("Documents")
// and then maybe we should also have the following, in case people want to be explicit.
// + make sure the docs say this is the default.
wn.path.private.directory("Documents")

Making paths private by default seems like the most safe option. The reason we haven't done this yet is because WNFS currently doesn't support moving things from /private/ to /public/. Which I think is a must have if we'd go private by default.

icidasset avatar Mar 25 '22 10:03 icidasset

In terms of API changes, we've also discussed something else in the past: The public and private partitions actually can't interact easily today. So e.g. a fs.mv(path.file("public", "file.txt"), path.file("private", "file.txt")) doesn't work.

So we've thought about making the API be this instead:

fs.public.read(path.file("file.txt"))
fs.private.read(path.file("file.txt"))

matheus23 avatar Mar 28 '22 17:03 matheus23

@icidasset can you update the Issue description when you have a sec? 🙏

expede avatar Mar 29 '22 17:03 expede

This is the behavior I expected as a developer playing with webnative for the first time. 👍 this change!

jessmartin avatar Nov 30 '22 12:11 jessmartin

Keeping as is, preferring explicitness over magic. We should improve the path creation however so that when a branch is needed one must actually be specified. There are also scenarios where paths are created without a branch, hence the current flexibility.

Tracking in #457

icidasset avatar Jan 06 '23 11:01 icidasset