auth0 / auth0-flutter

Auth0 SDK for Flutter
https://pub.dev/documentation/auth0_flutter/latest/
Apache License 2.0
61 stars 41 forks source link

Use Locale.US for Android #238

Closed Mecharyry closed 1 year ago

Mecharyry commented 1 year ago

📋 Changes

Offering as an alternative to https://github.com/auth0/auth0-flutter/pull/206.

This aligns how Android and iOS parse dates. iOS is already formatting dates to ISO8601String by directly taking the expiresAt property, without modification. Android on the other hand is modifying this expiresAt by allowing it to be localised to whatever the systems getDefaultLocale is.

To solve the issues we have seen with ar this ensures that all dates are using a consistent Locale of Locale.US. The test added credentials_test is used just to illustrate the breaking change is used to illustrate the issue with Arabic vs US English, but is actually fixed at the native level.

📎 References

No references.

🎯 Testing

Put your device into Arabic and attempt to use this library to log in. You'll notice that it works in this MR but doesn't on main

stevehobbsdev commented 1 year ago

Thankss @Mecharyry - can you elaborate on the breaking change? Does it introduce one?

Mecharyry commented 1 year ago

Thankss @Mecharyry - can you elaborate on the breaking change? Does it introduce one?

Sorry I goofed when writing the description. I meant to write that the test class illustrates why parsing an Arabic specific date time breaks vs a US one. I've updated the description.

poovamraj commented 1 year ago

@stevehobbsdev I'd prefer this solution as well over the other. LGTM 👍

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 97.36% and project coverage change: -0.08 :warning:

