-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add microsoft login support #35
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
Conversation
WalkthroughThe recent updates introduce Azure Active Directory (AAD) authentication to the backend and frontend applications. The backend now includes AAD configuration, dependencies, and authentication guards. The frontend integrates MSAL for enhanced security and user management. Additionally, the Angular app sees improved error handling and token management for API interactions. Changes
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: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
apps/api/package-lock.json
is excluded by!**/package-lock.json
apps/portal/package-lock.json
is excluded by!**/package-lock.json
Files selected for processing (21)
- apps/api/.env.example (1 hunks)
- apps/api/package.json (2 hunks)
- apps/api/src/app.module.ts (2 hunks)
- apps/api/src/auth/azure-ad.guard.ts (1 hunks)
- apps/api/src/auth/azure-ad.strategy.ts (1 hunks)
- apps/api/src/config/azure.config.ts (1 hunks)
- apps/api/src/modules/summary/summary.resolver.ts (1 hunks)
- apps/portal/angular.json (1 hunks)
- apps/portal/package.json (1 hunks)
- apps/portal/src/app/app-routing.module.ts (2 hunks)
- apps/portal/src/app/app.component.html (1 hunks)
- apps/portal/src/app/app.component.ts (1 hunks)
- apps/portal/src/app/app.module.ts (2 hunks)
- apps/portal/src/app/features/file-processor/file-processor.component.ts (4 hunks)
- apps/portal/src/app/features/transcript-analyzer/transcript-analyzer.component.ts (5 hunks)
- apps/portal/src/app/graphql/graphql.service.ts (1 hunks)
- apps/portal/src/assets/i18n/en.json (1 hunks)
- apps/portal/src/common/interface.ts (1 hunks)
- apps/portal/src/environments/environment.interface.ts (1 hunks)
- apps/portal/src/environments/environment.prod.ts (1 hunks)
- apps/portal/src/environments/environment.ts (1 hunks)
Files skipped from review due to trivial changes (5)
- apps/api/.env.example
- apps/portal/angular.json
- apps/portal/src/common/interface.ts
- apps/portal/src/environments/environment.prod.ts
- apps/portal/src/environments/environment.ts
Additional comments not posted (28)
apps/portal/src/environments/environment.interface.ts (4)
2-2
: The addition ofapiScope
is appropriate for Azure AD configuration.
3-3
: The addition ofredirectUri
is correctly implemented for handling OAuth redirections.
4-4
: The addition oftenantId
is crucial for Azure AD integration and is correctly implemented.
5-5
: The addition ofclientId
is essential for Azure AD application identification and is correctly implemented.apps/portal/src/app/app.component.html (1)
1-5
: The conditional rendering of the<h1>
element using*ngIf="loginDisplay"
is correctly implemented and aligns with the login display requirements.apps/api/src/auth/azure-ad.guard.ts (1)
8-11
: The implementation of thegetRequest
method inAzureADGuard
correctly extracts the request object from the GraphQL execution context, ensuring proper functioning of the guard.apps/portal/src/app/app-routing.module.ts (1)
13-13
: The addition ofMsalGuard
to thetranscript
route is correctly implemented, ensuring that the route is protected and requires authenticated access.apps/portal/src/assets/i18n/en.json (1)
19-19
: The addition of the "UNAUTHORIZED" message in the localization file is clear and aligns with the security enhancements of the application.apps/api/src/config/azure.config.ts (1)
8-24
: TheAzureConfig
object is well-structured and correctly usesconfigService.getOrThrow
to ensure all necessary Azure AD settings are provided, enhancing the application's robustness and security.apps/api/src/modules/summary/summary.resolver.ts (1)
15-15
: The use ofAzureADGuard
in thecreateSummary
andfindAll
methods is correctly implemented, ensuring that these GraphQL endpoints are secured and accessible only to authenticated users.Also applies to: 23-23
apps/api/src/auth/azure-ad.strategy.ts (1)
8-27
: TheAzureADStrategy
class is correctly implemented with comprehensive configuration for Azure AD authentication. The placeholder in thevalidate
method is appropriate, allowing for customization based on specific user validation needs.apps/api/src/app.module.ts (5)
17-19
: ConfiguringConfigModule
as global is a good practice for making environment variables available throughout the application.
21-21
: Setting the default strategy to 'azure-ad' aligns with the PR's goal to integrate Azure AD authentication.
25-26
: UsingConfigService
to dynamically set Redis configuration is a robust approach, ensuring flexibility and scalability.
35-35
: Conditionally enabling the GraphQL playground in development is a best practice to avoid exposing it in production environments.
11-13
: Ensure that the importedAzureADStrategy
is properly implemented and tested since it's crucial for the new authentication mechanism.Verification successful
The
AzureADStrategy
class is properly implemented inapps/api/src/auth/azure-ad.strategy.ts
. It extendsPassportStrategy
and includes a constructor that configures the Azure AD strategy using settings fromAzureConfig
. Thevalidate
method is also defined, which is crucial for user validation.
apps/api/src/auth/azure-ad.strategy.ts
: Contains the implementation ofAzureADStrategy
.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of AzureADStrategy. # Test: Search for the implementation of AzureADStrategy. Expect: Proper implementation details. ast-grep --lang typescript --pattern $'class AzureADStrategy { $$$ }'Length of output: 72
Script:
#!/bin/bash # Locate the file containing the AzureADStrategy class fd 'AzureADStrategy.ts' -t f # If the file is found, display its content to verify the implementation if [ -f $(fd 'AzureADStrategy.ts' -t f) ]; then cat $(fd 'AzureADStrategy.ts' -t f) fiLength of output: 107
Script:
#!/bin/bash # Search for the AzureADStrategy class definition within the codebase ast-grep --lang typescript --pattern 'class AzureADStrategy { $$$ }'Length of output: 70
Script:
#!/bin/bash # Search for any file containing the term AzureADStrategy rg 'AzureADStrategy'Length of output: 301
Script:
#!/bin/bash # Display the content of the AzureADStrategy implementation file cat apps/api/src/auth/azure-ad.strategy.tsLength of output: 1280
apps/portal/package.json (1)
27-28
: Adding@azure/msal-angular
and@azure/msal-browser
is essential for integrating Azure AD authentication in the Angular application.apps/portal/src/app/graphql/graphql.service.ts (3)
15-15
: InjectingMsalService
to handle authentication in GraphQL services aligns with the PR's objectives to secure API access.
29-49
: Properly handling tokens and usingcatchError
to manage errors in GraphQL queries enhances security and robustness.
53-79
: The implementation of token handling and error management in GraphQL mutations is well-executed, ensuring secure and reliable data mutations.apps/portal/src/app/app.module.ts (3)
7-15
: Importing and configuring MSAL-related modules and services is crucial for integrating Azure AD authentication in the Angular application.
30-61
: The factory functions for MSAL configuration are well-implemented, providing a clean and maintainable way to configure authentication.
79-83
: Initializing the MSAL module in the NgModule with custom factory functions ensures that the authentication configuration is centralized and easily manageable.apps/portal/src/app/app.component.ts (1)
1-17
: Importing and using MSAL services and related classes in the main component is essential for handling authentication lifecycle events and token management.apps/api/package.json (1)
Line range hint
42-61
: Adding@nestjs/passport
,passport
, andpassport-azure-ad
is crucial for integrating Azure AD authentication in the NestJS application.apps/portal/src/app/features/transcript-analyzer/transcript-analyzer.component.ts (2)
19-19
: InjectingMsalService
to handle authentication in theTranscriptAnalyzerComponent
aligns with the PR's objectives to secure feature components.
60-91
: Properly handling unauthorized errors in thesummarizeTranscript
method enhances security by ensuring that sensitive operations are protected.apps/portal/src/app/features/file-processor/file-processor.component.ts (1)
32-32
: InjectingMsalService
to handle authentication in theFileProcessorComponent
aligns with the PR's objectives to secure feature components.
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
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/portal/angular.json (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/portal/angular.json
apps/portal/src/app/features/transcript-analyzer/transcript-analyzer.component.ts
Outdated
Show resolved
Hide resolved
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.
Minor change to be done.
Approved for now to be able to merge after the issue is fixed.
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
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/portal/src/app/features/transcript-analyzer/transcript-analyzer.component.ts (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/portal/src/app/features/transcript-analyzer/transcript-analyzer.component.ts
Description
Backend
Frontend
Summary by CodeRabbit
New Features
Updates
angular.json
for better resource management.Bug Fixes
Documentation
Refactor