fix(user): fail-closed when admin seed would use dev defaults outside dev/test/e2e
Some checks failed
Some checks failed
Addresses Nora's review concern on #513/#516. The previous fix only made env-vars take effect — it did NOT close the fail-open default path. If an operator forgets APP_ADMIN_USERNAME / APP_ADMIN_PASSWORD on first prod boot, the seeded admin is the well-known `admin@familienarchiv.local` / `admin123` and is permanently locked (UserDataInitializer only seeds when the row is missing). Refuse to seed outside dev/test/e2e profiles when either credential matches the documented default. The startup fails fast with a clear message pointing at the env-var names and the permanence trap. Also adds Markus/Felix/Sara's "pin the Java side" coverage: a reflection test on the @Value placeholder catches a future rename of `${app.admin.email:...}` back to `${app.admin.username:...}`, which would otherwise pass the yaml-side test but silently break the binding. Tests: - AdminSeedFailClosedTest pins fail-closed for non-local profiles and verifies the dev/test/e2e bypass. - AdminSeedPropertyKeyTest now also asserts the @Value placeholder string on UserDataInitializer.adminEmail/adminPassword. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit was merged in pull request #516.
This commit is contained in:
@@ -20,6 +20,7 @@ import org.springframework.boot.CommandLineRunner;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.context.annotation.Profile;
|
||||
import org.springframework.core.env.Environment;
|
||||
import org.springframework.security.crypto.password.PasswordEncoder;
|
||||
|
||||
import java.time.LocalDate;
|
||||
@@ -31,19 +32,39 @@ import java.util.Set;
|
||||
@DependsOn("flyway")
|
||||
public class UserDataInitializer {
|
||||
|
||||
@Value("${app.admin.email:admin@familyarchive.local}")
|
||||
static final String DEFAULT_ADMIN_EMAIL = "admin@familienarchiv.local";
|
||||
static final String DEFAULT_ADMIN_PASSWORD = "admin123";
|
||||
|
||||
@Value("${app.admin.email:" + DEFAULT_ADMIN_EMAIL + "}")
|
||||
private String adminEmail;
|
||||
|
||||
@Value("${app.admin.password:admin123}")
|
||||
@Value("${app.admin.password:" + DEFAULT_ADMIN_PASSWORD + "}")
|
||||
private String adminPassword;
|
||||
|
||||
private final AppUserRepository userRepository;
|
||||
private final UserGroupRepository groupRepository;
|
||||
private final Environment environment;
|
||||
|
||||
@Bean
|
||||
public CommandLineRunner initAdminUser(PasswordEncoder passwordEncoder) {
|
||||
return args -> {
|
||||
if (userRepository.findByEmail(adminEmail).isEmpty()) {
|
||||
// Fail-closed in production: refuse to seed with the well-known
|
||||
// defaults. Otherwise an operator who forgets APP_ADMIN_USERNAME
|
||||
// / APP_ADMIN_PASSWORD locks production to admin@…/admin123 PERMANENTLY
|
||||
// (UserDataInitializer only seeds when the row is missing — see #513).
|
||||
// Allowed in dev/test/e2e because those run without secrets configured.
|
||||
boolean isLocalProfile = environment.matchesProfiles("dev", "test", "e2e");
|
||||
if (!isLocalProfile
|
||||
&& (DEFAULT_ADMIN_EMAIL.equals(adminEmail)
|
||||
|| DEFAULT_ADMIN_PASSWORD.equals(adminPassword))) {
|
||||
throw new IllegalStateException(
|
||||
"Refusing to seed admin user with default credentials outside "
|
||||
+ "the dev/test/e2e profiles. Set APP_ADMIN_USERNAME and "
|
||||
+ "APP_ADMIN_PASSWORD to non-default values before first boot — "
|
||||
+ "this lock-in is permanent."
|
||||
);
|
||||
}
|
||||
log.info("Kein Admin-User '{}' gefunden. Erstelle Default-Admin...", adminEmail);
|
||||
|
||||
UserGroup adminGroup = UserGroup.builder()
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
package org.raddatz.familienarchiv.user;
|
||||
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.extension.ExtendWith;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.junit.jupiter.MockitoExtension;
|
||||
import org.springframework.boot.CommandLineRunner;
|
||||
import org.springframework.core.env.Environment;
|
||||
import org.springframework.security.crypto.password.PasswordEncoder;
|
||||
import org.springframework.test.util.ReflectionTestUtils;
|
||||
|
||||
import java.util.Optional;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
import static org.mockito.ArgumentMatchers.anyString;
|
||||
import static org.mockito.Mockito.never;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
/**
|
||||
* UserDataInitializer must refuse to seed the admin user with the hardcoded
|
||||
* dev defaults when running outside the {@code dev} profile.
|
||||
*
|
||||
* <p>Why this matters: per DEPLOYMENT.md §3.5 and ADR-011, the admin password
|
||||
* is permanently locked on first deploy (UserDataInitializer only seeds when
|
||||
* the row is missing). If an operator forgets to set {@code APP_ADMIN_USERNAME}
|
||||
* / {@code APP_ADMIN_PASSWORD}, prod silently boots with the well-known dev
|
||||
* defaults — a credential-disclosure foot-gun, not a config typo. See #513.
|
||||
*/
|
||||
@ExtendWith(MockitoExtension.class)
|
||||
class AdminSeedFailClosedTest {
|
||||
|
||||
@Mock AppUserRepository userRepository;
|
||||
@Mock UserGroupRepository groupRepository;
|
||||
@Mock Environment environment;
|
||||
@Mock PasswordEncoder passwordEncoder;
|
||||
|
||||
UserDataInitializer initializer;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
initializer = new UserDataInitializer(userRepository, groupRepository, environment);
|
||||
}
|
||||
|
||||
@Test
|
||||
void refuses_to_seed_when_email_is_default_and_profile_is_not_dev() throws Exception {
|
||||
when(userRepository.findByEmail(anyString())).thenReturn(Optional.empty());
|
||||
when(environment.matchesProfiles("dev", "test", "e2e")).thenReturn(false);
|
||||
ReflectionTestUtils.setField(initializer, "adminEmail", UserDataInitializer.DEFAULT_ADMIN_EMAIL);
|
||||
ReflectionTestUtils.setField(initializer, "adminPassword", "operator-set-this-one");
|
||||
|
||||
CommandLineRunner runner = initializer.initAdminUser(passwordEncoder);
|
||||
|
||||
assertThatThrownBy(() -> runner.run())
|
||||
.isInstanceOf(IllegalStateException.class)
|
||||
.hasMessageContaining("default credentials")
|
||||
.hasMessageContaining("permanent");
|
||||
|
||||
verify(userRepository, never()).save(org.mockito.ArgumentMatchers.any());
|
||||
}
|
||||
|
||||
@Test
|
||||
void refuses_to_seed_when_password_is_default_and_profile_is_not_dev() throws Exception {
|
||||
when(userRepository.findByEmail(anyString())).thenReturn(Optional.empty());
|
||||
when(environment.matchesProfiles("dev", "test", "e2e")).thenReturn(false);
|
||||
ReflectionTestUtils.setField(initializer, "adminEmail", "admin@archiv.raddatz.cloud");
|
||||
ReflectionTestUtils.setField(initializer, "adminPassword", UserDataInitializer.DEFAULT_ADMIN_PASSWORD);
|
||||
|
||||
CommandLineRunner runner = initializer.initAdminUser(passwordEncoder);
|
||||
|
||||
assertThatThrownBy(() -> runner.run())
|
||||
.isInstanceOf(IllegalStateException.class)
|
||||
.hasMessageContaining("default credentials");
|
||||
}
|
||||
|
||||
@Test
|
||||
void allows_seed_when_both_values_are_set_and_profile_is_not_dev() throws Exception {
|
||||
when(userRepository.findByEmail(anyString())).thenReturn(Optional.empty());
|
||||
when(environment.matchesProfiles("dev", "test", "e2e")).thenReturn(false);
|
||||
when(passwordEncoder.encode(anyString())).thenReturn("$2a$10$stub");
|
||||
ReflectionTestUtils.setField(initializer, "adminEmail", "admin@archiv.raddatz.cloud");
|
||||
ReflectionTestUtils.setField(initializer, "adminPassword", "a-real-strong-password");
|
||||
|
||||
CommandLineRunner runner = initializer.initAdminUser(passwordEncoder);
|
||||
runner.run();
|
||||
|
||||
verify(userRepository).save(org.mockito.ArgumentMatchers.any(AppUser.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
void allows_seed_with_defaults_when_profile_is_dev() throws Exception {
|
||||
when(userRepository.findByEmail(anyString())).thenReturn(Optional.empty());
|
||||
when(environment.matchesProfiles("dev", "test", "e2e")).thenReturn(true);
|
||||
when(passwordEncoder.encode(anyString())).thenReturn("$2a$10$stub");
|
||||
ReflectionTestUtils.setField(initializer, "adminEmail", UserDataInitializer.DEFAULT_ADMIN_EMAIL);
|
||||
ReflectionTestUtils.setField(initializer, "adminPassword", UserDataInitializer.DEFAULT_ADMIN_PASSWORD);
|
||||
|
||||
CommandLineRunner runner = initializer.initAdminUser(passwordEncoder);
|
||||
runner.run();
|
||||
|
||||
verify(userRepository).save(org.mockito.ArgumentMatchers.any(AppUser.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
void does_not_check_defaults_when_admin_already_exists() throws Exception {
|
||||
AppUser existing = AppUser.builder()
|
||||
.email("someone@example.com")
|
||||
.password("$2a$10$stub")
|
||||
.build();
|
||||
when(userRepository.findByEmail(anyString())).thenReturn(Optional.of(existing));
|
||||
ReflectionTestUtils.setField(initializer, "adminEmail", UserDataInitializer.DEFAULT_ADMIN_EMAIL);
|
||||
ReflectionTestUtils.setField(initializer, "adminPassword", UserDataInitializer.DEFAULT_ADMIN_PASSWORD);
|
||||
|
||||
CommandLineRunner runner = initializer.initAdminUser(passwordEncoder);
|
||||
runner.run();
|
||||
|
||||
verify(userRepository, never()).save(org.mockito.ArgumentMatchers.any());
|
||||
// Importantly, no IllegalStateException — re-deploys must not panic over
|
||||
// historical default-seeded data they cannot retroactively fix.
|
||||
}
|
||||
}
|
||||
@@ -1,12 +1,14 @@
|
||||
package org.raddatz.familienarchiv.user;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.beans.factory.annotation.Value;
|
||||
import org.springframework.beans.factory.config.YamlPropertiesFactoryBean;
|
||||
import org.springframework.boot.context.properties.bind.Binder;
|
||||
import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
|
||||
import org.springframework.core.env.PropertiesPropertySource;
|
||||
import org.springframework.core.io.ClassPathResource;
|
||||
|
||||
import java.lang.reflect.Field;
|
||||
import java.util.Properties;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
@@ -57,6 +59,31 @@ class AdminSeedPropertyKeyTest {
|
||||
.isNotBlank();
|
||||
}
|
||||
|
||||
@Test
|
||||
void userDataInitializer_reads_app_admin_email_not_username() throws NoSuchFieldException {
|
||||
// Pin the Java side too: a future rename of the @Value placeholder
|
||||
// (e.g. back to `${app.admin.username:...}`) would silently break the
|
||||
// binding while the yaml-side assertions above still pass. See #513.
|
||||
Field field = UserDataInitializer.class.getDeclaredField("adminEmail");
|
||||
Value annotation = field.getAnnotation(Value.class);
|
||||
assertThat(annotation)
|
||||
.as("UserDataInitializer.adminEmail must be @Value-annotated")
|
||||
.isNotNull();
|
||||
assertThat(annotation.value())
|
||||
.as("UserDataInitializer must read app.admin.email — not username or any other key")
|
||||
.startsWith("${app.admin.email:");
|
||||
}
|
||||
|
||||
@Test
|
||||
void userDataInitializer_reads_app_admin_password() throws NoSuchFieldException {
|
||||
Field field = UserDataInitializer.class.getDeclaredField("adminPassword");
|
||||
Value annotation = field.getAnnotation(Value.class);
|
||||
assertThat(annotation).isNotNull();
|
||||
assertThat(annotation.value())
|
||||
.as("UserDataInitializer must read app.admin.password")
|
||||
.startsWith("${app.admin.password:");
|
||||
}
|
||||
|
||||
private Binder binderFromApplicationYaml() {
|
||||
YamlPropertiesFactoryBean yaml = new YamlPropertiesFactoryBean();
|
||||
yaml.setResources(new ClassPathResource("application.yaml"));
|
||||
|
||||
Reference in New Issue
Block a user