Retrieve mandatory email address from Github #127
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
WalkthroughThe 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
src/Libraries/GithubOAuth.php (1)
93-100: Update required inGoogleOAuth.php:The function
fetchUserInfoWithTokeninGoogleOAuth.phpdoes not include the logic for fetching the user's primary email if it is missing. Please update the function to include this logic, similar to the implementation inGithubOAuth.php.
src/Libraries/GoogleOAuth.php:
- Update
fetchUserInfoWithTokento fetch the user's primary email if it is missing.Analysis chain
LGTM! But verify the function usage in the codebase.
The code changes are approved.
However, ensure that all function calls to
fetchUserInfoWithTokencorrectly handle the new logic for fetching the user's primary email.
Let's verify the implementation of
fetchUserInfoWithTokeninGoogleOAuth.phpand ensure thatfetchUserEmailsWithTokenandgetUserPrimaryEmailmethods are correctly implemented and used.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `fetchUserInfoWithToken` correctly handle the new logic for fetching the user's primary email. # Test: Search for the function usage. Expect: Proper handling of the new logic. rg --type php -A 5 $'fetchUserInfoWithToken'Length of output: 2158
Script:
#!/bin/bash # Description: Verify the implementation of `fetchUserInfoWithToken` in `GoogleOAuth.php` and ensure `fetchUserEmailsWithToken` and `getUserPrimaryEmail` methods are correctly implemented and used. # Search for the implementation of `fetchUserInfoWithToken` in `GoogleOAuth.php` rg --type php -A 10 $'protected function fetchUserInfoWithToken()' src/Libraries/GoogleOAuth.php # Search for the `fetchUserEmailsWithToken` method rg --type php -A 5 $'function fetchUserEmailsWithToken' src/Libraries # Search for the `getUserPrimaryEmail` method rg --type php -A 5 $'function getUserPrimaryEmail' src/LibrariesLength of output: 1721
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/Libraries/GithubOAuth.php (2 hunks)
Additional context used
GitHub Check: PHP 8.1 Static Analysis
src/Libraries/GithubOAuth.php
[failure] 103-103:
Method Datamweb\ShieldOAuth\Libraries\GithubOAuth::fetchUserEmailsWithToken() has no return type specified.
[failure] 122-122:
Method Datamweb\ShieldOAuth\Libraries\GithubOAuth::getUserPrimaryEmail() has parameter $emailAddresses with no value type specified in iterable type array.
GitHub Check: PHP 8.0 Static Analysis
src/Libraries/GithubOAuth.php
[failure] 103-103:
Method Datamweb\ShieldOAuth\Libraries\GithubOAuth::fetchUserEmailsWithToken() has no return type specified.
[failure] 122-122:
Method Datamweb\ShieldOAuth\Libraries\GithubOAuth::getUserPrimaryEmail() has parameter $emailAddresses with no value type specified in iterable type array.
GitHub Check: PHP 7.4 Static Analysis
src/Libraries/GithubOAuth.php
[failure] 103-103:
Method Datamweb\ShieldOAuth\Libraries\GithubOAuth::fetchUserEmailsWithToken() has no return type specified.
[failure] 122-122:
Method Datamweb\ShieldOAuth\Libraries\GithubOAuth::getUserPrimaryEmail() has parameter $emailAddresses with no value type specified in iterable type array.
Additional comments not posted (1)
src/Libraries/GithubOAuth.php (1)
122-129: Specify the parameter type forgetUserPrimaryEmail.The function parameter lacks a type specification. Adding a type will improve code clarity and type safety.
- protected function getUserPrimaryEmail(array $emailAddresses): string + protected function getUserPrimaryEmail(array $emailAddresses): stringLikely invalid or redundant comment.
Tools
GitHub Check: PHP 8.1 Static Analysis
[failure] 122-122:
Method Datamweb\ShieldOAuth\Libraries\GithubOAuth::getUserPrimaryEmail() has parameter $emailAddresses with no value type specified in iterable type array.GitHub Check: PHP 8.0 Static Analysis
[failure] 122-122:
Method Datamweb\ShieldOAuth\Libraries\GithubOAuth::getUserPrimaryEmail() has parameter $emailAddresses with no value type specified in iterable type array.GitHub Check: PHP 7.4 Static Analysis
[failure] 122-122:
Method Datamweb\ShieldOAuth\Libraries\GithubOAuth::getUserPrimaryEmail() has parameter $emailAddresses with no value type specified in iterable type array.
| protected function fetchUserEmailsWithToken() | ||
| { | ||
| // send request to API URL | ||
| try { | ||
| $response = $this->client->request('GET', self::$API_USER_EMAILS_URL, [ | ||
| 'headers' => [ | ||
| 'User-Agent' => self::$APPLICATION_NAME . '/1.0', | ||
| 'Accept' => 'application/vnd.github+json', | ||
| 'Authorization' => 'Bearer ' . $this->getToken(), | ||
| ], | ||
| 'http_errors' => false, | ||
| ]); | ||
| } catch (Exception $e) { | ||
| exit($e->getMessage()); | ||
| } | ||
|
|
||
| return json_decode($response->getBody(), false); | ||
| } |
There was a problem hiding this comment.
Specify the return type for fetchUserEmailsWithToken.
The function lacks a return type specification. Adding a return type will improve code clarity and type safety.
- protected function fetchUserEmailsWithToken()
+ protected function fetchUserEmailsWithToken(): arrayCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| protected function fetchUserEmailsWithToken() | |
| { | |
| // send request to API URL | |
| try { | |
| $response = $this->client->request('GET', self::$API_USER_EMAILS_URL, [ | |
| 'headers' => [ | |
| 'User-Agent' => self::$APPLICATION_NAME . '/1.0', | |
| 'Accept' => 'application/vnd.github+json', | |
| 'Authorization' => 'Bearer ' . $this->getToken(), | |
| ], | |
| 'http_errors' => false, | |
| ]); | |
| } catch (Exception $e) { | |
| exit($e->getMessage()); | |
| } | |
| return json_decode($response->getBody(), false); | |
| } | |
| protected function fetchUserEmailsWithToken(): array | |
| { | |
| // send request to API URL | |
| try { | |
| $response = $this->client->request('GET', self::$API_USER_EMAILS_URL, [ | |
| 'headers' => [ | |
| 'User-Agent' => self::$APPLICATION_NAME . '/1.0', | |
| 'Accept' => 'application/vnd.github+json', | |
| 'Authorization' => 'Bearer ' . $this->getToken(), | |
| ], | |
| 'http_errors' => false, | |
| ]); | |
| } catch (Exception $e) { | |
| exit($e->getMessage()); | |
| } | |
| return json_decode($response->getBody(), false); | |
| } |
Tools
GitHub Check: PHP 8.1 Static Analysis
[failure] 103-103:
Method Datamweb\ShieldOAuth\Libraries\GithubOAuth::fetchUserEmailsWithToken() has no return type specified.
GitHub Check: PHP 8.0 Static Analysis
[failure] 103-103:
Method Datamweb\ShieldOAuth\Libraries\GithubOAuth::fetchUserEmailsWithToken() has no return type specified.
GitHub Check: PHP 7.4 Static Analysis
[failure] 103-103:
Method Datamweb\ShieldOAuth\Libraries\GithubOAuth::fetchUserEmailsWithToken() has no return type specified.
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
Improvements