Comparison is base (4fd061e) 98.74% compared to head (ead54f2) 98.67%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #238 +/- ## ============================================ - Coverage 98.74% 98.67% -0.08% - Complexity 82 85 +3 ============================================ Files 80 85 +5 Lines 1439 1506 +67 Branches 320 331 +11 ============================================ + Hits 1421 1486 +65 - Misses 6 7 +1 - Partials 12 13 +1 ``` | Flag | Coverage Δ | | |---|---|---| | auth0_flutter | `100.00% <100.00%> (ø)` | | | auth0_flutter_android | `96.74% <100.00%> (+0.14%)` | :arrow_up: | | auth0_flutter_ios | `99.84% <96.29%> (-0.16%)` | :arrow_down: | | auth0_flutter_platform_interface | `99.00% <94.44%> (-0.29%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://app.codecov.io/gh/auth0/auth0-flutter/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0) | Coverage Δ | | |---|---|---| | [...auth0\_flutter/AuthenticationExceptionExtensions.kt](https://app.codecov.io/gh/auth0/auth0-flutter/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0#diff-YXV0aDBfZmx1dHRlci9hbmRyb2lkL3NyYy9tYWluL2tvdGxpbi9jb20vYXV0aDAvYXV0aDBfZmx1dHRlci9BdXRoZW50aWNhdGlvbkV4Y2VwdGlvbkV4dGVuc2lvbnMua3Q=) | `100.00% <ø> (ø)` | | | [...ses/AuthAPI/AuthAPILoginWithOTPMethodHandler.swift](https://app.codecov.io/gh/auth0/auth0-flutter/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0#diff-YXV0aDBfZmx1dHRlci9pb3MvQ2xhc3Nlcy9BdXRoQVBJL0F1dGhBUElMb2dpbldpdGhPVFBNZXRob2RIYW5kbGVyLnN3aWZ0) | `100.00% <ø> (ø)` | | | [auth0\_flutter/lib/auth0\_flutter.dart](https://app.codecov.io/gh/auth0/auth0-flutter/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0#diff-YXV0aDBfZmx1dHRlci9saWIvYXV0aDBfZmx1dHRlci5kYXJ0) | `100.00% <ø> (ø)` | | | [...0\_flutter/ios/Classes/AuthAPI/AuthAPIHandler.swift](https://app.codecov.io/gh/auth0/auth0-flutter/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0#diff-YXV0aDBfZmx1dHRlci9pb3MvQ2xhc3Nlcy9BdXRoQVBJL0F1dGhBUElIYW5kbGVyLnN3aWZ0) | `97.67% <50.00%> (-2.33%)` | :arrow_down: | | [...latform\_interface/lib/src/auth/challenge\_type.dart](https://app.codecov.io/gh/auth0/auth0-flutter/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0#diff-YXV0aDBfZmx1dHRlcl9wbGF0Zm9ybV9pbnRlcmZhY2UvbGliL3NyYy9hdXRoL2NoYWxsZW5nZV90eXBlLmRhcnQ=) | `75.00% <75.00%> (ø)` | | | [...tlin/com/auth0/auth0\_flutter/Auth0FlutterPlugin.kt](https://app.codecov.io/gh/auth0/auth0-flutter/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0#diff-YXV0aDBfZmx1dHRlci9hbmRyb2lkL3NyYy9tYWluL2tvdGxpbi9jb20vYXV0aDAvYXV0aDBfZmx1dHRlci9BdXRoMEZsdXR0ZXJQbHVnaW4ua3Q=) | `72.22% <100.00%> (+0.79%)` | :arrow_up: | | [...ter/request\_handlers/api/LoginApiRequestHandler.kt](https://app.codecov.io/gh/auth0/auth0-flutter/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0#diff-YXV0aDBfZmx1dHRlci9hbmRyb2lkL3NyYy9tYWluL2tvdGxpbi9jb20vYXV0aDAvYXV0aDBfZmx1dHRlci9yZXF1ZXN0X2hhbmRsZXJzL2FwaS9Mb2dpbkFwaVJlcXVlc3RIYW5kbGVyLmt0) | `97.22% <100.00%> (ø)` | | | [...uest\_handlers/api/LoginWithOtpApiRequestHandler.kt](https://app.codecov.io/gh/auth0/auth0-flutter/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0#diff-YXV0aDBfZmx1dHRlci9hbmRyb2lkL3NyYy9tYWluL2tvdGxpbi9jb20vYXV0aDAvYXV0aDBfZmx1dHRlci9yZXF1ZXN0X2hhbmRsZXJzL2FwaS9Mb2dpbldpdGhPdHBBcGlSZXF1ZXN0SGFuZGxlci5rdA==) | `96.55% <100.00%> (ø)` | | | [...dlers/api/MultifactorChallengeApiRequestHandler.kt](https://app.codecov.io/gh/auth0/auth0-flutter/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0#diff-YXV0aDBfZmx1dHRlci9hbmRyb2lkL3NyYy9tYWluL2tvdGxpbi9jb20vYXV0aDAvYXV0aDBfZmx1dHRlci9yZXF1ZXN0X2hhbmRsZXJzL2FwaS9NdWx0aWZhY3RvckNoYWxsZW5nZUFwaVJlcXVlc3RIYW5kbGVyLmt0) | `100.00% <100.00%> (ø)` | | | [...ter/request\_handlers/api/RenewApiRequestHandler.kt](https://app.codecov.io/gh/auth0/auth0-flutter/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0#diff-YXV0aDBfZmx1dHRlci9hbmRyb2lkL3NyYy9tYWluL2tvdGxpbi9jb20vYXV0aDAvYXV0aDBfZmx1dHRlci9yZXF1ZXN0X2hhbmRsZXJzL2FwaS9SZW5ld0FwaVJlcXVlc3RIYW5kbGVyLmt0) | `96.66% <100.00%> (ø)` | | | ... and [9 more](https://app.codecov.io/gh/auth0/auth0-flutter/pull/238?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=auth0) | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

Mecharyry commented 1 year ago

@poovamraj Sorry for the delay, I've made the requested changes!

Mecharyry commented 1 year ago

@poovamraj @stevehobbsdev Can I get some eyes on this? It would be great to merge this so I can stop maintaining a fork 😄

stevehobbsdev commented 1 year ago

@Mecharyry Thanks for your patience here. We're gearing up for a new release that includes web support, this PR will be included along with that work. We're aiming for that release within the next week or so.

stevehobbsdev commented 1 year ago

@Mecharyry FYI This has been included in v1.2.0-beta.1, we're expecting this to come out of beta next week 👍🏻

Thanks again for your contrubution!