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

afpi: Ensure Dart expiresAt uses the UTC time zone #331

Merged
merged 1 commit into from Nov 1, 2023

Conversation

Widcket
Copy link
Collaborator

@Widcket Widcket commented Oct 31, 2023

  • 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

Currently, both native implementations (iOS and Android) provide credentials to the Dart layer that include an expiresAt value formatted as ISO 8601 date with UTC time zone. More specifically, the format used follows the RFC 3339's profile of ISO 8601 dates, which is somewhat more restrictive for the sake of simplicity:

Screenshot 2023-10-31 at 23 15 09

Among the fields that the RFC 3339 deems mandatory (that ISO 8601 does not) is the time zone. So, a RFC 3339 date must contain time zone information to be valid. The native implementations, in particular, use UTC as the time zone.

Besides passing RFC 3339 expiresAt dates to the Dart layer, the native implementations expect that any expiresAt dates received from the Dart layer also to be RFC 3339 dates. But, here comes the problem: the Dart layer generates the date strings from a DateTime object. A DateTime can either be set as "local" time, or can be set as UTC. That is, Dart DateTime objects do not support time zones other than UTC and "local". And by default, they'll be set as "local", meaning that when formatted as ISO 8601 dates, they won't be RFC 3339 compliant:

Screenshot 2023-10-31 at 23 43 40

We were not setting the expiresAt dates as UTC on the Dart layer. But still, this did not cause issues in most cases, because the Dart layer was dealing with expiresAt dates coming from the native implementations. As mentioned before, those are RFC 3339 compliant, and thus contain time zone information –the Z that signals UTC. It only caused issues when dealing with DateTime objects created externally, that were not set as UTC –e.g. as described in #304

To fix this issue, this PR enforces UTC for all expiresAt dates either coming from the native layer or going to the native layer, in the Dart layer.

📎 References

Fixes #304
See also https://ijmacd.github.io/rfc3339-iso8601/

🎯 Testing

Unit tests were added. The fix was also tested manually on both Android and iOS.

@Widcket Widcket requested a review from a team as a code owner October 31, 2023 23:46
@Widcket Widcket temporarily deployed to internal October 31, 2023 23:46 — with GitHub Actions Inactive
@Widcket Widcket temporarily deployed to internal October 31, 2023 23:46 — with GitHub Actions Inactive
@Widcket Widcket temporarily deployed to internal October 31, 2023 23:46 — with GitHub Actions Inactive
@Widcket Widcket temporarily deployed to internal October 31, 2023 23:46 — with GitHub Actions Inactive
@Widcket Widcket added the review:small Small review label Oct 31, 2023
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (253209a) 96.08% compared to head (7e08c9b) 95.79%.
Report is 14 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #331      +/-   ##
============================================
- Coverage     96.08%   95.79%   -0.30%     
============================================
  Files            97       76      -21     
  Lines          1611     1118     -493     
  Branches        331      281      -50     
============================================
- Hits           1548     1071     -477     
+ Misses           49       46       -3     
+ Partials         14        1      -13     
Flag Coverage Δ
auth0_flutter 100.00% <ø> (ø)
auth0_flutter_android ?
auth0_flutter_ios ?
auth0_flutter_platform_interface 87.03% <100.00%> (ø)

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

Files Coverage Δ
...lutter_platform_interface/lib/src/credentials.dart 100.00% <100.00%> (ø)

... and 23 files with indirect coverage changes

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

@Widcket Widcket merged commit b7c5a60 into main Nov 1, 2023
16 checks passed
@Widcket Widcket deleted the fix/utc-dart-dates branch November 1, 2023 08:45
@Widcket Widcket mentioned this pull request Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review:small Small review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in CredentialsManagerSaveMethodHandler
2 participants