Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File systems with authorization #33

Merged
merged 3 commits into from
Sep 6, 2024
Merged

Conversation

quasiyoke
Copy link
Collaborator

Let's say you need to implement a WebDAV server with access control to files, directories and various file system functions. Users must authenticate with a token or password specified in the authorization headers.

I tried to implement similar access control using a few different hacks around the dav_server, but it was unsuccessful. In particular, I don't recommend trying to encode anything to pass to the file system adapter in file paths, since the dav_server needs them to provide the full path to the files in its responses.

To provide access control to file systems of this kind, I organized access to file system adapters through the GuardedFileSystem<C> trait, whose methods require a reference to generic credentials C to operate. I kept the same stateless approach to handling file system requests. The DavHandler<C> receives credentials from the library user in the DavHandler::<C>::handle_guarded method.

Backward compatibility

I want to preserve the support of old DavFileSystem implementations. Writing new adapters to file systems without access control shouldn't become any more difficult either. For example, introducing a GuardedFileSystem doesn't require any changes to dav-server-opendalfs (can be compiled as is).

For this purpose, blanket implementation GuardedFileSystem<()> for DavFileSystem was made. It required figuring out how Box<dyn GuardedFileSystem<C>> would be cloned. The old approach with BoxCloneFs required casting a Box<dyn DavFileSystem> into Box<dyn GuardedFileSystem<()>> which is problematic. Instead, I took a crate from dtolnay — dyn-clone, which solves the problem of cloning Box<dyn X> — thanks to it, a trait can be object-safe, and at the same time impose a requirement on the cloning of its implementations.

Since BoxCloneFs is no longer needed, I suggest removing this trait. Even though it was marked as #[doc(hidden)], it's essentially a backwards-incompatible change, but it won't require many edits from those maintaining their DavFileSystem implementations and who used BoxCloneFs. Since we already use dyn-clone for file system types, I used the same approach for DavLockSystem and DavMetaData (BoxCloneLs and BoxCloneMd were removed too).

The old methods DavHandler::handle and others without access control retained their signature and the code with them doesn't require refactoring since the generic credentials parameter C has a default value C = ().

The litmus server example examples/sample-litmus-server.rs reads auth headers but doesn't use credentials in the file system adapter, so it doesn't require refactoring.

Examples

  • Simple server with access control as documentation — examples/auth.rs.

To do

  1. The DavHandler<C> could take the principal (to specify locks' owner) automatically from the credentials type parameter C if it implements the Principal trait (not implemented). I decided that the confusion that such a feature would introduce was not worth the loss of flexibility in specifying the principal.
  2. The if_match_get_tokens function needs to be refactored as the method of DavInner, since half of its 6 arguments are already DavInner's fields. I didn't do this because it would introduce too many changes outside the scope of the pull request.
  3. We have to clone all the DavConfig fields to construct the DavInner, then we clone them to handle asynchronous requests to the DavInner, after which we clone a bunch of them into PropWriter (which is also cloned) for directory traversal. Most likely we should relate to DavInner through self: Arc<Self> (or &self) and clone the Arc if necessary to prevent excess allocations.

@messense messense self-requested a review August 27, 2024 01:46
Copy link
Owner

@messense messense left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you!

@messense messense merged commit 084b56b into messense:main Sep 6, 2024
6 checks passed
@messense
Copy link
Owner

messense commented Sep 6, 2024

Sent you an invitation to colab on this project. :-)

@quasiyoke quasiyoke deleted the feature/fs-auth branch September 6, 2024 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants