@uppy/companion: fix authProvider property inconsistency
#4672
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
# Conflicts: # packages/@uppy/companion/src/server/controllers/refresh-token.js # packages/@uppy/companion/src/server/provider/index.js
this is useful because:
// onedrive gives some errors here that the user might want to know about
// e.g. these happen if you try to login to a users in an organization,
// without an Office365 licence or OneDrive account setup completed
// 400: Tenant does not have a SPO license
// 403: You do not have access to create this personal site or you do not have a valid license
always use the static property ignoring the instance propety fixes #4460
mifi
changed the title
authProvider property inconsistency
# Conflicts: # packages/@uppy/companion-client/src/Provider.js # packages/@uppy/companion/src/server/provider/error.d.ts
(cherry picked from commit 4be2b6f)
see discussion here: #4619 (comment)
Co-authored-by: Antoine du Hamel <[email protected]>
aduh95
changed the title
authProvider property inconsistencyauthProvider property inconsistency
|
Isn't that semver-major? If someone we relying on that property to be non-static, it would break them, no? |
| @@ -157,6 +152,7 @@ class Zoom extends Provider { | |||
| }) | |||
| } | |||
|
|
|||
| // eslint-disable-next-line class-methods-use-this | |||
There was a problem hiding this comment.
This indicates that we could move it out of the class (i.e. as a standalone function). Can we do that in this PR?
There was a problem hiding this comment.
Right, but this has to be done in all providers so I didn't feel it was worth the effort (and breakage of git blame/history). maybe a todo will do?
There was a problem hiding this comment.
I just tried to refactor one of the provider classes by moving it out of the class, but then we have to add 6 new // eslint-disable-next-line class-methods-use-this because each instance method gets the same error (due to no longer using this.#withErrorHandling). so i'm guessing this is why i didn't do it in the first place 😅
Maybe yes. I tried now to make it backwards compatible (so that you can specify authProvider either as a static or instance property), but I can't make it work. It seem to lead to a catch-22:
Additionally, as discussed in #4460 - So I'm not sure what's the best way forward. Without this PR it's broken unless you specify both the static and instance properties. |
|
Docs and the example seem to be accurate: https://uppy.io/docs/companion/#how-to-add-custom-providers and https://github.com/transloadit/uppy/blob/main/examples/custom-provider/server/CustomProvider.cjs. Let's merge and update the blog post @mifi found to only reference the static property? |
| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/aws-s3 | 3.6.0 | @uppy/instagram | 3.2.0 | | @uppy/aws-s3-multipart | 3.10.0 | @uppy/onedrive | 3.2.0 | | @uppy/box | 2.2.0 | @uppy/provider-views | 3.8.0 | | @uppy/companion | 4.12.0 | @uppy/store-default | 3.2.0 | | @uppy/companion-client | 3.7.0 | @uppy/tus | 3.5.0 | | @uppy/core | 3.8.0 | @uppy/url | 3.5.0 | | @uppy/dropbox | 3.2.0 | @uppy/utils | 5.7.0 | | @uppy/facebook | 3.2.0 | @uppy/xhr-upload | 3.6.0 | | @uppy/google-drive | 3.4.0 | @uppy/zoom | 2.2.0 | | @uppy/image-editor | 2.4.0 | uppy | 3.21.0 | - @uppy/provider-views: fix uploadRemoteFile undefined (Mikael Finstad / #4814) - @uppy/companion: fix double tus uploads (Mikael Finstad / #4816) - @uppy/companion: fix accelerated endpoints for presigned POST (Mikael Finstad / #4817) - @uppy/companion: fix `authProvider` property inconsistency (Mikael Finstad / #4672) - @uppy/companion: send certain onedrive errors to the user (Mikael Finstad / #4671) - meta: fix typo in `lockfile_check.yml` name (Antoine du Hamel) - @uppy/aws-s3: change Companion URL in tests (Antoine du Hamel) - @uppy/set-state: fix types (Antoine du Hamel) - @uppy/companion: Provider user sessions (Mikael Finstad / #4619) - meta: fix `js2ts` script on Node.js 20+ (Merlijn Vos / #4802) - @uppy/companion-client: avoid unnecessary preflight requests (Antoine du Hamel / #4462) - meta: Migrate to AWS-SDK V3 syntax (Artur Paikin / #4810) - @uppy/utils: fix import in test files (Antoine du Hamel / #4806) - @uppy/core: Fix onBeforeFileAdded with Golden Retriever (Merlijn Vos / #4799) - @uppy/image-editor: respect `cropperOptions.initialAspectRatio` (Lucklj521 / #4805)
make authProvider consistent
fixes #4460
NOTE: do no merge before #4619