Docker - #38
Closed
JinethBosilu wants to merge 2 commits into
Closed
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates runtime configuration to better support containerized/deployed environments by centralizing CORS behavior and making JWT signing key handling more tolerant of environment-provided secrets.
Changes:
- Add
cors.allowed-originsproperty (backed byCORS_ALLOWED_ORIGINS) and wire it into CORS configuration. - Remove per-controller
@CrossOriginannotations in favor of global CORS configuration. - Add fallback handling for non-Base64 JWT secrets when deriving the signing key.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/main/resources/application.properties |
Adds configurable cors.allowed-origins property with localhost defaults. |
src/main/java/com/crimeLink/analyzer/service/JwtService.java |
Attempts to support non-Base64 jwt.secret by deriving a stable HMAC key. |
src/main/java/com/crimeLink/analyzer/controller/WeaponIssueController.java |
Removes controller-level @CrossOrigin. |
src/main/java/com/crimeLink/analyzer/controller/WeaponController.java |
Removes controller-level @CrossOrigin. |
src/main/java/com/crimeLink/analyzer/controller/VehicleController.java |
Removes controller-level @CrossOrigin. |
src/main/java/com/crimeLink/analyzer/controller/SafetyLocationController.java |
Removes controller-level @CrossOrigin and related import. |
src/main/java/com/crimeLink/analyzer/controller/MobileDutyController.java |
Removes controller-level @CrossOrigin. |
src/main/java/com/crimeLink/analyzer/controller/LeaveController.java |
Removes controller-level @CrossOrigin. |
src/main/java/com/crimeLink/analyzer/controller/DutyScheduleController.java |
Removes controller-level @CrossOrigin. |
src/main/java/com/crimeLink/analyzer/controller/DutyRecommendationController.java |
Removes controller-level @CrossOrigin. |
src/main/java/com/crimeLink/analyzer/controller/CrimeReportController.java |
Removes controller-level @CrossOrigin and related import. |
src/main/java/com/crimeLink/analyzer/controller/BulletController.java |
Removes controller-level @CrossOrigin. |
src/main/java/com/crimeLink/analyzer/config/SecurityConfig.java |
Configures Spring Security CORS using cors.allowed-origins parsing. |
src/main/java/com/crimeLink/analyzer/config/CorsConfig.java |
Configures a CorsFilter using cors.allowed-origins parsing. |
.env.example |
Documents new CORS_ALLOWED_ORIGINS and Python service URL env vars. |
Comments suppressed due to low confidence (2)
src/main/java/com/crimeLink/analyzer/config/SecurityConfig.java:51
- CORS is being configured in two places: (1) Spring Security via
.cors(cors -> cors.configurationSource(corsConfigurationSource()))and (2) a separateCorsFilterbean inCorsConfig. Having both active can lead to duplicate/conflicting CORS headers (e.g., differentexposedHeaders), and makes it unclear which config is authoritative. Consider removing one of these and keeping a single shared CORS configuration source.
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
http
.csrf(csrf -> csrf.disable())
// CRITICAL FIX: Enable CORS using the bean configuration
.cors(cors -> cors.configurationSource(corsConfigurationSource()))
.authorizeHttpRequests(auth -> auth
src/main/java/com/crimeLink/analyzer/config/CorsConfig.java:32
- This
CorsFilterbean duplicates the CORS configuration already defined inSecurityConfig#corsConfigurationSource()(including separate parsing logic and different exposed headers). Running both can cause inconsistent or duplicated CORS response headers. Prefer a single CORS configuration approach (either keep the Security CORS config and delete this filter, or vice versa) and centralizeparseAllowedOriginsin one place.
@Bean
public CorsFilter corsFilter() {
CorsConfiguration config = new CorsConfiguration();
// Allow credentials (cookies, authorization headers, etc.)
config.setAllowCredentials(true);
// Use allowedOriginPatterns instead of allowedOrigins when credentials are enabled
config.setAllowedOriginPatterns(parseAllowedOrigins(allowedOrigins));
config.setAllowedHeaders(List.of("*"));
// Allow specific HTTP methods
config.setAllowedMethods(Arrays.asList(
"GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH"
));
💡 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
78
to
104
| private Claims extractAllClaims(String token) { | ||
| return Jwts | ||
| .parser() | ||
| .verifyWith(Keys.hmacShaKeyFor(Decoders.BASE64.decode(secretKey))) | ||
| .build() | ||
| .parseSignedClaims(token) | ||
| .getPayload(); | ||
| } | ||
|
|
||
| private Key getSignInKey() { | ||
| byte[] keyBytes = Decoders.BASE64.decode(secretKey); | ||
| byte[] keyBytes; | ||
| try { | ||
| keyBytes = Decoders.BASE64.decode(secretKey); | ||
| } catch (IllegalArgumentException ex) { | ||
| // Fallback for non-base64 secrets: derive a stable 32-byte key | ||
| byte[] raw = secretKey.getBytes(StandardCharsets.UTF_8); | ||
| if (raw.length < 32) { | ||
| try { | ||
| keyBytes = MessageDigest.getInstance("SHA-256").digest(raw); | ||
| } catch (Exception e) { | ||
| keyBytes = raw; | ||
| } | ||
| } else { | ||
| keyBytes = raw; | ||
| } | ||
| } | ||
| return Keys.hmacShaKeyFor(keyBytes); |
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.
No description provided.