Feature/35 upgrade to primeng v18 #37
Conversation
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 14015317 | Triggered | Generic Password | 6d1de1e | .run/IdentityProvider.run.xml | View secret |
| 14525404 | Triggered | Generic Password | 9bf6909 | sep490-idp/src/main/java/sep490/idp/SecurityConfig.java | View secret |
| 14525404 | Triggered | Generic Password | 9bf6909 | sep490-idp/src/main/resources/application.yml | View secret |
| 14525404 | Triggered | Generic Password | cd81a3e | sep490-idp/src/main/java/sep490/idp/SecurityConfig.java | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
WalkthroughThis pull request introduces several significant changes across multiple files. Key additions include the Changes
Possibly related PRs
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (15)
sep490-frontend/tailwind.config.js (1)
9-9: Consider specifying the plugin version explicitlyFor better maintainability and version control, consider specifying the plugin version in the config:
- plugins: [require('tailwindcss-primeui')] + plugins: [require('tailwindcss-primeui')({ version: '0.3.4' })]sep490-frontend/src/app/modules/shared/shared.module.ts (1)
Line range hint
44-49: LGTM! Well-structured module configurationThe NgModule configuration follows Angular best practices. Consider documenting the module's purpose and usage in a comment block above the class, as this is a shared module that other feature modules will depend on.
Add a documentation block like this:
/** * SharedModule provides common PrimeNG and Angular modules required across the application. * Import this module in feature modules that need these common functionalities. * * Note: This module is configured for PrimeNG v18 and includes: * - Common Angular modules (Forms, HTTP, Router) * - PrimeNG UI components * - Shared components, pipes, and services */.run/IdentityProvider.run.xml (2)
4-8: Consider using profiles for different environmentsThe database configuration is hardcoded for a local environment. Consider using Spring profiles to manage different environments.
19-24: Consider restricting coverage patternThe coverage pattern
sep490.idp.*is quite broad. Consider restricting it to specific packages or excluding test classes.sep490-frontend/src/styles/styles.css (1)
3-3: Well-structured layer ordering!The layer declaration follows best practices by:
- Placing PrimeNG between Tailwind base and utilities
- Allowing proper cascade and override capabilities
This structure will help maintain style precedence while allowing custom utilities to override PrimeNG styles when needed.
sep490-frontend/src/app/modules/dev/components/toolbox/toolbox.component.html (2)
22-75: Consider using the variant prop consistently across button types.While the code works, there's an inconsistency in how button variants are specified. Some use the new
variantprop while others use boolean props. Consider using thevariantprop consistently where applicable, as it's the preferred approach in PrimeNG v18.For example, consider updating raised buttons to:
-<p-button label="Primary" [raised]="true" /> +<p-button label="Primary" variant="raised" />
78-122: Simplify icon button styling using the variant prop.The icon buttons section uses multiple boolean props for styling, which could be simplified using the
variantprop. This would make the code more maintainable and align with PrimeNG v18's preferred styling approach.For example, consider updating:
-<p-button icon="pi pi-check" [rounded]="true" [text]="true" [raised]="true" /> +<p-button icon="pi pi-check" [rounded]="true" variant="raised-text" />sep490-frontend/src/app/modules/core/services/theme.service.ts (3)
7-57: Consider externalizing large configurations for maintainabilityThe
MyPresetconstant contains extensive theme configurations within the service file. To improve readability and maintainability, consider moving these configurations to a separate file or module.
85-86: Handle potential unsupported browsers formatchMediaWhile
window.matchMediais widely supported, consider adding a fallback for browsers that do not support it to prevent runtime errors.
103-107: Ensure consistent handling of ObservablesThe
isDarkMode()method returns different Observable types (ofandBehaviorSubject). Ensure that subscribers handle these consistently, or consider standardizing the return type.sep490-frontend/src/app/app.component.ts (2)
13-14: Consider initializing theme inngOnInitWhile initializing the theme in the constructor works, it's generally recommended to perform such initialization in the
ngOnInitlifecycle hook.
17-24: Handlewindowavailability for SSRUsing
window.matchMediadirectly may cause issues if Server-Side Rendering (SSR) is implemented, aswindowis not available on the server side. Consider adding checks or using Angular'sPLATFORM_IDfor compatibility.sep490-idp/src/main/java/sep490/idp/SecurityConfig.java (3)
Line range hint
45-58: Enhance CORS Configuration FlexibilityThe CORS configuration is well-structured but could be more flexible for different environments.
Consider:
- Making allowed origins configurable
- Adding commonly needed headers like 'Accept' and 'Origin'
- Extracting CORS settings to configuration
@Bean CorsConfigurationSource corsConfigurationSource() { CorsConfiguration corsConfig = new CorsConfiguration(); - corsConfig.setAllowedOrigins(List.of(CLIENT_URL)); + corsConfig.setAllowedOrigins(List.of(clientUrl)); corsConfig.setAllowedMethods(List.of("GET", "POST", "PUT", "DELETE", "OPTIONS")); - corsConfig.setAllowedHeaders(List.of("Authorization", "Content-Type")); + corsConfig.setAllowedHeaders(List.of("Authorization", "Content-Type", "Accept", "Origin")); corsConfig.setAllowCredentials(true); corsConfig.setMaxAge(3600L); UrlBasedCorsConfigurationSource source = new UrlBasedCorsConfigurationSource(); source.registerCorsConfiguration("/**", corsConfig); return source; }
61-90: LGTM with Security Enhancement SuggestionThe security filter chain configuration is well-structured for an OAuth2 authorization server.
Consider adding security headers to enhance protection:
public SecurityFilterChain defaultSecurityFilterChain(HttpSecurity http) throws Exception { http.cors(cors -> cors.configurationSource(corsConfigurationSource())); http.formLogin(Customizer.withDefaults()); + http.headers(headers -> headers + .frameOptions(HeadersConfigurer.FrameOptionsConfig::deny) + .xssProtection(HeadersConfigurer.XXssConfig::enable) + .contentSecurityPolicy(csp -> csp.policyDirectives("default-src 'self'"))); http.authorizeHttpRequests( c -> c.anyRequest().authenticated() ); return http.build(); }
Line range hint
118-147: Clean Up and Enhance JWT ConfigurationThe JWT token customizer needs improvement:
Hardcoded email domain
Large block of commented code
TODO comment without implementation
Externalize email domain configuration
Remove commented code
Implement proper user info service
public OAuth2TokenCustomizer<JwtEncodingContext> jwtCustomizer() { return context -> { - // TODO: implement OidcUserInfoService if (OidcParameterNames.ID_TOKEN.equals(context.getTokenType().getValue())) { var userInfo = OidcUserInfo.builder() .subject(UUID.randomUUID().toString()) - .email(USERNAME + "@elca.vn") + .email(USERNAME + "@" + emailDomain) .emailVerified(true) - // Remove all commented code .build() .getClaims(); context.getClaims().claims(claimsConsumer -> claimsConsumer.putAll(userInfo)); } }; }Would you like me to help implement the
OidcUserInfoService?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (38)
sep490-frontend/package-lock.jsonis excluded by!**/package-lock.jsonsep490-mobile/android/app/src/main/res/mipmap-hdpi/ic_launcher.pngis excluded by!**/*.pngsep490-mobile/android/app/src/main/res/mipmap-mdpi/ic_launcher.pngis excluded by!**/*.pngsep490-mobile/android/app/src/main/res/mipmap-xhdpi/ic_launcher.pngis excluded by!**/*.pngsep490-mobile/android/app/src/main/res/mipmap-xxhdpi/ic_launcher.pngis excluded by!**/*.pngsep490-mobile/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-1024x1024@1x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@1x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@2x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@3x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@1x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@2x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@3x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@1x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@2x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@3x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@2x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@3x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@1x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@2x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-83.5x83.5@2x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/LaunchImage.imageset/LaunchImage.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/LaunchImage.imageset/LaunchImage@2x.pngis excluded by!**/*.pngsep490-mobile/ios/Runner/Assets.xcassets/LaunchImage.imageset/LaunchImage@3x.pngis excluded by!**/*.pngsep490-mobile/macos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_1024.pngis excluded by!**/*.pngsep490-mobile/macos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_128.pngis excluded by!**/*.pngsep490-mobile/macos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_16.pngis excluded by!**/*.pngsep490-mobile/macos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_256.pngis excluded by!**/*.pngsep490-mobile/macos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_32.pngis excluded by!**/*.pngsep490-mobile/macos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_512.pngis excluded by!**/*.pngsep490-mobile/macos/Runner/Assets.xcassets/AppIcon.appiconset/app_icon_64.pngis excluded by!**/*.pngsep490-mobile/pubspec.lockis excluded by!**/*.locksep490-mobile/web/favicon.pngis excluded by!**/*.pngsep490-mobile/web/icons/Icon-192.pngis excluded by!**/*.pngsep490-mobile/web/icons/Icon-512.pngis excluded by!**/*.pngsep490-mobile/web/icons/Icon-maskable-192.pngis excluded by!**/*.pngsep490-mobile/web/icons/Icon-maskable-512.pngis excluded by!**/*.pngsep490-mobile/windows/runner/resources/app_icon.icois excluded by!**/*.ico
📒 Files selected for processing (25)
.run/IdentityProvider.run.xml(1 hunks).run/infrastructure.run.xml(1 hunks)sep490-common/local.properties(0 hunks)sep490-common/src/main/java/sep490/common/api/BaseDTO.java(1 hunks)sep490-frontend/angular.json(2 hunks)sep490-frontend/package.json(3 hunks)sep490-frontend/src/app/app.component.ts(3 hunks)sep490-frontend/src/app/app.module.ts(4 hunks)sep490-frontend/src/app/components/header/header.component.html(1 hunks)sep490-frontend/src/app/components/header/header.component.ts(2 hunks)sep490-frontend/src/app/modules/core/services/theme.service.ts(1 hunks)sep490-frontend/src/app/modules/dev/components/toolbox/toolbox.component.html(1 hunks)sep490-frontend/src/app/modules/shared/shared.module.ts(1 hunks)sep490-frontend/src/index.html(1 hunks)sep490-frontend/src/styles/styles.css(1 hunks)sep490-frontend/src/styles/theme-aura-dark-cyan.css(0 hunks)sep490-frontend/src/styles/theme-aura-light-cyan.css(0 hunks)sep490-frontend/tailwind.config.js(1 hunks)sep490-idp/.env.local.example(1 hunks)sep490-idp/local.properties(0 hunks)sep490-idp/src/main/java/sep490/idp/SecurityConfig.java(5 hunks)sep490-idp/src/main/java/sep490/idp/entity/AbstractAuditableEntity.java(1 hunks)sep490-idp/src/main/java/sep490/idp/entity/AbstractBaseEntity.java(2 hunks)sep490-idp/src/main/java/sep490/idp/entity/JpaUtils.java(1 hunks)sep490-idp/src/main/resources/application.yml(1 hunks)
💤 Files with no reviewable changes (4)
- sep490-frontend/src/styles/theme-aura-light-cyan.css
- sep490-idp/local.properties
- sep490-frontend/src/styles/theme-aura-dark-cyan.css
- sep490-common/local.properties
✅ Files skipped from review due to trivial changes (5)
- sep490-frontend/src/index.html
- sep490-idp/src/main/java/sep490/idp/entity/AbstractBaseEntity.java
- sep490-common/src/main/java/sep490/common/api/BaseDTO.java
- sep490-idp/src/main/java/sep490/idp/entity/JpaUtils.java
- sep490-idp/src/main/java/sep490/idp/entity/AbstractAuditableEntity.java
🔇 Additional comments (19)
sep490-frontend/tailwind.config.js (1)
9-9: Verify plugin compatibility with PrimeNG v18
The addition of tailwindcss-primeui plugin aligns with the PrimeNG upgrade objective.
Let's verify the plugin compatibility and installation:
✅ Verification successful
Plugin compatibility with PrimeNG v18 is confirmed
The tailwindcss-primeui plugin (v0.3.4) is properly installed and configured. The package is compatible with PrimeNG v18, as both dependencies are present and properly versioned in the project:
- PrimeNG: v18.0.0
- tailwindcss-primeui: v0.3.4
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the plugin is properly installed and get its version
cat package.json | jq '.dependencies["tailwindcss-primeui"] // .devDependencies["tailwindcss-primeui"]'
# Check if there are any compatibility notes in README
rg -i "primeng.*18|primeng.*compatibility" README.md
Length of output: 205
Script:
#!/bin/bash
# Let's try to find the package.json file first
fd package.json
# Then check for any PrimeNG or tailwind related files
fd -e json -e js -e ts -e md -i -g "*prime*|*tailwind*"
# Search for PrimeNG version references in any file
rg -i "primeng.*[0-9]+"
Length of output: 613
Script:
#!/bin/bash
# Let's check the actual package.json content for tailwindcss-primeui
cat sep490-frontend/package.json | jq '.dependencies["tailwindcss-primeui"] // .devDependencies["tailwindcss-primeui"]'
# Check if there are any tailwind-primeui related imports or configurations
rg -i "tailwindcss-primeui"
Length of output: 588
sep490-frontend/src/app/modules/shared/shared.module.ts (3)
Line range hint 36-42: LGTM! Common modules are well-organized
The common modules configuration follows Angular best practices and includes all necessary modules.
9-9: Verify DynamicDialog usage across the application
The change from DynamicDialogModule to DynamicDialog is correct for PrimeNG v18, but this is a breaking change that requires verification of all dynamic dialog implementations.
✅ Verification successful
Based on the search results, I can now generate a final response since I have sufficient information about the DynamicDialog usage in the codebase:
DynamicDialog migration appears to be properly implemented
The codebase shows no remaining instances of the old DynamicDialogModule, and the existing DynamicDialogRef usages in modal-provider.ts and confirm-dialog.component.ts are already using the correct imports from primeng/dynamicdialog. The implementation follows PrimeNG v18's breaking changes correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for potential DynamicDialogModule usages that need updating
# Search for old DynamicDialogModule usage
echo "Checking for old DynamicDialogModule usage that needs updating:"
rg -l "DynamicDialogModule" "sep490-frontend/src"
# Search for DynamicDialogRef usage patterns that might need updates
echo -e "\nChecking for DynamicDialogRef usage that might need review:"
rg "DynamicDialogRef" "sep490-frontend/src" -A 3
Length of output: 2429
Line range hint 22-34: Verify PrimeNG v18 module requirements
The PrimeNG modules configuration has been updated for v18, but please verify if any additional modules are required for the new version.
✅ Verification successful
Let me gather more information about the actual PrimeNG components being used in the templates.
PrimeNG modules configuration is complete for current usage
Based on the analysis of the templates and imports:
- All currently used PrimeNG components (Button, Toast, Dialog, InputSwitch, etc.) have their corresponding modules imported in
shared.module.ts - The core components like
DynamicDialogand services likeMessageServiceare properly configured - No missing PrimeNG modules were detected for the components currently in use
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for PrimeNG component usage to ensure all required modules are imported
# Search for PrimeNG component usage in templates
echo "Checking for PrimeNG component usage in templates:"
rg "p-[a-zA-Z]+" "sep490-frontend/src" --type html -l
# Search for PrimeNG imports that might be missing from SharedModule
echo -e "\nChecking for PrimeNG imports outside SharedModule:"
rg "from 'primeng/" "sep490-frontend/src" --glob '!**/shared.module.ts'
Length of output: 2029
Script:
#!/bin/bash
# Search for specific PrimeNG component usage in templates
echo "Checking for specific PrimeNG components in templates:"
rg "p-[a-zA-Z]+" "sep490-frontend/src" --type html -A 1
# Check the current shared.module.ts content
echo -e "\nChecking shared.module.ts content:"
cat "sep490-frontend/src/app/modules/shared/shared.module.ts"
Length of output: 18764
.run/infrastructure.run.xml (1)
25-25: Verify the docker-compose.yml path
The sourceFilePath is set to sep490-infrastructure/docker-compose.yml. Let's verify this path exists.
✅ Verification successful
Docker Compose file path is correctly configured
The path sep490-infrastructure/docker-compose.yml exists in the repository, confirming that the sourceFilePath configuration is valid.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the docker-compose file exists at the specified path
if [ -f "sep490-infrastructure/docker-compose.yml" ]; then
echo "docker-compose.yml found"
else
echo "docker-compose.yml not found at specified path"
fi
Length of output: 112
.run/IdentityProvider.run.xml (1)
18-18: Verify the Identity Provider main class
Let's verify the existence of the main class.
✅ Verification successful
Main class IdentityProvider exists and is properly configured
The main class exists at sep490-idp/src/main/java/sep490/idp/IdentityProvider.java and contains the required Spring Boot main method. The configuration in .run/IdentityProvider.run.xml correctly references this class.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the main class exists
ast-grep --pattern 'class IdentityProvider {
$$$
}'
Length of output: 55
Script:
#!/bin/bash
# Let's try to find the IdentityProvider class file first
fd IdentityProvider.java
# Then let's search for the main class pattern specifically
rg "public class IdentityProvider" -A 5
Length of output: 620
sep490-frontend/src/styles/styles.css (1)
1-1: Verify PrimeNG v18 icon import method
The PrimeIcons import looks correct, but let's ensure this is the recommended approach for PrimeNG v18.
✅ Verification successful
PrimeIcons import is correctly implemented
The import statement @import "primeicons/primeicons.css"; is the correct approach for PrimeNG v18. This is verified by:
- Project uses PrimeNG v18.0.0 with PrimeIcons v7.0.0
- The import is consistent with PrimeNG's latest version requirements
- No duplicate imports found in other style files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check package.json for PrimeNG version and related dependencies
cat package.json | jq '.dependencies | .["primeng"], .["primeicons"]'
# Look for any documentation or migration guides in the project
fd -t f -e md -e txt -H "UPGRADE|MIGRATION|README"
Length of output: 301
Script:
#!/bin/bash
# Check package.json in the frontend directory
cat sep490-frontend/package.json | jq '.dependencies | .["primeng"], .["primeicons"]'
# Search for any PrimeNG configuration or setup files
fd -t f "prime" -d 3 sep490-frontend
# Look for other files importing primeicons
rg -l "primeicons" sep490-frontend
Length of output: 279
sep490-frontend/src/app/modules/dev/components/toolbox/toolbox.component.html (3)
11-20: LGTM! Severity buttons section follows PrimeNG v18 conventions.
The severity naming and spacing are correctly implemented according to PrimeNG v18 standards.
125-129: LGTM! Badge implementation follows PrimeNG conventions.
The badge functionality is correctly implemented with appropriate use of styleClass for margin control.
Line range hint 1-129: Verify the necessity of all button variants.
The file includes a comprehensive set of button variants. Please confirm if all these variants are actually needed in the application or if this is meant to be a demo/documentation page.
sep490-idp/.env.local.example (1)
10-12: LGTM
Adding OIDC environment variables enhances configurability for authentication. Ensure that these variables are properly set in your deployment environments.
sep490-frontend/src/app/components/header/header.component.html (2)
1-6:
Incorrect template syntax: @if is not valid in Angular
The use of @if and @else is not standard in Angular templates. Angular uses *ngIf and *ngIf ... else for conditional rendering.
Apply this diff to correct the syntax:
-@if (authenticated | async) {
- <p-button label="Logout" [outlined]="true" (onClick)="logout()" />
-} @else {
- <p-button label="Sign in" [text]="true" (onClick)="login()" />
- <p-button label="Sign up" [outlined]="true" />
-}
+<ng-container *ngIf="authenticated | async; else notAuthenticated">
+ <p-button label="Logout" [outlined]="true" (onClick)="logout()" />
+</ng-container>
+<ng-template #notAuthenticated>
+ <p-button label="Sign in" [text]="true" (onClick)="login()" />
+ <p-button label="Sign up" [outlined]="true" />
+</ng-template>Likely invalid or redundant comment.
8-24:
Incorrect template syntax: @if is not valid in Angular
Similarly, update the dark mode toggle to use proper Angular directives.
Apply this diff to correct the syntax:
-@if (isDarkMode | async) {
- <p-button
- icon="pi pi-moon"
- [rounded]="true"
- [text]="true"
- (onClick)="toggleLightDark()"
- aria-label="Switch to light mode"
- />
-} @else {
- <p-button
- icon="pi pi-sun"
- [rounded]="true"
- [text]="true"
- (onClick)="toggleLightDark()"
- aria-label="Switch to dark mode"
- />
-}
+<ng-container *ngIf="isDarkMode | async; else lightMode">
+ <p-button
+ icon="pi pi-moon"
+ [rounded]="true"
+ [text]="true"
+ (onClick)="toggleLightDark()"
+ aria-label="Switch to light mode"
+ />
+</ng-container>
+<ng-template #lightMode>
+ <p-button
+ icon="pi pi-sun"
+ [rounded]="true"
+ [text]="true"
+ (onClick)="toggleLightDark()"
+ aria-label="Switch to dark mode"
+ />
+</ng-template>Likely invalid or redundant comment.
sep490-frontend/src/app/components/header/header.component.ts (1)
31-33: LGTM
Updating isDarkMode to return an Observable<boolean> aligns with Angular's reactive programming practices.
sep490-frontend/package.json (2)
41-41: Review TypeScript ESLint configuration
The ESLint configuration has been modified:
- Removed
@typescript-eslint/eslint-plugin - Added standalone
typescript-eslint
Please verify that the linting still works correctly with the new configuration.
Also applies to: 56-56, 57-57
22-22: Verify PrimeNG v18 migration steps
The upgrade to PrimeNG v18 includes new required dependencies. Please ensure you've followed all migration steps from the PrimeNG v18 migration guide.
Key changes to verify:
- Added
@primeng/themesfor the new theming system - Added
tailwindcss-primeuifor Tailwind integration - Updated core
primengto v18
Run this script to check for any missed PrimeNG imports or breaking changes:
Also applies to: 29-29, 31-31
✅ Verification successful
Let me verify the theme configuration since that's a major change in PrimeNG v18.
PrimeNG v18 migration appears correctly implemented
The codebase already follows the new PrimeNG v18 theming system:
- Uses
@primeng/themespackage - Properly configures theme through
PrimeNGconfig service - Has theme service implementation with dark mode support
- Correctly initializes theme at app startup
The migration requirements are satisfied:
- ✓ Added
@primeng/themes - ✓ Added
tailwindcss-primeui - ✓ Updated core
primengto v18 - ✓ Theme configuration is compatible with v18
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for PrimeNG imports that might need updates
rg "from 'primeng/" -t ts
Length of output: 2641
Script:
#!/bin/bash
# Check theme configuration in main files
rg -l "theme|Theme" --type ts src/
Length of output: 81
Script:
#!/bin/bash
# Check theme configuration in main files
rg -l "theme|Theme" --type ts sep490-frontend/src/
Length of output: 270
Script:
#!/bin/bash
# Check theme service implementation and usage
rg -A 10 "class ThemeService" sep490-frontend/src/app/modules/core/services/theme.service.ts
# Check app component theme initialization
rg -A 5 "theme" sep490-frontend/src/app/app.component.ts
Length of output: 1126
sep490-frontend/angular.json (1)
65-65: Verify PrimeNG styles configuration
The styles array has been simplified, removing direct PrimeNG CSS imports. This aligns with PrimeNG v18's new theming system using @primeng/themes. Ensure that:
- Theme is properly configured in your application
- Required PrimeNG styles are imported in
styles.css
✅ Verification successful
PrimeNG styles are correctly configured
The verification shows that:
- PrimeNG theme is properly configured using
@primeng/themespackage (v18.0.0) - The Material theme is imported from
@primeng/themes/materialin the theme service - Required PrimeNG styles are correctly imported in
styles.css:primeicons.cssfor icons- Proper layer configuration for PrimeNG and Tailwind integration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check styles.css for required PrimeNG imports
cat src/styles/styles.css
Length of output: 81
Script:
#!/bin/bash
# Find the correct styles file location
fd styles.css
# Check angular.json for styles configuration
cat angular.json
Length of output: 117
Script:
#!/bin/bash
# Check the content of styles.css file
cat sep490-frontend/src/styles/styles.css
# Check angular.json in the correct directory
cat sep490-frontend/angular.json
Length of output: 4583
Script:
#!/bin/bash
# Check package.json for PrimeNG theme dependencies
cat sep490-frontend/package.json
# Look for any PrimeNG theme imports in the codebase
rg "@primeng/themes" --type ts
Length of output: 2092
sep490-frontend/src/app/app.module.ts (2)
4-4: LGTM: Animation provider updated to async
The change from BrowserAnimationsModule to provideAnimationsAsync() is correct and follows Angular's new functional providers pattern.
Also applies to: 80-80
13-13: Verify PrimeNG configuration
The PrimeNG provider is correctly configured with ripple effect enabled. However, ensure that:
- Theme initialization is properly handled in your application
- Any PrimeNG services used in components are properly imported
Also applies to: 81-83
✅ Verification successful
Let me analyze the theme initialization by checking the theme service implementation.
PrimeNG configuration is properly implemented
The theme initialization and PrimeNG services are correctly configured:
- Custom theme preset is properly defined and initialized in
ThemeService - Theme initialization is handled in
AppComponentconstructor - System and user theme preferences are properly managed
- Required PrimeNG services are imported and provided in core/shared modules
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for theme service usage
rg "ThemeService" -t ts
# Check for PrimeNG service imports
rg "import.*from 'primeng/" -t ts
Length of output: 3457
Script:
#!/bin/bash
# Check theme service implementation
cat sep490-frontend/src/app/modules/core/services/theme.service.ts
# Check app component initialization
cat sep490-frontend/src/app/app.component.ts
Length of output: 4643
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (3)
sep490-idp/src/main/java/sep490/idp/SecurityConfig.java (2)
65-69: Improve code formatting consistencyThe indentation in the exception handling configuration is inconsistent with the rest of the code.
Suggested formatting:
- .defaultAuthenticationEntryPointFor( - new LoginUrlAuthenticationEntryPoint("/login"), - new MediaTypeRequestMatcher(MediaType.TEXT_HTML) - ) - ); + .defaultAuthenticationEntryPointFor( + new LoginUrlAuthenticationEntryPoint("/login"), + new MediaTypeRequestMatcher(MediaType.TEXT_HTML) + ) + );
116-116: Implement the TODO commentThe TODO comment indicates missing implementation for
OidcUserInfoService.Would you like me to help create a GitHub issue to track the implementation of the
OidcUserInfoService?sep490-frontend/src/app/app.component.ts (1)
17-24: Consider adding error handling for matchMediaWhile the implementation is correct, it's good practice to add a fallback for browsers that might not support
matchMedia.ngOnInit(): void { + try { // Listen for system theme changes this.systemThemeMediaQuery = window.matchMedia( this.themeService.SYSTEM_COLOR_SCHEME_QUERY ); this.systemThemeMediaQuery.addEventListener( 'change', this.handleThemeChange ); + } catch (error) { + console.warn('System theme detection not supported:', error); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
sep490-frontend/src/app/app.component.ts(3 hunks)sep490-frontend/src/app/modules/core/services/theme.service.ts(1 hunks)sep490-idp/src/main/java/sep490/idp/SecurityConfig.java(5 hunks)
🔇 Additional comments (5)
sep490-idp/src/main/java/sep490/idp/SecurityConfig.java (3)
Line range hint 40-52: Externalize CORS configuration
The CORS configuration contains hardcoded values that should be externalized to configuration properties for different environments.
Move the CORS configuration to application properties:
- corsConfig.setAllowedOrigins(List.of("http://localhost:4200"));
+ corsConfig.setAllowedOrigins(List.of(clientUrl));Add to application.yml:
client:
cors:
allowed-origins: ${CLIENT_URL:http://localhost:4200}
max-age: 360087-91:
Critical Security Issue: Replace NoOpPasswordEncoder
Using NoOpPasswordEncoder is a severe security risk as it stores passwords in plain text.
Replace with a secure password encoder:
@Bean
public PasswordEncoder passwordEncoder() {
- // return new BCryptPasswordEncoder();
- return NoOpPasswordEncoder.getInstance();
+ return new BCryptPasswordEncoder(12);
}60-61: Verify OIDC endpoint configuration
The default OIDC configuration is enabled, but we should verify that all required endpoints are properly configured.
✅ Verification successful
OIDC configuration is properly set up
The configuration is correctly implemented with:
- Default OIDC endpoints enabled through
OAuth2AuthorizationServerConfigurer - Proper client configuration in
application.ymlincluding:- Authorization grant type (authorization_code)
- Required scopes (openid, email, phone)
- Redirect URIs and post-logout redirect URIs
- Client authentication and PKCE settings
The use of Customizer.withDefaults() is appropriate here as Spring Security's OAuth2 Authorization Server provides a secure default configuration for all standard OIDC endpoints (authorization, token, userinfo, and jwks).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for OIDC endpoint configuration in application properties
# Search for OIDC-related configuration
rg -l "oidc|oauth2" --type yaml
# Search for endpoint configurations
rg "authorization-endpoint|token-endpoint|userinfo-endpoint|jwks-endpoint" --type yaml
Length of output: 168
Script:
#!/bin/bash
# Let's check the content of the application.yml file to see the OAuth2/OIDC configuration
cat sep490-idp/src/main/resources/application.yml
# Also, let's search for any OAuth2/OIDC related configurations in the Java code
ast-grep --pattern 'class SecurityConfig {
$$$
$_(HttpSecurity $http) {
$$$
}
}'
# Search for any custom endpoint configurations
rg -B2 -A2 "configure.*OAuth2" --type java
Length of output: 2492
sep490-frontend/src/app/app.component.ts (1)
12-14: LGTM: Theme initialization in constructor
Good practice to initialize the theme service in the constructor. This ensures the theme is set up before the component is fully initialized.
sep490-frontend/src/app/modules/core/services/theme.service.ts (1)
7-57: Validate semantic color values
The semantic color configuration looks comprehensive, but some values use template literals (e.g., {neutral.50}). Ensure these references are valid in PrimeNG v18's theme system.
✅ Verification successful
Template literals in semantic colors are valid PrimeNG syntax
The use of {neutral.50} template literals in the semantic color configuration is valid PrimeNG syntax for theme inheritance. This is a standard pattern in PrimeNG's Material theme system where surface colors can reference neutral palette values. The implementation correctly extends the Material preset using definePreset and follows PrimeNG's theming conventions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for PrimeNG theme documentation or similar implementations
rg -g '*.ts' -g '*.md' "definePreset.*semantic.*neutral"
Length of output: 58
Script:
#!/bin/bash
# Let's search for any PrimeNG theme-related imports and configurations
rg -g '*.ts' "from '@primeng/.*theme'" -A 5
# Also search for any neutral color definitions in theme files
rg -g '*.ts' "neutral: \{" -A 10
Length of output: 86
Script:
#!/bin/bash
# Let's check imports in the theme service file and its dependencies
fd "theme.service.ts" --exec cat {}
# Look for Material import which is used in definePreset
rg -g '*.ts' "import.*Material.*from"
Length of output: 3895
Summary by CodeRabbit
Release Notes
New Features
ProjectRunConfigurationManagerfor managing configurations for both Spring Boot and Docker deployments.application.yml.Bug Fixes
Documentation
.env.local.examplewith new environment variable declarations.Chores