add pre-get hook

[benitogf]

following up on #431

not sure if this feature would be welcomed, please advice @Acconut

[Acconut]

Thank you for the PR and apologies for my delayed response! I can see the value of a pre-get hook. You are probably using it for authorization/authentication as you mentioned in #431, aren't you? For those cases, I think it would make more sense to implement a proper authentication system using hooks where a hook is always invoked before a user tries to access an upload. The hook can then decide if this access should be granted or not.

Such a system is more complex to implement but would serve more use cases. What do you think?

[benitogf]

Hello! Yes this PR is trying to address the lack of content access control

a proper authentication system using hooks where a hook is always invoked before a user tries to access an upload

how is this different from the pre-get hook? I think that this could be used for logging, white/blacklisting, authentication, etc. same as current hooks

complex to implement but would serve more use cases

could you give some examples or the use cases? I think that tusd scope should be access control rather than something specific like authentication

[benitogf]

rebased with master branch and some further changes:

• call pre-get hook before getting file info so we don't return 404 without calling the hook
• added pre-get to the default enabled hooks flag

apologies for the commits mess 😅 maybe we can close and re-create the branch/PR

[Acconut]

a proper authentication system using hooks where a hook is always invoked before a user tries to access an upload

how is this different from the pre-get hook?

There are also other scenarios besides the GET request where a user accesses an upload. For example, the HEAD request where a user can get the file size, type and meta data. Such requests should also go through an access control system. Therefore, I think it's a bit weird if we have a system to check access before GET requests but not HEAD or PATCH requests.

Having said that, I think it would be beneficial to talk about how such a system should be implemented before we jump straight into code. Does that make sense?

I think that tusd scope should be access control rather than something specific like authentication

I used authentication in the meaning of access control. Maybe I should have said authorization or something else. However, the idea is that tusd invokes a hook, which then returns where a given request is allowed to access a specific upload.

[benitogf]

I think it's a bit weird if we have a system to check access before GET requests but not HEAD or PATCH requests.

Agreed, I understand the need for a unified hook, some points to discuss:

• renaming pre-get to access-control ? or authorization ?
• the hook should intercept all requests (GET, HEAD, PATCH, POST) ?
• there should be separation between read and write access access-write and access-read
• access-read will intercept GET and HEAD requests
• access-write will intercept PATCH and POST requests
• response on failure: on the http hook we could forward the status of the response, not sure if we could achieve the same with file hooks, or grpc. Maybe its better to return 403 on all cases instead?
• the hook both hooks should be invoked before getting any file information? meaning that the hook will not get access to the file information to determine its response (only the ID requested)

[Acconut]

That sounds all very good!

* both hooks should be invoked before getting any file information? meaning that the hook will not get access to the file information to determine its response (only the ID requested)

I think it would make sense to invoke them after the upload information has been retrieved. This avoids invoking the hook if the upload does not exist while also making the hook more useful.

However, I am currently working on a larger refactoring of the entire hook system in #516. There is not much there yet, but I want to make it easier to define response headers and the response body from a hook. Since that would be very useful for this work here, I think it makes sense to wait until that refactoring has been done.

[benitogf]

sounds good! thanks, we can close this one then and create an issue to document the requirements when possible

This avoids invoking the hook if the upload does not exist

not sure about this one, if there's no authorization it should not return a 404 just not authorized imho

[Acconut]

Superseeded by #1077, which implements a similar mechanism, which is not restricted to downloads.