feat: create building resource #124
Conversation
WalkthroughThe pull request introduces changes across backend, identity provider, database, and frontend layers. New constants, DTO and entity fields, mapping interfaces, service implementations, and REST endpoints are added to support building management operations. Additional modifications include updated dependency configurations, Docker and Maven changes for Keycloak test containers, SQL migration scripts enhancing the buildings table, and revised Angular module imports. Test configurations now leverage Testcontainers for integration tests, and a custom Keycloak protocol mapper has been introduced for token transformation. Changes
Sequence Diagram(s)Create Building FlowsequenceDiagram
participant C as Client
participant BC as BuildingController
participant ES as EnterpriseService
participant BM as BuildingMapper
participant BS as BuildingService
C->>BC: POST /buildings\nwith BuildingDTO
BC->>ES: getById(enterpriseId)
ES-->>BC: EnterpriseEntity
BC->>BM: Map BuildingDTO to BuildingEntity
BC->>BS: createBuilding(BuildingEntity)
BS-->>BC: Persisted BuildingEntity
BC->>C: 201 Created\nwith BuildingDTO
Search Enterprise Buildings FlowsequenceDiagram
participant C as Client
participant BC as BuildingController
participant BS as BuildingService
C->>BC: POST /buildings/search\nwith SearchCriteriaDTO
BC->>BS: getEnterpriseBuildings(enterpriseId, pageable)
BS-->>BC: Page\<BuildingEntity\>
BC->>C: 200 OK\nwith SearchResultDTO\<BuildingDTO\>
Poem
✨ Finishing Touches
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: 6
🧹 Nitpick comments (7)
sep490-enterprise/src/main/java/enterprise/services/EnterpriseService.java (1)
11-11: Add method documentation.Please add Javadoc to document the method's behavior, parameters, return value, and potential exceptions.
+ /** + * Retrieves an enterprise entity by its unique identifier. + * + * @param id the UUID of the enterprise to retrieve + * @return the enterprise entity + * @throws jakarta.persistence.EntityNotFoundException if no enterprise is found with the given id + */ EnterpriseEntity getById(UUID id);sep490-enterprise/src/main/java/enterprise/services/BuildingService.java (1)
9-14: Add Javadoc documentation for the service interface.Document the interface and its methods to improve code maintainability.
Add documentation:
+/** + * Service interface for managing building resources. + */ public interface BuildingService { + /** + * Creates a new building. + * + * @param building The building entity to create + * @throws IllegalArgumentException if building is null + */ void createBuilding(BuildingEntity building); + /** + * Retrieves a paginated list of buildings for a specific enterprise. + * + * @param enterpriseId The ID of the enterprise + * @param page The pagination information + * @return A page of building entities + * @throws IllegalArgumentException if enterpriseId is null + */ Page<BuildingEntity> getEnterpriseBuildings(UUID enterpriseId, Pageable page); }sep490-enterprise/src/main/java/enterprise/repositories/BuildingRepository.java (1)
10-14: Add Javadoc documentation for the repository interface.Document the interface and its methods to improve code maintainability.
Add documentation:
+/** + * Repository interface for managing building entities. + */ public interface BuildingRepository extends AbstractBaseRepository<BuildingEntity> { + /** + * Finds all buildings associated with a specific enterprise. + * + * @param enterpriseId The ID of the enterprise + * @param pageable The pagination information + * @return A page of building entities + */ Page<BuildingEntity> findByEnterpriseId(UUID enterpriseId, Pageable pageable); }sep490-enterprise/src/main/java/enterprise/mappers/BuildingMapper.java (1)
12-21: Add Javadoc documentation and fix formatting.
- Document the interface and its methods.
- Fix the formatting of the
partialUpdatemethod parameters.Apply these changes:
+/** + * Mapper interface for converting between BuildingDTO and BuildingEntity. + */ @Mapper(unmappedTargetPolicy = ReportingPolicy.IGNORE, componentModel = MappingConstants.ComponentModel.SPRING) public interface BuildingMapper { + /** + * Converts a BuildingDTO to a BuildingEntity. + * + * @param buildingDTO The DTO to convert + * @return The converted entity + */ BuildingEntity toEntity(BuildingDTO buildingDTO); + /** + * Converts a BuildingEntity to a BuildingDTO. + * + * @param buildingEntity The entity to convert + * @return The converted DTO + */ BuildingDTO toDto(BuildingEntity buildingEntity); + /** + * Updates an existing BuildingEntity with non-null values from a BuildingDTO. + * + * @param buildingDTO The DTO containing updates + * @param buildingEntity The entity to update + * @return The updated entity + */ @BeanMapping(nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.IGNORE) - BuildingEntity partialUpdate(BuildingDTO buildingDTO, @MappingTarget - BuildingEntity buildingEntity); + BuildingEntity partialUpdate(BuildingDTO buildingDTO, @MappingTarget BuildingEntity buildingEntity); }sep490-enterprise/src/main/java/enterprise/services/impl/BuildingServiceImpl.java (2)
21-24: Add validation and error handling for duplicate buildings.Consider adding validation to check for duplicate building names within the same enterprise before creation.
@Override public void createBuilding(BuildingEntity building) { + var existingBuilding = buildingRepository.findByEnterpriseIdAndName( + building.getEnterprise().getId(), + building.getName() + ); + if (existingBuilding.isPresent()) { + throw new DuplicateResourceException("Building with name " + building.getName() + " already exists"); + } buildingRepository.save(building); }
26-29: Add logging for better observability.Consider adding logging statements to track building creation and retrieval operations.
+private static final Logger log = LoggerFactory.getLogger(BuildingServiceImpl.class); @Override public Page<BuildingEntity> getEnterpriseBuildings(UUID enterpriseId, Pageable page) { + log.info("Retrieving buildings for enterprise: {}", enterpriseId); return buildingRepository.findByEnterpriseId(enterpriseId, page); }sep490-enterprise/src/main/java/enterprise/entities/BuildingEntity.java (1)
18-23: Consider adding database indexes.Consider adding an index on
enterprise_idand a unique index on the combination ofenterprise_idandnameto improve query performance and enforce uniqueness.@Entity @Table(name = "buildings") +@Table(name = "buildings", indexes = { + @Index(name = "idx_building_enterprise", columnList = "enterprise_id"), + @Index(name = "idx_building_enterprise_name", columnList = "enterprise_id,name", unique = true) +}) @Getter @Setter @NoArgsConstructor
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
sep490-commons/api/src/main/java/green_buildings/commons/api/SagaManager.java(1 hunks)sep490-enterprise/src/main/java/enterprise/dtos/BuildingDTO.java(1 hunks)sep490-enterprise/src/main/java/enterprise/entities/BuildingEntity.java(2 hunks)sep490-enterprise/src/main/java/enterprise/mappers/BuildingMapper.java(1 hunks)sep490-enterprise/src/main/java/enterprise/repositories/BuildingRepository.java(1 hunks)sep490-enterprise/src/main/java/enterprise/rest/BuildingController.java(2 hunks)sep490-enterprise/src/main/java/enterprise/rest/EnterpriseController.java(0 hunks)sep490-enterprise/src/main/java/enterprise/services/BuildingService.java(1 hunks)sep490-enterprise/src/main/java/enterprise/services/EnterpriseService.java(1 hunks)sep490-enterprise/src/main/java/enterprise/services/impl/BuildingServiceImpl.java(1 hunks)sep490-enterprise/src/main/java/enterprise/services/impl/EnterpriseServiceImpl.java(1 hunks)sep490-enterprise/src/main/resources/db/migration/V0.0.1.2__ManageBuildings.sql(1 hunks)sep490-idp/src/main/java/green_buildings/idp/service/impl/UserServiceImpl.java(1 hunks)
💤 Files with no reviewable changes (1)
- sep490-enterprise/src/main/java/enterprise/rest/EnterpriseController.java
✅ Files skipped from review due to trivial changes (1)
- sep490-commons/api/src/main/java/green_buildings/commons/api/SagaManager.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Java Modules (sep490-enterprise)
🔇 Additional comments (1)
sep490-idp/src/main/java/green_buildings/idp/service/impl/UserServiceImpl.java (1)
90-90: LGTM! Good improvement in maintainability.The change to use
TRANSACTION_TIMEOUTconstant fromSagaManagerinstead of a hardcoded value improves maintainability by centralizing the timeout configuration.Let's verify the constant's value and its usage across the codebase:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify the TRANSACTION_TIMEOUT constant and its usage. # Test 1: Find the constant definition echo "Finding TRANSACTION_TIMEOUT constant definition:" rg -p "TRANSACTION_TIMEOUT\s*=\s*\d+" --type java # Test 2: Find other usages of the constant echo -e "\nFinding other usages of TRANSACTION_TIMEOUT:" rg "TRANSACTION_TIMEOUT" --type javaLength of output: 803
LGTM! The change correctly replaces the hardcoded timeout with the centralized
TRANSACTION_TIMEOUTconstant.
- Constant Verification:
TRANSACTION_TIMEOUTis defined insep490-commons/api/src/main/java/green_buildings/commons/api/SagaManager.javawith a value of 10000.- Its utilization in
sep490-idp/src/main/java/green_buildings/idp/service/impl/UserServiceImpl.javais consistent with its intended usage for handling transaction timeouts.
sep490-enterprise/src/main/java/enterprise/services/impl/EnterpriseServiceImpl.java
Show resolved
Hide resolved
sep490-enterprise/src/main/java/enterprise/dtos/BuildingDTO.java
Outdated
Show resolved
Hide resolved
sep490-enterprise/src/main/java/enterprise/rest/BuildingController.java
Outdated
Show resolved
Hide resolved
sep490-enterprise/src/main/resources/db/migration/V0.0.1.2__ManageBuildings.sql
Outdated
Show resolved
Hide resolved
thongdanghoang
force-pushed
the
feature/77-userstory-enterprise
branch
2 times, most recently
from
add7c17 to
b239422
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 19
♻️ Duplicate comments (2)
sep490-enterprise/src/main/java/enterprise/rest/BuildingController.java (2)
49-57:⚠️ Potential issueAdd validation and improve error handling.
The createBuilding method needs several improvements:
- Add @Valid to validate request body
- Add error handling for null enterprise
- Add message for requireNonNull
- public ResponseEntity<BuildingDTO> createBuilding(@RequestBody BuildingDTO buildingDTO, + public ResponseEntity<BuildingDTO> createBuilding(@Valid @RequestBody BuildingDTO buildingDTO, @AuthenticationPrincipal UserContextData userContextData) { - var enterpriseIdFromContext = Objects.requireNonNull(userContextData.getEnterpriseId()); + var enterpriseIdFromContext = Objects.requireNonNull(userContextData.getEnterpriseId(), + "Enterprise ID is required"); - var enterprise = enterpriseService.getById(enterpriseIdFromContext); + var enterprise = enterpriseService.getById(enterpriseIdFromContext) + .orElseThrow(() -> new ResourceNotFoundException("Enterprise not found")); var building = buildingMapper.toEntity(buildingDTO); building.setEnterprise(enterprise); var createdBuilding = buildingService.createBuilding(building); return ResponseEntity.status(HttpStatus.CREATED).body(buildingMapper.toDto(createdBuilding)); }
49-57:⚠️ Potential issueAdd validation and error handling.
The create endpoint needs input validation and proper error handling.
- public ResponseEntity<BuildingDTO> createBuilding(@RequestBody BuildingDTO buildingDTO, + public ResponseEntity<BuildingDTO> createBuilding(@Valid @RequestBody BuildingDTO buildingDTO, @AuthenticationPrincipal UserContextData userContextData) { var enterpriseIdFromContext = Objects.requireNonNull(userContextData.getEnterpriseId()); - var enterprise = enterpriseService.getById(enterpriseIdFromContext); + var enterprise = enterpriseService.getById(enterpriseIdFromContext) + .orElseThrow(() -> new ResourceNotFoundException("Enterprise not found")); var building = buildingMapper.toEntity(buildingDTO); building.setEnterprise(enterprise); var createdBuilding = buildingService.createBuilding(building); return ResponseEntity.status(HttpStatus.CREATED).body(buildingMapper.toDto(createdBuilding)); }
🧹 Nitpick comments (7)
sep490-enterprise/src/test/resources/db/migration/V0.0.1.3__TestcontainerUsers.sql (1)
1-8: Consider using more obvious test data identifiers.While using real-world data like "FPT University" provides familiarity, it might cause confusion between test and production data. Consider:
- Prefixing test data with "TEST_" (e.g., "TEST_UNIVERSITY")
- Using obviously fake contact details (e.g., "test@example.com", "000-TEST-0000")
- Using different users for creation vs modification to better test audit functionality
sep490-infrastructure/keycloak-testcontainer-provider/pom.xml (1)
52-69: Specify the maven-assembly-plugin version.While the plugin configuration is correct, it's recommended to specify the plugin version for build reproducibility.
<plugin> + <version>3.6.0</version> <artifactId>maven-assembly-plugin</artifactId> <executions>sep490-infrastructure/keycloak-testcontainer-provider/src/main/java/keycloak/provider/Sep490IdentityProviderTokenMapper.java (2)
65-82: Improve display text and documentation.The display type and help text are currently using the same generic label. Consider:
- Providing more specific help text explaining the mapper's purpose
- Documenting why the priority is set to 1000
@Override public String getDisplayType() { - return getDisplayCategory(); + return "GreenBuildings Token Mapper"; } @Override public String getHelpText() { - return getDisplayCategory(); + return "Maps user roles and enterprise ID based on email prefix for GreenBuildings application"; }
1-94: Add unit tests for the token mapper.The token mapper contains critical authentication and authorization logic but lacks unit tests. Consider adding tests to verify:
- Token transformation for different email prefixes
- Handling of missing email claim
- Proper claim registration
- Configuration property handling
Would you like me to help generate unit tests for this class?
sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java (1)
49-67: Add error handling for token retrieval.The token retrieval implementation could be more robust:
- Consider extracting hard-coded values to constants or configuration
- Add error handling for failed requests
- Include specific error messages in the assertion
protected String getToken(String username, String password) { + private static final String CLIENT_ID = "testcontainer"; + private static final String GRANT_TYPE = "password"; var tokenUrl = StringSubstitutor.replace( "http://localhost:${mappedPort}/realms/greenbuildings/protocol/openid-connect/token", Map .of("mappedPort", getMappedPort(idP, 8180))); - var response = RestAssured - .given() - .contentType(ContentType.URLENC) - .formParam("client_id", "testcontainer") - .formParam("grant_type", "password") - .formParam("username", username) - .formParam("password", password) - .post(tokenUrl) - .then() - .statusCode(200) - .extract() - .as(Token.class); + try { + var response = RestAssured + .given() + .contentType(ContentType.URLENC) + .formParam("client_id", CLIENT_ID) + .formParam("grant_type", GRANT_TYPE) + .formParam("username", username) + .formParam("password", password) + .post(tokenUrl) + .then() + .statusCode(200) + .extract() + .as(Token.class); + return response.accessToken(); + } catch (Exception e) { + throw new RuntimeException("Failed to retrieve token: " + e.getMessage(), e); + } - return response.accessToken(); }sep490-enterprise/src/test/java/enterprise/rest/BuildingControllerTest.java (2)
26-48: Enhance test coverage for search endpoint.While the status code assertions are good, consider:
- Adding assertions for response body structure
- Testing with non-null search criteria
- Adding pagination tests
@Test void getEnterpriseBuildings_withValidToken_returns200() { RestAssured.given() .auth().oauth2(getToken("enterprise.employee@greenbuildings.com", "enterprise.employee")) .contentType(ContentType.JSON) - .body(new SearchCriteriaDTO<Void>(null, null, null)) + .body(new SearchCriteriaDTO<Void>(0, 10, null)) .when() .post("/buildings/search") .then() - .statusCode(200); + .statusCode(200) + .body("content", notNullValue()) + .body("totalElements", notNullValue()) + .body("totalPages", notNullValue()); }
50-89: Enhance test coverage for creation endpoint.While the current tests are good, consider adding:
- Assertions for the Location header in the 201 response
- Validation of the created resource via a GET request
- Tests for invalid input values (negative floors, etc.)
@Test void createBuilding_withValidToken_returns201() { var building = BuildingDTO.builder() .name("Building 1") .floors(1) .squareMeters(BigDecimal.valueOf(123.45)) .build(); + String location = RestAssured.given() .auth().oauth2(getToken("enterprise.owner@greenbuildings.com", "enterprise.owner")) .contentType(ContentType.JSON) .body(building) .when() .post("/buildings") .then() .statusCode(201) + .header("Location", matchesPattern("/buildings/[0-9a-f-]+")) + .extract() + .header("Location"); + // Verify the created resource + RestAssured.given() + .auth().oauth2(getToken("enterprise.owner@greenbuildings.com", "enterprise.owner")) + .when() + .get(location) + .then() + .statusCode(200) + .body("name", equalTo(building.name())) + .body("floors", equalTo(building.floors())) + .body("squareMeters", equalTo(building.squareMeters().toString())); } +@Test +void createBuilding_withInvalidValues_returns400() { + var building = BuildingDTO.builder() + .name("Building 1") + .floors(-1) // Invalid value + .squareMeters(BigDecimal.valueOf(-123.45)) // Invalid value + .build(); + RestAssured.given() + .auth().oauth2(getToken("enterprise.owner@greenbuildings.com", "enterprise.owner")) + .contentType(ContentType.JSON) + .body(building) + .when() + .post("/buildings") + .then() + .statusCode(400); +}
🛑 Comments failed to post (19)
sep490-infrastructure/keycloak-testcontainer-provider/pom.xml (1)
15-15: 💡 Verification agent
🧩 Analysis chain
Verify if a newer version of Keycloak is available.
Let's check if there's a newer version of Keycloak available that might include important security fixes or features.
🌐 Web query:
What is the latest version of Keycloak?💡 Result:
The latest version of Keycloak as of February 13, 2025 is 26.1.2, released on February 11, 2025[6][7]. This minor update follows the previous 26.1.1 release from February 5, 2025[1][4].
Key details about recent Keycloak versions:
Current supported releases:
- 26.1.x (active development branch)
- 26.0.x (ended support on January 15, 2025)[1]
Notable features in recent versions:
- Full OpenTelemetry tracing support[2][3]
- Persistent user sessions enabled by default[2]
- Improved high availability configurations for multi-site deployments[2]
- Enhanced admin account recovery mechanisms[2]
Only the latest major version (26.x) receives active development and security fixes[1]. Commercial long-term support options are available through Red Hat's offerings[1]. Users are advised to upgrade to v26.1.2 for the latest security patches and improvements[7].
Citations:
- 1: https://endoflife.date/keycloak
- 2: https://www.keycloak.org/docs/latest/release_notes/index.html
- 3: https://www.keycloak.org/2025/01/keycloak-2610-released
- 4: https://github.com/keycloak/keycloak/releases
- 5: https://www.keycloak.org/downloads
- 6: https://www.keycloak.org
- 7: https://www.keycloak.org/2025/02/keycloak-2612-released
Action: Update Keycloak Version
The Maven file currently sets the Keycloak version to 24.0.5, which is outdated. According to recent updates, the latest available version as of February 13, 2025 is 26.1.2. This release includes important security patches and new features (e.g., enhanced OpenTelemetry tracing, persistent user session improvements, and better high availability configurations).
- File:
sep490-infrastructure/keycloak-testcontainer-provider/pom.xmlat line 15- Current snippet:
<version.keycloak>24.0.5</version.keycloak>- Recommended update:
<version.keycloak>26.1.2</version.keycloak>Please update the Keycloak version accordingly.
sep490-infrastructure/keycloak-testcontainer-provider/src/main/java/keycloak/provider/Sep490IdentityProviderTokenMapper.java (3)
41-42:
⚠️ Potential issueAdd null check for email claim.
The code assumes the email claim exists. Add a null check to prevent
NullPointerException.- var email = token.getOtherClaims().get("email").toString(); + Object emailClaim = token.getOtherClaims().get("email"); + if (emailClaim == null) { + throw new IllegalStateException("Email claim is required"); + } + var email = emailClaim.toString();📝 Committable 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.Object emailClaim = token.getOtherClaims().get("email"); if (emailClaim == null) { throw new IllegalStateException("Email claim is required"); } var email = emailClaim.toString(); registerClaim(token, PERMISSIONS_CLAIM, Collections.emptyList());
47-55: 💡 Verification agent
⚠️ Potential issue🧩 Analysis chain
Security concern: Predictable enterprise ID.
Using a fixed enterprise ID for all enterprise users could lead to security issues. Consider:
- Using a unique ID per enterprise
- Fetching the ID from a secure configuration or database
- Using a more secure method for enterprise ID assignment
Let's verify if this enterprise ID is used elsewhere in the codebase:
🏁 Script executed:
#!/bin/bash # Search for hardcoded enterprise ID usage rg -l "664748fa-1312-4456-a88c-1ef187ec9510"Length of output: 252
Security Issue: Hardcoded Enterprise ID Usage Detected
We've confirmed that the value
"664748fa-1312-4456-a88c-1ef187ec9510"is hardcoded in two places:
- File:
sep490-infrastructure/keycloak-testcontainer-provider/src/main/java/keycloak/provider/Sep490IdentityProviderTokenMapper.java(Lines 47–55)- File:
sep490-enterprise/src/test/resources/db/migration/V0.0.1.3__TestcontainerUsers.sqlThis fixed enterprise ID is used for both "enterprise.owner" and "enterprise.employee" cases. Using a predictable, constant ID can pose security risks—especially if test configurations inadvertently leak into production environments.
Recommendations:
- Isolation: If this value is intended exclusively for testing, ensure that test configurations are segregated from production settings.
- Dynamic Assignment: For production usage, avoid hardcoding and instead:
- Generate or assign a unique enterprise ID for each tenant.
- Retrieve the ID from a secure configuration file or a trusted database.
- Implement a secure method for enterprise ID assignment that mitigates predictable patterns.
Please address these points to improve security and prevent potential misconfigurations.
24-28: 🛠️ Refactor suggestion
Consider adding configuration properties for enterprise ID.
The enterprise ID is currently hardcoded. Consider adding it as a configurable property to make the mapper more flexible and reusable.
Add this configuration property:
private final List<ProviderConfigProperty> configProperties = new ArrayList<>(); + +private static final String ENTERPRISE_ID_CONFIG = "enterprise.id"; + +public Sep490IdentityProviderTokenMapper() { + ProviderConfigProperty property = new ProviderConfigProperty(); + property.setName(ENTERPRISE_ID_CONFIG); + property.setLabel("Enterprise ID"); + property.setType(ProviderConfigProperty.STRING_TYPE); + property.setHelpText("Default enterprise ID for enterprise users"); + configProperties.add(property); +}📝 Committable 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.private static final String AUTHORITIES_CLAIM = "authorities"; private static final String ENTERPRISE_ID_CLAIM = "enterpriseId"; private static final String PERMISSIONS_CLAIM = "permissions"; private final List<ProviderConfigProperty> configProperties = new ArrayList<>(); private static final String ENTERPRISE_ID_CONFIG = "enterprise.id"; public Sep490IdentityProviderTokenMapper() { ProviderConfigProperty property = new ProviderConfigProperty(); property.setName(ENTERPRISE_ID_CONFIG); property.setLabel("Enterprise ID"); property.setType(ProviderConfigProperty.STRING_TYPE); property.setHelpText("Default enterprise ID for enterprise users"); configProperties.add(property); }sep490-enterprise/src/main/java/enterprise/services/impl/BuildingServiceImpl.java (3)
21-24:
⚠️ Potential issueAdd parameter validation and error handling.
The createBuilding method should validate the input parameter and ensure the enterprise exists.
@Override public BuildingEntity createBuilding(BuildingEntity building) { + Objects.requireNonNull(building, "Building cannot be null"); + Objects.requireNonNull(building.getEnterprise(), "Enterprise must be set"); + if (!enterpriseRepository.existsById(building.getEnterprise().getId())) { + throw new ResourceNotFoundException("Enterprise not found"); + } return buildingRepository.save(building); }📝 Committable 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.@Override public BuildingEntity createBuilding(BuildingEntity building) { Objects.requireNonNull(building, "Building cannot be null"); Objects.requireNonNull(building.getEnterprise(), "Enterprise must be set"); if (!enterpriseRepository.existsById(building.getEnterprise().getId())) { throw new ResourceNotFoundException("Enterprise not found"); } return buildingRepository.save(building); }
21-24: 🛠️ Refactor suggestion
Add parameter validation in createBuilding method.
The method should validate that the building parameter is not null and has a valid enterprise association before saving.
@Override public BuildingEntity createBuilding(BuildingEntity building) { + Objects.requireNonNull(building, "Building cannot be null"); + Objects.requireNonNull(building.getEnterprise(), "Building must be associated with an enterprise"); return buildingRepository.save(building); }📝 Committable 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.@Override public BuildingEntity createBuilding(BuildingEntity building) { Objects.requireNonNull(building, "Building cannot be null"); Objects.requireNonNull(building.getEnterprise(), "Building must be associated with an enterprise"); return buildingRepository.save(building); }
21-24: 🛠️ Refactor suggestion
Add null checks and error handling.
The
createBuildingmethod should validate input and handle potential errors.@Override public BuildingEntity createBuilding(BuildingEntity building) { + if (building == null) { + throw new IllegalArgumentException("Building cannot be null"); + } + if (building.getEnterprise() == null) { + throw new IllegalArgumentException("Enterprise must be set"); + } return buildingRepository.save(building); }📝 Committable 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.@Override public BuildingEntity createBuilding(BuildingEntity building) { if (building == null) { throw new IllegalArgumentException("Building cannot be null"); } if (building.getEnterprise() == null) { throw new IllegalArgumentException("Enterprise must be set"); } return buildingRepository.save(building); }sep490-enterprise/src/main/java/enterprise/rest/BuildingController.java (4)
38-45: 🛠️ Refactor suggestion
Add validation for search criteria.
The getEnterpriseBuildings method should validate the search criteria.
- public ResponseEntity<SearchResultDTO<BuildingDTO>> getEnterpriseBuildings(@RequestBody SearchCriteriaDTO<Void> searchCriteria, + public ResponseEntity<SearchResultDTO<BuildingDTO>> getEnterpriseBuildings(@Valid @RequestBody SearchCriteriaDTO<Void> searchCriteria, @AuthenticationPrincipal UserContextData userContextData) { - var enterpriseIdFromContext = Objects.requireNonNull(userContextData.getEnterpriseId()); + var enterpriseIdFromContext = Objects.requireNonNull(userContextData.getEnterpriseId(), + "Enterprise ID is required"); var pageable = CommonMapper.toPageable(searchCriteria.page(), searchCriteria.sort()); var searchResults = buildingService.getEnterpriseBuildings(enterpriseIdFromContext, pageable); var searchResultDTO = CommonMapper.toSearchResultDTO(searchResults, buildingMapper::toDto); return ResponseEntity.ok(searchResultDTO); }📝 Committable 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.public ResponseEntity<SearchResultDTO<BuildingDTO>> getEnterpriseBuildings(@Valid @RequestBody SearchCriteriaDTO<Void> searchCriteria, @AuthenticationPrincipal UserContextData userContextData) { var enterpriseIdFromContext = Objects.requireNonNull(userContextData.getEnterpriseId(), "Enterprise ID is required"); var pageable = CommonMapper.toPageable(searchCriteria.page(), searchCriteria.sort()); var searchResults = buildingService.getEnterpriseBuildings(enterpriseIdFromContext, pageable); var searchResultDTO = CommonMapper.toSearchResultDTO(searchResults, buildingMapper::toDto); return ResponseEntity.ok(searchResultDTO); }
38-45: 🛠️ Refactor suggestion
Add validation for search criteria.
The search endpoint should validate the input parameters.
- public ResponseEntity<SearchResultDTO<BuildingDTO>> getEnterpriseBuildings(@RequestBody SearchCriteriaDTO<Void> searchCriteria, + public ResponseEntity<SearchResultDTO<BuildingDTO>> getEnterpriseBuildings(@Valid @RequestBody SearchCriteriaDTO<Void> searchCriteria,📝 Committable 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.public ResponseEntity<SearchResultDTO<BuildingDTO>> getEnterpriseBuildings(@Valid @RequestBody SearchCriteriaDTO<Void> searchCriteria, @AuthenticationPrincipal UserContextData userContextData) { var enterpriseIdFromContext = Objects.requireNonNull(userContextData.getEnterpriseId()); var pageable = CommonMapper.toPageable(searchCriteria.page(), searchCriteria.sort()); var searchResults = buildingService.getEnterpriseBuildings(enterpriseIdFromContext, pageable); var searchResultDTO = CommonMapper.toSearchResultDTO(searchResults, buildingMapper::toDto); return ResponseEntity.ok(searchResultDTO); }
37-45: 🛠️ Refactor suggestion
Add @Valid annotation for request body validation.
The searchCriteria parameter should be validated using @Valid annotation.
- public ResponseEntity<SearchResultDTO<BuildingDTO>> getEnterpriseBuildings(@RequestBody SearchCriteriaDTO<Void> searchCriteria, + public ResponseEntity<SearchResultDTO<BuildingDTO>> getEnterpriseBuildings(@Valid @RequestBody SearchCriteriaDTO<Void> searchCriteria,📝 Committable 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.@PostMapping("/search") public ResponseEntity<SearchResultDTO<BuildingDTO>> getEnterpriseBuildings(@Valid @RequestBody SearchCriteriaDTO<Void> searchCriteria, @AuthenticationPrincipal UserContextData userContextData) { var enterpriseIdFromContext = Objects.requireNonNull(userContextData.getEnterpriseId()); var pageable = CommonMapper.toPageable(searchCriteria.page(), searchCriteria.sort()); var searchResults = buildingService.getEnterpriseBuildings(enterpriseIdFromContext, pageable); var searchResultDTO = CommonMapper.toSearchResultDTO(searchResults, buildingMapper::toDto); return ResponseEntity.ok(searchResultDTO); }
47-57:
⚠️ Potential issueAdd proper validation and error handling.
The method needs several improvements:
- Add @Valid for buildingDTO validation
- Handle potential null enterprise
- Validate that buildingDTO.id is null for creation
- public ResponseEntity<BuildingDTO> createBuilding(@RequestBody BuildingDTO buildingDTO, + public ResponseEntity<BuildingDTO> createBuilding(@Valid @RequestBody BuildingDTO buildingDTO, @AuthenticationPrincipal UserContextData userContextData) { + if (buildingDTO.id() != null) { + throw new IllegalArgumentException("Building ID must be null for creation"); + } var enterpriseIdFromContext = Objects.requireNonNull(userContextData.getEnterpriseId()); - var enterprise = enterpriseService.getById(enterpriseIdFromContext); + var enterprise = enterpriseService.getById(enterpriseIdFromContext) + .orElseThrow(() -> new ResourceNotFoundException("Enterprise not found")); var building = buildingMapper.toEntity(buildingDTO); building.setEnterprise(enterprise); var createdBuilding = buildingService.createBuilding(building); return ResponseEntity.status(HttpStatus.CREATED).body(buildingMapper.toDto(createdBuilding)); }📝 Committable 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.@PostMapping @RolesAllowed(UserRole.RoleNameConstant.ENTERPRISE_OWNER) - public ResponseEntity<BuildingDTO> createBuilding(@RequestBody BuildingDTO buildingDTO, + public ResponseEntity<BuildingDTO> createBuilding(@Valid @RequestBody BuildingDTO buildingDTO, @AuthenticationPrincipal UserContextData userContextData) { + if (buildingDTO.id() != null) { + throw new IllegalArgumentException("Building ID must be null for creation"); + } var enterpriseIdFromContext = Objects.requireNonNull(userContextData.getEnterpriseId()); - var enterprise = enterpriseService.getById(enterpriseIdFromContext); + var enterprise = enterpriseService.getById(enterpriseIdFromContext) + .orElseThrow(() -> new ResourceNotFoundException("Enterprise not found")); var building = buildingMapper.toEntity(buildingDTO); building.setEnterprise(enterprise); var createdBuilding = buildingService.createBuilding(building); return ResponseEntity.status(HttpStatus.CREATED).body(buildingMapper.toDto(createdBuilding)); }sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java (1)
19-32: 💡 Verification agent
🧩 Analysis chain
LGTM! Container setup looks good.
The container initialization and lifecycle management are well-implemented with proper version pinning.
Let's verify that these container versions are the latest stable releases:
🌐 Web query:
What is the latest stable version of PostgreSQL Alpine?💡 Result:
The latest stable version of PostgreSQL available on Alpine Linux as of February 2025 is PostgreSQL 16, specifically version 16.6 when used with Docker containers based on Alpine Linux[1][3].
Key Details:
Alpine Linux Official Packages:
- The Alpine Linux Wiki explicitly lists PostgreSQL 16 as the latest release[1].
- Installation instructions for
postgresql16are provided[1].Docker Integration:
- The
yobasystems/alpine-postgresDocker image uses PostgreSQL 16.6 on Alpine Linux v3.21 (latest as of January 2025)[3].This image supports multiple architectures (amd64/arm64)[3].docker run --name some-postgres -e POSTGRES_PASSWORD=RaNd0MpA55W0Rd -d yobasystems/alpine-postgres:16.6Version Support:
- While PostgreSQL itself has newer major versions (e.g., 17.x), these are not yet widely adopted in Alpine’s ecosystem[4].
- Older versions like 15.x and 14.x remain installable but are not recommended for new deployments[1][2].
For production use cases requiring stability and security updates, PostgreSQL 16 on Alpine Linux v3.21 is currently the best-supported combination[1][3][5].
Citations:
- 1: https://wiki.alpinelinux.org/wiki/Postgresql
- 2: https://azuremarketplace.microsoft.com/en/marketplace/apps/ntegralinc1586961136942.ntg_alpine_linux_3_16_postgres_14?tab=Overview
- 3: https://github.com/yobasystems/alpine-postgres
- 4: https://endoflife.date/postgresql
- 5: https://endoflife.date/alpine
- 6: https://hub.docker.com/_/postgres/tags
- 7: https://pkgs.alpinelinux.org/package/v3.13/main/x86/postgresql-client
- 8: https://hub.docker.com/_/postgres
- 9: https://hub.docker.com/_/alpine
🌐 Web query:
What is the latest stable version of Keycloak?💡 Result:
The latest stable version of Keycloak is 26.1.x, with 26.1.2 being the most recent minor release as of February 13, 2025[8][9]. This version includes security updates and new features like full OpenTelemetry tracing support[3][6], persistent user sessions by default[3], and improved logging configurations[3][6].
Key details about recent releases:
- Version support: Only the latest major version (currently 26.x) receives active development and security fixes[1].
- Red Hat builds: Commercial long-term support options are available through Red Hat build of Keycloak (RHBK), which tracks upstream releases with enterprise-grade support[2][4].
- Upgrade considerations: Recent changes include mandatory Java JDK 21 support[7], standalone JavaScript adapter deployment[3], and removal of deprecated methods/features like GELF logging[3].
For production environments:
# Recommended download command using official distribution curl -LO https://github.com/keycloak/keycloak/releases/download/26.1.2/keycloak-26.1.2.zipThe Keycloak team maintains a rapid release cycle (~4 major versions/year)[4], so regular updates are advised unless using Red Hat's commercially supported builds for extended maintenance[4].
Citations:
- 1: https://endoflife.date/keycloak
- 2: https://developers.redhat.com/articles/2023/11/15/whats-new-red-hat-build-keycloak-version-22
- 3: https://www.keycloak.org/docs/latest/release_notes/index.html
- 4: Release Long Term Support (LTS) version keycloak/keycloak#25688
- 5: https://www.keycloak.org/2025/01/keycloak-2610-released
- 6: https://github.com/keycloak/keycloak/releases
- 7: https://www.keycloak.org/2024/06/keycloak-2500-released
- 8: https://www.keycloak.org/downloads
- 9: https://www.keycloak.org
- 10: https://www.keycloak.org/2024/12/keycloak-2607-released
Container Setup Verified — Consider Upgrading Container Versions
- The container initialization and lifecycle management in
sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java(lines 19–32) is well implemented.- PostgreSQL Container: Currently using
postgres:16.4-alpine. However, as of February 2025 the latest stable PostgreSQL Alpine release is version 16.6.- Keycloak Container: Currently using
thongdh3401/keycloak:24.0.5. The latest stable Keycloak release is 26.1.2.- If there are no compatibility constraints, please consider updating these versions to leverage the latest improvements and security updates.
sep490-enterprise/src/test/java/enterprise/adapters/exchangerate_api/ExchangeRateApiCurrencyConverterTest.java (1)
3-3: 🛠️ Refactor suggestion
Remove unnecessary TestcontainersConfigs inheritance.
This test class doesn't use any features from TestcontainersConfigs (no database or Keycloak operations). Consider removing the inheritance to avoid starting unnecessary containers.
-import enterprise.TestcontainersConfigs; import green_buildings.commons.api.enums.Currency; @SpringBootTest -class ExchangeRateApiCurrencyConverterTest extends TestcontainersConfigs { +class ExchangeRateApiCurrencyConverterTest {Also applies to: 18-18
sep490-infrastructure/keycloak-testcontainer/Dockerfile (2)
3-5:
⚠️ Potential issue❓ Verification inconclusive
Security: Avoid running as root user and verify plugin path.
Two concerns:
- Running as root (USER 0) is a security risk. Consider using the default non-root Keycloak user.
- The plugin JAR file path needs to be verified.
Let's verify the plugin file exists:
🏁 Script executed:
#!/bin/bash # Description: Check if the plugin JAR exists fd --type f "greenbuildings-testcontainer-plugin.jar$"Length of output: 56
Attention: Security Risks and Plugin Presence Verification
- The Dockerfile continues to run as root (
USER 0), which introduces potential security vulnerabilities. Please update this to use Keycloak’s default non-root user.- The search for
greenbuildings-testcontainer-plugin.jardid not return any results. Kindly manually verify that the plugin JAR exists in the repository at the expected path or adjust the Dockerfile accordingly.
1-1: 💡 Verification agent
🧩 Analysis chain
LGTM! Using the latest stable version of Keycloak.
The Dockerfile uses Keycloak version 24.0.5, which is a recent stable version.
Let's verify this is indeed the latest stable version:
🌐 Web query:
What is the latest stable version of Keycloak?💡 Result:
The latest stable version of Keycloak as of February 13, 2025 is 26.1.2 according to its official website and downloads page[8][9]. This minor update follows 26.1.0 released on January 15, 2025[5], which introduced full OpenTelemetry tracing support and improvements to logging configuration[3][6].
Key details about recent versions:
Version Release Date Status 26.1.x Jan-Feb 2025 Latest stable branch 26.0.x Oct 2024-Jan 2025 Previous major version (now unsupported) 25.x Jun-Sep 2024 Older unsupported versions Notable features in recent updates include:
- Persistent user sessions enabled by default since v26[3]
- Standalone JavaScript adapter decoupled from server[3]
- Enhanced OpenTelemetry tracing with custom spans for database/LDAP operations[3][6]
- Support for ECDH-ES encryption algorithms[3]
For enterprise users needing long-term support, Red Hat provides commercial offerings through Red Hat build of Keycloak, which includes additional features like OpenShift operators[2][4]. The community edition follows a rapid release cycle with only the latest major version receiving updates[1].
Citations:
- 1: https://endoflife.date/keycloak
- 2: https://developers.redhat.com/articles/2023/11/15/whats-new-red-hat-build-keycloak-version-22
- 3: https://www.keycloak.org/docs/latest/release_notes/index.html
- 4: Release Long Term Support (LTS) version keycloak/keycloak#25688
- 5: https://www.keycloak.org/2025/01/keycloak-2610-released
- 6: https://github.com/keycloak/keycloak/releases
- 7: https://www.keycloak.org/2024/06/keycloak-2500-released
- 8: https://www.keycloak.org/downloads
- 9: https://www.keycloak.org
- 10: https://www.keycloak.org/2024/12/keycloak-2607-released
Action Required: Update Keycloak Version in Dockerfile
The Dockerfile currently specifies:
FROM quay.io/keycloak/keycloak:24.0.5However, according to the official Keycloak website as of February 2025, the latest stable version is 26.1.2. Please update the base image accordingly to ensure you're using the most recent stable release.
- File:
sep490-infrastructure/keycloak-testcontainer/Dockerfile- Line: 1
- Suggested Change: Update the version from
24.0.5to26.1.2sep490-enterprise/build.gradle (1)
34-35: 💡 Verification agent
🛠️ Refactor suggestion
🧩 Analysis chain
Pin test dependency versions for reproducible builds.
The versions for
testcontainers:postgresqlandrest-assuredare not pinned. This could lead to inconsistent behavior across different environments.Let's check the latest stable versions:
Apply this diff to pin the versions:
- testImplementation 'org.testcontainers:postgresql' - testImplementation 'io.rest-assured:rest-assured' + testImplementation 'org.testcontainers:postgresql:<version>' + testImplementation 'io.rest-assured:rest-assured:<version>'
🌐 Web query:
What are the latest stable versions of Testcontainers and Rest Assured?💡 Result:
The latest stable versions as of February 2025 are:
Testcontainers (Java):
Latest stable version: 1.20.4 (released November 20, 2024)[4].REST Assured (Java):
Latest stable version: 5.5.0 (released July 5, 2024)[8].Key Details:
- Testcontainers follows semantic versioning with frequent updates for Docker compatibility and new features like network reusability and improved Windows support[1][3]. The Maven repository lists active releases through late 2024[4].
- REST Assured focuses on API testing enhancements, including CSRF support and Spring Framework compatibility[2][8]. Recent updates prioritize bug fixes and dependency management[2][8].
Both libraries maintain backward compatibility where possible but recommend checking changelogs for breaking changes during major version upgrades[1][2][8].
Citations:
- 1: https://github.com/testcontainers/testcontainers-java/blob/main/CHANGELOG.md
- 2: https://github.com/rest-assured/rest-assured/wiki/OldNews
- 3: https://www.atomicjar.com/2022/04/new-milestone-new-look-new-release-same-testcontainers/
- 4: https://mvnrepository.com/artifact/org.testcontainers/testcontainers
- 5: https://www.ontestautomation.com/rest-assured-net-in-2024-a-review/
- 6: https://java.testcontainers.org/modules/webdriver_containers/
- 7: https://softwaremind.com/blog/introduction-to-testcontainers-for-java-developers/
- 8: https://rest-assured.io
- 9: https://blog.nashtechglobal.com/api-testing-with-testcontainers-and-rest-assured/
- 10: https://graphaware.com/blog/proofread-testcontainers/
Action: Pin test dependency versions for consistent builds
- In
sep490-enterprise/build.gradle(lines 34-35), update the test dependencies to use explicit version numbers. This ensures reproducible builds across environments.- Based on the latest stable releases (Testcontainers: 1.20.4, REST Assured: 5.5.0), please update the dependencies as follows:
- testImplementation 'org.testcontainers:postgresql' - testImplementation 'io.rest-assured:rest-assured' + testImplementation 'org.testcontainers:postgresql:1.20.4' + testImplementation 'io.rest-assured:rest-assured:5.5.0'📝 Committable 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.testImplementation 'org.testcontainers:postgresql:1.20.4' testImplementation 'io.rest-assured:rest-assured:5.5.0'sep490-infrastructure/keycloak-testcontainer/realm-export.json (3)
413-418:
⚠️ Potential issueSecurity: Restrict client redirect URIs.
The testcontainer client has overly permissive redirect URIs and web origins (
/*). This could enable phishing attacks through malicious redirects.Restrict the URIs to specific paths:
"redirectUris": [ - "/*" + "/auth/*", + "/api/*" ], "webOrigins": [ - "/*" + "http://localhost:*" ],📝 Committable 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."redirectUris": [ "/auth/*", "/api/*" ], "webOrigins": [ "http://localhost:*" ],
38-46:
⚠️ Potential issueEnable brute force protection.
Brute force protection is disabled (
bruteForceProtected: false). This makes the system vulnerable to password guessing attacks.Apply this diff to enable brute force protection with reasonable defaults:
- "bruteForceProtected": false, - "permanentLockout": false, - "maxTemporaryLockouts": 0, - "maxFailureWaitSeconds": 900, - "minimumQuickLoginWaitSeconds": 60, - "waitIncrementSeconds": 60, - "quickLoginCheckMilliSeconds": 1000, - "maxDeltaTimeSeconds": 43200, - "failureFactor": 30, + "bruteForceProtected": true, + "permanentLockout": false, + "maxTemporaryLockouts": 3, + "maxFailureWaitSeconds": 900, + "minimumQuickLoginWaitSeconds": 60, + "waitIncrementSeconds": 60, + "quickLoginCheckMilliSeconds": 1000, + "maxDeltaTimeSeconds": 43200, + "failureFactor": 3,📝 Committable 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."bruteForceProtected": true, "permanentLockout": false, "maxTemporaryLockouts": 3, "maxFailureWaitSeconds": 900, "minimumQuickLoginWaitSeconds": 60, "waitIncrementSeconds": 60, "quickLoginCheckMilliSeconds": 1000, "maxDeltaTimeSeconds": 43200, "failureFactor": 3,
8-24: 💡 Verification agent
🧩 Analysis chain
Review token lifespans and timeouts for security.
The current token and session timeouts need review:
- Access token lifespan (5 min) is reasonable
- SSO session idle timeout (30 min) is reasonable
- SSO session max lifespan (10 hours) might be too long for a secure environment
Let's check the recommended values:
🌐 Web query:
What are the recommended token lifespans and session timeouts for Keycloak in a secure environment?💡 Result:
For secure Keycloak environments, recommended configurations balance security with usability through controlled token lifespans and session timeouts:
Core Token Lifespans
Token Type Recommended Duration Key Security Rationale Access Token 5-15 minutes[1][6][7][9] Minimizes exposure if compromised Implicit Flow Token ≤30 minutes[1][8] No refresh mechanism in implicit flow Refresh Token 30 minutes - 4 hours[1][7] Single-use with "Max Reuse"=0[1][8] Offline Token ≤60 days idle[6][8] Requires explicit storage management[4] Session Management
Critical Settings:
- SSO Session Idle: 15-30 minutes[1][4][8]
- SSO Session Max: ≤8 hours[4][8]
- Client Session Idle: ≤50% of SSO idle time[5][8]
- Offline Session Max: ≤60 days[6][8]
Best Practice Configuration Steps
Access Control
SetAccess Token Lifespanto 15m andImplicit Flowvariant to 30m in Realm Settings → Tokens[1][8]Refresh Security
EnableRevoke Refresh Tokenand setRefresh Token Max Reuse=0[1][8]Session Hardening
ConfigureSSO Session Idle=30mandSSO Session Max=8hunder Realm Settings → Sessions[5][8]Offline Access
For long-lived processes:curl -X POST \ -H "Authorization: Bearer <offline_token>" \ https://keycloak/auth/realms/{realm}/protocol/openid-connect/tokenWith
offline_session_idle=30dand strict client-side storage[4][6]Security vs Usability Balance
- High-security systems: Use lower bounds (5m access tokens, 15m sessions)[6][7]
- User-centric apps: Upper bounds (15m tokens, 8h sessions) with active refresh flows[9]
- Critical systems: Implement token revocation webhooks + session monitoring[5][7]
These settings should be combined with HTTPS enforcement, regular key rotations, and client-side token validation for defense-in-depth security[2][5]. Always test configurations against your specific authentication patterns before deployment.
Citations:
- 1: https://blog.elest.io/configure-keycloak-with-saml/
- 2: https://skycloak.io/blog/8-default-configurations-to-adjust-right-away-on-your-keycloak-cluster/
- 3: https://docs.ezmeral.hpe.com/unified-analytics/15/Security/keycloak-change-auth-token-settings.html
- 4: https://www.janua.fr/understanding-token-usage-in-keycloak/
- 5: https://docs.redhat.com/en/documentation/red_hat_build_of_keycloak/22.0/html/server_administration_guide/managing_user_sessions
- 6: https://support.domino.ai/support/s/article/Keycloak-Recommended-settings-for-session-and-token-timeouts-1718868036368
- 7: https://blog.elest.io/keycloak-token-management-expiration-revocation-and-renewal/
- 8: https://wjw465150.gitbooks.io/keycloak-documentation/content/server_admin/topics/sessions/timeouts.html
- 9: https://coguard.io/post/session-tokens-balancing-usability-security
- 10: https://docs.cloudblue.com/cbc/21.0/Identity-Provider/Changing-ID-Token-Expiration-Period.htm
Action: Adjust SSO Session Maximum Lifespan for Stronger Security
- Access Token Lifespan (300 sec/5 min): Within a secure, recommended range.
- SSO Session Idle Timeout (1800 sec/30 min): Consistent with secure configuration best practices.
- SSO Session Max Lifespan (36000 sec/10 hours): Exceeds the secure recommendation. For enhanced security, consider reducing this value to around 28800 sec (8 hours) per industry guidelines.
- Other values (implicit flow, offline tokens, client sessions) appear aligned with secure settings.
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
sep490-enterprise/src/test/resources/db/migration/V0.0.1.4__TestcontainerUsers.sql (1)
1-9: 🛠️ Refactor suggestionAdd test data for buildings table.
Since new columns were added to the buildings table, corresponding test data should be added to validate the new fields.
Add test building data:
INSERT INTO buildings (created_date, created_by, last_modified_date, last_modified_by, id, version, enterprise_id, name, floors, square_meters) VALUES (CURRENT_TIMESTAMP, 'enterprise.owner', CURRENT_TIMESTAMP, 'enterprise.owner', 'b7f3fc5d-d0c9-4f8f-b6c9-dea8f58f5a4e', 0, '664748fa-1312-4456-a88c-1ef187ec9510', 'Alpha Building', 5, 1000.000);
🧹 Nitpick comments (5)
sep490-frontend/src/app/modules/enterprise/enterprise.module.ts (1)
9-9: Remove unused PaymentService import.The PaymentService import appears to be unused since it's already included in the providers array.
-import {PaymentService} from './services/payment.service';sep490-infrastructure/keycloak-testcontainer-provider/pom.xml (2)
7-9: Consider aligning artifactId with final JAR name for consistency.The artifactId
idp-testcontainer-providerdiffers from the final JAR namegreenbuildings-testcontainer-plugin. Consider using consistent naming across both to maintain clarity.- <artifactId>idp-testcontainer-provider</artifactId> + <artifactId>greenbuildings-testcontainer-plugin</artifactId>Also applies to: 66-66
52-69: Specify maven-assembly-plugin version for build reproducibility.While the plugin configuration is correct, it's recommended to specify the plugin version to ensure build reproducibility across different environments.
<plugin> <artifactId>maven-assembly-plugin</artifactId> + <version>3.6.0</version> <executions>sep490-enterprise/src/test/resources/db/migration/V0.0.1.4__TestcontainerUsers.sql (2)
1-4: Use current timestamp for test data.The created_date and last_modified_date values are set to 2025, which is in the future. Use CURRENT_TIMESTAMP instead.
Apply this diff:
INSERT INTO enterprises (created_date, created_by, last_modified_date, last_modified_by, id, version, hotline, name, email) -VALUES ('2025-02-12 23:11:26.875648', 'enterprise.owner', '2025-02-12 23:11:26.875648', 'enterprise.owner', +VALUES (CURRENT_TIMESTAMP, 'enterprise.owner', CURRENT_TIMESTAMP, 'enterprise.owner', '664748fa-1312-4456-a88c-1ef187ec9510', 0, '0123456789', 'FPT University', 'fptu.hcm@fpt.edu.vn');
6-8: Use current timestamp for wallet test data.The timestamps in wallet insert are also set to 2025.
Apply this diff:
INSERT INTO wallets (created_date, created_by, last_modified_date, last_modified_by, id, version, enterprise_id, balance) -VALUES ('2025-02-12 23:11:26.883390', 'enterprise.owner', '2025-02-12 23:11:26.883390', 'enterprise.owner', +VALUES (CURRENT_TIMESTAMP, 'enterprise.owner', CURRENT_TIMESTAMP, 'enterprise.owner',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
sep490-commons/api/src/main/java/green_buildings/commons/api/SagaManager.java(1 hunks)sep490-commons/springfw-impl/src/main/java/commons/springfw/impl/securities/JwtAuthenticationConverter.java(2 hunks)sep490-enterprise/build.gradle(1 hunks)sep490-enterprise/src/main/java/enterprise/dtos/BuildingDTO.java(1 hunks)sep490-enterprise/src/main/java/enterprise/entities/BuildingEntity.java(2 hunks)sep490-enterprise/src/main/java/enterprise/mappers/BuildingMapper.java(1 hunks)sep490-enterprise/src/main/java/enterprise/repositories/BuildingRepository.java(1 hunks)sep490-enterprise/src/main/java/enterprise/rest/BuildingController.java(2 hunks)sep490-enterprise/src/main/java/enterprise/rest/EnterpriseController.java(0 hunks)sep490-enterprise/src/main/java/enterprise/services/BuildingService.java(1 hunks)sep490-enterprise/src/main/java/enterprise/services/EnterpriseService.java(1 hunks)sep490-enterprise/src/main/java/enterprise/services/impl/BuildingServiceImpl.java(1 hunks)sep490-enterprise/src/main/java/enterprise/services/impl/EnterpriseServiceImpl.java(1 hunks)sep490-enterprise/src/main/resources/db/migration/V0.0.1.3__ManageBuildings.sql(1 hunks)sep490-enterprise/src/test/java/enterprise/EnterpriseApplication.java(0 hunks)sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java(1 hunks)sep490-enterprise/src/test/java/enterprise/adapters/exchangerate_api/ExchangeRateApiCurrencyConverterTest.java(1 hunks)sep490-enterprise/src/test/java/enterprise/rest/BuildingControllerTest.java(1 hunks)sep490-enterprise/src/test/resources/application.yml(0 hunks)sep490-enterprise/src/test/resources/db/migration/V0.0.1.4__TestcontainerUsers.sql(1 hunks)sep490-frontend/src/app/modules/enterprise/enterprise.module.ts(2 hunks)sep490-frontend/src/app/modules/enterprise/models/payment.ts(1 hunks)sep490-frontend/src/app/modules/shared/shared.module.ts(4 hunks)sep490-idp/src/main/java/green_buildings/idp/service/impl/UserServiceImpl.java(1 hunks)sep490-infrastructure/keycloak-testcontainer-provider/pom.xml(1 hunks)sep490-infrastructure/keycloak-testcontainer-provider/src/main/java/keycloak/provider/Sep490IdentityProviderTokenMapper.java(1 hunks)sep490-infrastructure/keycloak-testcontainer/Dockerfile(1 hunks)sep490-infrastructure/keycloak-testcontainer/realm-export.json(1 hunks)
💤 Files with no reviewable changes (3)
- sep490-enterprise/src/test/resources/application.yml
- sep490-enterprise/src/test/java/enterprise/EnterpriseApplication.java
- sep490-enterprise/src/main/java/enterprise/rest/EnterpriseController.java
🚧 Files skipped from review as they are similar to previous changes (16)
- sep490-enterprise/src/main/java/enterprise/services/impl/EnterpriseServiceImpl.java
- sep490-enterprise/src/main/java/enterprise/services/EnterpriseService.java
- sep490-infrastructure/keycloak-testcontainer/Dockerfile
- sep490-commons/api/src/main/java/green_buildings/commons/api/SagaManager.java
- sep490-idp/src/main/java/green_buildings/idp/service/impl/UserServiceImpl.java
- sep490-commons/springfw-impl/src/main/java/commons/springfw/impl/securities/JwtAuthenticationConverter.java
- sep490-enterprise/src/main/java/enterprise/repositories/BuildingRepository.java
- sep490-enterprise/src/test/java/enterprise/adapters/exchangerate_api/ExchangeRateApiCurrencyConverterTest.java
- sep490-enterprise/src/main/java/enterprise/services/BuildingService.java
- sep490-enterprise/build.gradle
- sep490-enterprise/src/test/java/enterprise/rest/BuildingControllerTest.java
- sep490-enterprise/src/main/java/enterprise/mappers/BuildingMapper.java
- sep490-enterprise/src/main/java/enterprise/entities/BuildingEntity.java
- sep490-enterprise/src/main/java/enterprise/services/impl/BuildingServiceImpl.java
- sep490-enterprise/src/main/java/enterprise/dtos/BuildingDTO.java
- sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java
🔇 Additional comments (11)
sep490-frontend/src/app/modules/enterprise/enterprise.module.ts (1)
20-20: LGTM! Good module organization.The simplified imports array follows Angular best practices by:
- Using SharedModule for common PrimeNG components
- Avoiding individual component imports
sep490-frontend/src/app/modules/shared/shared.module.ts (3)
7-7: LGTM! Good module organization.The addition of PrimeNG module imports (AccordionModule, CardModule, TagModule) follows Angular best practices by:
- Using module-level imports instead of individual component imports
- Centralizing UI component modules in SharedModule
Also applies to: 12-12, 31-31
45-45: LGTM! Well-organized module imports.The primeNgModules array is properly updated with the new modules while maintaining alphabetical order.
Also applies to: 50-50, 70-71
34-34:❓ Verification inconclusive
Verify component declarations.
Please ensure that the newly added components (CardTemplateComponent, FormFieldErrorComponent, PaymentStatusComponent) are properly tested and documented.
Also applies to: 36-36, 37-37
🏁 Script executed:
#!/bin/bash # Description: Check for component tests and documentation # Check for component test files echo "Checking for component test files..." fd -e spec.ts "card-template|form-field-error|payment-status" src/app/modules/shared/components # Check for component documentation echo "Checking for component documentation..." fd -e md "card-template|form-field-error|payment-status" src/app/modules/shared/componentsLength of output: 604
Manual Verification Required: Review and Confirm Component Tests & Documentation
The automated search for test and documentation files in
src/app/modules/shared/componentsdid not yield results because the expected directory was not found. Please manually verify that the following components have corresponding tests and documentation in the correct locations:
- CardTemplateComponent
- FormFieldErrorComponent
- PaymentStatusComponent
If these files are located in a different directory or have been renamed, update the tests and documentation references accordingly.
sep490-frontend/src/app/modules/enterprise/models/payment.ts (1)
6-6:❓ Verification inconclusive
LGTM! Type change follows TypeScript best practices.
The change from
PaymentStatustokeyof typeof PaymentStatusimproves type safety by ensuring that only valid enum keys can be assigned to the status property.Let's verify the impact on existing code:
🏁 Script executed:
#!/bin/bash # Description: Find all usages of PaymentDTO to ensure they're compatible with the type change # Search for files using PaymentDTO echo "Files using PaymentDTO:" rg -l "PaymentDTO" # Search for direct assignments to status property echo "\nAssignments to status property:" rg "status.*=.*PaymentStatus\." -A 2Length of output: 611
PaymentDTO Interface - Type Change Approved, Pending Manual Verification
The update to use
keyof typeof PaymentStatusfollows TypeScript best practices and improves type safety by limiting the property to valid enum keys. The initial usage search confirms thatPaymentDTOis used in multiple parts of the codebase. However, since the search for direct assignments to thestatusproperty returned no output, please manually verify that all indirect or dynamic assignments (if any) continue to work correctly with this updated type.
Verify that usages in:
sep490-frontend/src/app/modules/enterprise/services/payment.service.tssep490-frontend/src/app/modules/enterprise/components/payment/payment.component.tscorrectly handle the new union type.
Confirm that any interoperation with back-end components (e.g., in Java files) isn’t negatively impacted by this change.
sep490-infrastructure/keycloak-testcontainer-provider/pom.xml (1)
18-49: LGTM! Dependencies are well-configured.
- Keycloak dependencies are correctly marked as 'provided' since they'll be available in the runtime environment
- The optional flag for auto-service is appropriate for annotation processing
sep490-enterprise/src/main/java/enterprise/rest/BuildingController.java (1)
49-56: Fix validation and error handling.The method is missing validation annotations and proper error handling.
Apply this diff:
- public ResponseEntity<BuildingDTO> createBuilding(@RequestBody BuildingDTO buildingDTO, + public ResponseEntity<BuildingDTO> createBuilding(@Valid @RequestBody BuildingDTO buildingDTO, @AuthenticationPrincipal UserContextData userContextData) { var enterpriseIdFromContext = Objects.requireNonNull(userContextData.getEnterpriseId()); - var enterprise = enterpriseService.getById(enterpriseIdFromContext); + var enterprise = enterpriseService.getById(enterpriseIdFromContext) + .orElseThrow(() -> new ResourceNotFoundException("Enterprise not found"));sep490-infrastructure/keycloak-testcontainer-provider/src/main/java/keycloak/provider/Sep490IdentityProviderTokenMapper.java (2)
19-28: LGTM! Class structure and constants are well-defined.The class is properly annotated with
@AutoServicefor service registration and extends the appropriate base classes. Constants for claim labels are clearly defined.
61-92: LGTM! Boilerplate methods are properly implemented.The display and configuration methods are correctly implemented with appropriate return values.
sep490-infrastructure/keycloak-testcontainer/realm-export.json (2)
1031-1040: LGTM! Security headers are properly configured.The browser security headers include:
- XSS protection
- Content Security Policy
- X-Frame-Options
- HSTS
502-521:❓ Verification inconclusive
Verify the custom protocol mapper configuration.
The
testcontainerclient scope uses the customgreen-buildings-mapperprotocol mapper, which corresponds toSep490IdentityProviderTokenMapper. Ensure that:
- The mapper is properly registered and available in the Keycloak container
- The configuration is empty but should include the enterprise ID property as suggested in the mapper review
Run this script to verify the mapper registration:
🏁 Script executed:
#!/bin/bash # Description: Verify the custom protocol mapper registration # Check if the mapper class is in the provider JAR find . -name "*.jar" -exec sh -c 'echo "Checking {}" && jar tvf {} | grep -i "green-buildings-mapper"' \; # Check Keycloak logs for mapper registration find . -name "*.log" -exec grep -l "green-buildings-mapper" {} \;Length of output: 522
Custom Protocol Mapper Verification – Environment Issue Detected
It appears that the verification script wasn’t able to confirm the registration of the custom protocol mapper because the
jartool is missing in the current environment, and no relevant log entries were found. Please ensure that:
- The custom mapper (
green-buildings-mapperassociated withSep490IdentityProviderTokenMapper) is properly registered and available in the Keycloak container.- The verification is rerun in an environment where the
jarcommand is installed (or adjust the script accordingly) to validate that the mapper is present in the provider JAR.- The mapper configuration in
sep490-infrastructure/keycloak-testcontainer/realm-export.jsonincludes the required enterprise ID property rather than remaining empty.
sep490-enterprise/src/main/resources/db/migration/V0.0.1.3__ManageBuildings.sql
Show resolved
Hide resolved
...estcontainer-provider/src/main/java/keycloak/provider/Sep490IdentityProviderTokenMapper.java
Show resolved
Hide resolved
thongdanghoang
force-pushed
the
feature/77-userstory-enterprise
branch
3 times, most recently
from
9a71ffd to
2b5dffe
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/pull-request.yml(1 hunks).github/workflows/release.yml(0 hunks)sep490-enterprise/src/main/resources/application.yml(1 hunks)sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/release.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/pull-request.yml
- sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java
thongdanghoang
force-pushed
the
feature/77-userstory-enterprise
branch
3 times, most recently
from
dc85672 to
523af82
Compare
nganntqe170236
force-pushed
the
feature/77-userstory-enterprise
branch
from
91166ca to
51850de
Compare
test: integration BuildingControllerTest bugfix: fix PR feat: setup idp testcontainer feat: create building resource
nganntqe170236
force-pushed
the
feature/77-userstory-enterprise
branch
from
eb5d243 to
c44bad6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
.github/workflows/pull-request.yml(2 hunks).github/workflows/release.yml(0 hunks)sep490-commons/api/src/main/java/green_buildings/commons/api/SagaManager.java(1 hunks)sep490-commons/springfw-impl/src/main/java/commons/springfw/impl/securities/JwtAuthenticationConverter.java(2 hunks)sep490-enterprise/build.gradle(1 hunks)sep490-enterprise/src/main/java/enterprise/dtos/BuildingDTO.java(1 hunks)sep490-enterprise/src/main/java/enterprise/entities/BuildingEntity.java(2 hunks)sep490-enterprise/src/main/java/enterprise/mappers/BuildingMapper.java(1 hunks)sep490-enterprise/src/main/java/enterprise/repositories/BuildingRepository.java(1 hunks)sep490-enterprise/src/main/java/enterprise/rest/BuildingController.java(2 hunks)sep490-enterprise/src/main/java/enterprise/rest/EnterpriseController.java(0 hunks)sep490-enterprise/src/main/java/enterprise/services/BuildingService.java(1 hunks)sep490-enterprise/src/main/java/enterprise/services/EnterpriseService.java(1 hunks)sep490-enterprise/src/main/java/enterprise/services/impl/BuildingServiceImpl.java(1 hunks)sep490-enterprise/src/main/java/enterprise/services/impl/EnterpriseServiceImpl.java(1 hunks)sep490-enterprise/src/main/resources/application.yml(1 hunks)sep490-enterprise/src/main/resources/db/migration/V0.0.1.3__ManageBuildings.sql(1 hunks)sep490-enterprise/src/test/java/enterprise/EnterpriseApplication.java(0 hunks)sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java(1 hunks)sep490-enterprise/src/test/java/enterprise/adapters/exchangerate_api/ExchangeRateApiCurrencyConverterTest.java(1 hunks)sep490-enterprise/src/test/java/enterprise/rest/BuildingControllerTest.java(1 hunks)sep490-enterprise/src/test/resources/application.yml(0 hunks)sep490-enterprise/src/test/resources/db/migration/V0.0.1.4__TestcontainerUsers.sql(1 hunks)sep490-frontend/src/app/modules/enterprise/enterprise.module.ts(2 hunks)sep490-frontend/src/app/modules/enterprise/models/payment.ts(1 hunks)sep490-frontend/src/app/modules/shared/shared.module.ts(4 hunks)sep490-idp/src/main/java/green_buildings/idp/service/impl/UserServiceImpl.java(1 hunks)sep490-infrastructure/keycloak-testcontainer-provider/pom.xml(1 hunks)sep490-infrastructure/keycloak-testcontainer-provider/src/main/java/keycloak/provider/Sep490IdentityProviderTokenMapper.java(1 hunks)sep490-infrastructure/keycloak-testcontainer/Dockerfile(1 hunks)sep490-infrastructure/keycloak-testcontainer/realm-export.json(1 hunks)
💤 Files with no reviewable changes (4)
- sep490-enterprise/src/test/java/enterprise/EnterpriseApplication.java
- sep490-enterprise/src/test/resources/application.yml
- sep490-enterprise/src/main/java/enterprise/rest/EnterpriseController.java
- .github/workflows/release.yml
🚧 Files skipped from review as they are similar to previous changes (25)
- sep490-enterprise/src/main/java/enterprise/services/EnterpriseService.java
- sep490-commons/api/src/main/java/green_buildings/commons/api/SagaManager.java
- .github/workflows/pull-request.yml
- sep490-enterprise/src/main/resources/db/migration/V0.0.1.3__ManageBuildings.sql
- sep490-enterprise/src/main/resources/application.yml
- sep490-infrastructure/keycloak-testcontainer/Dockerfile
- sep490-idp/src/main/java/green_buildings/idp/service/impl/UserServiceImpl.java
- sep490-infrastructure/keycloak-testcontainer-provider/pom.xml
- sep490-enterprise/src/main/java/enterprise/dtos/BuildingDTO.java
- sep490-frontend/src/app/modules/enterprise/models/payment.ts
- sep490-enterprise/src/main/java/enterprise/mappers/BuildingMapper.java
- sep490-enterprise/src/test/java/enterprise/adapters/exchangerate_api/ExchangeRateApiCurrencyConverterTest.java
- sep490-enterprise/src/test/resources/db/migration/V0.0.1.4__TestcontainerUsers.sql
- sep490-enterprise/src/main/java/enterprise/services/impl/BuildingServiceImpl.java
- sep490-enterprise/src/test/java/enterprise/rest/BuildingControllerTest.java
- sep490-enterprise/build.gradle
- sep490-frontend/src/app/modules/enterprise/enterprise.module.ts
- sep490-enterprise/src/test/java/enterprise/TestcontainersConfigs.java
- sep490-commons/springfw-impl/src/main/java/commons/springfw/impl/securities/JwtAuthenticationConverter.java
- sep490-enterprise/src/main/java/enterprise/repositories/BuildingRepository.java
- sep490-enterprise/src/main/java/enterprise/services/impl/EnterpriseServiceImpl.java
- sep490-infrastructure/keycloak-testcontainer-provider/src/main/java/keycloak/provider/Sep490IdentityProviderTokenMapper.java
- sep490-frontend/src/app/modules/shared/shared.module.ts
- sep490-enterprise/src/main/java/enterprise/entities/BuildingEntity.java
- sep490-enterprise/src/main/java/enterprise/services/BuildingService.java
🔇 Additional comments (3)
sep490-enterprise/src/main/java/enterprise/rest/BuildingController.java (1)
47-57: Add input validation and error handling.The endpoint needs the following improvements:
- Input validation for building DTO
- Error handling for null enterprise and service exceptions
Apply this diff:
- @PostMapping @RolesAllowed(UserRole.RoleNameConstant.ENTERPRISE_OWNER) - public ResponseEntity<BuildingDTO> createBuilding(@RequestBody BuildingDTO buildingDTO, + public ResponseEntity<BuildingDTO> createBuilding(@Valid @RequestBody BuildingDTO buildingDTO, @AuthenticationPrincipal UserContextData userContextData) { + Objects.requireNonNull(buildingDTO, "Building DTO cannot be null"); var enterpriseIdFromContext = Objects.requireNonNull(userContextData.getEnterpriseId()); - var enterprise = enterpriseService.getById(enterpriseIdFromContext); - var building = buildingMapper.toEntity(buildingDTO); - building.setEnterprise(enterprise); - var createdBuilding = buildingService.createBuilding(building); - return ResponseEntity.status(HttpStatus.CREATED).body(buildingMapper.toDto(createdBuilding)); + try { + var enterprise = enterpriseService.getById(enterpriseIdFromContext) + .orElseThrow(() -> new ResourceNotFoundException("Enterprise not found")); + var building = buildingMapper.toEntity(buildingDTO); + building.setEnterprise(enterprise); + var createdBuilding = buildingService.createBuilding(building); + return ResponseEntity.status(HttpStatus.CREATED).body(buildingMapper.toDto(createdBuilding)); + } catch (ResourceNotFoundException e) { + throw e; + } catch (Exception e) { + log.error("Error creating building for enterprise {}: {}", enterpriseIdFromContext, e.getMessage(), e); + throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Error creating building", e); + } }sep490-infrastructure/keycloak-testcontainer/realm-export.json (2)
1-46: Enable brute force protection.The realm configuration has brute force protection disabled, which may expose the system to brute force attacks.
Apply this diff:
- "bruteForceProtected": false, + "bruteForceProtected": true,
402-452: Restrict testcontainer client configuration.The testcontainer client has overly permissive settings that could lead to security vulnerabilities:
- Wildcard redirect URIs
- Wildcard web origins
- Full scope allowed
Apply this diff:
"redirectUris": [ - "/*" + "http://localhost:*/*", + "https://*.your-domain.com/*" ], "webOrigins": [ - "/*" + "http://localhost:*", + "https://*.your-domain.com" ], - "fullScopeAllowed": true, + "fullScopeAllowed": false,
Summary by CodeRabbit
New Features
Refactor
Chores
Tests