Skip to content
This repository has been archived by the owner on Mar 14, 2024. It is now read-only.

Add Cryptomator-Icon to Drive #37

Open
kojid0 opened this issue Apr 6, 2020 · 8 comments
Open

Add Cryptomator-Icon to Drive #37

kojid0 opened this issue Apr 6, 2020 · 8 comments

Comments

@kojid0
Copy link

kojid0 commented Apr 6, 2020

Adding a Cryptomator Icon to unlocked vaults / drives would enhance UX by showing that a vault is unlocked. The feature would also make it easier to distinguish at first glance between a Crypotmator drive and a Windows drive.

cryptomator-issue2

@infeo
Copy link
Member

infeo commented Apr 6, 2020

Thanks for this suggestion!

Some possible solutions discussed in dokan-dev/dokany#51

Originally posted by @overheadhunter in https://github.com/cryptomator/dokany-nio-adapter/issues/36#issuecomment-609583179

There are different approaches to achieve this:

  1. Over the registry
    1. only for the current user:
    2. for all users
  2. Over a certain file:
    1. via an autorun.inf file
    2. via the desktop.ini file

Since a vault can be mounted as a volume or as folder, we should check if one of the above methods covers both.

Some thougts:
Method 1.ii is the least acceptable, since it needs admin rights and thus not really useable for us.
Method 1.i is from my point of view the preferred one, since we only need to execute one command for setting or unsetting (using reg )
The filebased methods are from my point of view not the right tool, because Cryptomator is a os independet tool and we would need to place some files inside a vault only for windows. This can confuse users and they can accidently delete them.

@overheadhunter
Copy link
Member

How would you change the icon for a drive via registry?

I only came across the solution facilitating the desktop ini file, which we could simply add to the dir listing of the root directory.

@infeo
Copy link
Member

infeo commented Apr 7, 2020

How would you change the icon for a drive via registry?

See for example here: https://www.tenforums.com/tutorials/74659-change-drive-icon-windows-10-a.html#option3

What do you mean add to the dir listing? Like sideloading the file from another location? And nonetheless the user would see this file and be able to interact it, which they shouldn't imho.

@overheadhunter
Copy link
Member

user would see this file and be able to interact it, which they shouldn't imho

Why not?

@infeo
Copy link
Member

infeo commented Apr 7, 2020

Because this is adapter specifc stuff, the end-user of any application, which uses this adpater, should not be able to alter anything here except through an intended way by a developer. (at least not in an easy way) But since this file is added to the directory listing, it is visible and hence, can be possible altered.

Also, due to the sideloading it is not clear how to handle this alteration process. We need to add a filtering to all other methods if we want to prevent it. Or need a case distinction between this special file and all other files.

Additionally think about the implications specifically for Cryptomator:

  1. A vault contains only encrypted data
  2. Using this adpater to serve this vault shows inside your vault data that is not encrypted.
  3. What if someone altered this side-loaded data? Made it executable? Maybe even replaced with Maleware? And it is still shown inside a "secure" vault

The last two points are of course only valid, if we use this "side-loading" strategy. But if we do not have a file system which is backed up by file storage (e.g. database), i cannot imagine any other way to do it.

@overheadhunter
Copy link
Member

In my opinion it is very much intended by the developer that the user can add a desktop.ini file. For us, any file should only be a file, we should not distinguish it.

If the user writes such a file, dokany-nio-adapter passes it through to the underlying file system.

If the user attempts to read this file (and it exists), dokany-nio-adapter reads it from the underlying file system.

Only if it doesn't exist, it will "add" it. Kind of like OverlayFS works but with a precedence for the underlying file.

@infeo
Copy link
Member

infeo commented Apr 8, 2020

Ah, sorry, i forgot that this is not the dokan-java projekt and just an dokan adapter for the nio-interface of Java, therefore we will always have a filesystem/ file storage.

But since this is only a nio-adapter, don't we violate the contract of the interface by showing files which do not belong to the underlying filesystem?

In my opinion it is very much intended by the developer that the user can add a desktop.ini file. For us, any file should only be a file, we should not distinguish it.
If the user writes such a file, dokany-nio-adapter passes it through to the underlying file system.
If the user attempts to read this file (and it exists), dokany-nio-adapter reads it from the underlying file system.

I agree with you here. If we decide to use the registry solution we should research what the precedence is.

For the "overlay" solution the handling question persits: How to handle calls like move/write/delete onto such "not existing" file? We cannot block them, we need to return some code. Success or a specific error?

@overheadhunter
Copy link
Member

How to handle calls like move/write/delete onto such "not existing" file?

Legitimate to ask this for move/delete. Regarding write, I'd say the user is allowed to write through to the underlying file system, thus creating an "existing" file.

don't we violate the contract of the interface by showing files which do not belong to the underlying filesystem?

This is after all an optional feature. I don't want this library to add a "Cryptomator Icon", but rather add a Map<Path, byte[]> overlayFiles containing file contents to show for certain paths, which will be empty by default.

Of course, we need to thoroughly document the exact behaviour

Underlying File Exists Underlying File is Absent
Read Read underlying file Read from overlay map
Write Write to underlying file Write to underlying file
Delete Delete underlying file no-op? TBD
Move from Move underlying file TBD
Move to Move to underlying file Move to underlying file
...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants