-
Notifications
You must be signed in to change notification settings - Fork 1
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(doc): passport #75
Conversation
Warning Rate Limit Exceeded@Romakita has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 55 minutes and 30 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. WalkthroughThe updates to 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 Configration 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: 17
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (20)
- docs/tutorials/passport.md (1 hunks)
- docs/tutorials/snippets/passport/AcceptRolesMiddleware.ts (1 hunks)
- docs/tutorials/snippets/passport/AzureBearerProtocol.ts (1 hunks)
- docs/tutorials/snippets/passport/BasicProtocol.spec.ts (1 hunks)
- docs/tutorials/snippets/passport/BasicProtocol.ts (1 hunks)
- docs/tutorials/snippets/passport/DiscordProtocol.ts (1 hunks)
- docs/tutorials/snippets/passport/FacebookProtocol.ts (1 hunks)
- docs/tutorials/snippets/passport/JwtProtocol.ts (1 hunks)
- docs/tutorials/snippets/passport/LoginLocalProtocol.spec.ts (1 hunks)
- docs/tutorials/snippets/passport/LoginLocalProtocol.ts (1 hunks)
- docs/tutorials/snippets/passport/OriginalDiscordProtocol.js (1 hunks)
- docs/tutorials/snippets/passport/OriginalJwtPassport.js (1 hunks)
- docs/tutorials/snippets/passport/PassportCtrl.ts (1 hunks)
- docs/tutorials/snippets/passport/PassportFacebookCtrl.ts (1 hunks)
- docs/tutorials/snippets/passport/SignupLocalProtocol.ts (1 hunks)
- docs/tutorials/snippets/passport/acceptRoles.ts (1 hunks)
- docs/tutorials/snippets/passport/guard-basic-auth.ts (1 hunks)
- docs/tutorials/snippets/passport/guard-ctrl.ts (1 hunks)
- docs/tutorials/snippets/passport/roles-usage.ts (1 hunks)
- docs/tutorials/snippets/passport/server.ts (1 hunks)
Additional Context Used
LanguageTool (5)
docs/tutorials/passport.md (5)
Near line 35: Did you mean: “By default,”?
Context: ...ppets/passport/server.ts ### UserInfo By default Ts.ED use a UserInfo model to serialize...
Rule ID: BY_DEFAULT_COMMA
Near line 89: The word “checkout” is a noun. The verb is spelled with a space.
Context: ...: tip For signup and basic flow you can checkout one of our examples: <Projects type="p...
Rule ID: NOUN_VERB_CONFUSION
Near line 110: Consider a more expressive alternative.
Context: ...also possible to use the Basic Auth. To do that, you have to create a Protocol bas...
Rule ID: DO_ACHIEVE
Near line 143: Consider inserting a comma after ‘that’.
Context: ... gives an example to refresh the token. To do that you have to create a new Strategy and r...
Rule ID: TO_VERB_COMMA
Near line 161: Consider a shorter alternative to avoid wordiness.
Context: ...sport/FacebookProtocol.ts ::: tip Note In order to use Facebook authentication, you must f...
Rule ID: IN_ORDER_TO_PREMIUM
Biome (66)
docs/tutorials/snippets/passport/AcceptRolesMiddleware.ts (1)
7-7: Decorators are not valid here.
docs/tutorials/snippets/passport/AzureBearerProtocol.ts (5)
13-13: Decorators are not valid here.
13-13: Decorators are not valid here.
1-2: Some named imports are only used as types.
2-3: Some named imports are only used as types.
3-4: All these imports are only used as types.
docs/tutorials/snippets/passport/BasicProtocol.spec.ts (3)
33-33: Unexpected any. Specify a different type.
60-60: Unexpected any. Specify a different type.
84-84: Unexpected any. Specify a different type.
docs/tutorials/snippets/passport/BasicProtocol.ts (6)
16-16: Decorators are not valid here.
16-16: Decorators are not valid here.
16-16: Decorators are not valid here.
1-2: Some named imports are only used as types.
2-3: All these imports are only used as types.
4-5: All these imports are only used as types.
docs/tutorials/snippets/passport/DiscordProtocol.ts (6)
19-19: Decorators are not valid here.
19-19: Decorators are not valid here.
19-19: Unexpected any. Specify a different type.
1-2: Some named imports are only used as types.
2-3: Some named imports are only used as types.
4-5: All these imports are only used as types.
docs/tutorials/snippets/passport/FacebookProtocol.ts (6)
21-21: Decorators are not valid here.
21-21: Decorators are not valid here.
21-21: Unexpected any. Specify a different type.
2-3: Some named imports are only used as types.
3-4: Some named imports are only used as types.
4-5: All these imports are only used as types.
docs/tutorials/snippets/passport/JwtProtocol.ts (6)
19-19: Decorators are not valid here.
19-19: Decorators are not valid here.
19-19: Unexpected any. Specify a different type.
1-2: Some named imports are only used as types.
2-3: Some named imports are only used as types.
3-4: All these imports are only used as types.
docs/tutorials/snippets/passport/LoginLocalProtocol.spec.ts (3)
33-33: Unexpected any. Specify a different type.
60-60: Unexpected any. Specify a different type.
84-84: Unexpected any. Specify a different type.
docs/tutorials/snippets/passport/LoginLocalProtocol.ts (6)
33-33: Decorators are not valid here.
33-33: Decorators are not valid here.
3-4: Some named imports are only used as types.
4-5: Some named imports are only used as types.
5-6: All these imports are only used as types.
6-7: All these imports are only used as types.
docs/tutorials/snippets/passport/OriginalJwtPassport.js (3)
17-20: This else clause can be omitted because previous branches break early.
11-21: This function expression can be turned into an arrow function.
10-22: This function expression can be turned into an arrow function.
docs/tutorials/snippets/passport/PassportCtrl.ts (3)
12-12: Decorators are not valid here.
12-12: Decorators are not valid here.
12-12: Decorators are not valid here.
docs/tutorials/snippets/passport/PassportFacebookCtrl.ts (2)
10-10: Decorators are not valid here.
17-17: Decorators are not valid here.
docs/tutorials/snippets/passport/SignupLocalProtocol.ts (5)
20-20: Decorators are not valid here.
20-20: Decorators are not valid here.
2-3: Some named imports are only used as types.
5-6: All these imports are only used as types.
6-7: All these imports are only used as types.
docs/tutorials/snippets/passport/guard-basic-auth.ts (5)
15-15: Decorators are not valid here.
15-15: Decorators are not valid here.
15-15: Decorators are not valid here.
4-5: All these imports are only used as types.
5-6: All these imports are only used as types.
docs/tutorials/snippets/passport/guard-ctrl.ts (5)
15-15: Decorators are not valid here.
15-15: Decorators are not valid here.
15-15: Decorators are not valid here.
4-5: All these imports are only used as types.
5-6: All these imports are only used as types.
docs/tutorials/snippets/passport/server.ts (1)
1-2: All these imports are only used as types.
Additional comments not posted (14)
docs/tutorials/snippets/passport/acceptRoles.ts (1)
5-7
: The implementation of theAcceptRoles
decorator looks correct and aligns well with the PR's objectives to enhance role-based access control.docs/tutorials/snippets/passport/roles-usage.ts (1)
6-12
: TheCalendarController
is correctly set up with role-based access control for its POST endpoint.docs/tutorials/snippets/passport/PassportCtrl.ts (1)
7-15
: TheAuthCtrl
class and itslogin
method are correctly implemented. The static analysis hint about decorators is a false positive; the usage here is valid.docs/tutorials/snippets/passport/PassportFacebookCtrl.ts (1)
6-21
: TheAuthCtrl
class for Facebook authentication is correctly implemented. The static analysis hints about decorators are false positives; the usage here is valid.docs/tutorials/snippets/passport/AcceptRolesMiddleware.ts (1)
5-18
: TheAcceptRolesMiddleware
is correctly implemented for role-based access control. The static analysis hint about decorators is a false positive; the usage here is valid.
[APROVED]docs/tutorials/snippets/passport/guard-ctrl.ts (1)
8-17
: The implementation ofCalendarController
with authorization looks correct.docs/tutorials/snippets/passport/guard-basic-auth.ts (1)
8-17
: The implementation ofCalendarController
with basic authentication is correctly done.docs/tutorials/snippets/passport/BasicProtocol.ts (1)
16-30
: The$onVerify
method implementation looks correct and adheres to the expected logic for a basic authentication strategy.docs/tutorials/snippets/passport/SignupLocalProtocol.ts (1)
20-28
: The$onVerify
method correctly handles the registration logic by checking for existing users and throwing an appropriate exception if the email is already registered.docs/tutorials/snippets/passport/AzureBearerProtocol.ts (1)
13-34
: The$onVerify
method is well-implemented with comprehensive error handling and logging, ensuring robust token verification and user authentication.docs/tutorials/snippets/passport/LoginLocalProtocol.ts (1)
33-48
: The$onVerify
method correctly handles user verification based on credentials.docs/tutorials/snippets/passport/BasicProtocol.spec.ts (3)
11-39
: The test case for$onVerify
method is well-implemented, covering scenarios with valid user credentials.
39-65
: The test case correctly handles the scenario where the password does not match, ensuring$onVerify
returns false as expected.
66-89
: The test case effectively tests the scenario where no user is found, ensuring$onVerify
returns false as expected.
async $onVerify(@Req() req: Req, @Arg(0) jwtPayload: any) { | ||
const user = await this.authService.findOne({id: jwtPayload.sub}); | ||
|
||
return user ? user : false; |
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.
Consider specifying a more explicit type instead of any
for jwtPayload
to enhance type safety.
- async $onVerify(@Req() req: Req, @Arg(0) jwtPayload: any) {
+ async $onVerify(@Req() req: Req, @Arg(0) jwtPayload: JwtPayload) {
Committable suggestion was skipped due low confidence.
]); | ||
|
||
// WHEN | ||
const result = await protocol.$onVerify(request as any, {email, password}); |
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.
Replace any
with a more specific type to enhance type safety.
- const result = await protocol.$onVerify(request as any, {email, password});
+ const result = await protocol.$onVerify(request as Request, {email, password});
Committable suggestion was skipped due low confidence.
]); | ||
|
||
// WHEN | ||
const result = await protocol.$onVerify(request as any, {email, password}); |
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.
Replace any
with a more specific type to enhance type safety.
- const result = await protocol.$onVerify(request as any, {email, password});
+ const result = await protocol.$onVerify(request as Request, {email, password});
Committable suggestion was skipped due low confidence.
$onInstall(strategy: Strategy): void { | ||
// intercept the strategy instance to adding extra configuration | ||
} |
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.
Consider implementing the intended extra configuration for the strategy or clarify the purpose of this method with a comment if it's meant to be extended in the future.
$onInstall(strategy: Strategy): void { | ||
// intercept the strategy instance to adding extra configuration | ||
} |
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.
Consider implementing the intended extra configuration for the strategy or clarify the purpose of this method with a comment if it's meant to be extended in the future.
async $beforeInstall(settings: IStrategyOptions): Promise<IStrategyOptions> { | ||
// load something from backend | ||
// settings.usernameField = await this.usersService.loadFieldConfiguration() | ||
|
||
return settings; |
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.
Consider implementing the backend settings loading logic or clarify this method's purpose if the commented code is not intended for use.
$onInstall(strategy: Strategy): void { | ||
// intercept the strategy instance to adding extra configuration | ||
} |
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.
Consider implementing the intended extra configuration for the strategy or clarify the purpose of this method with a comment if it's meant to be extended in the future.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
||
Before using Passport, we need to install the [Passport.js](https://www.npmjs.com/package/passport) and the Passport-local. | ||
|
||
::: code-group |
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.
::: code-group
npm install --save passport
yarn add passport
pnpm add passport
bun add passport
:::
Fix: #48
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores