feat: add sso #358
feat: add sso #358
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #358 +/- ##
==========================================
- Coverage 45.68% 45.44% -0.24%
==========================================
Files 23 23
Lines 1902 1934 +32
==========================================
+ Hits 869 879 +10
- Misses 1033 1055 +22
☔ View full report in Codecov by Sentry. |
|
Probably should test this end to end once |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Type: Enhancement
PR Summary: This pull request introduces the functionality for single sign-on (SSO) authentication, allowing users to authenticate using an enterprise Identity Provider. The changes include the implementation of SSO in both asynchronous and synchronous GoTrue client classes, the addition of necessary types and response parsers, and updates to the documentation to reflect the new feature.
Decision: Comment
📝 Type: 'Enhancement' - not supported yet.
- Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
✅ Small diff: the diff is small enough to approve with confidence.
No details provided.
General suggestions:
- Review the usage of the 'captcha_token' variable within the 'sign_in_with_sso' methods as it is currently assigned but not utilized. If it is meant for future use, it should be documented accordingly.
- Ensure that the 'parse_sso_response' function is thoroughly tested to handle the expected SSO response structure and any edge cases that may occur during the SSO process.
- The 'skip_http_redirect' field should be typed as a boolean to reflect its intended usage, rather than as a string. This change will improve code clarity and prevent potential type-related bugs.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
| domain = credentials.get("domain") | ||
| options = credentials.get("options", {}) | ||
| redirect_to = options.get("redirect_to") | ||
| captcha_token = options.get("captcha_token") |
There was a problem hiding this comment.
issue (llm): The variable 'captcha_token' is assigned but not used within the 'sign_in_with_sso' method. If it's intended for future use or has side effects, consider adding a comment to clarify its purpose.
| domain = credentials.get("domain") | ||
| options = credentials.get("options", {}) | ||
| redirect_to = options.get("redirect_to") | ||
| captcha_token = options.get("captcha_token") |
There was a problem hiding this comment.
issue (llm): As in the async client, the 'captcha_token' is assigned but not used in the synchronous 'sign_in_with_sso' method. If this is intentional, please add a comment to explain its presence.
gotrue/types.py
Outdated
| class SignInWithSSOOptions(TypedDict): | ||
| redirect_to: NotRequired[str] | ||
| captcha_token: NotRequired[str] | ||
| skip_http_redirect: NotRequired[str] |
There was a problem hiding this comment.
issue (llm): The 'skip_http_redirect' field in 'SignInWithSSOOptions' is typed as a string, but its usage suggests a boolean value. Consider correcting the type to 'NotRequired[bool]' for clarity and correctness.
| return model_validate(UserResponse, data) | ||
|
|
||
|
|
||
| def parse_sso_response(data: Any) -> SSOResponse: |
There was a problem hiding this comment.
question (llm): The 'parse_sso_response' function is added to handle SSO responses. Verify that this function correctly parses the expected data structure and handles any potential edge cases or errors that may arise from the SSO process.
What kind of change does this PR introduce?
Address #340
Tested using flask-notes. Do note that we don't follow redirects on this method to mirror JS + also because httpx doesn't allow following of redirects