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

Authentication #669

Open
meidlinga opened this issue Feb 24, 2022 · 7 comments · May be fixed by #1077
Open

Authentication #669

meidlinga opened this issue Feb 24, 2022 · 7 comments · May be fixed by #1077

Comments

@meidlinga
Copy link

meidlinga commented Feb 24, 2022

Is your feature request related to a problem? Please describe.
When using tusd for multiple users, different users should not be able to access other users uploads in any way.
In my current situation, this applies to all possible operations, tusd supports on uploads.
Eventually in other situations, get could be whitelisted, while I would keep it simple initially.
I am aware, that the currently suggested approach is: "Deal with this in a proxy" But I think providing a reusable example would be a good idea. Also I would like to focus on the bridge between authentication and tusd, since the actual authentication implementation could differ.

Describe the solution you'd like
The basic idea would be:

  • Intercept postCreate, where we have an upload id and store it together with a user id, or an authentication token.
  • Other operations check the referenced upload id against the authentication token and reject any mismatches.

My currently favoured way would be implementing it in go using the approach described in "Use tusd as package", because it would mean less moving parts. But I am not sure if this is possible, or advisable.

Describe alternatives you've considered
As an alternative I considered combining a proxy and a hook.
The hook would be postCreate, since we have an upload id to register with an Authentication token (hash).
The proxy would be HAProxy with a custom Lua extension, blocking any request, where the uploadid does not match the authentication token.

The benefit here would be, that a proxy probably has better isolation possibilities than implementing it directly in tusd.

Can you provide help with implementing this feature?
Yes, since this could be a problem for other developers to, I would like to create and contribute a reusable solution.

Additional context
Details, like storing the token securely (in a serverside cache, jwt, cookie, etc), dealing with token refresh must be handled, but are currently not described further.

@Acconut
Copy link
Member

Acconut commented Feb 26, 2022

Yes, that is a very good idea. We have discussed and agreed on implementing additional hooks to facilitate authentication and authorization. Feel free to have a look at the discussions in #431 and #483. However, no work has been done so far as we first want to merge #516 which is a major overhaul of the entire hook system. Making any changes before this is complete is not sensible.

@meidlinga
Copy link
Author

Linking for future ref: #95

@G2G2G2G
Copy link

G2G2G2G commented Mar 14, 2023

Is this for the uploading too? There's a hook but uppy (for example) doesn't even handle status codes properly nor give you a method of reading the status code

               PreUploadCreateCallback: func(hook tusd.HookEvent) error {
			return checkauth(hook.Upload)
		},

func checkauth(u tusd.FileInfo) error {

//fancy checks up here

//metadata database query to check on some stuff sent by client via the u.MetaData["keys"]

    switch err {
	case nil:
		return nil
	case pgx.ErrNoRows:
		return tusd.NewHTTPError(errors.New("authentication failed"), 401)
	default:
		log.Print("ERROR checkauth:", err)
		return tusd.NewHTTPError(errors.New("error checking auth"), 500)
	}
}

Uppy does fail instantly with an 401 error though, so obviously they know what the error is about. There's a few mentions of it in their logic I see.

@Acconut
Copy link
Member

Acconut commented Mar 16, 2023

There's a hook but uppy (for example) doesn't even handle status codes properly nor give you a method of reading the status code

tus-js-client (and thus Uppy) should stop retrying if you return a 4XX status code. The error events from Uppy then allow you to access the response using the error objects. So this is all possible.

@G2G2G2G
Copy link

G2G2G2G commented Mar 17, 2023

where? https://uppy.io/docs/uppy/#upload-error this gives out a block of unusable and unlabeled text items and none of them have the status code.

Have an example of where the status code is? I read the uppy.js file too don't see where it's output

@Acconut
Copy link
Member

Acconut commented Mar 20, 2023

The error object should have additional properties to inspect the response. See the documentation for tus-js-client here: https://github.com/tus/tus-js-client/blob/main/docs/api.md#onerror

@Acconut
Copy link
Member

Acconut commented Feb 8, 2024

An implementation of a pre-access hook for checking access permissions to uploads has begun in #1077.

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

Successfully merging a pull request may close this issue.

3 participants