diff --git a/src/main/java/com/example/bankapp/config/SecurityConfig.java b/src/main/java/com/example/bankapp/config/SecurityConfig.java index 4dbd1572..1505447a 100644 --- a/src/main/java/com/example/bankapp/config/SecurityConfig.java +++ b/src/main/java/com/example/bankapp/config/SecurityConfig.java @@ -27,7 +27,6 @@ public static PasswordEncoder passwordEncoder() { @Bean public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { http - .csrf(csrf -> csrf.disable()) .authorizeHttpRequests(authz -> authz .requestMatchers("/register").permitAll() .anyRequest().authenticated() @@ -41,7 +40,7 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti .logout(logout -> logout .invalidateHttpSession(true) .clearAuthentication(true) - .logoutRequestMatcher(new AntPathRequestMatcher("/logout")) + .logoutRequestMatcher(new AntPathRequestMatcher("/logout", "POST")) .logoutSuccessUrl("/login?logout") .permitAll() ) diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 08663a63..fad0b857 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -9,3 +9,7 @@ spring.datasource.driver-class-name=com.mysql.cj.jdbc.Driver spring.jpa.hibernate.ddl-auto=update spring.jpa.properties.hibernate.dialect=org.hibernate.dialect.MySQL8Dialect spring.jpa.show-sql=true + +# Session cookie hardening +server.servlet.session.cookie.same-site=lax +server.servlet.session.cookie.http-only=true diff --git a/src/main/resources/templates/dashboard.html b/src/main/resources/templates/dashboard.html index 23a0cc7f..1101535d 100644 --- a/src/main/resources/templates/dashboard.html +++ b/src/main/resources/templates/dashboard.html @@ -26,7 +26,8 @@ border-bottom: 2px solid #B08D57; } - .navbar-custom a { + .navbar-custom a, + .navbar-custom .btn-link { color: #ffffff !important; } @@ -107,6 +108,7 @@ text-decoration: underline; } + @@ -141,7 +148,7 @@

Account Details

Deposit
-
+
@@ -157,7 +164,7 @@

Account Details

Withdraw
- +
@@ -173,7 +180,7 @@

Account Details

Transfer Money
- +
diff --git a/src/main/resources/templates/login.html b/src/main/resources/templates/login.html index 7b40190d..412a68eb 100644 --- a/src/main/resources/templates/login.html +++ b/src/main/resources/templates/login.html @@ -105,7 +105,7 @@

Login

- +
diff --git a/src/main/resources/templates/register.html b/src/main/resources/templates/register.html index e410b74e..7555094b 100644 --- a/src/main/resources/templates/register.html +++ b/src/main/resources/templates/register.html @@ -105,7 +105,7 @@

Register a New Account

- +
diff --git a/src/main/resources/templates/transactions.html b/src/main/resources/templates/transactions.html index 307bc5de..85d4a28e 100644 --- a/src/main/resources/templates/transactions.html +++ b/src/main/resources/templates/transactions.html @@ -26,7 +26,8 @@ border-bottom: 2px solid #B08D57; } - .navbar-custom a { + .navbar-custom a, + .navbar-custom .btn-link { color: #ffffff !important; } @@ -86,6 +87,7 @@ text-decoration: underline; } + diff --git a/src/test/java/com/example/bankapp/controller/BankControllerCsrfTest.java b/src/test/java/com/example/bankapp/controller/BankControllerCsrfTest.java new file mode 100644 index 00000000..308c10c6 --- /dev/null +++ b/src/test/java/com/example/bankapp/controller/BankControllerCsrfTest.java @@ -0,0 +1,118 @@ +package com.example.bankapp.controller; + +import com.example.bankapp.config.SecurityConfig; +import com.example.bankapp.model.Account; +import com.example.bankapp.service.AccountService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.web.servlet.MockMvc; + +import java.math.BigDecimal; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@WebMvcTest(BankController.class) +@Import(SecurityConfig.class) +class BankControllerCsrfTest { + + @Autowired + private MockMvc mockMvc; + + @MockBean + private AccountService accountService; + + private final Account account = new Account(); + + @BeforeEach + void setUp() { + account.setUsername("alice"); + account.setBalance(new BigDecimal("1000")); + when(accountService.findAccountByUsername("alice")).thenReturn(account); + } + + @Test + @WithMockUser(username = "alice") + void transferWithoutCsrfTokenIsForbidden() throws Exception { + mockMvc.perform(post("/transfer").param("toUsername", "mallory").param("amount", "500")) + .andExpect(status().isForbidden()); + verify(accountService, never()).transferAmount(any(), anyString(), any()); + } + + @Test + @WithMockUser(username = "alice") + void transferWithCsrfTokenSucceeds() throws Exception { + mockMvc.perform(post("/transfer").param("toUsername", "bob").param("amount", "500").with(csrf())) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("/dashboard")); + verify(accountService).transferAmount(account, "bob", new BigDecimal("500")); + } + + @Test + @WithMockUser(username = "alice") + void withdrawWithoutCsrfTokenIsForbidden() throws Exception { + mockMvc.perform(post("/withdraw").param("amount", "500")) + .andExpect(status().isForbidden()); + verify(accountService, never()).withdraw(any(), any()); + } + + @Test + @WithMockUser(username = "alice") + void withdrawWithCsrfTokenSucceeds() throws Exception { + mockMvc.perform(post("/withdraw").param("amount", "500").with(csrf())) + .andExpect(redirectedUrl("/dashboard")); + verify(accountService).withdraw(account, new BigDecimal("500")); + } + + @Test + @WithMockUser(username = "alice") + void depositWithoutCsrfTokenIsForbidden() throws Exception { + mockMvc.perform(post("/deposit").param("amount", "500")) + .andExpect(status().isForbidden()); + verify(accountService, never()).deposit(any(), any()); + } + + @Test + @WithMockUser(username = "alice") + void depositWithCsrfTokenSucceeds() throws Exception { + mockMvc.perform(post("/deposit").param("amount", "500").with(csrf())) + .andExpect(redirectedUrl("/dashboard")); + verify(accountService).deposit(account, new BigDecimal("500")); + } + + @Test + @WithMockUser(username = "alice") + void dashboardFormsCarryCsrfToken() throws Exception { + mockMvc.perform(get("/dashboard")) + .andExpect(status().isOk()) + .andExpect(content().string(org.hamcrest.Matchers.containsString("name=\"_csrf\""))); + } + + @Test + @WithMockUser(username = "alice") + void logoutViaGetIsNotHandled() throws Exception { + mockMvc.perform(get("/logout")) + .andExpect(status().isNotFound()); + } + + @Test + @WithMockUser(username = "alice") + void logoutViaPostWithCsrfTokenSucceeds() throws Exception { + mockMvc.perform(post("/logout").with(csrf())) + .andExpect(redirectedUrl("/login?logout")); + } +}