Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package ca.gc.tbs.config;

import java.io.IOException;
import java.net.URI;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
Expand All @@ -31,7 +32,9 @@ public void onAuthenticationSuccess(

SavedRequest savedRequest = requestCache.getRequest(request, response);

if (savedRequest == null || savedRequest.getRedirectUrl().contains("signin")) {
if (savedRequest == null
|| savedRequest.getRedirectUrl().contains("signin")
|| !isSameOrigin(savedRequest.getRedirectUrl(), request)) {
for (GrantedAuthority auth : authentication.getAuthorities()) {
if ("ADMIN".equals(auth.getAuthority())) {
response.sendRedirect("/u/index");
Expand All @@ -52,4 +55,16 @@ public void onAuthenticationSuccess(
public void setRequestCache(RequestCache requestCache) {
this.requestCache = requestCache;
}

private boolean isSameOrigin(String url, HttpServletRequest request) {
if (url.startsWith("/")) {
return true;
}
try {
URI uri = URI.create(url);
return !uri.isAbsolute() || request.getServerName().equalsIgnoreCase(uri.getHost());
} catch (IllegalArgumentException e) {
return false;
}
}
}
7 changes: 5 additions & 2 deletions src/main/java/ca/gc/tbs/config/WebSecurityConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
Expand All @@ -18,6 +19,7 @@

@Configuration
@EnableWebSecurity
@EnableMethodSecurity
public class WebSecurityConfig {

private final CustomizeAuthenticationSuccessHandler customizeAuthenticationSuccessHandler;
Expand All @@ -34,13 +36,14 @@ public WebSecurityConfig(
@Bean
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
http
.csrf(csrf -> csrf.disable())
.csrf(csrf -> csrf
.ignoringRequestMatchers("/authenticate"))
.authorizeHttpRequests(auth -> auth
.requestMatchers("/createApiUser").hasAuthority("ADMIN")
.requestMatchers("/authenticate").permitAll()
.requestMatchers("/actuator/health").permitAll()
.requestMatchers("/api/user/**").hasRole("USER")
.requestMatchers("/", "/checkExists", "/error", "/enableAdmin", "/login", "/signup", "/success").permitAll()
.requestMatchers("/", "/checkExists", "/error", "/login", "/signup", "/success").permitAll()
.requestMatchers("/u/**").hasAnyAuthority("ADMIN")
.requestMatchers("/keywords/**").hasAnyAuthority("ADMIN")
.requestMatchers("/python/**", "/reports/**", "/dashboard/**").hasAnyAuthority("USER", "ADMIN")
Expand Down
11 changes: 5 additions & 6 deletions src/main/java/ca/gc/tbs/controller/AuthController.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,17 @@ public ResponseEntity<String> createAuthenticationToken(@RequestBody AuthRequest
}
}

@GetMapping("/createApiUser")
public ResponseEntity<String> createApiUser(
@RequestParam String username, @RequestParam String password) {
@PostMapping("/createApiUser")
public ResponseEntity<String> createApiUser(@RequestBody CreateUserRequest body) {
// Check if user already exists
User existingUser = userService.findUserByEmail(username);
User existingUser = userService.findUserByEmail(body.username());
if (existingUser != null) {
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("User already exists.");
}

User user = new User();
user.setEmail(username);
user.setPassword(password);
user.setEmail(body.username());
user.setPassword(body.password());
user.setEnabled(true);

Role apiRole = userService.findRoleByName("API");
Expand Down
10 changes: 7 additions & 3 deletions src/main/java/ca/gc/tbs/controller/UserController.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,24 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.util.HtmlUtils;
import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.servlet.ModelAndView;
import org.springframework.web.servlet.View;
import org.springframework.web.servlet.mvc.support.RedirectAttributes;
import org.springframework.web.servlet.view.RedirectView;
import org.springframework.security.access.prepost.PreAuthorize;

@Controller
@PreAuthorize("hasAuthority('ADMIN')")
public class UserController {

private static final Logger LOG = LoggerFactory.getLogger(UserController.class);

@Autowired private UserService service;

@GetMapping(value = "/u/update")
@PostMapping(value = "/u/update")
public @ResponseBody String updateUser(HttpServletRequest request) {
try {
this.service.enable(request.getParameter("id"));
Expand All @@ -33,7 +37,7 @@ public class UserController {
}
}

@GetMapping(value = "/u/delete")
@PostMapping(value = "/u/delete")
public @ResponseBody String deleteUser(HttpServletRequest request) {
try {
this.service.deleteUserById(request.getParameter("id"));
Expand Down Expand Up @@ -100,7 +104,7 @@ public String getData(String lang) {
</div>
</td>
</tr>""".formatted(
email, institution, roles, dateCreated, status,
HtmlUtils.htmlEscape(email), HtmlUtils.htmlEscape(institution), HtmlUtils.htmlEscape(roles.toString()), dateCreated, status,
toggleIdPrefix, id, toggleClass, toggleLabel,
id, deleteLabel));
}
Expand Down
67 changes: 38 additions & 29 deletions src/main/java/ca/gc/tbs/filter/GcIpFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ public class GcIpFilter implements Filter {
@Value("${gc.ip.filter.whitelist.file:}")
private String whitelistFilePath;

// Deployment-specific: controls how the real client IP is extracted.
// X_REAL_IP — nginx/Kubernetes ingress (default for this repo)
// X_FORWARDED_FOR_LAST — AWS ALB/ECS: add GC_IP_FILTER_CLIENT_IP_SOURCE=X_FORWARDED_FOR_LAST
// to container_environment in terragrunt/aws/ecs/ecs.tf when porting
// REMOTE_ADDR — no proxy (direct connection / local)
@Value("${gc.ip.filter.client-ip-source:X_REAL_IP}")
private String clientIpSource;

private Set<String> fileWhitelistIps = new HashSet<>();

@Override
Expand Down Expand Up @@ -105,37 +113,37 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
}

/**
* Extract client IP address from request
* Handles X-Forwarded-For header for requests behind load balancer
* Extract the real client IP using the strategy configured by gc.ip.filter.client-ip-source.
*
* X-Forwarded-For is intentionally NOT used as a generic source: it is a client-controlled
* header and trusting it directly allows an attacker to spoof any IP.
*/
private String getClientIpAddress(HttpServletRequest request) {
String[] headerCandidates = {
"X-Forwarded-For",
"X-Real-IP",
"Proxy-Client-IP",
"WL-Proxy-Client-IP",
"HTTP_X_FORWARDED_FOR",
"HTTP_X_FORWARDED",
"HTTP_X_CLUSTER_CLIENT_IP",
"HTTP_CLIENT_IP",
"HTTP_FORWARDED_FOR",
"HTTP_FORWARDED",
"HTTP_VIA",
"REMOTE_ADDR"
};

for (String header : headerCandidates) {
String ip = request.getHeader(header);
if (ip != null && !ip.isEmpty() && !"unknown".equalsIgnoreCase(ip)) {
// X-Forwarded-For can contain multiple IPs, take the first one
if (ip.contains(",")) {
ip = ip.split(",")[0].trim();
return switch (clientIpSource) {
case "X_REAL_IP" -> {
// X-Real-IP is set by nginx ingress to the verified TCP source IP.
// It is not forwarded from the client and cannot be spoofed.
String xRealIp = request.getHeader("X-Real-IP");
yield (xRealIp != null && !xRealIp.trim().isEmpty() && !"unknown".equalsIgnoreCase(xRealIp.trim()))
? xRealIp.trim()
: request.getRemoteAddr();
}
case "X_FORWARDED_FOR_LAST" -> {
// AWS ALB always appends the real client IP as the rightmost entry.
// Taking the last value is safe because only the ALB can append to the right.
String xff = request.getHeader("X-Forwarded-For");
if (xff != null && !xff.isEmpty()) {
String[] parts = xff.split(",");
String last = parts[parts.length - 1].trim();
if (!last.isEmpty() && !"unknown".equalsIgnoreCase(last)) {
yield last;
}
}
return ip;
yield request.getRemoteAddr();
}
}

return request.getRemoteAddr();
// REMOTE_ADDR or any unrecognised value — direct connection, no proxy.
default -> request.getRemoteAddr();
};
}

/**
Expand Down Expand Up @@ -215,8 +223,9 @@ public void init(FilterConfig filterConfig) throws ServletException {
// Load IPs from file if configured
loadWhitelistFromFile();

logger.info("GC IP Filter initialized - filter enabled: {}, property whitelist: {}, file whitelist: {} IPs",
filterEnabled,
logger.info("GC IP Filter initialized - filter enabled: {}, client-ip-source: {}, property whitelist: {}, file whitelist: {} IPs",
filterEnabled,
clientIpSource,
whitelistIps != null && !whitelistIps.isEmpty() ? "configured" : "none",
fileWhitelistIps.size());
}
Expand Down
6 changes: 2 additions & 4 deletions src/main/java/ca/gc/tbs/security/JWTUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@ public class JWTUtil {

private SecretKey getSigningKey() {
byte[] keyBytes = SECRET_KEY.getBytes(StandardCharsets.UTF_8);
// Ensure key is at least 256 bits (32 bytes) for HS256
if (keyBytes.length < 32) {
byte[] paddedKey = new byte[32];
System.arraycopy(keyBytes, 0, paddedKey, 0, keyBytes.length);
return Keys.hmacShaKeyFor(paddedKey);
throw new IllegalStateException(
"jwt.secret.key must be at least 32 characters; got " + keyBytes.length);
}
return Keys.hmacShaKeyFor(keyBytes);
}
Expand Down
Binary file modified src/main/resources/application.properties.gpg
Binary file not shown.
13 changes: 10 additions & 3 deletions src/main/resources/templates/users_en.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,18 @@
</th:block>
<th:block layout:fragment="script">
<script th:inline="javascript">
var _csrfToken = /*[[${_csrf.token}]]*/ null;
var _csrfHeader = /*[[${_csrf.headerName}]]*/ null;
$.ajaxSetup({
beforeSend: function (xhr) {
xhr.setRequestHeader(_csrfHeader, _csrfToken);
}
});
$(document).on("wb-ready.wb", function () {
$(".deleteBtn").on("click", function (e) {
var id = $(e.target).attr("id").replace("delete", "");
$.ajax({
type: "get",
type: "post",
data: {
"id": id
},
Expand All @@ -44,7 +51,7 @@
$("body").on("click", ".enableBtn", function (e) {
var id = $(e.target).attr("id").replace("enable", "");
$.ajax({
type: "get",
type: "post",
data: {
"id": id,
"enabled": true
Expand All @@ -68,7 +75,7 @@
$("body").on("click", ".disableBtn", function (e) {
var id = $(e.target).attr("id").replace("disable", "");
$.ajax({
type: "get",
type: "post",
data: {
"id": id,
"enabled": false
Expand Down
13 changes: 10 additions & 3 deletions src/main/resources/templates/users_fr.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,18 @@
</th:block>
<th:block layout:fragment="script">
<script th:inline="javascript">
var _csrfToken = /*[[${_csrf.token}]]*/ null;
var _csrfHeader = /*[[${_csrf.headerName}]]*/ null;
$.ajaxSetup({
beforeSend: function (xhr) {
xhr.setRequestHeader(_csrfHeader, _csrfToken);
}
});
$(document).on("wb-ready.wb", function () {
$(".deleteBtn").on("click", function (e) {
var id = $(e.target).attr("id").replace("delete", "");
$.ajax({
type: "get",
type: "post",
data: {
"id": id
},
Expand All @@ -44,7 +51,7 @@
$("body").on("click", ".enableBtn", function (e) {
var id = $(e.target).attr("id").replace("enable", "");
$.ajax({
type: "get",
type: "post",
data: {
"id": id,
"enabled": true
Expand All @@ -68,7 +75,7 @@
$("body").on("click", ".disableBtn", function (e) {
var id = $(e.target).attr("id").replace("disable", "");
$.ajax({
type: "get",
type: "post",
data: {
"id": id,
"enabled": false
Expand Down
Loading
Loading