Revert "Revert "Refactor/dedupe cookie/session logic"" #4426
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 0e7b16a.
arturi
approved these changes
Apr 24, 2023
This was referenced Jun 14, 2023
This was referenced Jun 22, 2023
This was referenced Jul 8, 2023
mifi
added a commit
that referenced
this pull request
Aug 13, 2023
This was referenced Nov 27, 2023
mifi
added a commit
that referenced
this pull request
Dec 7, 2023
* remove useless line * fix broken cookie removal logic related #4426 * fix mime type of thumbnails not critical but some browsers might have problems * simplify/speedup token generation so we don't have to decode/decrypt/encode/encrypt so many times * use instanceof instead of prop check * Implement alternative provider auth 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`. * refactor * use respondWithError also for thumbnails for consistency * fix prepareStream it wasn't returning the status code (like `got` does on error) it's needed to respond properly with a http error * don't throw when missing i18n key instead log error and show the key this in on par with other i18n frameworks * fix bugged try/catch * allow aborting login too 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 json http error support 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 tightly couple auth form with html form don't force the user to use html form and use preact for it, for flexibility * fix i18n * make contentType parameterized * allow sending certain errors to the user 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 * sending certain onedrive errors to the user 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 * don't have default content-type * make a loginSimpleAuth api too * make removeAuthToken protected (cherry picked from commit 4be2b6f) * fix lint * run yarn format * Apply suggestions from code review Co-authored-by: Antoine du Hamel <antoine@transloadit.com> * fix broken merge conflict * improve inheritance * fix bug * fix bug with dynamic grant config * use duck typing for error checks see discussion here: #4619 (comment) * Apply suggestions from code review Co-authored-by: Antoine du Hamel <antoine@transloadit.com> * fix broken lint fix script * fix broken merge code * try to fix flakey tets * fix lint --------- Co-authored-by: Antoine du Hamel <antoine@transloadit.com>
mifi
added a commit
that referenced
this pull request
Dec 12, 2023
* remove useless line * fix broken cookie removal logic related #4426 * fix mime type of thumbnails not critical but some browsers might have problems * simplify/speedup token generation so we don't have to decode/decrypt/encode/encrypt so many times * use instanceof instead of prop check * Implement alternative provider auth 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`. * refactor * use respondWithError also for thumbnails for consistency * fix prepareStream it wasn't returning the status code (like `got` does on error) it's needed to respond properly with a http error * don't throw when missing i18n key instead log error and show the key this in on par with other i18n frameworks * fix bugged try/catch * allow aborting login too 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 json http error support 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 tightly couple auth form with html form don't force the user to use html form and use preact for it, for flexibility * fix i18n * make contentType parameterized * allow sending certain errors to the user 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 * make `authProvider` consistent always use the static property ignoring the instance propety fixes #4460 * fix bug * fix test also * don't have default content-type * make a loginSimpleAuth api too * make removeAuthToken protected (cherry picked from commit 4be2b6f) * fix lint * run yarn format * Apply suggestions from code review Co-authored-by: Antoine du Hamel <antoine@transloadit.com> * fix broken merge conflict * improve inheritance * fix bug * fix bug with dynamic grant config * use duck typing for error checks see discussion here: #4619 (comment) * Apply suggestions from code review Co-authored-by: Antoine du Hamel <antoine@transloadit.com> * fix broken lint fix script * fix broken merge code * try to fix flakey tets * fix lint * fix merge issue --------- Co-authored-by: Antoine du Hamel <antoine@transloadit.com>
|
Closing. If this was needed that we would have pressed for it more. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #4425
NOTE: WIP
I think grant/oauth would have to use query string to store state if we remove this.