Équipe 3 – Phase 3: Code Quality, Security Hardening & Docker Optimization - #57
Merged
Conversation
…tion - Remove MySQL/Eureka/Gateway dependencies, migrate to MongoDB-only - Externalize all secrets to .env (JWT, Google OAuth2, MongoDB) - Add .env.example template, .gitignore protects .env from commits - Rename testapi Java package org.requests -> ca.etsmtl.taf.testapi - Rename RequestController -> TestRequestExecutor - Extract AuthController business logic to UserRegistrationService - Extract GatlingApiController logic to GatlingExecutionService - Add Dashboard MongoDB collections (TestRun, TestCaseResult, TestResultService) - Fix OAuth2 auto-logout (auth interceptor skips refresh-token endpoint) - Optimize all 4 Dockerfiles (multi-stage, .dockerignore, layer caching) - Fix frontend nginx proxy_pass and envsubst config - Add JaCoCo to testapi module (surefire 3.2.5 + jacoco 0.8.12) - Restore jaxb-api 2.3.1 (required by jjwt 0.9.1 on Java 17+) - Fix AuthControllerTest and WebSecurityConfigTest after refactoring - Clean unused dependencies (mysql, h2, eureka, zuul, ribbon, gateway) - Rename TS models (test_model -> TestDefinition, testmodel2 -> TestExecution) - Update README (remove Eureka, add OAuth2 setup guide, .env workflow) - Update TEST-REPORT.md (add UserRegistrationService coverage) - 87 tests passing, 0 failures, 98% line coverage on team classes
There was a problem hiding this comment.
Pull request overview
This PR delivers Phase 3 hardening/cleanup across the TAF microservices: externalizing secrets, removing legacy/dead code, improving authentication flows (OAuth2 + refresh tokens), optimizing Docker builds, and adding new API-test persistence + UI improvements (progressive execution + Gherkin mode).
Changes:
- Externalize previously hardcoded credentials into environment variables and update local run scripts/docker ignores.
- Add OAuth2 Google login + refresh-token support (backend + frontend) and refactor controller logic into services.
- Optimize Dockerfiles to multi-stage builds and enhance test infrastructure (JUnit5/Mockito conventions + JaCoCo).
Reviewed changes
Copilot reviewed 145 out of 147 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| user/src/main/resources/application.yml | Externalizes Eureka config to env vars and adds enable flag. |
| user/src/main/java/ca/etsmtl/taf/user/services/UserService.java | Fixes update logic and adds null guard for findById. |
| user/src/main/java/ca/etsmtl/taf/user/services/JwtService.java | Removes unused imports/fields. |
| user/src/main/java/ca/etsmtl/taf/user/repository/UserRepository.java | Adds @NonNull contract for existsById. |
| user/src/main/java/ca/etsmtl/taf/user/payload/request/PasswordRequest.java | Removes unused import. |
| user/src/main/java/ca/etsmtl/taf/user/jwt/JwtUtil.java | Removes unused token methods/claims. |
| user/src/main/java/ca/etsmtl/taf/user/jwt/JwtAuthenticationFilter.java | Adds @NonNull annotations on filter method params. |
| user/src/main/java/ca/etsmtl/taf/user/controller/UserController.java | Cleans unused auth/jwt wiring from controller. |
| user/src/main/java/ca/etsmtl/taf/user/AuthGatewayApplication.java | Removes unused imports. |
| user/Dockerfile.local | Improves Gradle layer caching by pre-downloading deps. |
| user/.dockerignore | Reduces Docker build context size. |
| testapi-Service/testapi/src/test/java/ca/etsmtl/taf/testapi/util/JsonComparatorTest.java | Adds a JUnit5 test for JsonComparator. |
| testapi-Service/testapi/src/main/resources/application.yml | Normalizes YAML + adds timeout config. |
| testapi-Service/testapi/src/main/java/org/utils/JsonComparator.java | Removes legacy package/class (migrated to new namespace). |
| testapi-Service/testapi/src/main/java/org/requests/TestApiController.java | Removes legacy controller (migrated to new namespace). |
| testapi-Service/testapi/src/main/java/ca/etsmtl/taf/testapi/util/JsonComparator.java | Reintroduces comparator under new package. |
| testapi-Service/testapi/src/main/java/ca/etsmtl/taf/testapi/payload/request/TestApiRequest.java | Migrates package + Jakarta validation imports. |
| testapi-Service/testapi/src/main/java/ca/etsmtl/taf/testapi/payload/request/Answer.java | Adds actualResponseTime output field. |
| testapi-Service/testapi/src/main/java/ca/etsmtl/taf/testapi/config/TimeoutConfig.java | Configures RestAssured timeouts via Spring config. |
| testapi-Service/testapi/src/main/java/ca/etsmtl/taf/testapi/config/DevCorsConfiguration.java | Migrates config package name. |
| testapi-Service/testapi/src/main/java/ca/etsmtl/taf/testapi/TestRequestExecutor.java | Refactors request execution (headers/body optional) + timeout reporting. |
| testapi-Service/testapi/src/main/java/ca/etsmtl/taf/testapi/TestManager.java | Migrates main class package. |
| testapi-Service/testapi/src/main/java/ca/etsmtl/taf/testapi/TestApiController.java | Migrates controller + adds slow endpoint for timeout testing. |
| testapi-Service/testapi/src/main/java/ca/etsmtl/taf/testapi/Method.java | Migrates enum/package cleanup. |
| testapi-Service/testapi/pom.xml | Updates groupId/start-class and adds Surefire + JaCoCo. |
| testapi-Service/testapi/Dockerfile | Adds multi-stage build (Maven builder → JRE runtime). |
| testapi-Service/selenium/src/test/java/ca/etsmtl/selenium/SeleniumApplicationTests.java | Removes unused test import. |
| testapi-Service/selenium/src/main/java/ca/etsmtl/selenium/requests/UseSelenium.java | Updates Selenium API usage (Duration + getDomAttribute). |
| testapi-Service/selenium/src/main/java/ca/etsmtl/selenium/config/DevCorsConfiguration.java | Adds @NonNull annotation to config callback. |
| testapi-Service/run-tests-testapi.ps1 | Adds PowerShell runner for backend unit tests. |
| testapi-Service/pom.xml | Adds UTF-8 build/reporting encodings. |
| testapi-Service/frontend/src/environments/environment.ts | Adds oauth2 backend URL for local dev. |
| testapi-Service/frontend/src/environments/environment.prod.ts | Adds oauth2 backend URL for prod env config. |
| testapi-Service/frontend/src/app/register/register.component.spec.ts | Fixes test module wiring (HttpClientTestingModule/Forms). |
| testapi-Service/frontend/src/app/project/project.component.spec.ts | Fixes test module wiring (router/forms/http). |
| testapi-Service/frontend/src/app/profile/profile.component.spec.ts | Adds TokenStorageService mock for profile test. |
| testapi-Service/frontend/src/app/performance-test-api/gatling-api/gatling-api.component.spec.ts | Fixes HttpClient dependency for test. |
| testapi-Service/frontend/src/app/oauth2-callback/oauth2-callback.component.ts | Adds OAuth2 callback handler to persist tokens/user info. |
| testapi-Service/frontend/src/app/oauth2-callback/oauth2-callback.component.html | Adds OAuth2 callback UI template. |
| testapi-Service/frontend/src/app/oauth2-callback/oauth2-callback.component.css | Adds OAuth2 callback styling. |
| testapi-Service/frontend/src/app/models/testmodel2.ts | Renames interface and adds response timing + pending fields. |
| testapi-Service/frontend/src/app/models/testResponseModel.ts | Adds actualResponseTime to response model. |
| testapi-Service/frontend/src/app/models/test-model.ts | Renames class to PascalCase. |
| testapi-Service/frontend/src/app/login/login.component.ts | Supports alternate token field names + adds Google login redirect. |
| testapi-Service/frontend/src/app/login/login.component.spec.ts | Fixes test module wiring. |
| testapi-Service/frontend/src/app/login/login.component.html | Adds “Login with Google” button UI. |
| testapi-Service/frontend/src/app/login/login.component.css | Styles Google login button. |
| testapi-Service/frontend/src/app/interface-test-api/test-api/test-api.component.ts | Adds progressive execution, persistence calls, and Gherkin mode. |
| testapi-Service/frontend/src/app/interface-test-api/test-api/test-api.component.spec.ts | Expands coverage for Gherkin mode + dialog/edit + timeout display. |
| testapi-Service/frontend/src/app/interface-test-api/test-api/test-api.component.html | Adds Gherkin editor toggle + actual response time + pending spinner. |
| testapi-Service/frontend/src/app/interface-test-api/test-api/delete-test-dialog/delete-test-dialog.component.ts | Updates model naming imports. |
| testapi-Service/frontend/src/app/interface-test-api/test-api/add-test-dialog/add-test-dialog.component.ts | Adds edit mode + input body support + update flow. |
| testapi-Service/frontend/src/app/interface-test-api/test-api/add-test-dialog/add-test-dialog.component.html | Updates labels and supports edit-mode submit text. |
| testapi-Service/frontend/src/app/interface-test-api/gherkin-editor/gherkin-editor.component.ts | Adds new Gherkin editor component with highlighting + import/export. |
| testapi-Service/frontend/src/app/interface-test-api/gherkin-editor/gherkin-editor.component.spec.ts | Adds unit tests for editor parsing/highlighting/actions. |
| testapi-Service/frontend/src/app/interface-test-api/gherkin-editor/gherkin-editor.component.html | Adds editor UI + preview + syntax help. |
| testapi-Service/frontend/src/app/interface-test-api/gherkin-editor/gherkin-editor.component.css | Adds styling for editor/preview/highlighting. |
| testapi-Service/frontend/src/app/home/home.component.spec.ts | Fixes HttpClient dependency for test. |
| testapi-Service/frontend/src/app/gatling/gatling.component.spec.ts | Fixes HttpClient dependency for test. |
| testapi-Service/frontend/src/app/board-user/board-user.component.spec.ts | Fixes HttpClient dependency for test. |
| testapi-Service/frontend/src/app/board-admin/board-admin.component.spec.ts | Fixes HttpClient dependency for test. |
| testapi-Service/frontend/src/app/app.module.ts | Registers OAuth2Callback + Gherkin editor + tooltip/spinner modules. |
| testapi-Service/frontend/src/app/app.component.ts | Changes logout to redirect to /login instead of reload. |
| testapi-Service/frontend/src/app/app.component.spec.ts | Adjusts test schema and removes obsolete title tests. |
| testapi-Service/frontend/src/app/app-routing.module.ts | Adds route for OAuth2 callback. |
| testapi-Service/frontend/src/app/_services/user.service.spec.ts | Adds HttpClientTestingModule. |
| testapi-Service/frontend/src/app/_services/token-storage.service.ts | Adds refresh token storage helpers. |
| testapi-Service/frontend/src/app/_services/performance-test-api.service.spec.ts | Adds HttpClientTestingModule. |
| testapi-Service/frontend/src/app/_services/auth.service.ts | Adds refresh-token API call (direct backend URL). |
| testapi-Service/frontend/src/app/_services/auth.service.spec.ts | Adds HttpClientTestingModule. |
| testapi-Service/frontend/src/app/_helpers/auth.interceptor.ts | Adds refresh-on-401 logic with queueing and recursion guard. |
| testapi-Service/frontend/package.json | Formatting-only newline fix. |
| testapi-Service/frontend/nginx.conf | Fixes proxy routes and headers; splits backend/auth/user upstreams. |
| testapi-Service/frontend/envsubst-file.sh | Updates envsubst variables. |
| testapi-Service/frontend/angular.json | Relaxes bundle/style budgets. |
| testapi-Service/frontend/Dockerfile | Adds multi-stage build and reduces layers/permissions. |
| testapi-Service/documentation/CONVENTIONS.md | Documents unit-test conventions and recommended commands. |
| testapi-Service/backend/src/test/resources/application.yml | Adds dedicated test config disabling Mongo/Eureka autoconfig. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/security/services/UserDetailsServiceImplTest.java | Adds unit tests for user details service. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/security/services/UserDetailsImplTest.java | Adds unit tests for user details wrapper. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/security/jwt/JwtUtilsTest.java | Adds unit tests for JWT generation/validation/refresh. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/security/jwt/AuthTokenFilterTest.java | Adds unit tests for JWT filter behavior. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/security/jwt/AuthEntryPointJwtTest.java | Adds unit tests for 401 JSON response body. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/security/WebSecurityConfigTest.java | Adds MVC slice tests for security rules/beans. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/payload/response/MessageResponseTest.java | Adds DTO tests. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/payload/response/JwtResponseTest.java | Adds DTO tests (incl refresh token). |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/entity/UserEntityTest.java | Adds entity tests for local/oauth2 constructors and roles. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/entity/RoleTest.java | Adds Role entity tests. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/controller/TestApiControllerTest.java | Adds controller tests including HTTP timeout behavior. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/controller/OAuth2ControllerTest.java | Adds controller test for login-url endpoint. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/controller/ApiTestDefinitionControllerTest.java | Adds CRUD tests for persisted API test definitions. |
| testapi-Service/backend/src/test/java/ca/etsmtl/taf/TestAutomationFrameworkApplicationTests.java | Disables full context test requiring MongoDB. |
| testapi-Service/backend/src/main/resources/application.yml | Adds OAuth2 config + refresh expiry + timeouts; externalizes Eureka. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/service/UserRegistrationService.java | Extracts signup business logic from controller. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/service/TestResultService.java | Persists test run summaries + case results for dashboard. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/service/GatlingExecutionService.java | Extracts Gatling process execution into service. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/security/oauth2/OAuth2LoginSuccessHandler.java | Creates/links OAuth2 users and redirects with JWTs. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/security/jwt/JwtUtils.java | Adds refresh token support + expired-token username extraction. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/security/WebSecurityConfig.java | Enables OAuth2 login flow and updates security matchers/session policy. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/repository/UserRepository.java | Adds findByEmail/findByGoogleId for OAuth2 linkage. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/repository/TestRunRepository.java | Adds repository for dashboard test run docs. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/repository/TestCaseResultRepository.java | Adds repository for dashboard test case docs. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/repository/ApiTestDefinitionRepository.java | Adds repository for saved API test definitions. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/payload/response/JwtResponse.java | Adds refresh token to auth response DTO. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/payload/request/TestApiRequest.java | Adds responseTime/expectedHeaders to backend request DTO. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/payload/request/RefreshTokenRequest.java | Adds refresh token request DTO. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/eureka/EurekaItem.java | Removes dead Eureka exploration service. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/entity/User.java | Extends user model for OAuth2 provider/googleId and adjusts username length. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/entity/TestRun.java | Adds dashboard test run MongoDB document schema. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/entity/TestCaseResult.java | Adds dashboard test case MongoDB document schema. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/entity/ApiTestDefinition.java | Adds saved API test definition MongoDB document schema. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/controller/TestResultController.java | Adds endpoint to persist test execution results. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/controller/TestController.java | Removes unused demo controller. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/controller/TestApiController.java | Adds configurable timeout for forwarding requests to testapi microservice. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/controller/OAuth2Controller.java | Adds helper endpoint exposing OAuth2 login URL info. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/controller/GatlingApiController.java | Delegates Gatling execution to service class. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/controller/AuthController.java | Adds refresh-token endpoint and delegates signup to service. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/controller/ApiTestDefinitionController.java | Adds CRUD endpoints for saved API test definitions. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/config/OpenApiConfig.java | Updates OpenAPI auth docs for JWT + OAuth2. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/config/MongoAuditingConfig.java | Moves Mongo auditing out of main app class for test slicing. |
| testapi-Service/backend/src/main/java/ca/etsmtl/taf/TestAutomationFrameworkApplication.java | Removes discovery client/auditing from main app class. |
| testapi-Service/backend/pom.xml | Removes legacy deps, adds OAuth2 client, adds JaCoCo, adds frontend profile. |
| testapi-Service/backend/pom.docker.xml | Adds OAuth2 client dependency for Docker build variant. |
| testapi-Service/backend/.env | Updates local env values (but remains tracked). |
| testapi-Service/Dockerfile.local | Optimizes local Docker Maven build flags. |
| testapi-Service/CONTRIBUTING.md | Adds explicit backend unit test instructions. |
| testapi-Service/.dockerignore | Reduces Docker build context (frontend/dist/node_modules, backend/target, etc.). |
| test-performance-Service/backend/src/main/resources/application.yml | Externalizes Eureka config to env vars and adds enable flag. |
| start-taf-local.ps1 | Adds stop/restart/status switches and updates service lists/phases. |
| selenium-test-Service/backend/src/main/resources/application.yml | Externalizes Eureka config to env vars and adds enable flag. |
| gateway/src/main/java/ca/etsmtl/taf/gateway/GatewayApplication.java | Removes unused import. |
| auth/src/main/resources/application.yml | Externalizes Eureka config to env vars and adds enable flag. |
| auth/src/main/java/ca/etsmtl/taf/auth/services/JwtService.java | Removes unused imports. |
| auth/src/main/java/ca/etsmtl/taf/auth/services/CustomUserDetailsService.java | Removes unused import. |
| auth/Dockerfile | Switches auth service to multi-stage Gradle build. |
| auth/.dockerignore | Reduces Docker build context size. |
| .gitignore | Adds .env ignore patterns and (now) ignores *.py. |
| .env.example | Adds template for required env vars and secret handling. |
Comments suppressed due to low confidence (2)
testapi-Service/testapi/src/main/java/ca/etsmtl/taf/testapi/TestRequestExecutor.java:74
headersOK = this.checkResponseHeaders()is used to decide pass/fail, butcheckResponseHeaders()currently does not fail when a required header is missing (it records a message but keepsoktrue). This can cause false positives where header assertions are silently ignored.
testapi-Service/testapi/src/main/java/ca/etsmtl/taf/testapi/TestRequestExecutor.java:33- This class logs extensively with
System.out.println(...)(including per-request debug). In a Spring Boot service, prefer a logger (slf4j) with appropriate log levels so output can be managed/disabled in production and doesn’t flood container logs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+126
to
+130
| // Redirect to frontend with JWT token, refresh token, and user info | ||
| String redirectUrl = frontendRedirectUrl + "/oauth2/callback?token=" + jwtToken + "&refreshToken=" + refreshToken + "&userInfo=" + userInfoBase64; | ||
| log.info("Redirecting OAuth2 user to: {}", frontendRedirectUrl + "/oauth2/callback?token=***"); | ||
|
|
||
| getRedirectStrategy().sendRedirect(request, response, redirectUrl); |
There was a problem hiding this comment.
Access/refresh tokens are sent to the frontend in URL query parameters. This leaks tokens via browser history, logs, proxies, and the Referer header on subsequent requests. Prefer returning tokens in an HttpOnly/SameSite cookie or redirecting with the token in the URL fragment (#) and/or using a one-time authorization code exchanged via POST.
| /** | ||
| * Refresh token endpoint. | ||
| * Accepts a valid refresh token and returns a new access token + refresh token pair. | ||
| * The old refresh token is invalidated by issuing a new one (rotation strategy). |
There was a problem hiding this comment.
The Javadoc claims the old refresh token is "invalidated" via rotation, but the implementation doesn’t persist refresh tokens server-side (no DB store/blacklist), so previously-issued refresh tokens remain valid until expiry. Either adjust the comment, or implement real rotation (store current refresh token per user and reject older ones).
Suggested change
| * The old refresh token is invalidated by issuing a new one (rotation strategy). | |
| * Note: Previously issued refresh tokens remain valid until they expire; this method does | |
| * not persist or invalidate older refresh tokens on the server side. |
Comment on lines
+81
to
+90
| // Validate the refresh token | ||
| if (!jwtUtils.validateJwtToken(requestRefreshToken)) { | ||
| logger.warn("Refresh token validation failed"); | ||
| return ResponseEntity.status(401) | ||
| .body(new MessageResponse("Error: Refresh token is invalid or expired. Please sign in again.")); | ||
| } | ||
|
|
||
| // Extract username from the refresh token | ||
| String username = jwtUtils.getUserNameFromJwtToken(requestRefreshToken); | ||
|
|
There was a problem hiding this comment.
/refresh-token accepts any valid JWT signed with the same key (including a non-expired access token), because refresh tokens are not distinguishable from access tokens. Consider adding a claim like typ=refresh (or a dedicated tokenType claim) and verifying it here, or signing refresh tokens with a different key/algorithm.
Comment on lines
+22
to
+31
| @PostConstruct | ||
| public void configureRestAssuredTimeout() { | ||
| RestAssured.config = RestAssuredConfig.config() | ||
| .httpClient(HttpClientConfig.httpClientConfig() | ||
| .setParam("http.connection.timeout", connectionTimeout) | ||
| .setParam("http.socket.timeout", socketTimeout) | ||
| .setParam("http.connection-manager.timeout", connectionManagerTimeout)); | ||
|
|
||
| System.out.println("✅ RestAssured timeout configured: connection=" + connectionTimeout | ||
| + "ms, socket=" + socketTimeout + "ms, connectionManager=" + connectionManagerTimeout + "ms"); |
There was a problem hiding this comment.
Avoid System.out.println in service configuration; use the project logging framework (slf4j) so logs can be controlled via levels/appenders and don’t pollute stdout in containers.
Comment on lines
10
to
15
| JWT_SECRET=bezKoderSecretKey | ||
| JWT_EXPIRES=86400000 | ||
| JWT_EXPIRES=1800000 | ||
| JWT_REFRESH_EXPIRES=604800000 | ||
| DB_URI=mongodb://admin:admin123@localhost:27017/taf?authSource=admin | ||
| DB_NAME=taf_database | ||
| DB_AUTH=admin |
There was a problem hiding this comment.
This committed .env file contains secrets/credentials (JWT secret, MongoDB admin password, etc.). Since .env is now in .gitignore, it should be removed from version control and replaced by a sanitized .env.example (or moved to docs).
Comment on lines
+3
to
+4
| import org.junit.jupiter.api.Test; | ||
| import org.springframework.boot.test.context.SpringBootTest; | ||
| import org.junit.jupiter.api.condition.DisabledIfEnvironmentVariable; |
There was a problem hiding this comment.
Unused import DisabledIfEnvironmentVariable will fail compilation. Remove it or switch to a conditional disable annotation that actually uses it (e.g., disable unless an env var is set).
Comment on lines
+23
to
+28
| for (int i = 0; i < array1.size(); i++) { | ||
| if (array1.get(i).isObject() && array2.get(i).isObject()) { | ||
| arrayResult.add(compareJson(array1.get(i), array2.get(i), result.objectNode())); | ||
| } else { | ||
| arrayResult.add(array1.get(i).equals(array2.get(i))); | ||
| } |
There was a problem hiding this comment.
compareJson iterates using array1.size() but accesses array2.get(i) without checking bounds/null. When array2 is shorter, array2.get(i) can be null and isObject() will throw. Consider checking i < array2.size() (and treating missing elements as false) before dereferencing.
Comment on lines
+24
to
+40
| private static final int MAX_DELAY_MS = 120_000; | ||
|
|
||
| /** | ||
| * Simulates a slow API endpoint to test timeout behavior. | ||
| * Sleeps for the given number of milliseconds (default 15000, max 120000). | ||
| */ | ||
| @GetMapping("/slow") | ||
| public ResponseEntity<Map<String, Object>> slow( | ||
| @RequestParam(value = "delay", defaultValue = "15000") int delay) { | ||
| if (delay < 0 || delay > MAX_DELAY_MS) { | ||
| return ResponseEntity.badRequest().body(Map.of( | ||
| "error", "delay must be between 0 and " + MAX_DELAY_MS + " ms")); | ||
| } | ||
| long start = System.currentTimeMillis(); | ||
| try { | ||
| Thread.sleep(delay); | ||
| } catch (InterruptedException e) { |
There was a problem hiding this comment.
The /slow endpoint intentionally sleeps and can be abused to tie up request threads (easy DoS) if exposed beyond local testing. Consider guarding it behind a local profile, a feature flag, or removing it from production builds.
Comment on lines
1
to
5
| export const environment = { | ||
| production: true, | ||
| apiUrl: '/my-backend' | ||
| apiUrl: '/my-backend', | ||
| oauth2BackendUrl: 'http://localhost:8084' | ||
| }; |
There was a problem hiding this comment.
oauth2BackendUrl is hardcoded to http://localhost:8084 even in environment.prod.ts, which will break OAuth2 login in real deployments. Consider making this relative (same origin) or injecting it at build/runtime (e.g., via nginx envsubst like the other backend URLs).
Comment on lines
+90
to
+93
| // Assign default ROLE_USER | ||
| Set<Role> roles = new HashSet<>(); | ||
| roleRepository.findByName(ERole.ROLE_USER).ifPresent(roles::add); | ||
| newUser.setRoles(roles); |
There was a problem hiding this comment.
When creating a new OAuth2 user, ROLE_USER is added only if present (ifPresent). If the role collection is missing/empty, the user will be persisted with no roles, which can break authorization flows. Consider using orElseThrow(...) (like signup) or ensuring roles are seeded before allowing OAuth2 signup.
…mprovements - Fix AuthController javadoc: clarify refresh tokens are not server-side invalidated - Replace System.out.println with slf4j in TimeoutConfig and TestRequestExecutor - Remove backend/.env from git tracking (secrets file, already in .gitignore) - Remove unused DisabledIfEnvironmentVariable import in ApplicationTests - Fix JsonComparator array bounds check (prevent NPE when array2 is shorter) - Fix checkResponseHeaders: missing header now correctly fails the assertion - Extract /slow endpoint to SlowEndpointController with @Profile guard (DoS prevention) - Fix environment.prod.ts: use relative path instead of hardcoded localhost:8084 - Fix OAuth2 role assignment: ifPresent → orElseThrow (prevent users with no roles) - Update OAuth2LoginSuccessHandlerTest for new orElseThrow behavior - 87 tests passing, 0 failures
BEDDAR
approved these changes
Mar 12, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add /my-backend/oauth2/ nginx route for Google OAuth2 authorization flow - Add /my-backend/api/ nginx route for direct backend API calls (e.g. refresh-token) - Add explicit redirect-uri in application.yml to ensure correct Google callback URL when proxied through nginx (OAUTH2_REDIRECT_URI env var, defaults to localhost:8084) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
BEDDAR
approved these changes
Mar 13, 2026
Valeeeu
self-requested a review
March 13, 2026 17:44
Valeeeu
approved these changes
Mar 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Équipe 3 – Phase 3: Code Quality, Security Hardening & Docker Optimization
Summary
This PR delivers the Phase 3 milestone for Équipe 3 — Code quality improvements, security hardening, and Docker optimization. The objective was to eliminate technical debt accumulated during Phases 1 and 2, harden secret management, optimize Docker builds, and ensure the codebase follows consistent conventions.
When we started Phase 3, the codebase had hardcoded secrets in Git (JWT key, MongoDB credentials, Google OAuth2 client secret), unused legacy dependencies (MySQL, Eureka, Zuul, Ribbon, Gateway), inconsistent Java package naming (
org.requestsvsca.etsmtl.taf), controller classes containing business logic, and unoptimized Docker images (single-stage builds, no layer caching). The OAuth2 login also had a critical bug causing automatic logout after Google authentication.After our changes, all secrets are externalized to
.env(protected by.gitignore), 12 unused dependencies have been removed, the testapi Java package follows the project convention (ca.etsmtl.taf.testapi), controller business logic is extracted into service classes, all 4 Dockerfiles use multi-stage builds with.dockerignore, the OAuth2 auto-logout bug is fixed, and all 87 backend tests pass with 98% line coverage.Problems Found & Changes Made
1. Hardcoded secrets committed to Git
Problem: The codebase contained hardcoded sensitive values directly in configuration files tracked by Git:
JWT_SECREThardcoded inapplication.yml(used for HS512 signing)MONGO_ROOT_USERNAME,MONGO_ROOT_PASSWORD) indocker-compose-local-test.ymlclient-idandclient-secretinapplication.ymlapplication.ymlThis is a critical security vulnerability — anyone with repository access could extract production secrets.
Changes:
.env.example— template file with placeholder values and instructions.gitignore— added.envand**/.envpatterns to prevent accidental commitsdocker-compose-local-test.yml— all hardcoded credentials replaced with${VAR}references that Docker Compose reads from.envapplication.yml(backend, auth, user, selenium, test-performance) — replaced hardcoded values with${ENV_VAR}or${ENV_VAR:default}syntax.env.example→.envand fill in their own values2. Unused legacy dependencies bloating the build
Problem: The backend
pom.xmlcontained 12+ dependencies from a previous MySQL/Eureka/Gateway architecture that was migrated to MongoDB in Phase 2. These dependencies added unnecessary JAR weight, potential CVE surface, and confusion for developers:mysql-connector-j,h2(database drivers — now using MongoDB)spring-cloud-starter-netflix-eureka-client(service discovery — removed)spring-cloud-starter-gateway,spring-cloud-starter-netflix-zuul,spring-cloud-starter-netflix-ribbon(API gateway — not used)jakarta.xml.bind-api,jaxb-runtime(marshalling — not needed)commons-codec,httpclient5(redundant with Spring's built-in HTTP client)javax.xml.bind:jaxb-apiwas initially removed but restored becausejjwt 0.9.1internally usesjavax.xml.bind.DatatypeConverterfor Base64 operations, which was removed from the JDK in Java 17Changes (
backend/pom.xml):javax.xml.bind:jaxb-api:2.3.1with explanatory comment<dependencyManagement>section (removed Spring Cloud BOM)EurekaItem.java(dead code — Eureka DTO no longer needed)@EnableDiscoveryClientfromTestAutomationFrameworkApplication.java3. Inconsistent Java package naming in testapi module
Problem: The testapi microservice used the package
org.requestsfor all its classes (e.g.,org.requests.RequestController,org.utils.JsonComparator), while the rest of the project usesca.etsmtl.taf.*. This violated Java naming conventions and made it confusing to identify which module a class belongs to.Changes (testapi module):
org.requests→ca.etsmtl.taf.testapiorg.config→ca.etsmtl.taf.testapi.configorg.utils→ca.etsmtl.taf.testapi.utilorg.requests.payload.request→ca.etsmtl.taf.testapi.payload.requestRequestController→TestRequestExecutor(clearer responsibility)testapi/pom.xml<start-class>to match new package4. Controller classes containing business logic
Problem:
AuthControllercontained 50+ lines of business logic directly in the signup endpoint: user existence checks, password encoding, role resolution, and database persistence. This made it untestable (required mocking 3 repositories + PasswordEncoder) and violated the Single Responsibility Principle. Similarly,GatlingApiControllercontained inline Gatling process execution logic.Changes:
UserRegistrationService— extractedexistsByUsername(),existsByEmail(),registerUser()methods fromAuthControllerAuthController— signup endpoint now delegates touserRegistrationService.registerUser()GatlingExecutionService— extracted Gatling process builder logic fromGatlingApiControllerAuthControllerTest— rewrote from mockingUserRepository/RoleRepository/PasswordEncoderto mockingUserRegistrationServiceWebSecurityConfigTest— added@MockitoBean UserRegistrationService5. OAuth2 Google login causes immediate auto-logout
Problem: After successful Google OAuth2 login, the user was immediately logged out. Root cause: the
AuthInterceptorwas intercepting the/api/auth/refresh-tokenrequest itself. When the access token expired, the interceptor tried to refresh it by calling/api/auth/refresh-token, but depending on timing, this request was also intercepted, creating a recursive loop that cleared the session.Changes (
auth.interceptor.ts):/api/auth/refresh-tokenrequests6. Unoptimized Docker images
Problem: All 4 Dockerfiles (backend, testapi, frontend, auth) used single-stage builds:
.dockerignore—target/,node_modules/,.git/were included in the build contextChanges:
envsubstconfig andnginx.confproxy settings.dockerignorefor auth and user modules7. Frontend nginx misconfiguration
Problem: The frontend nginx config had incorrect
proxy_passtargets and theenvsubstscript wasn't properly replacing environment variables at container startup.Changes:
proxy_passURLs for/api/routes, added properproxy_set_headerdirectives8. Missing JaCoCo coverage for testapi module
Problem: The testapi module had no test infrastructure — no surefire plugin version, no JaCoCo, and no unit tests beyond a single
JsonComparatorTest.Changes (
testapi/pom.xml):maven-surefire-plugin3.2.5jacoco-maven-plugin0.8.12 (prepare-agent + report phases)testapi/target/site/jacoco/index.html9. Dashboard integration for test results
Problem: Test execution results were only kept in frontend memory — a page refresh lost all results. There was no server-side persistence or dashboard for historical test data.
Changes:
TestRunentity — MongoDB document storing test run metadata (timestamp, total/passed/failed counts)TestCaseResultentity — MongoDB document storing individual test case resultsTestRunRepositoryandTestCaseResultRepository** — Spring Data MongoDB repositoriesTestResultService— service layer for persisting and querying test resultsTestResultController— REST endpoints for dashboard data access10. TypeScript model naming inconsistencies
Problem: Frontend models used inconsistent naming:
test_model(snake_case file,testModelclass) andtestmodel2(no separator). These didn't follow Angular/TypeScript conventions.Changes:
TestDefinitionandTestExecution(PascalCase, semantic names)11. Test breakages after refactoring
Problem: The controller refactoring (extracting
UserRegistrationService) broke existing tests:AuthControllerTeststill mockedUserRepository,RoleRepository,PasswordEncoderdirectly — these are no longer injected intoAuthControllerWebSecurityConfigTestwas missing the newUserRegistrationServicedependencyJwtUtilsTestfailed withNoClassDefFound javax/xml/bind/DatatypeConverterafter jaxb-api was initially removedChanges:
AuthControllerTest— 13 tests now mockUserRegistrationServiceinstead of individual repositoriesWebSecurityConfigTest— added@MockitoBean UserRegistrationServicejaxb-api:2.3.1inbackend/pom.xml— jjwt 0.9.1 requires it on Java 17+Testing & Validation
Backend Test Results
Coverage Summary (Team-Owned Classes)
security(WebSecurityConfig)security.servicessecurity.oauth2security.jwtcontrollerservice(UserRegistrationService)entitypayload.requestpayload.responseTestapi Module
Files Modified
New Files
.env.exampleauth/.dockerignore,user/.dockerignorebackend/.../service/UserRegistrationService.javabackend/.../service/GatlingExecutionService.javabackend/.../service/TestResultService.javabackend/.../controller/TestResultController.javabackend/.../entity/TestRun.javabackend/.../entity/TestCaseResult.javabackend/.../repository/TestRunRepository.javabackend/.../repository/TestCaseResultRepository.javatestapi/src/test/.../JsonComparatorTest.javaModified Files
.gitignore.envpatternsdocker-compose-local-test.ymlstart-taf-local.ps1backend/pom.xmlbackend/.../AuthController.javabackend/.../GatlingApiController.javabackend/.../TestAutomationFrameworkApplication.java@EnableDiscoveryClientbackend/src/main/resources/application.ymlbackend/.../AuthControllerTest.javabackend/.../WebSecurityConfigTest.javatestapi/pom.xmltestapi/Dockerfilefrontend/Dockerfilefrontend/nginx.conffrontend/envsubst-file.shfrontend/.../auth.interceptor.tsfrontend/.../gherkin-parser.service.tsfrontend/.../test-api.service.tsfrontend/.../test-api.component.tsfrontend/src/app/models/test-model.tsfrontend/src/app/models/testmodel2.tsauth/Dockerfileauth/src/main/resources/application.ymluser/Dockerfile.localuser/src/main/resources/application.ymlREADME.mdTEST-REPORT.mdRenamed Files (testapi package migration)
org/requests/RequestController.javaca/etsmtl/taf/testapi/TestRequestExecutor.javaorg/requests/TestApiController.javaca/etsmtl/taf/testapi/TestApiController.javaorg/requests/Method.javaca/etsmtl/taf/testapi/Method.javaorg/requests/TestManager.javaca/etsmtl/taf/testapi/TestManager.javaorg/config/DevCorsConfiguration.javaca/etsmtl/taf/testapi/config/DevCorsConfiguration.javaorg/config/TimeoutConfig.javaca/etsmtl/taf/testapi/config/TimeoutConfig.javaorg/requests/payload/request/Answer.javaca/etsmtl/taf/testapi/payload/request/Answer.javaorg/requests/payload/request/TestApiRequest.javaca/etsmtl/taf/testapi/payload/request/TestApiRequest.javaorg/utils/JsonComparator.javaca/etsmtl/taf/testapi/util/JsonComparator.javaDeleted Files
backend/.../eureka/EurekaItem.java