Query params are unnecessary cleared in redirect-uri when using redirection strategy #13
Conversation
wojciechdudek-livechat
changed the title
| @@ -114,9 +118,12 @@ export default class AccountsSDK { | |||
| authorizeURL(options = {}, flow = '') { | |||
There was a problem hiding this comment.
Can we add a new option called preserveOriginalHref set by default to false and run the persister logic only when explicitly set to true by the customer?
There was a problem hiding this comment.
As we discussed - IMO it isn't feature, it's fix and it should apply to default behaviour because if not then communication between agent-app and developer-app gets broken.
| if (!localOptions.redirect_uri) { | ||
| localOptions.redirect_uri = | ||
| window.location.origin + window.location.pathname; | ||
| localOptions.redirect_uri = window.location.href; |
There was a problem hiding this comment.
This change is no longer required. Href contains document fragments, we shouldn't use it here.
Let's use old redirect URIs and persist href in a different way.
There was a problem hiding this comment.
Also as we discussed - it's actually needed, but it doesn't matter because later on rediect_uri gets cut properly (query and hash params are removed).
| @@ -176,7 +183,10 @@ export default class AccountsSDK { | |||
| } | |||
|
|
|||
| this.transaction.generate(params); | |||
| this.redirectUriParamsPersister.persist(params); | |||
There was a problem hiding this comment.
Use window.location.href here and change the underlying implementation.
| this.redirectUriParamsPersister.persist(params); | |
| if (preserveOriginalHref) { | |
| this.hrefPerister.persist(window.location.href); | |
| } |
There was a problem hiding this comment.
We shouldn't rely on window.location.href. We should persist params directly taken from redirect_uri which can be passed by:
- sdk config
- or autorization strategy config
- or is taken from window.location.href as fallback
wojciechdudek-livechat
force-pushed
the
crearing-query-params-in-redirect-uri
branch
from
94a242d to
c4a295e
Compare
Products SDK issue:
livechat/products-sdk#19
Also reported by 99bits.