-
Notifications
You must be signed in to change notification settings - Fork 179
Adds support to manage Native to SSO configuration on clients #885
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #885 +/- ##
==========================================
+ Coverage 79.18% 79.20% +0.02%
==========================================
Files 456 457 +1
Lines 5908 5915 +7
Branches 277 277
==========================================
+ Hits 4678 4685 +7
Misses 1136 1136
Partials 94 94
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Claude finished @kailash-b's task —— View job Code Review Complete ✅
Review Summary: Found issues with type safety and documentation that should be addressed. The implementation is functionally correct but could benefit from using enums for string properties to follow existing patterns and improve validation. |
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.
Overall Assessment
❌ There are several issues that should be addressed for better type safety and code quality.
The implementation correctly adds Native to SSO configuration support with proper JSON serialization and comprehensive integration tests. However, the code could benefit from stronger typing using enums instead of string literals for validation and consistency with existing patterns in the codebase.
Key concerns:
- String properties lack validation and type safety
- XML documentation formatting issues
- Missing enums for constrained string values
The integration tests are thorough and cover all scenarios appropriately.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
b7cf17e to
7f05091
Compare
|
Claude finished @kailash-b's task —— View job Code Review Complete ✅
Review Summary: The implementation is functional and well-tested but has minor type safety concerns. The |
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.
Overall Assessment
✅ The implementation is functional and well-tested, but has minor type safety concerns.
The PR successfully adds Native to SSO configuration support with comprehensive integration tests and proper JSON serialization. The enum approach for DeviceBindingType is correctly implemented. However, the AllowedAuthenticationMethods property lacks type safety and validation, which could lead to runtime issues with invalid values.
Key recommendations:
- Create an enum for authentication methods to match the pattern used for
DeviceBindingType - This will improve maintainability and prevent invalid values from being set
The integration tests are thorough and cover all scenarios appropriately.
|
Can I suggest using an enum for AllowedAuthenticationMethods, i.e. /// /// Allowed authentication methods for session transfer. /// public enum SessionTransferAuthenticationMethod { [EnumMember(Value = "cookie")] Cookie, [EnumMember(Value = "query")] Query } |
|
Also, I would appreciate being notified when this is merged and pushed to the public nuget server if possible. |
|
Hi @mfolker-sage 👋
This would have ideally been the approach if the allowed values were clearly mentioned on the auth0/docs. Since we do not know what the allowed values are, we let it be as is. If in the future these values are mentioned in the docs, then we will also update the SDK accordingly.
Sure! We will keep the original issue updated as soon as this fix is released and is available for use. |
https://auth0.com/docs/authenticate/single-sign-on/native-to-web/configure-implement-native-to-web If it helps, I think it is pretty clear in the documentation above. |
That is correct. However, the SDK implementation is bound to the API's schema. Since the schema does not restrict the users from sending any string, we will not be able to add that constraint in the SDK for now. In the meantime, we will initiate an internal discussion on whether this can be made explicit in the schema as well. That will make it clear for everyone. Once that reaches a conclusion, we will update the type to an |


✏️ Changes
This change adds support for managing Native to SSO (Single Sign-On) configuration on Auth0 clients through the Management API.
Classes and methods added:
SessionTransfermodel class with properties for configuring native-to-web SSO behaviorSessionTransferproperty toClientBaseclassProperties added to SessionTransfer model:
CanCreateSessionTransferToken- Controls whether an app can issue session tokens through Token ExchangeAllowedAuthenticationMethods- Specifies which methods can create sessions from session tokensEnforceDeviceBinding- Configures device binding security (by IP, ASN, or none)AllowRefreshToken- Controls refresh token issuance during session transfer authenticationEnforceOnlineRefreshTokens- Ties refresh tokens to session lifetimeEnforceCascadeRevocation- Enables cascade revocation of dependent tokensUsage summary:
This feature allows developers to configure Native to Web SSO settings when creating or updating Auth0 clients. The
SessionTransferproperty can be set on client creation/update requests to control how native applications can transfer authentication sessions to web applications.🔗 References
🎯 Testing
This change has been tested through integration tests that verify:
Testing coverage:
Integration tests added in
ClientTests.cscovering full CRUD operations with session transfer configurationTests verify all boolean, string, and array properties are handled correctly
Tests cover both enabled and disabled states for all configuration options
This change adds unit test coverage
This change adds integration test coverage
This change has been tested on the latest version of the platform/language
✅ Checklist