-
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
feat: add support for microsoft services #171
base: develop
Are you sure you want to change the base?
Conversation
Warning Rate limit exceeded@warcooft has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 8 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes introduced in the pull request enhance the documentation and functionality of the OAuth library by adding support for Microsoft OAuth. This includes updates to the documentation files to clarify the process of obtaining OAuth keys and integrating Microsoft OAuth. A new Changes
Possibly related PRs
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 using PR comments)
Other keywords and placeholders
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: 3
Outside diff range and nitpick comments (7)
src/Language/en/ShieldOAuthLang.php (1)
39-42
: Rename the 'google' key to 'microsoft' for consistency.The addition of the Microsoft entry in the language localization file is a great enhancement to support Microsoft OAuth integration. It follows the same structure as the existing entries for other OAuth providers.
However, please consider renaming the 'google' key to 'microsoft' to accurately reflect the provider and maintain consistency with the other entries.
Apply this diff to rename the key:
'Microsoft' => [ - 'google' => 'Microsoft', + 'microsoft' => 'Microsoft', 'not_allow' => "Now you can't login or register with Microsoft!", ],src/Language/fa/ShieldOAuthLang.php (1)
39-42
: Translate the message and remove commented code.The addition of the
'Microsoft'
array looks good and follows the existing structure. However, please consider the following suggestions:
- Translate the
'not_allow'
message to Persian for consistency with the rest of the file.- Remove the commented out
'Yahoo'
array at the end of the file to keep the codebase clean.docs/index.md (1)
12-12
: Enhance the writing style.The addition of Microsoft OAuth to the list of supported services is consistent with the PR objective and provides clear information to the users.
To improve the writing style, consider the following suggestions:
- Rephrase "very easy" to avoid using a weak adjective. For example, "Adding support for other OAuth services is straightforward."
- Remove the exclamation mark at the end of the sentence to avoid excessive punctuation.
Tools
LanguageTool
[style] ~12-~12: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...tter, LinkedIn, GitLab and ..., this is very easy. Just create a class in route `app\Libr...(EN_WEAK_ADJECTIVE)
[style] ~12-~12: Using many exclamation marks might seem excessive (in this case: 6 exclamation marks for a text that’s 1540 characters long)
Context: ...ass in routeapp\Libraries\ShieldOAuth
! more info see [How to add other service...(EN_EXCESSIVE_EXCLAMATION)
docs/quickstart.md (4)
5-5
: Adjust the heading level and remove trailing punctuationTo maintain a consistent heading hierarchy and adhere to the Markdownlint style guide, consider the following changes:
- Use h2 (##) instead of h3 (###) for the heading.
- Remove the trailing colon (
:
) from the heading.Apply this diff:
-### Step 1 : +## Step 1Tools
Markdownlint
5-5: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
5-5: Punctuation: ' :'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
21-25
: LGTM! Minor nitpick on the heading punctuation.The instructions for adding OAuth buttons to login and registration pages are clear and concise.
To adhere to the Markdownlint style guide, remove the trailing colon (
:
) from the heading "Step 3 :".Tools
Markdownlint
21-21: Punctuation: ' :'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
Line range hint
36-58
: LGTM! The addition of Microsoft OAuth configuration is a great example.The instructions for configuring OAuth keys in the
ShieldOAuthConfig.php
file are clear and provide a good example of adding a new OAuth service (Microsoft).To adhere to the Markdownlint style guide, remove the trailing colon (
:
) from the heading "Step 4 :".Tools
Markdownlint
21-21: Punctuation: ' :'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
36-36: Punctuation: ' :'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
Line range hint
69-84
: LGTM! Clear instructions for completing the setup.The instructions for canceling filters for
Shield OAuth
routes and accessing the login and registration pages are clear and provide the necessary code snippet.To adhere to the Markdownlint style guide, remove the trailing colon (
:
) from the heading "Step 5 :".Tools
Markdownlint
69-69: Punctuation: ' :'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- docs/get_keys.md (2 hunks)
- docs/index.md (2 hunks)
- docs/install.md (6 hunks)
- docs/quickstart.md (5 hunks)
- src/Config/ShieldOAuthConfig.php (1 hunks)
- src/Language/en/ShieldOAuthLang.php (1 hunks)
- src/Language/fa/ShieldOAuthLang.php (1 hunks)
- src/Language/fr/ShieldOAuthLang.php (1 hunks)
- src/Language/id/ShieldOAuthLang.php (1 hunks)
- src/Libraries/MicrosoftOAuth.php (1 hunks)
Additional context used
LanguageTool
docs/get_keys.md
[typographical] ~3-~3: The word “Therefore” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...he documentation and convenience of our users, Therefore, we have provided helpful links on how t...(THUS_SENTENCE)
docs/index.md
[style] ~12-~12: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...tter, LinkedIn, GitLab and ..., this is very easy. Just create a class in route `app\Libr...(EN_WEAK_ADJECTIVE)
[style] ~12-~12: Using many exclamation marks might seem excessive (in this case: 6 exclamation marks for a text that’s 1540 characters long)
Context: ...ass in routeapp\Libraries\ShieldOAuth
! more info see [How to add other service...(EN_EXCESSIVE_EXCLAMATION)
Markdownlint
docs/get_keys.md
16-16: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
docs/index.md
15-15: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
docs/quickstart.md
5-5: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
5-5: Punctuation: ' :'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
21-21: Punctuation: ' :'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
36-36: Punctuation: ' :'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
69-69: Punctuation: ' :'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
GitHub Check: PHP 8.1 Static Analysis
src/Libraries/MicrosoftOAuth.php
[failure] 15-15:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_CODE_URL has no type specified.
[failure] 16-16:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_TOKEN_URL has no type specified.
[failure] 17-17:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_USER_INFO_URL has no type specified.
[failure] 18-18:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$APPLICATION_NAME has no type specified.
[failure] 53-53:
Method Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::fetchAccessTokenWithAuthCode() has parameter $allGet with no value type specified in iterable type array.
[failure] 100-100:
Method Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::setColumnsName() return type has no value type specified in iterable type array.
[failure] 116-116:
Call to deprecated function random_string():
The type 'basic', 'md5', and 'sha1' are deprecated. They are not cryptographically secure.
GitHub Check: PHP 8.0 Static Analysis
src/Libraries/MicrosoftOAuth.php
[failure] 15-15:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_CODE_URL has no type specified.
[failure] 16-16:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_TOKEN_URL has no type specified.
[failure] 17-17:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_USER_INFO_URL has no type specified.
[failure] 18-18:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$APPLICATION_NAME has no type specified.
[failure] 53-53:
Method Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::fetchAccessTokenWithAuthCode() has parameter $allGet with no value type specified in iterable type array.
[failure] 100-100:
Method Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::setColumnsName() return type has no value type specified in iterable type array.
[failure] 116-116:
Call to deprecated function random_string():
The type 'basic', 'md5', and 'sha1' are deprecated. They are not cryptographically secure.
GitHub Check: PHP 7.4 Static Analysis
src/Libraries/MicrosoftOAuth.php
[failure] 15-15:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_CODE_URL has no type specified.
[failure] 16-16:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_TOKEN_URL has no type specified.
[failure] 17-17:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_USER_INFO_URL has no type specified.
[failure] 18-18:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$APPLICATION_NAME has no type specified.
[failure] 53-53:
Method Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::fetchAccessTokenWithAuthCode() has parameter $allGet with no value type specified in iterable type array.
[failure] 100-100:
Method Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::setColumnsName() return type has no value type specified in iterable type array.
[failure] 116-116:
Call to deprecated function random_string():
The type 'basic', 'md5', and 'sha1' are deprecated. They are not cryptographically secure.
Additional comments not posted (16)
src/Language/id/ShieldOAuthLang.php (1)
39-42
: LGTM!The addition of the
'Microsoft'
array with the'microsoft'
and'not_allow'
keys is consistent with the existing structure and style of the language file. The changes enhance the functionality by providing localized support for Microsoft login, thereby expanding the authentication options available to users. The translations for the keys seem appropriate.src/Language/fr/ShieldOAuthLang.php (1)
39-42
: LGTM!The code segment correctly adds support for Microsoft OAuth functionality. The added array follows the same structure and naming conventions as the existing OAuth options. The messages are consistent with the messages for other providers.
docs/get_keys.md (2)
9-9
: LGTM!The addition of the Microsoft keys section aligns with the PR objective and enhances the comprehensiveness of the documentation.
40-43
: LGTM!The addition of the Microsoft keys section with links to guides aligns with the PR objective and enhances the comprehensiveness of the documentation.
docs/index.md (1)
17-20
: LGTM!The reformatting of the list items improves the readability of the documentation.
docs/quickstart.md (1)
33-35
: LGTM!The instructions for adding Bootstrap scripts to login and registration pages are clear and provide the necessary code snippet.
src/Config/ShieldOAuthConfig.php (1)
49-55
: LGTM!The new configuration array for Microsoft OAuth is consistent with the existing OAuth configurations in the
ShieldOAuthConfig
class. The placeholders provide clear instructions for obtaining the required 'client_id' and 'client_secret' values from Microsoft. Enabling login and registration for Microsoft OAuth aligns with the stated objective of adding support for Microsoft services.src/Libraries/MicrosoftOAuth.php (5)
15-26
: LGTM!The class properties are correctly defined with appropriate access modifiers and type specifications.
The static analysis hints about missing type specifications for the private static properties can be ignored as these properties are unlikely to change.
Tools
GitHub Check: PHP 8.1 Static Analysis
[failure] 15-15:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_CODE_URL has no type specified.
[failure] 16-16:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_TOKEN_URL has no type specified.
[failure] 17-17:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_USER_INFO_URL has no type specified.
[failure] 18-18:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$APPLICATION_NAME has no type specified.GitHub Check: PHP 8.0 Static Analysis
[failure] 15-15:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_CODE_URL has no type specified.
[failure] 16-16:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_TOKEN_URL has no type specified.
[failure] 17-17:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_USER_INFO_URL has no type specified.
[failure] 18-18:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$APPLICATION_NAME has no type specified.GitHub Check: PHP 7.4 Static Analysis
[failure] 15-15:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_CODE_URL has no type specified.
[failure] 16-16:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_TOKEN_URL has no type specified.
[failure] 17-17:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$API_USER_INFO_URL has no type specified.
[failure] 18-18:
Property Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::$APPLICATION_NAME has no type specified.
27-35
: LGTM!The constructor correctly initializes the necessary properties and retrieves the
client_id
andclient_secret
from environment variables or config.
37-51
: LGTM!The
makeGoLink
method correctly generates the Microsoft OAuth authorization URL with the necessary query parameters.
53-76
: LGTM!The
fetchAccessTokenWithAuthCode
method correctly exchanges the authorization code for an access token and handles exceptions appropriately.The static analysis hint about the missing value type specification for the
$allGet
parameter can be ignored as it is not critical for the functionality.Tools
GitHub Check: PHP 8.1 Static Analysis
[failure] 53-53:
Method Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::fetchAccessTokenWithAuthCode() has parameter $allGet with no value type specified in iterable type array.GitHub Check: PHP 8.0 Static Analysis
[failure] 53-53:
Method Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::fetchAccessTokenWithAuthCode() has parameter $allGet with no value type specified in iterable type array.GitHub Check: PHP 7.4 Static Analysis
[failure] 53-53:
Method Datamweb\ShieldOAuth\Libraries\MicrosoftOAuth::fetchAccessTokenWithAuthCode() has parameter $allGet with no value type specified in iterable type array.
78-98
: LGTM!The
fetchUserInfoWithToken
method correctly retrieves the user information from the Microsoft Graph API using the access token and handles exceptions appropriately.docs/install.md (4)
30-38
: LGTM!The note about manual installation provides a helpful alternative for users who cannot use Composer. The instructions are clear and accurate.
Line range hint
63-69
: LGTM!The note about configuring custom column names for user data is helpful for users who need this flexibility. The instructions and code snippet are clear and accurate.
Line range hint
85-122
: LGTM!The instructions for setting OAuth keys are clear and comprehensive. The addition of the Microsoft OAuth section aligns with the PR objective of adding support for Microsoft services.
Line range hint
127-147
: LGTM!The instructions for adding OAuth login buttons are clear and easy to follow. The addition of the Microsoft OAuth button aligns with the PR objective of adding support for Microsoft services.
Need update : shield-oauth/src/Libraries/Basic/ShieldOAuth.php Lines 87 to 148 in 4d31367
|
Microsoft oauth by default does not provide avatar in its payload, if we want it we can fetch it from a different endpoint. example: private static $API_USER_AVATAR_URL = 'https://graph.microsoft.com/v1.0/me/photos/240x240/$value';
private function getUserAvatarWithToken(): string|null
{
try {
$response = $this->client->request('GET', self::$API_USER_AVATAR_URL, [
'headers' => [
'Authorization' => 'Bearer ' . $this->getToken(),
'Content-Type' => 'image/jpg',
],
'http_errors' => false,
]);
} catch (Exception $e) {
die($e->getMessage());
}
$body = $response->getBody();
$base64 = base64_encode($body);
$avatar = $this->createAvatar($base64);
return $avatar;
}
private function createAvatar(string $base64, string $pathToSave = 'ShieldOauth/avatar/', string|null $newName = null): string|null
{
helper('text');
$decodedBase64 = base64_decode($base64);
if ($decodedBase64 === false) {
return null;
}
$pathToSave .= date('Y/m/');
$newName = $newName ?? 'avatar_' . random_string('alnum', 8) . '.jpg';
if (!is_dir($pathToSave) && !mkdir($pathToSave, 0777, true)) {
return null;
}
if (file_put_contents($pathToSave . $newName, $decodedBase64) === false) {
return null;
}
return $pathToSave . $newName;
} then in protected function fetchUserInfoWithToken(): object
{
try {
$response = $this->client->request('GET', self::$API_USER_INFO_URL, [
'headers' => [
'Accept' => 'application/json',
'User-Agent' => self::$APPLICATION_NAME . '/1.0',
'Authorization' => 'Bearer ' . $this->getToken(),
],
'http_errors' => false,
]);
} catch (Exception $e) {
exit($e->getMessage());
}
$userInfo = json_decode($response->getBody());
$userInfo->email = $userInfo->mail;
$userInfo->avatar = $this->getUserAvatarWithToken(); // add this
return $userInfo;
} |
See: |
Do you mean we should save the base64 data in the avatar column instead of just saving the file path? |
Hi, and thanks for the submitted PR! |
I think the avatar column should be left null, if anyone wants to implement avatar, they can send PR. |
Official support for Microsoft services.
If you have any questions feel free to ask.
Thanks.
Summary by CodeRabbit
New Features
Documentation