Snyk fix 259390acd7312584b8bb38869d3538c9#195
Conversation
* Fix apple auth scope * Fix Apple auth scope test
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
|
Reviewer's GuideApple authentication handling is corrected across the wallet WDK and related services, versions are bumped to 3.0.0-beta.8, type exports for the dapp client are expanded, and Apple OAuth scopes are suppressed for Apple signups with corresponding tests and changeset metadata added. Sequence diagram for Apple OAuth authorization URL generationsequenceDiagram
actor User
participant DappClient
participant WalletWDK as Wallet_WDK_AuthCodeHandler
participant Apple as Apple_OAuth_Server
User->>DappClient: Selects_Apple_login
DappClient->>WalletWDK: startSignup(signupKind_apple)
WalletWDK->>WalletWDK: build_auth_code_url(state)
Note over WalletWDK: signupKind == apple
WalletWDK->>WalletWDK: Create_search_params_without_scope
WalletWDK->>DappClient: return_oauth_url
DappClient->>User: Redirect_to_oauth_url
User->>Apple: GET_authorize_without_scope_param
Apple-->>User: Apple_signin_UI
User->>Apple: Approve_signin
Apple-->>DappClient: Redirect_back_with_auth_code
Flow diagram for conditional OAuth scope handling in AuthCodeHandlerflowchart TD
A[Start_build_auth_code_url] --> B[Set_base_params_client_id_redirect_uri_response_type]
B --> C[Check_signup_kind]
C -->|signupKind_is_apple| D[Do_not_add_scope_param]
C -->|signupKind_is_not_apple| E[Add_scope_openid_profile_email]
D --> F[Create_search_params_from_params]
E --> F[Create_search_params_from_params]
F --> G[Return_oauthUrl_with_query_string]
G --> H[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Summary of ChangesHello @Dargon789, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates a Snyk-identified security fix primarily focused on refining Apple authentication processes across the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The auth code handler’s special-casing of
signupKind === 'apple'to omitscopecould benefit from an inline comment or reference to Apple’s OAuth/OpenID behavior so future maintainers understand why this divergence from other providers is required. - There are two new changeset files (
new-turkeys-doubleandnice-tips-slide) that appear to describe the same Apple auth change set; consider consolidating them into a single changeset to avoid duplicate patch bumps and potential version noise.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The auth code handler’s special-casing of `signupKind === 'apple'` to omit `scope` could benefit from an inline comment or reference to Apple’s OAuth/OpenID behavior so future maintainers understand why this divergence from other providers is required.
- There are two new changeset files (`new-turkeys-double` and `nice-tips-slide`) that appear to describe the same Apple auth change set; consider consolidating them into a single changeset to avoid duplicate patch bumps and potential version noise.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue with Apple authentication by correctly omitting the scope parameter from the OAuth authorization URL. The change is implemented cleanly and is accompanied by a new test to verify the fix. Additionally, the PR includes version bumps for multiple packages and exposes new types from the dapp-client API. I have one minor suggestion to improve maintainability by extracting a magic string to a constant. Overall, the changes are correct and well-implemented.
Dargon789
added
bug
Dargon789
left a comment
Dargon789
left a comment
There was a problem hiding this comment.
Snyk fix 259390acd7312584b8bb38869d3538c9#195
4 participants
Summary by Sourcery
Apply Apple authentication fixes across wallet and services packages and align related client exports and versions.
Bug Fixes:
Enhancements:
Tests: