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

[DAT-75] Correctly handle refreshToken for dataproxy replication #1587

Merged
merged 3 commits into from
Feb 11, 2025

Conversation

paultranvan
Copy link
Contributor

This includes:

  • Proper refreshToken detection in PouchLink
  • Proper handling of the refreshToken in worker env

@paultranvan paultranvan requested a review from Merkur39 as a code owner February 7, 2025 17:48
@Ldoppea
Copy link
Member

Ldoppea commented Feb 7, 2025

In order to ease history tracking, the failing test on the expired token message is a side effect of this previous commit where we introduced errors aggregation 5373877

The refreshToken detection was failing in replication, because an
AggregateError is made. We now handle such situation.
@paultranvan paultranvan force-pushed the refreshtokenworker branch 5 times, most recently from c1f1fe5 to 753cad4 Compare February 11, 2025 11:14
Comment on lines 212 to 214
if (token) {
token = token || data.cozyToken
token = token || cozyData.cozyToken
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block can be removed, data.cozyToken is from the appNode.dataset you removed.

Copy link
Member

@Ldoppea Ldoppea Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the result when calling document.querySelector('div[role="application"]').dataset:
image

The dataset.cozy value comes from the data-cozy attribute

All the dataset.cozyXxx values come from the deprecated data-cozy-xxx attributes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for the info, I was unsure of that

BREAKING CHANGE: The `data-cozy-token` injection is no longer supported
for refreshToken.

We used to rely on DOMParser to extract the new token during a
refreshToken procedure. However, DOMParser is a web API, which is not
available in web workers, nor in node env.  Therefore, we implement our
own HTML parsing, relying on the `data-cozy` attribute in HTML. We tried
using external libraries such as JSDom, fauxdom or linkedom, but got
build issues with all those libs. It was somehow manageable, but
required some additional config in consuming apps. As the HTML parsing
is quite basic, we decided that it is not worth the effort, and we now
do the parsing ourselves, making it available in web, workers, and node
envs.

As a consequence, we do not support the `data-cozy-token` existence
anymore, as it would require extra work and is seen as deprecated for
several years now. If your app still somehow require it, you need to
migrate the app template to rely on `data-cozy` like this:
https://github.com/cozy/cozy-drive/blob/master/src/targets/browser/index.ejs#L37
@paultranvan paultranvan merged commit 00742fb into master Feb 11, 2025
3 checks passed
@paultranvan paultranvan deleted the refreshtokenworker branch February 11, 2025 13:04
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Feb 11, 2025
`cozy-client` and `cozy-pouch-link` have been upgraded to `54.0.0` in
order to retrieve a fix on the refreshToken mechanism that did not work
when token expired during a Pouch replication

Related PR: cozy/cozy-client#1587
paultranvan pushed a commit to cozy/cozy-flagship-app that referenced this pull request Feb 11, 2025
`cozy-client` and `cozy-pouch-link` have been upgraded to `54.0.0` in
order to retrieve a fix on the refreshToken mechanism that did not work
when token expired during a Pouch replication

Related PR: cozy/cozy-client#1587
paultranvan pushed a commit to cozy/cozy-flagship-app that referenced this pull request Feb 11, 2025
`cozy-client` and `cozy-pouch-link` have been upgraded to `54.0.0` in
order to retrieve a fix on the refreshToken mechanism that did not work
when token expired during a Pouch replication

Related PR: cozy/cozy-client#1587
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Feb 11, 2025
`cozy-client` and `cozy-pouch-link` have been upgraded to `54.0.0` in
order to retrieve a fix on the refreshToken mechanism that did not work
when token expired during a Pouch replication

Related PR: cozy/cozy-client#1587
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Feb 12, 2025
`cozy-client` and `cozy-pouch-link` have been upgraded to `54.0.0` in
order to retrieve a fix on the refreshToken mechanism that did not work
when token expired during a Pouch replication

Related PR: cozy/cozy-client#1587
Ldoppea added a commit to cozy/cozy-flagship-app that referenced this pull request Feb 12, 2025
`cozy-client` and `cozy-pouch-link` have been upgraded to `54.0.0` in
order to retrieve a fix on the refreshToken mechanism that did not work
when token expired during a Pouch replication

Related PR: cozy/cozy-client#1587
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants