remove secrets from credentials (#198)

Co-authored-by: Michael Hoennig <michael@hoennig.de> Reviewed-on: https://dev.hostsharing.net/hostsharing/hs.hsadmin.ng/pulls/198 Reviewed-by: Marc Sandlus <marc.sandlus@hostsharing.net>
2025-09-09 11:47:08 +02:00
parent d7a78d0a79
commit 27b4f59a97
18 changed files with 19 additions and 186 deletions
@@ -1,7 +1,5 @@
 package net.hostsharing.hsadminng.hs.accounts;

-import java.time.LocalDateTime;
-import java.time.ZoneOffset;
 import java.util.List;
 import java.util.UUID;
 import java.util.function.BiConsumer;
@@ -37,7 +35,6 @@ import org.springframework.web.servlet.mvc.method.annotation.MvcUriComponentsBui
 import jakarta.persistence.EntityNotFoundException;
 import jakarta.validation.ValidationException;

-import static java.util.Optional.ofNullable;
 import static java.util.Optional.of;

@RestController
@@ -191,22 +188,6 @@ public class HsCredentialsController implements CredentialsApi {
        return ResponseEntity.ok(result);
    }

-    @Override
-    @Transactional
-    @Timed("app.credentials.credentials.credentialsUsed")
-    public ResponseEntity<CredentialsResource> credentialsUsed(final UUID credentialsUuid) {
-        context.define();
-
-        val current = credentialsRepo.findByUuid(credentialsUuid).orElseThrow();
-
-        current.setOnboardingToken(null);
-        current.setLastUsed(LocalDateTime.now());
-
-        val saved = credentialsRepo.save(current);
-        val mapped = mapper.map(saved, CredentialsResource.class, ENTITY_TO_RESOURCE_POSTMAPPER);
-        return ResponseEntity.ok(mapped);
-    }
-
    private void validateOnCreate(final HsCredentialsEntity newCredentialsEntity) {
        validateReferencedPersonToBeRepresentedByLoginUserPerson(newCredentialsEntity);
        validateNormalUsersOnlyAccessPublicContexts(newCredentialsEntity);
@@ -324,8 +305,6 @@ public class HsCredentialsController implements CredentialsApi {
    }

    final BiConsumer<HsCredentialsEntity, CredentialsResource> ENTITY_TO_RESOURCE_POSTMAPPER = (entity, resource) -> {
-        ofNullable(entity.getLastUsed()).ifPresent(
-            dt -> resource.setLastUsed(dt.atOffset(ZoneOffset.UTC)));
        of(entity.getSubject()).ifPresent(
                subject -> resource.setNickname(subject.getName())
        );
@@ -9,9 +9,7 @@ import net.hostsharing.hsadminng.repr.Stringify;
 import net.hostsharing.hsadminng.repr.Stringifyable;
 // import net.hostsharing.hsadminng.rbac.RbacSubjectEntity; // Assuming RbacSubjectEntity exists for the FK relationship

-import java.time.LocalDateTime;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 import java.util.UUID;

@@ -31,8 +29,6 @@ public class HsCredentialsEntity implements BaseEntity<HsCredentialsEntity>, Str
    protected static Stringify<HsCredentialsEntity> stringify = stringify(HsCredentialsEntity.class, "credentials")
            .withProp(HsCredentialsEntity::isActive)
            .withProp(HsCredentialsEntity::getEmailAddress)
-            .withProp(HsCredentialsEntity::getTotpSecrets)
-            .withProp(HsCredentialsEntity::getPhonePassword)
            .withProp(HsCredentialsEntity::getSmsNumber)
            .quotedValues(false);

@@ -51,9 +47,6 @@ public class HsCredentialsEntity implements BaseEntity<HsCredentialsEntity>, Str
    @Version
    private int version;

-    @Column
-    private LocalDateTime lastUsed;
-
    @Column
    private boolean active;

@@ -63,15 +56,6 @@ public class HsCredentialsEntity implements BaseEntity<HsCredentialsEntity>, Str
    @Column
    private Integer globalGid;

-    @Column
-    private String onboardingToken;
-
-    @Column
-    private List<String> totpSecrets;
-
-    @Column
-    private String phonePassword;
-
    @Column
    private String emailAddress;

@@ -4,7 +4,6 @@ import net.hostsharing.hsadminng.accounts.generated.api.v1.model.CredentialsPatc
 import net.hostsharing.hsadminng.mapper.EntityPatcher;
 import net.hostsharing.hsadminng.mapper.OptionalFromJson;

-import java.util.Optional;

 public class HsCredentialsEntityPatcher implements EntityPatcher<CredentialsPatchResource> {

@@ -23,12 +22,8 @@ public class HsCredentialsEntityPatcher implements EntityPatcher<CredentialsPatc
        }
        OptionalFromJson.of(resource.getEmailAddress())
                .ifPresent(entity::setEmailAddress);
-        Optional.ofNullable(resource.getTotpSecrets())
-                .ifPresent(entity::setTotpSecrets);
        OptionalFromJson.of(resource.getSmsNumber())
                .ifPresent(entity::setSmsNumber);
-        OptionalFromJson.of(resource.getPhonePassword())
-                .ifPresent(entity::setPhonePassword);
        if (resource.getContexts() != null) {
            contextMapper.syncCredentialsContextEntities(resource.getContexts(), entity.getLoginContexts());
        }
@@ -21,9 +21,6 @@ paths:

  # Credentials

-  /api/hs/accounts/credentials/{credentialsUuid}/used:
-    $ref: "credentials-with-uuid-used.yaml"
-
  /api/hs/accounts/credentials/{credentialsUuid}:
    $ref: "credentials-with-uuid.yaml"

@@ -24,12 +24,6 @@ components:
                nickname:
                    type: string
                    pattern: '^[a-z][a-z0-9]{1,8}-[a-z0-9]{1,10}$' # TODO.spec: pattern for login nickname
-                totpSecrets:
-                    type: array
-                    items:
-                        type: string
-                phonePassword:
-                    type: string
                emailAddress:
                    type: string
                smsNumber:
@@ -40,15 +34,10 @@ components:
                    type: number
                globalGid:
                    type: number
-                onboardingToken:
-                    type: string
                contexts:
                    type: array
                    items:
                        $ref: 'context-schemas.yaml#/components/schemas/Context'
-                lastUsed:
-                    type: string
-                    format: date-time
            required:
                - uuid
                - active
@@ -58,13 +47,6 @@ components:
        CredentialsPatch:
            type: object
            properties:
-                totpSecrets:
-                    type: array
-                    items:
-                        type: string
-                phonePassword:
-                    type: string
-                    nullable: true
                emailAddress:
                    type: string
                    nullable: true
@@ -88,12 +70,6 @@ components:
                nickname:
                    type: string
                    pattern: '^[a-z][a-z0-9]{1,8}-[a-z0-9]{1,10}$' # TODO.spec: pattern for login nickname
-                totpSecrets:
-                    type: array
-                    items:
-                        type: string
-                phonePassword:
-                    type: string
                emailAddress:
                    type: string
                smsNumber:
@@ -104,8 +80,6 @@ components:
                    type: number
                globalGid:
                    type: number
-                onboardingToken:
-                    type: string
                contexts:
                    type: array
                    items:
@@ -1,23 +0,0 @@
-post:
-    tags:
-        - credentials
-    description: 'Is called when credentials got used for a login.'
-    operationId: credentialsUsed
-    parameters:
-        -   name: credentialsUuid
-            in: path
-            required: true
-            schema:
-                type: string
-                format: uuid
-    responses:
-        "200":
-            description: OK
-            content:
-                'application/json':
-                    schema:
-                        $ref: 'credentials-schemas.yaml#/components/schemas/Credentials'
-        "401":
-            $ref: 'error-responses.yaml#/components/responses/Unauthorized'
-        "403":
-            $ref: 'error-responses.yaml#/components/responses/Forbidden'
@@ -13,13 +13,9 @@ create table hs_accounts.credentials
    person_uuid      uuid not null references hs_office.person(uuid),

    active           bool,
-    last_used        timestamp,
    global_uid       int unique,     -- w/o
    global_gid       int unique,     -- w/o
-    onboarding_token text,           -- w/o, but can be set to null to invalidate

-    totp_secrets     text[],
-    phone_password   text,
    email_address    text,
    sms_number       text
 );
@@ -68,10 +68,10 @@ begin
 --     call rbac.grantRoleToRole(hs_accounts.context_REFERRER(context_MATRIX_internal), rbac.global_ADMIN());

    -- Add test credentials (linking to assumed rbac.subject UUIDs)
-    INSERT INTO hs_accounts.credentials (uuid, version, person_uuid, active, global_uid, global_gid, onboarding_token, totp_secrets, phone_password, email_address, sms_number) VALUES
-        ( superuserAlexSubjectUuid, 0, personAlexUuid, true, 1001, 1001, 'token-abc', ARRAY['otp-secret-1a', 'otp-secret-1b'], 'phone-pw-1', 'alex@example.com', '111-222-3333'),
-        ( superuserFranSubjectUuid, 0, personFranUuid, true, 1002, 1002, 'token-def', ARRAY['otp-secret-2'], 'phone-pw-2', 'fran@example.com', '444-555-6666'),
-        ( userDrewSubjectUuid, 0, personDrewUuid, true, 1003, 1003, 'token-def', ARRAY['otp-secret-3'], 'phone-pw-3', 'drew@example.org', '999-888-7777');
+    INSERT INTO hs_accounts.credentials (uuid, version, person_uuid, active, global_uid, global_gid, email_address, sms_number) VALUES
+        ( superuserAlexSubjectUuid, 0, personAlexUuid, true, 1001, 1001, 'alex@example.com', '111-222-3333'),
+        ( superuserFranSubjectUuid, 0, personFranUuid, true, 1002, 1002, 'fran@example.com', '444-555-6666'),
+        ( userDrewSubjectUuid, 0, personDrewUuid, true, 1003, 1003, 'drew@example.org', '999-888-7777');

    -- Map credentials to contexts
    INSERT INTO hs_accounts.context_mapping (credentials_uuid, context_uuid) VALUES
@@ -27,7 +27,6 @@ import jakarta.persistence.PersistenceContext;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.function.Consumer;
-import java.util.stream.Collectors;

 import static net.hostsharing.hsadminng.config.JwtFakeBearer.bearer;
 import static net.hostsharing.hsadminng.hs.office.person.HsOfficePersonType.LEGAL_PERSON;
@@ -36,9 +35,6 @@ import static net.hostsharing.hsadminng.test.JsonMatcher.lenientlyEquals;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.not;
-import static org.hamcrest.Matchers.nullValue;

@Tag("generalIntegrationTest")
@Transactional
@@ -136,14 +132,11 @@ class HsCredentialsControllerAcceptanceTest extends ContextBasedTestWithCleanup
                                    "familyName": null
                                },
                                "nickname": "test-subject1",
-                                "totpSecrets": null,
-                                "phonePassword": null,
                                "emailAddress": null,
                                "smsNumber": null,
                                "active": false,
                                "globalUid": null,
                                "globalGid": null,
-                                "onboardingToken": null,
                                "contexts": [
                                    {
                                        "uuid": "33333333-3333-3333-3333-333333333333",
@@ -166,8 +159,7 @@ class HsCredentialsControllerAcceptanceTest extends ContextBasedTestWithCleanup
                                        "onlyForNaturalPersons": false,
                                        "publicAccess": true
                                    }
-                                ],
-                                "lastUsed": null
+                                ]
                            }
                            """));
            // @formatter:on
@@ -369,37 +361,6 @@ class HsCredentialsControllerAcceptanceTest extends ContextBasedTestWithCleanup
        }
    }

-    @Nested
-    class MarkCredentialsAsUsed {
-
-        @Test
-        void markCredentialsAsUsed() {
-            // given
-            val testPerson = givenNaturalPerson("selfregistered-user-drew@hostsharing.org");
-            val credentialsEntity = givenNewCredentials("selfregistered-user-drew@hostsharing.org",
-                    "test-subject2",
-                    testPerson, builder -> {
-                builder.onboardingToken("some-onboarding-token");
-                builder.loginContexts(contextRepo.findAll().stream()
-                        .filter(HsCredentialsContext::isPublicAccess).collect(Collectors.toSet()));
-            });
-
-            RestAssured // @formatter:off
-                    .given()
-                        .header("Authorization", bearer("superuser-alex@hostsharing.net"))
-                        .port(port)
-                    .when()
-                        .post("http://localhost/api/hs/accounts/credentials/" + credentialsEntity.getUuid() + "/used")
-                    .then().log().all().assertThat()
-                        .statusCode(200)
-                        .contentType("application/json")
-                        .body("uuid", is(credentialsEntity.getUuid().toString()))
-                        .body("onboardingToken", is(nullValue()))
-                        .body("lastUsed", is(not(nullValue())));
-            // @formatter:on
-        }
-    }
-
    // Helper methods
    private HsOfficePersonRealEntity givenLegalPerson(final String executingSubjectName) {
        return jpaAttempt.transacted(() -> {
@@ -33,15 +33,11 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase<

    private static final Boolean INITIAL_ACTIVE = true;
    private static final String INITIAL_EMAIL_ADDRESS = "initial@example.com";
-    private static final List<String> INITIAL_TOTP_SECRETS = List.of("initial_2fa");
    private static final String INITIAL_SMS_NUMBER = "initial_sms";
-    private static final String INITIAL_PHONE_PASSWORD = "initial_phone_pw";

    private static final Boolean PATCHED_ACTIVE = false;
    private static final String PATCHED_EMAIL_ADDRESS = "patched@example.com";
-    private static final List<String> PATCHED_TOTP_SECRETS = List.of("patched_2fa");
    private static final String PATCHED_SMS_NUMBER = "patched_sms";
-    private static final String PATCHED_PHONE_PASSWORD = "patched_phone_pw";

    // Contexts
    private static final UUID CONTEXT_UUID_1 = UUID.randomUUID();
@@ -102,9 +98,7 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase<
        entity.setUuid(INITIAL_CREDENTIALS_UUID);
        entity.setActive(INITIAL_ACTIVE);
        entity.setEmailAddress(INITIAL_EMAIL_ADDRESS);
-        entity.setTotpSecrets(INITIAL_TOTP_SECRETS);
        entity.setSmsNumber(INITIAL_SMS_NUMBER);
-        entity.setPhonePassword(INITIAL_PHONE_PASSWORD);
        // Ensure loginContexts is a mutable set for the patcher
        entity.setLoginContexts(new HashSet<>(initialContextEntities));
        return entity;
@@ -137,25 +131,12 @@ class HsCredentialsEntityPatcherUnitTest extends PatchUnitTestBase<
                        PATCHED_EMAIL_ADDRESS,
                        HsCredentialsEntity::setEmailAddress,
                        PATCHED_EMAIL_ADDRESS),
-                new SimpleProperty<>(
-                        "totpSecret",
-                        CredentialsPatchResource::setTotpSecrets,
-                        PATCHED_TOTP_SECRETS,
-                        HsCredentialsEntity::setTotpSecrets,
-                        PATCHED_TOTP_SECRETS)
-                        .notNullable(),
                new JsonNullableProperty<>(
                        "smsNumber",
                        CredentialsPatchResource::setSmsNumber,
                        PATCHED_SMS_NUMBER,
                        HsCredentialsEntity::setSmsNumber,
                        PATCHED_SMS_NUMBER),
-                new JsonNullableProperty<>(
-                        "phonePassword",
-                        CredentialsPatchResource::setPhonePassword,
-                        PATCHED_PHONE_PASSWORD,
-                        HsCredentialsEntity::setPhonePassword,
-                        PATCHED_PHONE_PASSWORD),
                new SimpleProperty<>(
                        "contexts",
                        CredentialsPatchResource::setContexts,
@@ -36,11 +36,8 @@ public class CreateCredentials extends BaseCredentialsUseCase<CreateCredentials>
                     "person.uuid": ${Person: %{personGivenName} %{personFamilyName}},
                     "nickname": ${nickname},
                     "active": %{active},
-                     "totpSecrets": @{totpSecrets},
                     "emailAddress": ${emailAddress},
-                     "phonePassword": ${phonePassword},
                     "smsNumber": ${smsNumber},
-                     "onboardingToken": ${onboardingToken},
                     "globalUid": %{globalUid},
                     "globalGid": %{globalGid},
                     "contexts": @{resolvedContexts}
@@ -58,9 +55,7 @@ public class CreateCredentials extends BaseCredentialsUseCase<CreateCredentials>
                        .expecting(OK).expecting(JSON),
                path("uuid").contains("%{newCredentials}"),
                path("nickname").contains("%{nickname}"),
-                path("person.uuid").contains("%{Person: %{personGivenName} %{personFamilyName}}"),
-                path("totpSecrets").contains("@{totpSecrets}"),
-                path("onboardingToken").contains("%{onboardingToken}")
+                path("person.uuid").contains("%{Person: %{personGivenName} %{personFamilyName}}")
        );
    }
 }
@@ -75,9 +75,7 @@ class CredentialsScenarioTests extends ScenarioTest {
                    .given("nickname", "firby-susan")
                    // initial credentials
                    .given("active", true)
-                    .given("totpSecrets", Array.of("initialSecret"))
                    .given("emailAddress", "susan.firby@example.com")
-                    .given("phonePassword", "securePass123")
                    .given("smsNumber", "+49123456789")
                    .given("globalUid", 21011)
                    .given("globalGid", 21011)
@@ -85,7 +83,6 @@ class CredentialsScenarioTests extends ScenarioTest {
                            "contexts", Array.of(
                                    Pair.of("HSADMIN", "prod")
                            ))
-                    .given("onboardingToken", "fake-unboarding-token")
                    .doRun()
                    .keep();
        }
@@ -99,9 +96,7 @@ class CredentialsScenarioTests extends ScenarioTest {
                    .given("credentialsUuid", "%{Credentials@hsadmin: firby-susan}")
                    // updated credentials
                    .given("active", false)
-                    .given("totpSecrets", Array.of("initialSecret", "additionalSecret"))
                    .given("emailAddress", "susan.firby@example.org")
-                    .given("phonePassword", "securePass987")
                    .given("smsNumber", "+49987654321")
                    .given(
                            "contexts", Array.of(
@@ -27,16 +27,12 @@ public class UpdateCredentials extends BaseCredentialsUseCase<UpdateCredentials>
            httpPatch("/api/hs/accounts/credentials/%{credentialsUuid}", usingJsonBody("""
                {
                     "active": %{active},
-                     "totpSecrets": @{totpSecrets},
-                     "emailAddress": ${emailAddress},
-                     "phonePassword": ${phonePassword},
                     "smsNumber": ${smsNumber},
                     "contexts": @{resolvedContexts}
                }
                """))
                .reportWithResponse().expecting(HttpStatus.OK).expecting(ContentType.JSON)
                .extractValue("nickname", "nickname")
-                .extractValue("totpSecrets", "totpSecrets")
        );

        return null;
@@ -49,8 +45,7 @@ public class UpdateCredentials extends BaseCredentialsUseCase<UpdateCredentials>
                () -> httpGet("/api/hs/accounts/credentials/%{credentialsUuid}")
                        .expecting(OK).expecting(JSON),
                path("uuid").contains("%{newCredentials}"),
-                path("nickname").contains("%{nickname}"),
-                path("totpSecrets").contains("%{totpSecrets}")
+                path("nickname").contains("%{nickname}")
        );
    }
 }
@@ -31,12 +31,12 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.startsWith;

-@Transactional
@Tag("officeIntegrationTest")
@SpringBootTest(
        webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
        classes = HsadminNgApplication.class)
@ActiveProfiles("fake-jwt")
+@Transactional
 class HsOfficeBankAccountControllerAcceptanceTest extends ContextBasedTestWithCleanup {

    @LocalServerPort