Draft: Webdav provider poc #4621
Conversation
not critical but some browsers might have problems
so we don't have to decode/decrypt/encode/encrypt so many times
New concept "simple auth" - authentication that happens immediately (in one http request) without redirecting to any third party. uppyAuthToken initially used to simply contain an encrypted & json encoded OAuth2 access_token for a specific provider. Then we added refresh tokens as well inside uppyAuthToken #4448. Now we also allow storing other state or parameters needed for that specific provider, like username, password, host name, webdav URL etc... This is needed for providers like webdav, ftp etc, where the user needs to give some more input data while authenticating Companion: - `providerTokens` has been renamed to `providerUserSession` because it now includes not only tokens, but a user's session with a provider. Companion `Provider` class: - New `hasSimpleAuth` static boolean property - whether this provider uses simple auth - uppyAuthToken expiry default 24hr again for providers that don't support refresh tokens - make uppyAuthToken expiry configurable per provider - new `authStateExpiry` static property (defaults to 24hr) - new static property `grantDynamicToUserSession`, allows providers to specify which state from Grant `dynamic` to include into the provider's `providerUserSession`.
also for thumbnails for consistency
it wasn't returning the status code (like `got` does on error) it's needed to respond properly with a http error
instead log error and show the key this in on par with other i18n frameworks
and don't replace the whole view with a loader when plugin state loading it will cause auth views to lose state an inter-view loading text looks much more graceful and is how SearchProviderView works too
add support for passing objects and messages from companion to uppy this allows companion to for example give a more detailed error when authenticating
don't force the user to use html form and use preact for it, for flexibility
heavily based on owncloud#1 but modified to work with the new provider user sessions
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
|
I can certainly help with finding/setting up a server. You could find me on talk.owncloud.com or you could open up your DMs on Twitter/X and we'll figure out something |
| transport: 'session', | ||
| // use 'subdomain' for full domain (hostname) similar to mastodon: | ||
| // https://github.com/simov/grant/blob/6e0692dfdd83edbc4ee82629ba0fe8f986d5879d/config/oauth.json#L631 | ||
| dynamic: ['subdomain'], |
There was a problem hiding this comment.
I'm not sure we should really set this as default/encourage it, it might be possible to craft a malicious redirect url with that and use it for phishing... haven't come up with anything clever for that so far (one of the reasons we did not enable this in production so far)
There was a problem hiding this comment.
Hmm, good point, but I wonder why grant does it for Mastodon if it's possible to abuse it for malicious purposes... Do you have any example of how one could abuse it?
| @@ -0,0 +1,94 @@ | |||
| // eslint-disable-next-line import/no-extraneous-dependencies | |||
There was a problem hiding this comment.
I don't know. I think something wrong with eslint or its import resolver (eslint gives an error if removed)
packages/@uppy/companion/src/server/provider/webdav/WebdavSimpleAuthProvider.js
Show resolved
Hide resolved
| <form onSubmit={onSubmit}> | ||
| <label htmlFor="uppy-Provider-publicLinkURL"> | ||
| <span>{i18n('publicLinkURLLabel')}</span> | ||
| <input id="uppy-Provider-publicLinkURL" name="webdavUrl" type="text" value={webdavUrl} onChange={(e) => setWebdavUrl(e.target.value)} disabled={loading} /> | ||
| </label> | ||
| <span>{i18n('publicLinkURLDescription')}</span> | ||
|
|
||
| <button style={{ display: 'block' }} disabled={loading} type="submit">Submit</button> | ||
| </form> |
There was a problem hiding this comment.
For the Oauth variant publicLinkURL in specified names probably doesn't make sense, we rather want to enter just a "server url" I think
|
I haven't tested the OAuth variant, as it's a bit cumbersome to setup - but I've tested the SimpleAuth variant and it does not seem to work. If you like, you can test with https://cloud.owncloud.com/s/29tljtuBOX6yHO9 |
|
WebDAV dependency should be bumped: |
|
We're still not sure whether we want to take on the maintenance burden of this plugin. It's quite a lot of code for a provider I never heard anyone needing except for your use case. I'm all for integrating the primitives into uppy/companion to make this happen, but perhaps the plugin belongs in user land. |
|
I understand. It's a bit unfortunate, as that will require me to maintain our fork and build our own docker images forever, but as long as we get the primitives integrated as you say I can live with that. Maybe we can make customizing the docker image easier as well, so that a new provider is automatically picked up if placed in a certain directory (or something like that) and doesn't need code changes otherwise. Then I don't need to maintain a full fork, but just a repository with the provider and a docker file which extends the official one. |
Maybe I misunderstand, but it shouldn't be too hard to maintain your own docker image so you don't have to maintain a fork of Companion?
This is pretty interesting, as it would mean standalone can be used with custom providers! @mifi what do you think? |
# Conflicts: # packages/@uppy/companion/src/server/controllers/refresh-token.js # packages/@uppy/companion/src/server/provider/index.js
(cherry picked from commit 4be2b6f)
Co-authored-by: Antoine du Hamel <antoine@transloadit.com>
see discussion here: #4619 (comment)
Co-authored-by: Antoine du Hamel <antoine@transloadit.com>
heavily based on owncloud#1 but modified to work with the new provider user sessions. not finished (missing some things from owncloud#1)
tldr: 2 new providers
problem: I'm not able to test these plugins because I don't know where to get WebDAV OAuth server credentials
note: don't merge before #4619 has been merged