fix platform security and SMTP boundaries
This commit is contained in:
@@ -9,6 +9,7 @@ import org.springframework.security.crypto.factory.PasswordEncoderFactories;
|
|||||||
import org.springframework.security.crypto.password.PasswordEncoder;
|
import org.springframework.security.crypto.password.PasswordEncoder;
|
||||||
import org.springframework.security.web.SecurityFilterChain;
|
import org.springframework.security.web.SecurityFilterChain;
|
||||||
import org.springframework.security.web.access.intercept.AuthorizationFilter;
|
import org.springframework.security.web.access.intercept.AuthorizationFilter;
|
||||||
|
import org.springframework.security.web.header.writers.ReferrerPolicyHeaderWriter.ReferrerPolicy;
|
||||||
|
|
||||||
@Configuration(proxyBeanMethods = false)
|
@Configuration(proxyBeanMethods = false)
|
||||||
class SecurityConfiguration {
|
class SecurityConfiguration {
|
||||||
@@ -27,10 +28,13 @@ class SecurityConfiguration {
|
|||||||
throws Exception {
|
throws Exception {
|
||||||
return http
|
return http
|
||||||
.authorizeHttpRequests(authorize -> authorize
|
.authorizeHttpRequests(authorize -> authorize
|
||||||
.requestMatchers("/bootstrap/**", "/activate/**", "/login", "/error", "/actuator/health")
|
.requestMatchers(
|
||||||
|
"/bootstrap/**", "/activate/**", "/login", "/error", "/assets/**",
|
||||||
|
"/actuator/health")
|
||||||
.permitAll()
|
.permitAll()
|
||||||
.requestMatchers("/admin/**").hasRole("ADMIN")
|
.requestMatchers("/admin/**").hasRole("ADMIN")
|
||||||
.anyRequest().authenticated())
|
.anyRequest().authenticated())
|
||||||
|
.headers(headers -> headers.referrerPolicy(policy -> policy.policy(ReferrerPolicy.NO_REFERRER)))
|
||||||
.formLogin(form -> form.loginPage("/login").defaultSuccessUrl("/", true))
|
.formLogin(form -> form.loginPage("/login").defaultSuccessUrl("/", true))
|
||||||
.logout(logout -> logout.logoutSuccessUrl("/login?logout"))
|
.logout(logout -> logout.logoutSuccessUrl("/login?logout"))
|
||||||
.addFilterBefore(bootstrapAccessFilter, AuthorizationFilter.class)
|
.addFilterBefore(bootstrapAccessFilter, AuthorizationFilter.class)
|
||||||
|
|||||||
+1
-1
@@ -29,7 +29,7 @@ public class BootstrapAccessFilter extends OncePerRequestFilter {
|
|||||||
|
|
||||||
private static boolean allowedBeforeBootstrap(String path) {
|
private static boolean allowedBeforeBootstrap(String path) {
|
||||||
return path.equals("/bootstrap") || path.startsWith("/bootstrap/")
|
return path.equals("/bootstrap") || path.startsWith("/bootstrap/")
|
||||||
|| path.equals("/actuator/health") || path.startsWith("/bootstrap-assets/")
|
|| path.equals("/actuator/health") || path.startsWith("/assets/")
|
||||||
|| path.equals("/error");
|
|| path.equals("/error");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -113,6 +113,14 @@ public class AccountService {
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Moves an active Intern's internship from {@code NOT_STARTED} to {@code ACTIVE} once the configured
|
||||||
|
* internship start date has arrived in the application's business timezone.
|
||||||
|
*
|
||||||
|
* @param internUserId Intern account whose internship should start
|
||||||
|
* @param adminId active Admin authorizing the state transition
|
||||||
|
* @throws IllegalStateException when the internship start date has not arrived
|
||||||
|
*/
|
||||||
@Transactional
|
@Transactional
|
||||||
public void activateInternship(long internUserId, long adminId) {
|
public void activateInternship(long internUserId, long adminId) {
|
||||||
AppUser admin = users.findById(adminId)
|
AppUser admin = users.findById(adminId)
|
||||||
@@ -124,9 +132,12 @@ public class AccountService {
|
|||||||
if (intern.getGlobalRole() != GlobalRole.INTERN || intern.getAccountStatus() != AccountStatus.ACTIVE) {
|
if (intern.getGlobalRole() != GlobalRole.INTERN || intern.getAccountStatus() != AccountStatus.ACTIVE) {
|
||||||
throw new IllegalArgumentException("An active Intern account is required");
|
throw new IllegalArgumentException("An active Intern account is required");
|
||||||
}
|
}
|
||||||
internProfiles.findForUpdateByUserId(internUserId)
|
InternProfile profile = internProfiles.findForUpdateByUserId(internUserId)
|
||||||
.orElseThrow(() -> new IllegalArgumentException("Intern profile not found"))
|
.orElseThrow(() -> new IllegalArgumentException("Intern profile not found"));
|
||||||
.activate(clock.instant());
|
if (LocalDate.now(clock).isBefore(profile.getInternshipStartDate())) {
|
||||||
|
throw new IllegalStateException("Internship cannot activate before its start date");
|
||||||
|
}
|
||||||
|
profile.activate(clock.instant());
|
||||||
}
|
}
|
||||||
|
|
||||||
@Transactional(readOnly = true)
|
@Transactional(readOnly = true)
|
||||||
|
|||||||
+39
-7
@@ -1,18 +1,40 @@
|
|||||||
package com.lab.labtimesheet.feature.integration.service;
|
package com.lab.labtimesheet.feature.integration.service;
|
||||||
|
|
||||||
|
import java.io.UnsupportedEncodingException;
|
||||||
|
import java.nio.charset.StandardCharsets;
|
||||||
import java.util.Properties;
|
import java.util.Properties;
|
||||||
|
import java.util.function.Supplier;
|
||||||
|
|
||||||
import com.lab.labtimesheet.feature.integration.model.dto.SmtpConnection;
|
import com.lab.labtimesheet.feature.integration.model.dto.SmtpConnection;
|
||||||
import com.lab.labtimesheet.feature.integration.model.SecurityMode;
|
import com.lab.labtimesheet.feature.integration.model.SecurityMode;
|
||||||
import org.springframework.mail.SimpleMailMessage;
|
import jakarta.mail.MessagingException;
|
||||||
|
import jakarta.mail.internet.MimeMessage;
|
||||||
import org.springframework.mail.javamail.JavaMailSenderImpl;
|
import org.springframework.mail.javamail.JavaMailSenderImpl;
|
||||||
|
import org.springframework.mail.javamail.MimeMessageHelper;
|
||||||
import org.springframework.stereotype.Component;
|
import org.springframework.stereotype.Component;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sends immediate SMTP messages through a freshly configured JavaMail client.
|
||||||
|
* Connections are bounded by finite network timeouts so an Admin test or
|
||||||
|
* activation delivery cannot block a request indefinitely.
|
||||||
|
*/
|
||||||
@Component
|
@Component
|
||||||
class JavaMailSmtpProbe implements SmtpProbe {
|
class JavaMailSmtpProbe implements SmtpProbe {
|
||||||
|
private static final String TIMEOUT_MILLIS = "5000";
|
||||||
|
|
||||||
|
private final Supplier<JavaMailSenderImpl> senderFactory;
|
||||||
|
|
||||||
|
JavaMailSmtpProbe() {
|
||||||
|
this(JavaMailSenderImpl::new);
|
||||||
|
}
|
||||||
|
|
||||||
|
JavaMailSmtpProbe(Supplier<JavaMailSenderImpl> senderFactory) {
|
||||||
|
this.senderFactory = senderFactory;
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void send(SmtpConnection connection, String recipient, String subject, String body) {
|
public void send(SmtpConnection connection, String recipient, String subject, String body) {
|
||||||
JavaMailSenderImpl sender = new JavaMailSenderImpl();
|
JavaMailSenderImpl sender = senderFactory.get();
|
||||||
sender.setHost(connection.host());
|
sender.setHost(connection.host());
|
||||||
sender.setPort(connection.port());
|
sender.setPort(connection.port());
|
||||||
sender.setUsername(connection.username());
|
sender.setUsername(connection.username());
|
||||||
@@ -24,11 +46,21 @@ class JavaMailSmtpProbe implements SmtpProbe {
|
|||||||
} else if (connection.securityMode() == SecurityMode.TLS) {
|
} else if (connection.securityMode() == SecurityMode.TLS) {
|
||||||
sender.setProtocol("smtps");
|
sender.setProtocol("smtps");
|
||||||
}
|
}
|
||||||
SimpleMailMessage message = new SimpleMailMessage();
|
String propertyPrefix = connection.securityMode() == SecurityMode.TLS ? "mail.smtps" : "mail.smtp";
|
||||||
message.setFrom(connection.fromAddress());
|
properties.setProperty(propertyPrefix + ".connectiontimeout", TIMEOUT_MILLIS);
|
||||||
message.setTo(recipient);
|
properties.setProperty(propertyPrefix + ".timeout", TIMEOUT_MILLIS);
|
||||||
message.setSubject(subject);
|
properties.setProperty(propertyPrefix + ".writetimeout", TIMEOUT_MILLIS);
|
||||||
message.setText(body);
|
|
||||||
|
MimeMessage message = sender.createMimeMessage();
|
||||||
|
try {
|
||||||
|
MimeMessageHelper helper = new MimeMessageHelper(message, false, StandardCharsets.UTF_8.name());
|
||||||
|
helper.setFrom(connection.fromAddress(), connection.fromName());
|
||||||
|
helper.setTo(recipient);
|
||||||
|
helper.setSubject(subject);
|
||||||
|
helper.setText(body);
|
||||||
|
} catch (MessagingException | UnsupportedEncodingException exception) {
|
||||||
|
throw new IllegalStateException("Unable to construct SMTP message", exception);
|
||||||
|
}
|
||||||
sender.send(message);
|
sender.send(message);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,55 @@
|
|||||||
|
package com.lab.labtimesheet.config;
|
||||||
|
|
||||||
|
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.anonymous;
|
||||||
|
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
|
||||||
|
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
|
||||||
|
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
|
||||||
|
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
|
||||||
|
|
||||||
|
import com.lab.labtimesheet.feature.account.service.BootstrapService;
|
||||||
|
import org.junit.jupiter.api.Test;
|
||||||
|
import org.springframework.beans.factory.annotation.Autowired;
|
||||||
|
import org.springframework.boot.test.context.SpringBootTest;
|
||||||
|
import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc;
|
||||||
|
import org.springframework.context.annotation.Import;
|
||||||
|
import org.springframework.test.annotation.DirtiesContext;
|
||||||
|
import org.springframework.test.context.ActiveProfiles;
|
||||||
|
import org.springframework.test.web.servlet.MockMvc;
|
||||||
|
|
||||||
|
@Import(TestcontainersConfiguration.class)
|
||||||
|
@SpringBootTest
|
||||||
|
@AutoConfigureMockMvc
|
||||||
|
@ActiveProfiles("test")
|
||||||
|
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)
|
||||||
|
class SecurityResponseIntegrationTest {
|
||||||
|
@Autowired
|
||||||
|
private MockMvc mockMvc;
|
||||||
|
|
||||||
|
@Autowired
|
||||||
|
private BootstrapService bootstrap;
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void assetsRemainPublicBeforeAndAfterBootstrap() throws Exception {
|
||||||
|
mockMvc.perform(get("/assets/review-test.css").with(anonymous()))
|
||||||
|
.andExpect(status().isOk())
|
||||||
|
.andExpect(content().string(org.hamcrest.Matchers.containsString("asset")));
|
||||||
|
|
||||||
|
bootstrap.bootstrap("admin@example.com", "Admin", "correct horse battery staple");
|
||||||
|
|
||||||
|
mockMvc.perform(get("/assets/review-test.css").with(anonymous()))
|
||||||
|
.andExpect(status().isOk())
|
||||||
|
.andExpect(content().string(org.hamcrest.Matchers.containsString("asset")));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void authenticationAndActivationResponsesDoNotSendReferrers() throws Exception {
|
||||||
|
bootstrap.bootstrap("admin@example.com", "Admin", "correct horse battery staple");
|
||||||
|
|
||||||
|
mockMvc.perform(get("/login"))
|
||||||
|
.andExpect(status().isOk())
|
||||||
|
.andExpect(header().string("Referrer-Policy", "no-referrer"));
|
||||||
|
mockMvc.perform(get("/activate").param("token", "non-secret-test-fixture"))
|
||||||
|
.andExpect(status().isOk())
|
||||||
|
.andExpect(header().string("Referrer-Policy", "no-referrer"));
|
||||||
|
}
|
||||||
|
}
|
||||||
+21
@@ -33,11 +33,13 @@ import org.springframework.context.annotation.Bean;
|
|||||||
import org.springframework.context.annotation.Import;
|
import org.springframework.context.annotation.Import;
|
||||||
import org.springframework.context.annotation.Primary;
|
import org.springframework.context.annotation.Primary;
|
||||||
import org.springframework.security.crypto.password.PasswordEncoder;
|
import org.springframework.security.crypto.password.PasswordEncoder;
|
||||||
|
import org.springframework.test.annotation.DirtiesContext;
|
||||||
import org.springframework.test.context.ActiveProfiles;
|
import org.springframework.test.context.ActiveProfiles;
|
||||||
|
|
||||||
@Import({TestcontainersConfiguration.class, AccountActivationIntegrationTest.MailProbeConfiguration.class})
|
@Import({TestcontainersConfiguration.class, AccountActivationIntegrationTest.MailProbeConfiguration.class})
|
||||||
@SpringBootTest
|
@SpringBootTest
|
||||||
@ActiveProfiles("test")
|
@ActiveProfiles("test")
|
||||||
|
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)
|
||||||
class AccountActivationIntegrationTest {
|
class AccountActivationIntegrationTest {
|
||||||
|
|
||||||
@Autowired
|
@Autowired
|
||||||
@@ -138,6 +140,25 @@ class AccountActivationIntegrationTest {
|
|||||||
assertThat(summary.activeInternships()).isEqualTo(1);
|
assertThat(summary.activeInternships()).isEqualTo(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void internshipCannotActivateBeforeItsBusinessStartDate() {
|
||||||
|
bootstrap.bootstrap("admin@example.com", "Admin", "correct horse battery staple");
|
||||||
|
long adminId = accounts.requireActiveAdminId("admin@example.com");
|
||||||
|
activateSmtp(adminId);
|
||||||
|
mail.messages.clear();
|
||||||
|
|
||||||
|
var creation = accounts.create(new CreateAccountCommand(
|
||||||
|
"future-intern@example.com", "Future Intern", GlobalRole.INTERN, "STU-FUTURE",
|
||||||
|
LocalDate.of(2026, 8, 15), LocalDate.of(2026, 12, 31)), adminId);
|
||||||
|
assertThat(accounts.activate(mail.onlyActivationToken(), "future secure password")).isTrue();
|
||||||
|
|
||||||
|
assertThatThrownBy(() -> accounts.activateInternship(creation.userId(), adminId))
|
||||||
|
.isInstanceOf(IllegalStateException.class)
|
||||||
|
.hasMessageContaining("start date");
|
||||||
|
assertThat(internProfiles.findById(creation.userId()).orElseThrow().getInternshipStatus())
|
||||||
|
.isEqualTo(InternshipStatus.NOT_STARTED);
|
||||||
|
}
|
||||||
|
|
||||||
private void activateSmtp(long adminId) {
|
private void activateSmtp(long adminId) {
|
||||||
long draftId = smtp.saveDraft(adminId, new SmtpDraft(
|
long draftId = smtp.saveDraft(adminId, new SmtpDraft(
|
||||||
"mailpit", 1025, SecurityMode.NONE, null, null, "admin@example.com", "Lab Timesheet"));
|
"mailpit", 1025, SecurityMode.NONE, null, null, "admin@example.com", "Lab Timesheet"));
|
||||||
|
|||||||
+58
@@ -0,0 +1,58 @@
|
|||||||
|
package com.lab.labtimesheet.feature.integration.service;
|
||||||
|
|
||||||
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
|
|
||||||
|
import com.lab.labtimesheet.feature.integration.model.SecurityMode;
|
||||||
|
import com.lab.labtimesheet.feature.integration.model.dto.SmtpConnection;
|
||||||
|
import jakarta.mail.internet.InternetAddress;
|
||||||
|
import jakarta.mail.internet.MimeMessage;
|
||||||
|
import org.junit.jupiter.api.Test;
|
||||||
|
import org.springframework.mail.javamail.JavaMailSenderImpl;
|
||||||
|
|
||||||
|
class JavaMailSmtpProbeTest {
|
||||||
|
@Test
|
||||||
|
void appliesFiniteTimeoutsAndConfiguredFromName() throws Exception {
|
||||||
|
var sender = new CapturingMailSender();
|
||||||
|
var probe = new JavaMailSmtpProbe(() -> sender);
|
||||||
|
var connection = new SmtpConnection(
|
||||||
|
"smtp.example.com", 587, SecurityMode.STARTTLS, "user", "password",
|
||||||
|
"noreply@example.com", "Lab Timesheet");
|
||||||
|
|
||||||
|
probe.send(connection, "admin@example.com", "Subject", "Body");
|
||||||
|
|
||||||
|
assertThat(sender.getJavaMailProperties())
|
||||||
|
.containsEntry("mail.smtp.connectiontimeout", "5000")
|
||||||
|
.containsEntry("mail.smtp.timeout", "5000")
|
||||||
|
.containsEntry("mail.smtp.writetimeout", "5000");
|
||||||
|
var from = (InternetAddress) sender.message.getFrom()[0];
|
||||||
|
assertThat(from.getAddress()).isEqualTo("noreply@example.com");
|
||||||
|
assertThat(from.getPersonal()).isEqualTo("Lab Timesheet");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void appliesFiniteTimeoutsToImplicitTlsTransport() {
|
||||||
|
var sender = new CapturingMailSender();
|
||||||
|
var probe = new JavaMailSmtpProbe(() -> sender);
|
||||||
|
var connection = new SmtpConnection(
|
||||||
|
"smtp.example.com", 465, SecurityMode.TLS, null, null,
|
||||||
|
"noreply@example.com", "Lab Timesheet");
|
||||||
|
|
||||||
|
probe.send(connection, "admin@example.com", "Subject", "Body");
|
||||||
|
|
||||||
|
assertThat(sender.getProtocol()).isEqualTo("smtps");
|
||||||
|
assertThat(sender.getJavaMailProperties())
|
||||||
|
.containsEntry("mail.smtps.connectiontimeout", "5000")
|
||||||
|
.containsEntry("mail.smtps.timeout", "5000")
|
||||||
|
.containsEntry("mail.smtps.writetimeout", "5000");
|
||||||
|
}
|
||||||
|
|
||||||
|
static final class CapturingMailSender extends JavaMailSenderImpl {
|
||||||
|
private MimeMessage message;
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void send(MimeMessage... mimeMessages) {
|
||||||
|
assertThat(mimeMessages).hasSize(1);
|
||||||
|
message = mimeMessages[0];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
asset
|
||||||
Reference in New Issue
Block a user