From b56582a8e236832b01bbba4632e8a45a9c4fab8d Mon Sep 17 00:00:00 2001 From: Lubos Racansky Date: Thu, 29 Sep 2022 07:15:12 +0200 Subject: [PATCH] Fix #351: Service for automatic cleanup of activations with failed onboarding --- docs/sql/mysql/onboarding/create-schema.sql | 1 + docs/sql/oracle/onboarding/create-schema.sql | 1 + .../postgresql/onboarding/create-schema.sql | 1 + .../database/OnboardingProcessRepository.java | 25 +++++ .../entity/OnboardingProcessEntity.java | 7 ++ .../impl/service/ActivationService.java | 20 ++++ .../impl/service/OnboardingServiceImpl.java | 5 +- .../cleaning/ActivationCleaningService.java | 90 +++++++++++++++++ .../task/cleaning/CleaningTask.java | 18 +++- .../ActivationCleaningServiceTest.java | 98 +++++++++++++++++++ .../ActivationCleaningServiceTest.sql | 4 + 11 files changed, 266 insertions(+), 4 deletions(-) create mode 100644 enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/task/cleaning/ActivationCleaningService.java create mode 100644 enrollment-server-onboarding/src/test/java/com/wultra/app/onboardingserver/task/cleaning/ActivationCleaningServiceTest.java create mode 100644 enrollment-server-onboarding/src/test/resources/com/wultra/app/onboardingserver/task/cleaning/ActivationCleaningServiceTest.sql diff --git a/docs/sql/mysql/onboarding/create-schema.sql b/docs/sql/mysql/onboarding/create-schema.sql index 50c0163f6..52f38b0c2 100644 --- a/docs/sql/mysql/onboarding/create-schema.sql +++ b/docs/sql/mysql/onboarding/create-schema.sql @@ -22,6 +22,7 @@ CREATE TABLE es_onboarding_process ( user_id VARCHAR(256), activation_id VARCHAR(36), status VARCHAR(32) NOT NULL, + activation_removed TINYINT DEFAULT 0, error_detail VARCHAR(256), error_origin VARCHAR(256), error_score INTEGER NOT NULL DEFAULT 0, diff --git a/docs/sql/oracle/onboarding/create-schema.sql b/docs/sql/oracle/onboarding/create-schema.sql index a8eed2466..19e34312a 100644 --- a/docs/sql/oracle/onboarding/create-schema.sql +++ b/docs/sql/oracle/onboarding/create-schema.sql @@ -24,6 +24,7 @@ CREATE TABLE ES_ONBOARDING_PROCESS ( USER_ID VARCHAR2(256 CHAR), ACTIVATION_ID VARCHAR2(36 CHAR), STATUS VARCHAR2(32 CHAR) NOT NULL, + ACTIVATION_REMOVED NUMBER(1) DEFAULT 0, ERROR_DETAIL VARCHAR2(256 CHAR), ERROR_ORIGIN VARCHAR2(256 CHAR), ERROR_SCORE INTEGER DEFAULT 0 NOT NULL, diff --git a/docs/sql/postgresql/onboarding/create-schema.sql b/docs/sql/postgresql/onboarding/create-schema.sql index 233bab984..7002b3882 100644 --- a/docs/sql/postgresql/onboarding/create-schema.sql +++ b/docs/sql/postgresql/onboarding/create-schema.sql @@ -28,6 +28,7 @@ CREATE TABLE es_onboarding_process ( user_id VARCHAR(256), activation_id VARCHAR(36), status VARCHAR(32) NOT NULL, + activation_removed BOOLEAN DEFAULT FALSE, error_detail VARCHAR(256), error_origin VARCHAR(256), error_score INTEGER NOT NULL DEFAULT 0, diff --git a/enrollment-server-onboarding-common/src/main/java/com/wultra/app/onboardingserver/common/database/OnboardingProcessRepository.java b/enrollment-server-onboarding-common/src/main/java/com/wultra/app/onboardingserver/common/database/OnboardingProcessRepository.java index d0daadab4..96f9ce2f0 100644 --- a/enrollment-server-onboarding-common/src/main/java/com/wultra/app/onboardingserver/common/database/OnboardingProcessRepository.java +++ b/enrollment-server-onboarding-common/src/main/java/com/wultra/app/onboardingserver/common/database/OnboardingProcessRepository.java @@ -21,6 +21,8 @@ import com.wultra.app.enrollmentserver.model.enumeration.ErrorOrigin; import com.wultra.app.enrollmentserver.model.enumeration.OnboardingStatus; import com.wultra.app.onboardingserver.common.database.entity.OnboardingProcessEntity; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.CrudRepository; @@ -96,4 +98,27 @@ public interface OnboardingProcessRepository extends CrudRepository com.wultra.app.enrollmentserver.model.enumeration.OnboardingStatus.FAILED " + "AND p.timestampCreated < :dateCreatedBefore") List findExpiredProcessIdsByCreatedDate(Date dateCreatedBefore); + + /** + * Return onboarding processes to remove activation. + * + * @param pageable pagination information + * @return onboarding processes + * @see #findProcessesToRemoveActivation(int) + */ + @Query("SELECT p FROM OnboardingProcessEntity p " + + "WHERE p.status = com.wultra.app.enrollmentserver.model.enumeration.OnboardingStatus.FAILED " + + "AND p.activationId is not null " + + "AND p.activationRemoved = false") + List findProcessesToRemoveActivation(Pageable pageable); + + /** + * Return onboarding processes to remove activation. + * + * @param maxCount max count of results + * @return onboarding processes + */ + default List findProcessesToRemoveActivation(int maxCount) { + return findProcessesToRemoveActivation(PageRequest.of(0, maxCount)); + } } diff --git a/enrollment-server-onboarding-common/src/main/java/com/wultra/app/onboardingserver/common/database/entity/OnboardingProcessEntity.java b/enrollment-server-onboarding-common/src/main/java/com/wultra/app/onboardingserver/common/database/entity/OnboardingProcessEntity.java index e7c8df131..a55ffb8ac 100644 --- a/enrollment-server-onboarding-common/src/main/java/com/wultra/app/onboardingserver/common/database/entity/OnboardingProcessEntity.java +++ b/enrollment-server-onboarding-common/src/main/java/com/wultra/app/onboardingserver/common/database/entity/OnboardingProcessEntity.java @@ -75,6 +75,13 @@ public class OnboardingProcessEntity implements Serializable { @Column(name = "status", nullable = false) private OnboardingStatus status; + /** + * When the status is {@link OnboardingStatus#FAILED}, the activation specified be {@link #activationId} should be removed at PowerAuth server. + * This flag indicates that the task has been done. + */ + @Column(name = "activation_removed") + private boolean activationRemoved; + @Column(name = "error_detail") private String errorDetail; diff --git a/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/impl/service/ActivationService.java b/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/impl/service/ActivationService.java index 4c40ede33..406187fa0 100644 --- a/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/impl/service/ActivationService.java +++ b/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/impl/service/ActivationService.java @@ -22,6 +22,8 @@ import com.wultra.app.onboardingserver.common.errorhandling.RemoteCommunicationException; import com.wultra.security.powerauth.client.PowerAuthClient; import com.wultra.security.powerauth.client.model.error.PowerAuthClientException; +import com.wultra.security.powerauth.client.v3.ActivationStatus; +import com.wultra.security.powerauth.client.v3.GetActivationStatusRequest; import com.wultra.security.powerauth.client.v3.RemoveActivationRequest; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; @@ -62,4 +64,22 @@ public void removeActivation(final String activationId) throws RemoteCommunicati throw new RemoteCommunicationException("Communication with PowerAuth server failed: " + e.getMessage(), e); } } + + /** + * Return activation status. + * + * @param activationId Activation ID. + * @return activation status + * @throws RemoteCommunicationException Thrown when communication with PowerAuth server fails. + */ + public ActivationStatus fetchActivationStatus(final String activationId) throws RemoteCommunicationException { + final GetActivationStatusRequest request = new GetActivationStatusRequest(); + request.setActivationId(activationId); + + try { + return powerAuthClient.getActivationStatus(request).getActivationStatus(); + } catch (PowerAuthClientException e) { + throw new RemoteCommunicationException("Communication with PowerAuth server failed: " + e.getMessage(), e); + } + } } diff --git a/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/impl/service/OnboardingServiceImpl.java b/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/impl/service/OnboardingServiceImpl.java index 06671ce51..038eae86a 100644 --- a/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/impl/service/OnboardingServiceImpl.java +++ b/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/impl/service/OnboardingServiceImpl.java @@ -227,15 +227,16 @@ public Response performCleanup(OnboardingCleanupRequest request) throws Onboardi otpService.cancelOtp(process, OtpType.ACTIVATION); otpService.cancelOtp(process, OtpType.USER_VERIFICATION); + removeActivation(process); + process.setStatus(OnboardingStatus.FAILED); process.setErrorDetail(OnboardingProcessEntity.ERROR_PROCESS_CANCELED); process.setErrorOrigin(ErrorOrigin.USER_REQUEST); process.setTimestampLastUpdated(new Date()); process.setTimestampFailed(new Date()); + process.setActivationRemoved(true); onboardingProcessRepository.save(process); - removeActivation(process); - return new Response(); } diff --git a/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/task/cleaning/ActivationCleaningService.java b/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/task/cleaning/ActivationCleaningService.java new file mode 100644 index 000000000..10e823052 --- /dev/null +++ b/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/task/cleaning/ActivationCleaningService.java @@ -0,0 +1,90 @@ +/* + * PowerAuth Enrollment Server + * Copyright (C) 2022 Wultra s.r.o. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package com.wultra.app.onboardingserver.task.cleaning; + +import com.wultra.app.onboardingserver.common.database.OnboardingProcessRepository; +import com.wultra.app.onboardingserver.common.database.entity.OnboardingProcessEntity; +import com.wultra.app.onboardingserver.common.errorhandling.RemoteCommunicationException; +import com.wultra.app.onboardingserver.impl.service.ActivationService; +import com.wultra.security.powerauth.client.v3.ActivationStatus; +import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; + +import java.util.List; + +/** + * Service to cleaning activations. + * + * @author Lubos Racansky, lubos.racansky@wultra.com + */ +@Service +@Slf4j +class ActivationCleaningService { + + private static final int BATCH_SIZE = 100; + + private final OnboardingProcessRepository onboardingProcessRepository; + + private final ActivationService activationService; + + @Autowired + public ActivationCleaningService( + final OnboardingProcessRepository onboardingProcessRepository, + final ActivationService activationService) { + + this.onboardingProcessRepository = onboardingProcessRepository; + this.activationService = activationService; + } + + /** + * Cleanup activations of failed onboarding processes. + */ + public void cleanupActivations() { + final List processes = onboardingProcessRepository.findProcessesToRemoveActivation(BATCH_SIZE); + if (processes.isEmpty()) { + logger.debug("No onboarding processes to remove activation"); + return; + } + + processes.forEach(this::cleanupActivation); + } + + private void cleanupActivation(final OnboardingProcessEntity process) { + final String activationId = process.getActivationId(); + logger.info("Removing activation ID: {} of process ID: {}", activationId, process.getId()); + + try { + removeActivation(activationId); + process.setActivationRemoved(true); + onboardingProcessRepository.save(process); + } catch (RemoteCommunicationException e) { + logger.error("Unable to remove activation ID: {}", activationId, e); + } + } + + private void removeActivation(String activationId) throws RemoteCommunicationException { + final ActivationStatus activationStatus = activationService.fetchActivationStatus(activationId); + if (activationStatus == ActivationStatus.REMOVED) { + logger.debug("Activation ID: {} has been already removed", activationId); + return; + } + + activationService.removeActivation(activationId); + } +} diff --git a/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/task/cleaning/CleaningTask.java b/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/task/cleaning/CleaningTask.java index 2ab220fae..305b33cc1 100644 --- a/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/task/cleaning/CleaningTask.java +++ b/enrollment-server-onboarding/src/main/java/com/wultra/app/onboardingserver/task/cleaning/CleaningTask.java @@ -25,7 +25,7 @@ import org.springframework.stereotype.Component; /** - * Task to clean expired processes, identity verifications, documents and OTPs. + * Task to clean expired processes, identity verifications, documents, activations and OTPs. * * @author Lubos Racansky, lubos.racansky@wultra.com */ @@ -35,9 +35,12 @@ public class CleaningTask { private final CleaningService cleaningService; + private final ActivationCleaningService activationCleaningService; + @Autowired - public CleaningTask(final CleaningService cleaningService) { + public CleaningTask(final CleaningService cleaningService, final ActivationCleaningService activationCleaningService) { this.cleaningService = cleaningService; + this.activationCleaningService = activationCleaningService; } /** @@ -110,4 +113,15 @@ public void terminateExpiredIdentityVerifications() { logger.debug("terminateExpiredIdentityVerifications"); cleaningService.terminateExpiredIdentityVerifications(); } + + /** + * Cleanup activations of failed onboarding processes. + */ + @Scheduled(fixedDelayString = "PT60S", initialDelayString = "PT60S") + @SchedulerLock(name = "cleanupActivations", lockAtLeastFor = "1s", lockAtMostFor = "5m") + public void cleanupActivations() { + LockAssert.assertLocked(); + logger.debug("cleanupActivations"); + activationCleaningService.cleanupActivations(); + } } diff --git a/enrollment-server-onboarding/src/test/java/com/wultra/app/onboardingserver/task/cleaning/ActivationCleaningServiceTest.java b/enrollment-server-onboarding/src/test/java/com/wultra/app/onboardingserver/task/cleaning/ActivationCleaningServiceTest.java new file mode 100644 index 000000000..90c23dcb3 --- /dev/null +++ b/enrollment-server-onboarding/src/test/java/com/wultra/app/onboardingserver/task/cleaning/ActivationCleaningServiceTest.java @@ -0,0 +1,98 @@ +/* + * PowerAuth Enrollment Server + * Copyright (C) 2022 Wultra s.r.o. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package com.wultra.app.onboardingserver.task.cleaning; + +import com.wultra.app.onboardingserver.EnrollmentServerTestApplication; +import com.wultra.app.onboardingserver.common.database.entity.OnboardingProcessEntity; +import com.wultra.app.onboardingserver.common.errorhandling.RemoteCommunicationException; +import com.wultra.app.onboardingserver.impl.service.ActivationService; +import com.wultra.security.powerauth.client.v3.ActivationStatus; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.jdbc.Sql; +import org.springframework.transaction.annotation.Transactional; + +import javax.persistence.EntityManager; + +import static org.mockito.Mockito.*; +import static org.springframework.test.util.AssertionErrors.assertFalse; +import static org.springframework.test.util.AssertionErrors.assertTrue; + +/** + * Test for {@link ActivationCleaningService}. + * + * @author Lubos Racansky, lubos.racansky@wultra.com + */ +@SpringBootTest(classes = EnrollmentServerTestApplication.class) +@ActiveProfiles("mock") +@Transactional +@Sql +class ActivationCleaningServiceTest { + + @Autowired + private ActivationCleaningService tested; + + @MockBean + private ActivationService activationService; + + @Autowired + private EntityManager entityManager; + + @Test + void testSuccessful() throws Exception { + when(activationService.fetchActivationStatus("a2")) + .thenReturn(ActivationStatus.ACTIVE); + + tested.cleanupActivations(); + + final OnboardingProcessEntity process = fetchOnboardingProcess("22222222-df91-4053-bb3d-3970979baf5d"); + assertTrue("activation should be marked as removed", process.isActivationRemoved()); + verify(activationService).removeActivation("a2"); + } + + @Test + void testAlreadyDeleted() throws Exception { + when(activationService.fetchActivationStatus("a2")) + .thenReturn(ActivationStatus.REMOVED); + + tested.cleanupActivations(); + + final OnboardingProcessEntity process = fetchOnboardingProcess("22222222-df91-4053-bb3d-3970979baf5d"); + assertTrue("activation should be marked as removed", process.isActivationRemoved()); + verify(activationService, never()).removeActivation("a2"); + } + + @Test + void testCommunicationException() throws Exception { + when(activationService.fetchActivationStatus("a2")) + .thenThrow(new RemoteCommunicationException("test exception")); + + tested.cleanupActivations(); + + final OnboardingProcessEntity process = fetchOnboardingProcess("22222222-df91-4053-bb3d-3970979baf5d"); + assertFalse("activation should not be marked as removed", process.isActivationRemoved()); + verify(activationService, never()).removeActivation("a2"); + } + + private OnboardingProcessEntity fetchOnboardingProcess(final String id) { + return entityManager.find(OnboardingProcessEntity.class, id); + } +} diff --git a/enrollment-server-onboarding/src/test/resources/com/wultra/app/onboardingserver/task/cleaning/ActivationCleaningServiceTest.sql b/enrollment-server-onboarding/src/test/resources/com/wultra/app/onboardingserver/task/cleaning/ActivationCleaningServiceTest.sql new file mode 100644 index 000000000..dc30c06b0 --- /dev/null +++ b/enrollment-server-onboarding/src/test/resources/com/wultra/app/onboardingserver/task/cleaning/ActivationCleaningServiceTest.sql @@ -0,0 +1,4 @@ +INSERT INTO es_onboarding_process(id, identification_data, status, activation_id, activation_removed, error_score, timestamp_created) VALUES + ('11111111-df91-4053-bb3d-3970979baf5d', '{}', 'ACTIVATION_IN_PROGRESS', 'a1', false, 0, now()), + ('22222222-df91-4053-bb3d-3970979baf5d', '{}', 'FAILED', 'a2', false, 0, now()), + ('33333333-df91-4053-bb3d-3970979baf5d', '{}', 'FAILED', 'a3', true, 0, now());