-
Notifications
You must be signed in to change notification settings - Fork 16
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
Retrieve mandatory email address from Github #128
Conversation
Retrieve the user's email addresses regardless if their status is set public or not
Update GithubOAuth.php
The /user API returns only the data the user explicitly set as public and null for those he didn't. If the user did not set the email address public (me for example), the email address being mandatory for the process, the authentication fails returning an error message. By using the /user/emails API in conjunction, it is possible to retrieve all the email addresses the user set in Github, regardless if they are set public or not, and select one of those (ex: primary one), for the login process to complete successfully.
…-Github Retrieve mandatory email address from Github
The /user API returns only the data the user explicitly set as public and null for those he didn't. If the user did not set the email address public (me for example), the email address being mandatory for the process, the authentication fails returning an error message. By using the /user/emails API in conjunction, it is possible to retrieve all the email addresses the user set in Github, regardless if they are set public or not, and select one of those (ex: primary one), for the login process to complete successfully.
WalkthroughThe recent updates to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration 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.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
src/Libraries/GithubOAuth.php (1)
94-101
: Missing Email Retrieval Logic inGoogleOAuth.php
The
fetchUserInfoWithToken
method inGoogleOAuth.php
does not handle cases where the user's email is missing from the initial response. UnlikeGithubOAuth.php
, which includes logic to fetch the primary email if it is not present,GoogleOAuth.php
should implement similar logic to ensure the email is always retrieved.
- File:
src/Libraries/GoogleOAuth.php
- Add logic to handle missing email similar to
GithubOAuth.php
.$userInfo = json_decode($response->getBody(), false); // the email address is mandatory if (empty($userInfo->email)) { $userInfo->email = $this->getUserPrimaryEmail($this->fetchUserEmailsWithToken()); } return $userInfo;Please ensure this logic is implemented to avoid issues with missing email addresses.
Analysis chain
LGTM! Enhanced email retrieval logic.
The changes ensure that if the user's email is not present in the initial response, it fetches the email using the new method
fetchUserEmailsWithToken
.However, ensure that all function calls to
fetchUserInfoWithToken
handle the possibility of the email being retrieved from a secondary source.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `fetchUserInfoWithToken`. # Test: Search for the function usage. Expect: Proper handling of email retrieval. rg --type php -A 5 $'fetchUserInfoWithToken'Length of output: 2158
Script:
#!/bin/bash # Description: Verify all subclass implementations and usages of `fetchUserInfoWithToken`. # Find all PHP files that extend the AbstractOAuth class fd --type f --name '*.php' | xargs grep -l 'extends AbstractOAuth' | xargs -I {} sh -c ' echo "File: {}" # Check the implementation of fetchUserInfoWithToken in the subclasses rg --type php -A 5 "fetchUserInfoWithToken" {} 'Length of output: 485
Script:
#!/bin/bash # Description: Verify all subclass implementations and usages of `fetchUserInfoWithToken`. # Find all PHP files and filter those that extend the AbstractOAuth class fd -e php | xargs grep -l 'extends AbstractOAuth' | while read -r file; do echo "File: $file" # Check the implementation of fetchUserInfoWithToken in the subclasses rg --type php -A 5 "fetchUserInfoWithToken" "$file" doneLength of output: 1200
Script:
#!/bin/bash # Description: Verify email retrieval logic in the fetchUserInfoWithToken method. # Check the email retrieval logic in GithubOAuth.php echo "File: src/Libraries/GithubOAuth.php" rg --type php -A 10 "fetchUserInfoWithToken" src/Libraries/GithubOAuth.php # Check the email retrieval logic in GoogleOAuth.php echo "File: src/Libraries/GoogleOAuth.php" rg --type php -A 10 "fetchUserInfoWithToken" src/Libraries/GoogleOAuth.phpLength of output: 1293
Script:
#!/bin/bash # Description: Retrieve the complete implementation of the fetchUserInfoWithToken method. # Get the complete implementation in GithubOAuth.php echo "File: src/Libraries/GithubOAuth.php" rg --type php -A 20 "fetchUserInfoWithToken" src/Libraries/GithubOAuth.php # Get the complete implementation in GoogleOAuth.php echo "File: src/Libraries/GoogleOAuth.php" rg --type php -A 20 "fetchUserInfoWithToken" src/Libraries/GoogleOAuth.phpLength of output: 1865
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/Libraries/GithubOAuth.php (2 hunks)
Additional comments not posted (3)
src/Libraries/GithubOAuth.php (3)
27-27
: LGTM! New constant for user emails API URL.The new constant
API_USER_EMAILS_URL
is correctly defined and aligns with the intended functionality.
107-124
: LGTM! New method for fetching user emails.The new method
fetchUserEmailsWithToken
is correctly implemented to handle the API call to fetch user emails, including error handling.
129-136
: LGTM! New method for determining primary email.The new method
getUserPrimaryEmail
correctly identifies the primary email from an array of email addresses, defaulting to the first email if no primary is marked.
The /user API returns only the data the user explicitly set as public and null for those he didn't.
If the user did not set the email address public (me for example), the email address being mandatory for the process, the authentication fails returning an error message.
By using the /user/emails API in conjunction, it is possible to retrieve all the email addresses the user set in Github, regardless if they are set public or not, and select one of those (ex: primary one), for the login process to complete successfully.
Summary by CodeRabbit
New Features
Bug Fixes