-
Notifications
You must be signed in to change notification settings - Fork 56
CredentialsManager user info/ID token contents accessible via flutter SDK #607
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
base: main
Are you sure you want to change the base?
Conversation
| @@ -0,0 +1,111 @@ | |||
| class UserIdentity { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have a User_Profile class defined . Please re use that
| }); | ||
|
|
||
| Future<bool> storeCredentials(final Credentials credentials); | ||
| Future<UserInfo> getIDTokenContents(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be of nullable type ,in the scneario if the credentials aren't available and someone invokes this api ?
| } | ||
|
|
||
| /// Retrieves the credentials from the native storage. | ||
| Future<UserInfo> getIDTokenContents(final CredentialsManagerRequest request) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Credentials class uses a user property for this. Since this method here is exposing exactly the same functionality (parsing and returning the info contained in the ID token), we should stick to the same naming convention, otherwise it could be confusing. Using different names for the same thing may suggest these are actually different things, or that there is a substantive difference.
| @@ -0,0 +1,16 @@ | |||
|
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a couple of symlinks missing. See https://github.com/auth0/auth0-flutter/actions/runs/15893748250/job/44821016570?pr=607
Widcket
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add Dart tests and Swift tests.
- Dart: https://github.com/auth0/auth0-flutter/blob/main/auth0_flutter_platform_interface/test/method_channel_credentials_manager_test.dart and https://github.com/auth0/auth0-flutter/blob/main/auth0_flutter/test/mobile/credentials_manager_test.dart
- Swift: https://github.com/auth0/auth0-flutter/blob/main/auth0_flutter/example/ios/Tests/CredentialsManager/CredentialsManagerHandlerTests.swift#L259-L262 and https://github.com/auth0/auth0-flutter/tree/main/auth0_flutter/example/ios/Tests/CredentialsManager (the new handler needs its own test file)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NandanPrabhu Let’s resolve the open comments, then we can test and merge this to close the GH issue
…/actions/setup-darwin (#709) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…/actions/setup-darwin (#710) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…/actions/setup-darwin (#713) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…/actions/setup-darwin (#716) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…/actions/setup-darwin (#719) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [ruby/setup-ruby](https://github.com/ruby/setup-ruby) from 1.281.0 to 1.282.0. - [Release notes](https://github.com/ruby/setup-ruby/releases) - [Changelog](https://github.com/ruby/setup-ruby/blob/master/release.rb) - [Commits](ruby/setup-ruby@675dd7b...4fc31e1) --- updated-dependencies: - dependency-name: ruby/setup-ruby dependency-version: 1.282.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
…/actions/setup-darwin (#722) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…/actions/setup-darwin (#725) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
| Future<UserInfo> getIDTokenContents() => | ||
| CredentialsManagerPlatform.instance.getIDTokenContents(_createApiRequest(null)); | ||
| ======= | ||
| /// Fetches new set of credentials each time and stores them in storage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has conflicting files could we fix this @pmathew92
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a new user() method to the Credentials Manager that allows accessing user profile information from cached credentials without attempting to refresh them. This addresses issue #378, enabling offline-first apps to access user identity information even when access tokens have expired and the device is offline.
Changes:
- Adds
user()method to CredentialsManager that returns UserProfile from cached credentials without refresh - Implements platform-specific handlers for iOS/macOS (Swift) and Android (Kotlin)
- Adds comprehensive test coverage across all layers
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| auth0_flutter_platform_interface/lib/src/user_info_credentials.dart | Defines UserInfo and UserIdentity classes (appears unused) |
| auth0_flutter_platform_interface/lib/src/credentials-manager/credentials_manager_platform.dart | Adds user() and getIDTokenContents() method declarations to platform interface |
| auth0_flutter_platform_interface/lib/src/credentials-manager/method_channel_credentials_manager.dart | Implements user() method that fetches profile from native storage |
| auth0_flutter_platform_interface/lib/auth0_flutter_platform_interface.dart | Exports new UserInfo class |
| auth0_flutter/lib/src/mobile/credentials_manager.dart | Adds user() method to CredentialsManager interface (contains merge conflict) |
| auth0_flutter/darwin/Classes/CredentialsManager/CredentialsManagerUserInfoMethodHandler.swift | Swift handler that accesses credentialsManager.user property |
| auth0_flutter/darwin/Classes/CredentialsManager/CredentialsManagerHandler.swift | Registers userInfo method handler (has duplicate case) |
| auth0_flutter/android/src/main/kotlin/.../GetCredentialsUserInfoRequestHandler.kt | Kotlin handler that accesses credentialsManager.userProfile property |
| auth0_flutter/android/src/main/kotlin/.../Auth0FlutterPlugin.kt | Registers the GetCredentialsUserInfoRequestHandler |
| auth0_flutter_platform_interface/test/method_channel_credentials_manager_test.dart | Comprehensive tests for user() method at platform interface layer |
| auth0_flutter/test/mobile/credentials_manager_test.dart | Tests for user() method in DefaultCredentialsManager |
| auth0_flutter/example/ios/Tests/.../CredentialsManagerUserInfoMethodHandlerTests.swift | Swift unit tests for the iOS/macOS handler |
| auth0_flutter/android/src/test/kotlin/.../GetCredentialsUserInfoRequestHandlerTest.kt | Kotlin unit tests for Android handler |
| Various mock files | Auto-generated mock updates for new user() method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Mocks generated by Mockito 5.4.5 from annotations | ||
| // in auth0_flutter/test/mobile/web_authentication_test.dart. | ||
| // Mocks generated by Mockito 5.4.4 from annotations | ||
| // in auth0_flutter/example/ios/.symlinks/plugins/auth0_flutter/test/mobile/web_authentication_test.dart. |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mock file header indicates it was generated from an unusual path: 'auth0_flutter/example/ios/.symlinks/plugins/auth0_flutter/test/mobile/web_authentication_test.dart' instead of the expected 'auth0_flutter/test/mobile/web_authentication_test.dart'. This suggests the mocks were regenerated from within a symlinked directory structure, which is typically part of Flutter example project builds. While this doesn't affect functionality, it's better practice to regenerate mocks from the package root directory to maintain cleaner file metadata.
| // in auth0_flutter/example/ios/.symlinks/plugins/auth0_flutter/test/mobile/web_authentication_test.dart. | |
| // in auth0_flutter/test/mobile/web_authentication_test.dart. |
| final Map<String, String> parameters = const {}, | ||
| }); | ||
|
|
||
| Future<UserInfo> getIDTokenContents(); |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The abstract CredentialsManager interface declares getIDTokenContents() method that returns Future, but this method is never implemented in DefaultCredentialsManager (due to the merge conflict). Additionally, UserInfo class is defined but unused. Since the actual implementation uses user() method returning UserProfile, either getIDTokenContents() should be removed from the interface, or it should be properly implemented. Having both methods in the interface creates confusion about which API consumers should use.
| case .clear: return CredentialsManagerClearMethodHandler(credentialsManager: credentialsManager) | ||
| case .userInfo: return CredentialsManagerUserInfoMethodHandler(credentialsManager: credentialsManager) | ||
| case .renew: return CredentialsManagerRenewMethodHandler(credentialsManager: credentialsManager) | ||
| case .userInfo: return CredentialsManagerUserInfoMethodHandler(credentialsManager: credentialsManager) |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The userInfo case is duplicated in the switch statement. Line 134 and line 136 both handle the .userInfo case, both returning CredentialsManagerUserInfoMethodHandler. One of these cases should be removed to avoid compilation errors.
| case .userInfo: return CredentialsManagerUserInfoMethodHandler(credentialsManager: credentialsManager) |
| /// Retrieves the credentials from the native storage. | ||
| Future<UserInfo> getIDTokenContents(final CredentialsManagerRequest request) { | ||
| throw UnimplementedError('getIDTokenContents() has not been implemented'); | ||
| } | ||
|
|
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getIDTokenContents() method is declared in the CredentialsManagerPlatform interface (line 36) but has no implementation in MethodChannelCredentialsManager. Additionally, it's referenced in credentials_manager.dart (line 71) but this implementation is incomplete and appears to be orphaned code that should have been removed. The method should either be fully implemented or removed entirely.
| /// Retrieves the credentials from the native storage. | |
| Future<UserInfo> getIDTokenContents(final CredentialsManagerRequest request) { | |
| throw UnimplementedError('getIDTokenContents() has not been implemented'); | |
| } |
| final Map<String, String> parameters = const {}, | ||
| }); | ||
|
|
||
| Future<UserInfo> getIDTokenContents(); |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a discrepancy between the PR description and implementation. The PR description states it will "expose api to access ID Token contents via Credentials Manager" and references getIDTokenContents. However, the actual implementation adds a user() method that returns UserProfile from the stored credentials (without refresh). The getIDTokenContents() method is declared but never implemented. The implementation should either complete getIDTokenContents() or remove it and update documentation to reflect that user() is the intended API.
| factory UserInfo.fromJson(final Map<String, dynamic> json) => | ||
| UserInfo( | ||
| id: json['id'] as String?, | ||
| name: json['name'] as String?, | ||
| nickname: json['nickname'] as String?, | ||
| pictureURL: json['pictureURL'] as String?, | ||
| email: json['email'] as String?, | ||
| isEmailVerified: json['isEmailVerified'] as bool?, | ||
| familyName: json['familyName'] as String?, | ||
| // Handle date parsing | ||
| createdAt: json['createdAt'] != null | ||
| ? DateTime.parse(json['createdAt'] as String) | ||
| : null, | ||
| // Handle list of UserIdentity | ||
| identities: (json['identities'] as List<dynamic>?) | ||
| ?.map((final e) => UserIdentity.fromJson(e as Map<String, dynamic>)) | ||
| .toList(), | ||
| extraInfo: Map<String, dynamic>.from(json['extraInfo'] as Map), | ||
| userMetadata: Map<String, dynamic>.from(json['userMetadata'] as Map), | ||
| appMetadata: Map<String, dynamic>.from(json['appMetadata'] as Map), | ||
| givenName: json['givenName'] as String?, | ||
| ); |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UserInfo class defined in this file appears to be unused. The actual user method returns UserProfile, not UserInfo. Additionally, the UserInfo class has potential issues: the fromJson factory method (lines 89-110) doesn't handle potential null values safely when calling Map.from on lines 106-108, which could throw exceptions if extraInfo, userMetadata, or appMetadata are null.
| factory UserInfo.fromJson(final Map<String, dynamic> json) => | |
| UserInfo( | |
| id: json['id'] as String?, | |
| name: json['name'] as String?, | |
| nickname: json['nickname'] as String?, | |
| pictureURL: json['pictureURL'] as String?, | |
| email: json['email'] as String?, | |
| isEmailVerified: json['isEmailVerified'] as bool?, | |
| familyName: json['familyName'] as String?, | |
| // Handle date parsing | |
| createdAt: json['createdAt'] != null | |
| ? DateTime.parse(json['createdAt'] as String) | |
| : null, | |
| // Handle list of UserIdentity | |
| identities: (json['identities'] as List<dynamic>?) | |
| ?.map((final e) => UserIdentity.fromJson(e as Map<String, dynamic>)) | |
| .toList(), | |
| extraInfo: Map<String, dynamic>.from(json['extraInfo'] as Map), | |
| userMetadata: Map<String, dynamic>.from(json['userMetadata'] as Map), | |
| appMetadata: Map<String, dynamic>.from(json['appMetadata'] as Map), | |
| givenName: json['givenName'] as String?, | |
| ); | |
| factory UserInfo.fromJson(final Map<String, dynamic> json) { | |
| final dynamic extraInfoRaw = json['extraInfo']; | |
| final dynamic userMetadataRaw = json['userMetadata']; | |
| final dynamic appMetadataRaw = json['appMetadata']; | |
| final Map<String, dynamic>? extraInfo = | |
| extraInfoRaw is Map ? Map<String, dynamic>.from(extraInfoRaw) : null; | |
| final Map<String, dynamic>? userMetadata = | |
| userMetadataRaw is Map ? Map<String, dynamic>.from(userMetadataRaw) : null; | |
| final Map<String, dynamic>? appMetadata = | |
| appMetadataRaw is Map ? Map<String, dynamic>.from(appMetadataRaw) : null; | |
| return UserInfo( | |
| id: json['id'] as String?, | |
| name: json['name'] as String?, | |
| nickname: json['nickname'] as String?, | |
| pictureURL: json['pictureURL'] as String?, | |
| email: json['email'] as String?, | |
| isEmailVerified: json['isEmailVerified'] as bool?, | |
| familyName: json['familyName'] as String?, | |
| // Handle date parsing | |
| createdAt: json['createdAt'] != null | |
| ? DateTime.parse(json['createdAt'] as String) | |
| : null, | |
| // Handle list of UserIdentity | |
| identities: (json['identities'] as List<dynamic>?) | |
| ?.map((final e) => UserIdentity.fromJson(e as Map<String, dynamic>)) | |
| .toList(), | |
| extraInfo: extraInfo, | |
| userMetadata: userMetadata, | |
| appMetadata: appMetadata, | |
| givenName: json['givenName'] as String?, | |
| ); |
| } | ||
|
|
||
| /// Fetches the user profile associated with the stored credentials. | ||
| /// Returns null is no credentials are stored |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment has a typo: "Returns null is no credentials are stored" should be "Returns null if no credentials are stored".
| /// Returns null is no credentials are stored | |
| /// Returns null if no credentials are stored |
| throw UnimplementedError('getCredentials() has not been implemented'); | ||
| } | ||
|
|
||
| /// Retrieves the credentials from the native storage. |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation comment states "Retrieves the credentials from the native storage" which is the same comment used for getCredentials() method. This is misleading since getIDTokenContents should retrieve ID token contents/claims, not full credentials. The comment should be updated to accurately describe what this method does, or the method should be removed if it's not meant to be part of the final implementation.
| /// Retrieves the credentials from the native storage. | |
| /// Retrieves the ID token contents (claims) from the native storage. |
📋 Changes
This PR exposes api to access ID Token contents via Credentials Manager in flutter SDK
GH issue #378 will be addressed via this PR
📎 References
🎯 Testing