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

Fix access token expiry android #315

Merged
merged 3 commits into from
Oct 20, 2023
Merged

Conversation

poovamraj
Copy link
Contributor

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

📋 Changes

This error is happening because in Java we were parsing the credentials result as Z which refers to timezone parsing in Java

But this was later changed to escape as shown below.

Screenshot 2023-10-12 at 13 38 20

This means we were initially parsing the timestamp as UTC at the dart layer when credentials were retrieved and to be same as that we had to set timezone while saving the credentials.

But after that we have removed parsing timezone by escaping the Z with quotes ''. But we haven't removed the timezone being set in save credentials handler. This caused the time to jump to the offset of UTC.

We have fixed this by removing the timezone in save credentials handler.

📎 References

#286
#162
v1.0.1...main

🎯 Testing

Existing unit test has been fixed to avoid this issue

@poovamraj poovamraj requested a review from a team as a code owner October 12, 2023 11:43
@poovamraj poovamraj mentioned this pull request Oct 12, 2023
6 tasks
@Widcket
Copy link
Contributor

Widcket commented Oct 12, 2023

Wasn't the Z escaped in this PR: #238?

Copy link

@aprzedecki aprzedecki left a comment

Choose a reason for hiding this comment

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

Thats a fix we are looking for 👍

@Widcket
Copy link
Contributor

Widcket commented Oct 12, 2023

In fact, we requested the escaped format:

Screenshot 2023-10-12 at 1 07 04 PM

Are we sure this change won't re-introduce this issue? #160

@poovamraj
Copy link
Contributor Author

Yes, escaping it would be the right thing. But while we escaped the timezone "Z" which will parse the date to local time, we have forgotten to remove

format.timeZone = TimeZone.getTimeZone("UTC")

Also this would have caused issues for API login and other places since it was set only here and not in the other places where we format date

@Widcket
Copy link
Contributor

Widcket commented Oct 12, 2023

It'd be great to have a single date formatter (or format string) in the entire Android native layer, to avoid these kinds of issues.

@joymyr
Copy link

joymyr commented Oct 19, 2023

This fixes the token refesh issue for me. Please merge it

@Widcket Widcket temporarily deployed to internal October 20, 2023 06:54 — with GitHub Actions Inactive
@Widcket Widcket temporarily deployed to internal October 20, 2023 06:55 — with GitHub Actions Inactive
@Widcket Widcket temporarily deployed to internal October 20, 2023 06:55 — with GitHub Actions Inactive
@Widcket Widcket temporarily deployed to internal October 20, 2023 06:55 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0186ee5) 95.88% compared to head (4ece863) 95.79%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #315      +/-   ##
==========================================
- Coverage   95.88%   95.79%   -0.09%     
==========================================
  Files          75       76       +1     
  Lines        1117     1118       +1     
  Branches      281      281              
==========================================
  Hits         1071     1071              
- Misses         45       46       +1     
  Partials        1        1              
Flag Coverage Δ
auth0_flutter 100.00% <ø> (ø)
auth0_flutter_ios ?
auth0_flutter_platform_interface 87.03% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Widcket Widcket merged commit acc5d90 into main Oct 20, 2023
16 checks passed
@Widcket Widcket deleted the fix-access-token-expiry-android branch October 20, 2023 07:58
@aprzedecki
Copy link

Great! Can we release it also on pub.dev ?

@mng-prophero
Copy link

Great! Can we release it also on pub.dev ?

We're needing this released as well as it's affecting our users

@Widcket
Copy link
Contributor

Widcket commented Oct 24, 2023

👋🏼 I expect to cut a release by EOW.

This was referenced Oct 27, 2023
@poovamraj poovamraj mentioned this pull request Mar 5, 2024
2 tasks
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.

5 participants