Implement department CRUD with DTOs, specifications, and REST endpoints

[RedAvocado22]

No description provided.

[github-actions]

🔍 PR Review

Summary

This PR adds CRUD operations for the Department entity, introduces DTOs, and establishes proper REST endpoints with Spring Security. It also makes minor updates to other modules (AuthController, AccountResponse, etc.).

Issues Found

[File: AuthController.java, ~line 46]

• Problem: The signUp method exposes AccountResponse directly, which contains sensitive data such as email and fullName. This might inadvertently expose sensitive information to unauthorized users.
• Scenario: If a malicious actor were able to trigger this endpoint, they could extract personally identifiable information (PII).
• Suggestion: Use a reduced-response DTO that excludes sensitive data unless explicitly required. Additionally, review and ensure the endpoint is protected by authentication/authorization rules.

---

[File: DepartmentController.java, ~line 27]

• Problem: The SearchDepartmentRequest is annotated with @ModelAttribute, which means it's passing potentially unvalidated input directly into the service layer where SQL queries are constructed.
• Scenario: This could allow injection of invalid/bad input which could lead to code or behavior vulnerabilities (like injection attacks).
• Suggestion: Use a @Valid annotation on SearchDepartmentRequest and validate its fields thoroughly using Java Bean Validation (@NotNull, @Pattern, etc.).

---

[File: DepartmentController.java, ~line 40]

• Problem: The createDepartment endpoint does not explicitly handle exceptions. If the CreateDepartmentService throws an exception (e.g., data constraints, database issues), it will result in an unhandled 500 error.
• Scenario: Sending malformed CreateDepartmentRequest data (e.g., null name or duplicate values) can crash the system.
• Suggestion: Add exception handling (e.g., @ControllerAdvice) to catch validation or service errors and map them to appropriate HTTP responses (400/409).

---

[File: Department.java, ~line 25]

• Problem: The isActive field was changed from Boolean (wrapper) to boolean (primitive). While this simplifies the code, it introduces an issue with flexibility since boolean cannot represent null values.
• Scenario: Existing records in the database with NULL values for is_active will cause an exception when fetched.
• Suggestion: Consider whether isActive should allow nullability. If null is valid, revert to Boolean. If not, ensure a database migration sets default values for records with NULL.

---

[File: DepartmentRepository.java]

• Problem: Missing a custom query or method for verifying unique department names (e.g., findByName).
• Scenario: The current code does not appear to enforce unique department names before saving, which could lead to duplicate entries being created.
• Suggestion: Add a method like Optional<Department> findByName(String name) in the repository and validate uniqueness inside the CreateDepartmentService.

---

[File: AccountResponse.java, ~line 11]

• Problem: The AccountResponse directly maps fields from the Account entity, exposing potentially sensitive data like role.
• Scenario: If the role is exposed more broadly, it could be exploited to infer application access logic or profiles.
• Suggestion: Limit exposure by filtering/obfuscating such fields where not required or authorized in downstream use cases.

---

What's Done Well

• Use of DTOs for both request and response is well-implemented. This avoids direct exposure of entity models.
• The introduction of specifications with JpaSpecificationExecutor for filtering/querying the Department entity is a good design choice for scalability.
• Proper use of annotations like @PreAuthorize helps secure sensitive endpoints.

Verdict

🔴 Needs Changes — The PR contains several security, validation, and design issues that need to be addressed before merging.

---

🤖 Automated review by GitHub Models

[RedAvocado22]

ok, lead tell I'm gud to go!