From 2c8ddc425056ed012eecf7f015652c1d3b48feeb Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 20 May 2025 17:37:35 +0200 Subject: [PATCH 1/4] fix ArchTest --- .../credentials/HsCredentialsContextsController.java | 4 ++++ .../hsadminng/credentials/HsCredentialsController.java | 9 +++++++++ .../net/hostsharing/hsadminng/arch/ArchitectureTest.java | 6 ++++-- ...sCredentialsContextRbacRepositoryIntegrationTest.java | 2 +- 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsContextsController.java b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsContextsController.java index cc9e1249..59438ecf 100644 --- a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsContextsController.java +++ b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsContextsController.java @@ -2,6 +2,8 @@ package net.hostsharing.hsadminng.credentials; import java.util.List; +import io.micrometer.core.annotation.Timed; +import net.hostsharing.hsadminng.config.NoSecurityRequirement; import net.hostsharing.hsadminng.context.Context; import net.hostsharing.hsadminng.credentials.generated.api.v1.api.LoginContextsApi; import net.hostsharing.hsadminng.credentials.generated.api.v1.model.LoginContextResource; @@ -11,6 +13,7 @@ import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.RestController; @RestController +@NoSecurityRequirement public class HsCredentialsContextsController implements LoginContextsApi { @Autowired @@ -23,6 +26,7 @@ public class HsCredentialsContextsController implements LoginContextsApi { private HsCredentialsContextRbacRepository contextRepo; @Override + @Timed("app.credentials.contexts.getListOfLoginContexts") public ResponseEntity> getListOfLoginContexts(final String assumedRoles) { context.assumeRoles(assumedRoles); diff --git a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsController.java b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsController.java index 62f55dc7..c917f10b 100644 --- a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsController.java +++ b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsController.java @@ -2,6 +2,9 @@ package net.hostsharing.hsadminng.credentials; import java.util.List; import java.util.UUID; + +import io.micrometer.core.annotation.Timed; +import io.swagger.v3.oas.annotations.security.SecurityRequirement; import net.hostsharing.hsadminng.context.Context; import net.hostsharing.hsadminng.credentials.generated.api.v1.api.LoginCredentialsApi; import net.hostsharing.hsadminng.credentials.generated.api.v1.model.LoginCredentialsInsertResource; @@ -15,6 +18,7 @@ import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.RestController; @RestController +@SecurityRequirement(name = "casTicket") public class HsCredentialsController implements LoginCredentialsApi { @Autowired @@ -33,6 +37,7 @@ public class HsCredentialsController implements LoginCredentialsApi { private HsCredentialsRepository loginCredentialsRepo; @Override + @Timed("app.credentials.credentials.getSingleLoginCredentialsByUuid") public ResponseEntity getSingleLoginCredentialsByUuid( final String assumedRoles, final UUID loginCredentialsUuid) { @@ -44,6 +49,7 @@ public class HsCredentialsController implements LoginCredentialsApi { } @Override + @Timed("app.credentials.credentials.getListOfLoginCredentialsByPersonUuid") public ResponseEntity> getListOfLoginCredentialsByPersonUuid( final String assumedRoles, final UUID personUuid @@ -57,6 +63,7 @@ public class HsCredentialsController implements LoginCredentialsApi { } @Override + @Timed("app.credentials.credentials.postNewLoginCredentials") public ResponseEntity postNewLoginCredentials( final String assumedRoles, final LoginCredentialsInsertResource body @@ -70,6 +77,7 @@ public class HsCredentialsController implements LoginCredentialsApi { } @Override + @Timed("app.credentials.credentials.deleteLoginCredentialsByUuid") public ResponseEntity deleteLoginCredentialsByUuid(final String assumedRoles, final UUID loginCredentialsUuid) { context.assumeRoles(assumedRoles); final var loginCredentialsEntity = em.getReference(HsCredentialsEntity.class, loginCredentialsUuid); @@ -78,6 +86,7 @@ public class HsCredentialsController implements LoginCredentialsApi { } @Override + @Timed("app.credentials.credentials.patchLoginCredentials") public ResponseEntity patchLoginCredentials( final String assumedRoles, final UUID loginCredentialsUuid, diff --git a/src/test/java/net/hostsharing/hsadminng/arch/ArchitectureTest.java b/src/test/java/net/hostsharing/hsadminng/arch/ArchitectureTest.java index 051bfe91..950d425a 100644 --- a/src/test/java/net/hostsharing/hsadminng/arch/ArchitectureTest.java +++ b/src/test/java/net/hostsharing/hsadminng/arch/ArchitectureTest.java @@ -52,6 +52,7 @@ public class ArchitectureTest { "..credentials", "..hash", "..lambda", + "..journal", "..generated..", "..persistence..", "..reflection", @@ -155,14 +156,14 @@ public class ArchitectureTest { public static final ArchRule testPackagesRule = classes() .that().resideInAPackage("..test.(*)..") .should().onlyBeAccessed().byClassesThat() - .resideInAnyPackage("..test.(*).."); + .resideInAnyPackage("..test.(*)..", "..journal.."); @ArchTest @SuppressWarnings("unused") public static final ArchRule testPackagePackageRule = classes() .that().resideInAPackage("..test.pac..") .should().onlyBeAccessed().byClassesThat() - .resideInAnyPackage("..test.pac.."); + .resideInAnyPackage("..test.pac..", "..journal.."); @ArchTest @SuppressWarnings("unused") @@ -174,6 +175,7 @@ public class ArchitectureTest { "..hs.office.(*)..", "..hs.booking.(*)..", "..hs.hosting.(*)..", + "..credentials..", "..hs.scenarios", "..hs.migration", "..rbacgrant" // TODO.test: just because of RbacGrantsDiagramServiceIntegrationTest diff --git a/src/test/java/net/hostsharing/hsadminng/credentials/HsCredentialsContextRbacRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/credentials/HsCredentialsContextRbacRepositoryIntegrationTest.java index d99f6b8c..2927c6e0 100644 --- a/src/test/java/net/hostsharing/hsadminng/credentials/HsCredentialsContextRbacRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/credentials/HsCredentialsContextRbacRepositoryIntegrationTest.java @@ -11,10 +11,10 @@ import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; import org.springframework.context.annotation.Import; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.transaction.annotation.Transactional; import jakarta.persistence.PersistenceException; import jakarta.servlet.http.HttpServletRequest; -import jakarta.transaction.Transactional; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; From f4bf614d77ea502fd555752e2179156a5fd7a2d7 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 20 May 2025 17:38:20 +0200 Subject: [PATCH 2/4] dd column seqTxId BIGINT to table base.tx_context --- .../db/changelog/0-base/020-audit-log.sql | 42 +++++- .../TransactionContextIntegrationTest.java | 122 ++++++++++++++++++ 2 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 src/test/java/net/hostsharing/hsadminng/journal/TransactionContextIntegrationTest.java diff --git a/src/main/resources/db/changelog/0-base/020-audit-log.sql b/src/main/resources/db/changelog/0-base/020-audit-log.sql index 2603f131..ca784399 100644 --- a/src/main/resources/db/changelog/0-base/020-audit-log.sql +++ b/src/main/resources/db/changelog/0-base/020-audit-log.sql @@ -34,6 +34,33 @@ create table base.tx_context create index on base.tx_context using brin (txTimestamp); --// + +-- ============================================================================ +--changeset michael.hoennig:audit-TX-CONTEXT-TABLE-COLUMN-SEQUENTIAL-TX-ID endDelimiter:--// +-- ---------------------------------------------------------------------------- +/* + Adds a column to base.tx_context which keeps a strictly sequentially ordered tx-id. + */ + +alter table base.tx_context + add column seqTxId BIGINT; + +CREATE OR REPLACE FUNCTION set_next_sequential_txid() + RETURNS TRIGGER AS $$ +BEGIN + LOCK TABLE base.tx_context IN EXCLUSIVE MODE; + SELECT COALESCE(MAX(seqTxId)+1, 0) INTO NEW.seqTxId FROM base.tx_context; + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER set_commit_order_trigger + BEFORE INSERT ON base.tx_context + FOR EACH ROW +EXECUTE FUNCTION set_next_sequential_txid(); +--// + + -- ============================================================================ --changeset michael.hoennig:audit-TX-JOURNAL-TABLE endDelimiter:--// -- ---------------------------------------------------------------------------- @@ -53,13 +80,24 @@ create index on base.tx_journal (targetTable, targetUuid); --// -- ============================================================================ ---changeset michael.hoennig:audit-TX-JOURNAL-VIEW endDelimiter:--// +--changeset michael.hoennig:audit-TX-JOURNAL-VIEW runOnChange:true validCheckSum:ANY endDelimiter:--// -- ---------------------------------------------------------------------------- /* A view combining base.tx_journal with base.tx_context. */ +drop view if exists base.tx_journal_v; create view base.tx_journal_v as -select txc.*, txj.targettable, txj.targetop, txj.targetuuid, txj.targetdelta +select txc.seqTxId, + txc.txId, + txc.txTimeStamp, + txc.currentSubject, + txc.assumedRoles, + txc.currentTask, + txc.currentRequest, + txj.targetTable, + txj.targeTop, + txj.targetUuid, + txj.targetDelta from base.tx_journal txj left join base.tx_context txc using (txId) order by txc.txtimestamp; diff --git a/src/test/java/net/hostsharing/hsadminng/journal/TransactionContextIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/journal/TransactionContextIntegrationTest.java new file mode 100644 index 00000000..abd0d764 --- /dev/null +++ b/src/test/java/net/hostsharing/hsadminng/journal/TransactionContextIntegrationTest.java @@ -0,0 +1,122 @@ +package net.hostsharing.hsadminng.journal; + +import lombok.SneakyThrows; +import net.hostsharing.hsadminng.context.Context; +import net.hostsharing.hsadminng.rbac.test.ContextBasedTestWithCleanup; +import net.hostsharing.hsadminng.rbac.test.JpaAttempt; +import net.hostsharing.hsadminng.rbac.test.cust.TestCustomerEntity; +import net.hostsharing.hsadminng.rbac.test.cust.TestCustomerRepository; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.context.annotation.Import; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.Transactional; + +import jakarta.servlet.http.HttpServletRequest; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.transaction.annotation.Propagation.NEVER; + +@DataJpaTest +@Import({ Context.class, JpaAttempt.class }) +@Tag("generalIntegrationTest") +class TransactionContextIntegrationTest extends ContextBasedTestWithCleanup { + + @Autowired + private PlatformTransactionManager transactionManager; + + @Autowired + JpaAttempt jpaAttempt; + + @MockitoBean + HttpServletRequest request; + + @Autowired + private TestCustomerRepository repository; + + @Test + @Transactional(propagation = NEVER) + void testConcurrentCommitOrder() { + + // determine initial row count + final var rowCount = jpaAttempt.transacted(() -> { + context("superuser-alex@hostsharing.net"); + return em.createQuery("SELECT e FROM TestCustomerEntity e", TestCustomerEntity.class).getResultList(); + }).assertSuccessful().returnedValue().size(); + + // when 3 transactions with different runtime run concurrently + runThreads( + // starts first, ends last (because it's slow) + createTransactionThread("t01", 91001, 500), + + // starts second, ends first (because it's faster than the one that got started first) + createTransactionThread("t02", 91002, 0), + + // starts third, ends second + createTransactionThread("t03", 91003, 100) + ); + + // then all 3 threads did insert one row each + jpaAttempt.transacted(() -> { + context("superuser-alex@hostsharing.net"); + var all = em.createQuery("SELECT e FROM TestCustomerEntity e", TestCustomerEntity.class).getResultList(); + assertThat(all).hasSize(rowCount + 3); + }).assertSuccessful(); + + // and seqTxId order is in correct order + final var txContextsX = em.createNativeQuery( + "select concat(c.txId, ':', c.currentTask) from base.tx_context c order by c.seqTxId" + ).getResultList(); + final var txContextTasks = last(3, txContextsX).stream().map(Object::toString).toList(); + assertThat(txContextTasks.get(0)).endsWith( + ":TestCustomerEntity(uuid=null, version=0, prefix=t02, reference=91002, adminUserName=null)"); + assertThat(txContextTasks.get(1)).endsWith( + "TestCustomerEntity(uuid=null, version=0, prefix=t03, reference=91003, adminUserName=null)"); + assertThat(txContextTasks.get(2)).endsWith( + "TestCustomerEntity(uuid=null, version=0, prefix=t01, reference=91001, adminUserName=null)"); + } + + private @NotNull Thread createTransactionThread(final String t01, final int reference, final int millis) { + return new Thread(() -> { + jpaAttempt.transacted(() -> { + final var entity1 = new TestCustomerEntity(); + entity1.setPrefix(t01); + entity1.setReference(reference); + + context.define(entity1.toString(), null, "superuser-alex@hostsharing.net", null); + entity1.setReference(80000 + toInt(em.createNativeQuery("SELECT txid_current()").getSingleResult())); + repository.save(entity1); + sleep(millis); // simulate a delay + }).assertSuccessful(); + }); + } + + private int toInt(final Object singleResult) { + return ((Long)singleResult).intValue(); + } + + @SneakyThrows + private void sleep(final int millis) { + Thread.sleep(millis); + } + + @SneakyThrows + private void runThreads(final Thread... threads) { + for (final Thread thread : threads) { + thread.start(); + sleep(100); + } + for (final Thread thread : threads) { + thread.join(); + } + + } + private List last(final int n, final List list) { + return list.subList(Math.max(list.size() - n, 0), list.size()); + } +} From 037bbf5a4c31384cab591594f30583e1e01bcd2b Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Wed, 21 May 2025 10:25:18 +0200 Subject: [PATCH 3/4] fix credentials-api-paths+includes --- .../login-credentials-data-model.mermaid | 22 +++---- .../HsCredentialsContextsController.java | 10 +-- .../credentials/HsCredentialsController.java | 66 +++++++++---------- .../credentials/HsCredentialsEntity.java | 2 +- .../HsCredentialsEntityPatcher.java | 14 ++-- .../api-definition/credentials/api-paths.yaml | 2 +- .../credentials/credentials-schemas.yaml | 6 +- ...CredentialsContextsControllerRestTest.java | 2 +- .../HsCredentialsEntityPatcherUnitTest.java | 37 +++++------ ...sCredentialsRepositoryIntegrationTest.java | 28 ++++---- 10 files changed, 94 insertions(+), 95 deletions(-) diff --git a/doc/ideas/login-credentials-data-model.mermaid b/doc/ideas/login-credentials-data-model.mermaid index 45722bf2..5d811421 100644 --- a/doc/ideas/login-credentials-data-model.mermaid +++ b/doc/ideas/login-credentials-data-model.mermaid @@ -1,13 +1,13 @@ classDiagram direction LR - OfficePerson o.. "*" LoginCredentials - LoginCredentials "1" o-- "1" RbacSubject + OfficePerson o.. "*" Credentials + Credentials "1" o-- "1" RbacSubject - LoginContext "1..n" --o "1" LoginContextMapping - LoginCredentials "1..n" --o "1" LoginContextMapping + CredentialsContext "1..n" --o "1" CredentialsContextMapping + Credentials "1..n" --o "1" CredentialsContextMapping - class LoginCredentials{ + class Credentials{ +twoFactorAuth: text +telephonePassword: text +emailAdress: text @@ -18,14 +18,14 @@ classDiagram -onboardingToken: text [w/o] } - class LoginContext{ + class CredentialsContext{ -type: Enum [SSH, Matrix, Mastodon, ...] -qualifier: text } - class LoginContextMapping{ + class CredentialsContextMapping{ } - note for LoginContextMapping "Assigns LoginCredentials to LoginContexts" + note for CredentialsContextMapping "Assigns Credentials to CredentialsContexts" class RbacSubject{ +uuid: uuid @@ -41,9 +41,9 @@ classDiagram +salutation: text } - style LoginContext fill:#00f,color:#fff - style LoginContextMapping fill:#00f,color:#fff - style LoginCredentials fill:#00f,color:#fff + style CredentialsContext fill:#00f,color:#fff + style CredentialsContextMapping fill:#00f,color:#fff + style Credentials fill:#00f,color:#fff style RbacSubject fill:#f96,color:#fff style OfficePerson fill:#f66,color:#000 diff --git a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsContextsController.java b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsContextsController.java index 59438ecf..bedcb441 100644 --- a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsContextsController.java +++ b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsContextsController.java @@ -5,8 +5,8 @@ import java.util.List; import io.micrometer.core.annotation.Timed; import net.hostsharing.hsadminng.config.NoSecurityRequirement; import net.hostsharing.hsadminng.context.Context; -import net.hostsharing.hsadminng.credentials.generated.api.v1.api.LoginContextsApi; -import net.hostsharing.hsadminng.credentials.generated.api.v1.model.LoginContextResource; +import net.hostsharing.hsadminng.credentials.generated.api.v1.api.ContextsApi; +import net.hostsharing.hsadminng.credentials.generated.api.v1.model.ContextResource; import net.hostsharing.hsadminng.mapper.StrictMapper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.ResponseEntity; @@ -14,7 +14,7 @@ import org.springframework.web.bind.annotation.RestController; @RestController @NoSecurityRequirement -public class HsCredentialsContextsController implements LoginContextsApi { +public class HsCredentialsContextsController implements ContextsApi { @Autowired private Context context; @@ -27,11 +27,11 @@ public class HsCredentialsContextsController implements LoginContextsApi { @Override @Timed("app.credentials.contexts.getListOfLoginContexts") - public ResponseEntity> getListOfLoginContexts(final String assumedRoles) { + public ResponseEntity> getListOfContexts(final String assumedRoles) { context.assumeRoles(assumedRoles); final var loginContexts = contextRepo.findAll(); - final var result = mapper.mapList(loginContexts, LoginContextResource.class); + final var result = mapper.mapList(loginContexts, ContextResource.class); return ResponseEntity.ok(result); } } diff --git a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsController.java b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsController.java index c917f10b..b91914e3 100644 --- a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsController.java +++ b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsController.java @@ -6,10 +6,10 @@ import java.util.UUID; import io.micrometer.core.annotation.Timed; import io.swagger.v3.oas.annotations.security.SecurityRequirement; import net.hostsharing.hsadminng.context.Context; -import net.hostsharing.hsadminng.credentials.generated.api.v1.api.LoginCredentialsApi; -import net.hostsharing.hsadminng.credentials.generated.api.v1.model.LoginCredentialsInsertResource; -import net.hostsharing.hsadminng.credentials.generated.api.v1.model.LoginCredentialsPatchResource; -import net.hostsharing.hsadminng.credentials.generated.api.v1.model.LoginCredentialsResource; +import net.hostsharing.hsadminng.credentials.generated.api.v1.api.CredentialsApi; +import net.hostsharing.hsadminng.credentials.generated.api.v1.model.CredentialsInsertResource; +import net.hostsharing.hsadminng.credentials.generated.api.v1.model.CredentialsPatchResource; +import net.hostsharing.hsadminng.credentials.generated.api.v1.model.CredentialsResource; import net.hostsharing.hsadminng.hs.office.person.HsOfficePersonRbacRepository; import net.hostsharing.hsadminng.mapper.StrictMapper; import net.hostsharing.hsadminng.persistence.EntityManagerWrapper; @@ -19,7 +19,7 @@ import org.springframework.web.bind.annotation.RestController; @RestController @SecurityRequirement(name = "casTicket") -public class HsCredentialsController implements LoginCredentialsApi { +public class HsCredentialsController implements CredentialsApi { @Autowired private Context context; @@ -34,72 +34,72 @@ public class HsCredentialsController implements LoginCredentialsApi { private HsOfficePersonRbacRepository personRepo; @Autowired - private HsCredentialsRepository loginCredentialsRepo; + private HsCredentialsRepository credentialsRepo; @Override - @Timed("app.credentials.credentials.getSingleLoginCredentialsByUuid") - public ResponseEntity getSingleLoginCredentialsByUuid( + @Timed("app.credentials.credentials.getSingleCredentialsByUuid") + public ResponseEntity getSingleCredentialsByUuid( final String assumedRoles, - final UUID loginCredentialsUuid) { + final UUID credentialsUuid) { context.assumeRoles(assumedRoles); - final var credentials = loginCredentialsRepo.findByUuid(loginCredentialsUuid); - final var result = mapper.map(credentials, LoginCredentialsResource.class); + final var credentials = credentialsRepo.findByUuid(credentialsUuid); + final var result = mapper.map(credentials, CredentialsResource.class); return ResponseEntity.ok(result); } @Override - @Timed("app.credentials.credentials.getListOfLoginCredentialsByPersonUuid") - public ResponseEntity> getListOfLoginCredentialsByPersonUuid( + @Timed("app.credentials.credentials.getListOfCredentialsByPersonUuid") + public ResponseEntity> getListOfCredentialsByPersonUuid( final String assumedRoles, final UUID personUuid ) { context.assumeRoles(assumedRoles); final var person = personRepo.findByUuid(personUuid).orElseThrow(); // FIXME: use proper exception - final var credentials = loginCredentialsRepo.findByPerson(person); - final var result = mapper.mapList(credentials, LoginCredentialsResource.class); + final var credentials = credentialsRepo.findByPerson(person); + final var result = mapper.mapList(credentials, CredentialsResource.class); return ResponseEntity.ok(result); } @Override - @Timed("app.credentials.credentials.postNewLoginCredentials") - public ResponseEntity postNewLoginCredentials( + @Timed("app.credentials.credentials.postNewCredentials") + public ResponseEntity postNewCredentials( final String assumedRoles, - final LoginCredentialsInsertResource body + final CredentialsInsertResource body ) { context.assumeRoles(assumedRoles); - final var newLoginCredentialsEntity = mapper.map(body, HsCredentialsEntity.class); - final var savedLoginCredentialsEntity = loginCredentialsRepo.save(newLoginCredentialsEntity); - final var newLoginCredentialsResource = mapper.map(savedLoginCredentialsEntity, LoginCredentialsResource.class); - return ResponseEntity.ok(newLoginCredentialsResource); + final var newCredentialsEntity = mapper.map(body, HsCredentialsEntity.class); + final var savedCredentialsEntity = credentialsRepo.save(newCredentialsEntity); + final var newCredentialsResource = mapper.map(savedCredentialsEntity, CredentialsResource.class); + return ResponseEntity.ok(newCredentialsResource); } @Override - @Timed("app.credentials.credentials.deleteLoginCredentialsByUuid") - public ResponseEntity deleteLoginCredentialsByUuid(final String assumedRoles, final UUID loginCredentialsUuid) { + @Timed("app.credentials.credentials.deleteCredentialsByUuid") + public ResponseEntity deleteCredentialsByUuid(final String assumedRoles, final UUID credentialsUuid) { context.assumeRoles(assumedRoles); - final var loginCredentialsEntity = em.getReference(HsCredentialsEntity.class, loginCredentialsUuid); - em.remove(loginCredentialsEntity); + final var credentialsEntity = em.getReference(HsCredentialsEntity.class, credentialsUuid); + em.remove(credentialsEntity); return ResponseEntity.noContent().build(); } @Override - @Timed("app.credentials.credentials.patchLoginCredentials") - public ResponseEntity patchLoginCredentials( + @Timed("app.credentials.credentials.patchCredentials") + public ResponseEntity patchCredentials( final String assumedRoles, - final UUID loginCredentialsUuid, - final LoginCredentialsPatchResource body + final UUID credentialsUuid, + final CredentialsPatchResource body ) { context.assumeRoles(assumedRoles); - final var current = loginCredentialsRepo.findByUuid(loginCredentialsUuid).orElseThrow(); + final var current = credentialsRepo.findByUuid(credentialsUuid).orElseThrow(); new HsCredentialsEntityPatcher(em, current).apply(body); - final var saved = loginCredentialsRepo.save(current); - final var mapped = mapper.map(saved, LoginCredentialsResource.class); + final var saved = credentialsRepo.save(current); + final var mapped = mapper.map(saved, CredentialsResource.class); return ResponseEntity.ok(mapped); } } diff --git a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsEntity.java b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsEntity.java index fad26941..cac48352 100644 --- a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsEntity.java @@ -26,7 +26,7 @@ import static net.hostsharing.hsadminng.repr.Stringify.stringify; @AllArgsConstructor public class HsCredentialsEntity implements BaseEntity, Stringifyable { - protected static Stringify stringify = stringify(HsCredentialsEntity.class, "loginCredentials") + protected static Stringify stringify = stringify(HsCredentialsEntity.class, "credentials") .withProp(HsCredentialsEntity::isActive) .withProp(HsCredentialsEntity::getEmailAddress) .withProp(HsCredentialsEntity::getTwoFactorAuth) diff --git a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsEntityPatcher.java b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsEntityPatcher.java index 935cc1d8..164fcc12 100644 --- a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsEntityPatcher.java +++ b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsEntityPatcher.java @@ -1,7 +1,7 @@ package net.hostsharing.hsadminng.credentials; -import net.hostsharing.hsadminng.credentials.generated.api.v1.model.LoginContextResource; -import net.hostsharing.hsadminng.credentials.generated.api.v1.model.LoginCredentialsPatchResource; +import net.hostsharing.hsadminng.credentials.generated.api.v1.model.ContextResource; +import net.hostsharing.hsadminng.credentials.generated.api.v1.model.CredentialsPatchResource; import net.hostsharing.hsadminng.mapper.EntityPatcher; import net.hostsharing.hsadminng.mapper.OptionalFromJson; @@ -11,7 +11,7 @@ import java.util.List; import java.util.Set; import java.util.stream.Collectors; -public class HsCredentialsEntityPatcher implements EntityPatcher { +public class HsCredentialsEntityPatcher implements EntityPatcher { private final EntityManager em; private final HsCredentialsEntity entity; @@ -22,7 +22,7 @@ public class HsCredentialsEntityPatcher implements EntityPatcher resources, + List resources, Set entities ) { final var resourceUuids = resources.stream() - .map(LoginContextResource::getUuid) + .map(ContextResource::getUuid) .collect(Collectors.toSet()); final var entityUuids = entities.stream() @@ -61,7 +61,7 @@ public class HsCredentialsEntityPatcher implements EntityPatcher { @@ -58,8 +57,8 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase< .qualifier("dev") .build(); - private LoginContextResource patchContextResource2; - private LoginContextResource patchContextResource3; + private ContextResource patchContextResource2; + private ContextResource patchContextResource3; // This is what em.find should return for CONTEXT_UUID_3 private final HsCredentialsContextRealEntity newContextEntity3 = HsCredentialsContextRealEntity.builder() @@ -69,7 +68,7 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase< .build(); private final Set initialContextEntities = Set.of(initialContextEntity1, initialContextEntity2); - private List patchedContextResources; + private List patchedContextResources; private final Set expectedPatchedContextEntities = Set.of(initialContextEntity2, newContextEntity3); @Mock @@ -82,14 +81,14 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase< lenient().when(em.find(eq(HsCredentialsContextRealEntity.class), eq(CONTEXT_UUID_2))).thenReturn(initialContextEntity2); lenient().when(em.find(eq(HsCredentialsContextRealEntity.class), eq(CONTEXT_UUID_3))).thenReturn(newContextEntity3); - patchContextResource2 = new LoginContextResource(); + patchContextResource2 = new ContextResource(); patchContextResource2.setUuid(CONTEXT_UUID_2); - patchContextResource2.setType(LoginContextTypeResource.SSH); + patchContextResource2.setType("SSH"); patchContextResource2.setQualifier("dev"); - patchContextResource3 = new LoginContextResource(); + patchContextResource3 = new ContextResource(); patchContextResource3.setUuid(CONTEXT_UUID_3); - patchContextResource3.setType(LoginContextTypeResource.HSADMIN); + patchContextResource3.setType("HSADMIN"); patchContextResource3.setQualifier("test"); patchedContextResources = List.of(patchContextResource2, patchContextResource3); @@ -110,8 +109,8 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase< } @Override - protected LoginCredentialsPatchResource newPatchResource() { - return new LoginCredentialsPatchResource(); + protected CredentialsPatchResource newPatchResource() { + return new CredentialsPatchResource(); } @Override @@ -124,38 +123,38 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase< return Stream.of( new SimpleProperty<>( "active", - LoginCredentialsPatchResource::setActive, + CredentialsPatchResource::setActive, PATCHED_ACTIVE, HsCredentialsEntity::setActive, PATCHED_ACTIVE) .notNullable(), new JsonNullableProperty<>( "emailAddress", - LoginCredentialsPatchResource::setEmailAddress, + CredentialsPatchResource::setEmailAddress, PATCHED_EMAIL_ADDRESS, HsCredentialsEntity::setEmailAddress, PATCHED_EMAIL_ADDRESS), new JsonNullableProperty<>( "twoFactorAuth", - LoginCredentialsPatchResource::setTwoFactorAuth, + CredentialsPatchResource::setTwoFactorAuth, PATCHED_TWO_FACTOR_AUTH, HsCredentialsEntity::setTwoFactorAuth, PATCHED_TWO_FACTOR_AUTH), new JsonNullableProperty<>( "smsNumber", - LoginCredentialsPatchResource::setSmsNumber, + CredentialsPatchResource::setSmsNumber, PATCHED_SMS_NUMBER, HsCredentialsEntity::setSmsNumber, PATCHED_SMS_NUMBER), new JsonNullableProperty<>( "phonePassword", - LoginCredentialsPatchResource::setPhonePassword, + CredentialsPatchResource::setPhonePassword, PATCHED_PHONE_PASSWORD, HsCredentialsEntity::setPhonePassword, PATCHED_PHONE_PASSWORD), new SimpleProperty<>( "contexts", - LoginCredentialsPatchResource::setContexts, + CredentialsPatchResource::setContexts, patchedContextResources, HsCredentialsEntity::setLoginContexts, expectedPatchedContextEntities) diff --git a/src/test/java/net/hostsharing/hsadminng/credentials/HsCredentialsRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/credentials/HsCredentialsRepositoryIntegrationTest.java index c9daa72f..96c8571d 100644 --- a/src/test/java/net/hostsharing/hsadminng/credentials/HsCredentialsRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/credentials/HsCredentialsRepositoryIntegrationTest.java @@ -42,7 +42,7 @@ class HsCredentialsRepositoryIntegrationTest extends ContextBasedTest { HttpServletRequest request; @Autowired - private HsCredentialsRepository loginCredentialsRepository; + private HsCredentialsRepository credentialsRepository; @Autowired private HsCredentialsContextRealRepository loginContextRealRepo; @@ -88,7 +88,7 @@ class HsCredentialsRepositoryIntegrationTest extends ContextBasedTest { @Test void shouldFindByUuidUsingTestData() { // when - final var foundEntityOptional = loginCredentialsRepository.findByUuid(alexSubject.getUuid()); + final var foundEntityOptional = credentialsRepository.findByUuid(alexSubject.getUuid()); // then assertThat(foundEntityOptional).isPresent() @@ -96,7 +96,7 @@ class HsCredentialsRepositoryIntegrationTest extends ContextBasedTest { } @Test - void shouldSaveLoginCredentialsWithExistingContext() { + void shouldSaveCredentialsWithExistingContext() { // given final var existingContext = loginContextRealRepo.findByTypeAndQualifier("HSADMIN", "prod") .orElseThrow(); @@ -111,12 +111,12 @@ class HsCredentialsRepositoryIntegrationTest extends ContextBasedTest { .build(); // when - loginCredentialsRepository.save(newCredentials); + credentialsRepository.save(newCredentials); em.flush(); em.clear(); // then - final var foundEntityOptional = loginCredentialsRepository.findByUuid(drewSubject.getUuid()); + final var foundEntityOptional = credentialsRepository.findByUuid(drewSubject.getUuid()); assertThat(foundEntityOptional).isPresent(); final var foundEntity = foundEntityOptional.get(); assertThat(foundEntity.getEmailAddress()).isEqualTo("drew.new@example.com"); @@ -129,7 +129,7 @@ class HsCredentialsRepositoryIntegrationTest extends ContextBasedTest { } @Test - void shouldNotSaveLoginCredentialsWithNewContext() { + void shouldNotSaveCredentialsWithNewContext() { // given final var newContext = HsCredentialsContextRealEntity.builder() .type("MATRIX") @@ -146,7 +146,7 @@ class HsCredentialsRepositoryIntegrationTest extends ContextBasedTest { // when final var exception = catchThrowable(() -> { - loginCredentialsRepository.save(newCredentials); + credentialsRepository.save(newCredentials); em.flush(); }); @@ -155,7 +155,7 @@ class HsCredentialsRepositoryIntegrationTest extends ContextBasedTest { } @Test - void shouldSaveNewLoginCredentialsWithoutContext() { + void shouldSaveNewCredentialsWithoutContext() { // given final var newCredentials = HsCredentialsEntity.builder() .subject(testUserSubject) @@ -167,12 +167,12 @@ class HsCredentialsRepositoryIntegrationTest extends ContextBasedTest { .build(); // when - loginCredentialsRepository.save(newCredentials); + credentialsRepository.save(newCredentials); em.flush(); em.clear(); // then - final var foundEntityOptional = loginCredentialsRepository.findByUuid(testUserSubject.getUuid()); + final var foundEntityOptional = credentialsRepository.findByUuid(testUserSubject.getUuid()); assertThat(foundEntityOptional).isPresent(); final var foundEntity = foundEntityOptional.get(); assertThat(foundEntity.getEmailAddress()).isEqualTo("test.user.new@example.com"); @@ -183,21 +183,21 @@ class HsCredentialsRepositoryIntegrationTest extends ContextBasedTest { } @Test - void shouldUpdateExistingLoginCredentials() { + void shouldUpdateExistingCredentials() { // given - final var entityToUpdate = loginCredentialsRepository.findByUuid(alexSubject.getUuid()).orElseThrow(); + final var entityToUpdate = credentialsRepository.findByUuid(alexSubject.getUuid()).orElseThrow(); final var initialVersion = entityToUpdate.getVersion(); // when entityToUpdate.setActive(false); entityToUpdate.setEmailAddress("updated.user1@example.com"); - final var savedEntity = loginCredentialsRepository.save(entityToUpdate); + final var savedEntity = credentialsRepository.save(entityToUpdate); em.flush(); em.clear(); // then assertThat(savedEntity.getVersion()).isGreaterThan(initialVersion); - final var updatedEntityOptional = loginCredentialsRepository.findByUuid(alexSubject.getUuid()); + final var updatedEntityOptional = credentialsRepository.findByUuid(alexSubject.getUuid()); assertThat(updatedEntityOptional).isPresent(); final var updatedEntity = updatedEntityOptional.get(); assertThat(updatedEntity.isActive()).isFalse(); From 17f620c7b8a9e85c87d239ba9ced919df55b4f4b Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Wed, 21 May 2025 11:10:08 +0200 Subject: [PATCH 4/4] fix fixme's for credentials error messages --- .../credentials/HsCredentialsController.java | 15 +++++++++++++-- .../credentials/HsCredentialsEntityPatcher.java | 14 +++++++++----- src/main/resources/i18n/messages_de.properties | 3 +++ .../HsCredentialsEntityPatcherUnitTest.java | 4 +++- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsController.java b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsController.java index b91914e3..fef10830 100644 --- a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsController.java +++ b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsController.java @@ -5,6 +5,7 @@ import java.util.UUID; import io.micrometer.core.annotation.Timed; import io.swagger.v3.oas.annotations.security.SecurityRequirement; +import net.hostsharing.hsadminng.config.MessageTranslator; import net.hostsharing.hsadminng.context.Context; import net.hostsharing.hsadminng.credentials.generated.api.v1.api.CredentialsApi; import net.hostsharing.hsadminng.credentials.generated.api.v1.model.CredentialsInsertResource; @@ -17,6 +18,8 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.RestController; +import jakarta.persistence.EntityNotFoundException; + @RestController @SecurityRequirement(name = "casTicket") public class HsCredentialsController implements CredentialsApi { @@ -30,6 +33,9 @@ public class HsCredentialsController implements CredentialsApi { @Autowired private StrictMapper mapper; + @Autowired + private MessageTranslator messageTranslator; + @Autowired private HsOfficePersonRbacRepository personRepo; @@ -56,7 +62,12 @@ public class HsCredentialsController implements CredentialsApi { ) { context.assumeRoles(assumedRoles); - final var person = personRepo.findByUuid(personUuid).orElseThrow(); // FIXME: use proper exception + final var person = personRepo.findByUuid(personUuid).orElseThrow( + () -> new EntityNotFoundException( + messageTranslator.translate("{0} \"{1}\" not found or not accessible", "personUuid", personUuid) + ) + + ); // FIXME: use proper exception final var credentials = credentialsRepo.findByPerson(person); final var result = mapper.mapList(credentials, CredentialsResource.class); return ResponseEntity.ok(result); @@ -96,7 +107,7 @@ public class HsCredentialsController implements CredentialsApi { final var current = credentialsRepo.findByUuid(credentialsUuid).orElseThrow(); - new HsCredentialsEntityPatcher(em, current).apply(body); + new HsCredentialsEntityPatcher(em, messageTranslator, current).apply(body); final var saved = credentialsRepo.save(current); final var mapped = mapper.map(saved, CredentialsResource.class); diff --git a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsEntityPatcher.java b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsEntityPatcher.java index 164fcc12..44f1ccd2 100644 --- a/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsEntityPatcher.java +++ b/src/main/java/net/hostsharing/hsadminng/credentials/HsCredentialsEntityPatcher.java @@ -1,5 +1,6 @@ package net.hostsharing.hsadminng.credentials; +import net.hostsharing.hsadminng.config.MessageTranslator; import net.hostsharing.hsadminng.credentials.generated.api.v1.model.ContextResource; import net.hostsharing.hsadminng.credentials.generated.api.v1.model.CredentialsPatchResource; import net.hostsharing.hsadminng.mapper.EntityPatcher; @@ -14,10 +15,12 @@ import java.util.stream.Collectors; public class HsCredentialsEntityPatcher implements EntityPatcher { private final EntityManager em; + private MessageTranslator messageTranslator; private final HsCredentialsEntity entity; - public HsCredentialsEntityPatcher(final EntityManager em, final HsCredentialsEntity entity) { + public HsCredentialsEntityPatcher(final EntityManager em, MessageTranslator messageTranslator, final HsCredentialsEntity entity) { this.em = em; + this.messageTranslator = messageTranslator; this.entity = entity; } @@ -57,14 +60,15 @@ public class HsCredentialsEntityPatcher implements EntityPatcher