Skip to content

Commit

Permalink
Combine CERN HR logic with internal life-cycle (#844)
Browse files Browse the repository at this point in the history
* This combination enables the grace period in case a non valid participation is found from HR database
* Update end-time with CERN HR voPerson participation endDate value
* Compare user's endTime only with current date part
* Consider as not expired all the endTimes those expire during the current date
* Removed CERN labels such as Action and Timestamp
* Events raised only if the update is effective
* Added and fixed tests
  • Loading branch information
enricovianello authored Sep 27, 2024
1 parent db2ee2c commit 30284b9
Show file tree
Hide file tree
Showing 23 changed files with 910 additions and 594 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public void setLabel(@PathVariable String id, @RequestBody @Validated LabelDTO l
handleValidationError(validationResult);
IamAccount account = service.findByUuid(id).orElseThrow(noSuchAccountError(id));

service.setLabel(account, converter.entityFromDto(label));
service.addLabel(account, converter.entityFromDto(label));
}

@RequestMapping(method = DELETE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,17 @@

import static it.infn.mw.iam.api.utils.ValidationErrorUtils.stringifyValidationError;
import static java.lang.String.format;
import static org.springframework.web.bind.annotation.RequestMethod.PUT;

import java.util.function.Supplier;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.http.converter.HttpMessageNotReadableException;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.validation.BindingResult;
import org.springframework.validation.annotation.Validated;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.ResponseStatus;
Expand All @@ -53,7 +52,6 @@ public class AccountLifecycleController {
private final IamAccountService service;
private final LifecycleProperties properties;

@Autowired
public AccountLifecycleController(IamAccountService accountService,
LifecycleProperties properties) {
this.service = accountService;
Expand All @@ -71,7 +69,7 @@ private void handleValidationError(BindingResult result) {
}
}

@RequestMapping(method = PUT)
@PutMapping
public void setEndTime(@PathVariable String id, @RequestBody @Validated AccountLifecycleDTO dto,
BindingResult validationResult) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
@Profile("cern")
public interface CernHrDBApiService {

boolean hasValidExperimentParticipation(String personId);

VOPersonDTO getHrDbPersonRecord(String personId);

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Profile;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
Expand Down Expand Up @@ -48,7 +47,6 @@ public class DefaultCernHrDBApiService implements CernHrDBApiService {
final RestTemplateFactory rtFactory;
final CernProperties properties;

@Autowired
public DefaultCernHrDBApiService(RestTemplateFactory rtFactory, CernProperties properties) {
this.rtFactory = rtFactory;
this.properties = properties;
Expand All @@ -64,26 +62,6 @@ private HttpHeaders buildAuthHeaders() {
return headers;
}

@Override
public boolean hasValidExperimentParticipation(String personId) {
RestTemplate rt = rtFactory.newRestTemplate();

String personValidUrl = String.format("%s%s", properties.getHrApi().getUrl(),
format(PARTICIPATION_API_PATH_TEMPLATE, properties.getExperimentName(), personId));

LOG.debug("Querying HR db participation API for person {} at URL {}", personId, personValidUrl);

try {

ResponseEntity<Boolean> response = rt.exchange(personValidUrl, HttpMethod.GET,
new HttpEntity<>(buildAuthHeaders()), Boolean.class);
return response.getBody();
} catch (RestClientException e) {
final String errorMsg = "HR db api error: " + e.getMessage();
throw new CernHrDbApiError(errorMsg, e);
}
}

@Override
public VOPersonDTO getHrDbPersonRecord(String personId) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import static it.infn.mw.iam.authn.ExternalAuthenticationHandlerSupport.EXT_AUTHN_UNREGISTERED_USER_AUTH;
import static org.springframework.security.web.context.HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY;
import static org.springframework.web.bind.annotation.RequestMethod.GET;

import java.time.Instant;
import java.util.Date;
Expand All @@ -30,7 +29,7 @@
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.stereotype.Controller;
import org.springframework.stereotype.Service;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.GetMapping;

import it.infn.mw.iam.api.registration.cern.CernHrDBApiService;
import it.infn.mw.iam.api.registration.cern.dto.InstituteDTO;
Expand All @@ -47,7 +46,7 @@ public class MockCernAuthController implements CernHrDBApiService {
@Autowired
CernProperties properties;

@RequestMapping(method = GET, path = "/mock-cern-auth")
@GetMapping("/mock-cern-auth")
public String mockCernAuthentication(HttpSession session) {

OidcSecurityContextBuilder builder = new OidcSecurityContextBuilder();
Expand All @@ -69,11 +68,6 @@ public String mockCernAuthentication(HttpSession session) {
return "redirect:/start-registration";
}

@Override
public boolean hasValidExperimentParticipation(String personId) {
return false;
}

@Override
public VOPersonDTO getHrDbPersonRecord(String personId) {
VOPersonDTO dto = new VOPersonDTO();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
public class ExpiredAccountsHandler implements Runnable {

public enum AccountLifecycleStatus {
OK, PENDING_SUSPENSION, PENDING_REMOVAL, SUSPENDED
PENDING_SUSPENSION, PENDING_REMOVAL, SUSPENDED
}

public static final String LIFECYCLE_TIMESTAMP_LABEL = "lifecycle.timestamp";
Expand Down Expand Up @@ -96,7 +96,7 @@ private boolean pastRemovalGracePeriod(IamAccount expiredAccount) {
}

private void addStatusLabel(IamAccount expiredAccount, AccountLifecycleStatus status) {
accountService.setLabel(expiredAccount,
accountService.addLabel(expiredAccount,
IamLabel.builder().name(LIFECYCLE_STATUS_LABEL).value(status.name()).build());
}

Expand All @@ -112,12 +112,13 @@ private void suspendAccount(IamAccount expiredAccount) {

if (expiredAccount.isActive()) {
LOG.info("Suspending account {} expired on {} ({} days ago)", expiredAccount.getUsername(),
expiredAccount.getEndTime(),
ChronoUnit.DAYS.between(expiredAccount.getEndTime().toInstant(), checkTime));
expiredAccount.getEndTime(),
ChronoUnit.DAYS.between(expiredAccount.getEndTime().toInstant(), checkTime));
accountService.disableAccount(expiredAccount);
} else {
// nothing to do
LOG.debug("Account {} expired on {} has been already suspended", expiredAccount.getUsername(), expiredAccount.getEndTime());
LOG.debug("Account {} expired on {} has been already suspended", expiredAccount.getUsername(),
expiredAccount.getEndTime());
}
if (properties.getAccount().getExpiredAccountPolicy().isRemoveExpiredAccounts()) {
markAsPendingRemoval(expiredAccount);
Expand Down Expand Up @@ -187,20 +188,21 @@ private void handleExpiredAccount(IamAccount expiredAccount) {

public void handleExpiredAccounts() {

LOG.info("Expired accounts handler ... [START]");

accountsScheduledForRemoval.clear();

LOG.debug("Starting...");
checkTime = clock.instant();
Date now = Date.from(checkTime);
checkTime = clock.instant().truncatedTo(ChronoUnit.DAYS);
LOG.debug("Comparing end-time with {}", checkTime);

Pageable pageRequest = PageRequest.of(0, PAGE_SIZE, Sort.by(Direction.ASC, "endTime"));

while (true) {
Page<IamAccount> expiredAccountsPage =
accountRepo.findExpiredAccountsAtTimestamp(now, pageRequest);
LOG.debug("expiredAccountsPage: {}", expiredAccountsPage);
accountRepo.findExpiredAccountsAtTimestamp(Date.from(checkTime), pageRequest);

if (expiredAccountsPage.hasContent()) {
LOG.debug("expiredAccountsPage [{}/{}]", expiredAccountsPage.getNumber()+1, expiredAccountsPage.getNumberOfElements());

for (IamAccount expiredAccount : expiredAccountsPage.getContent()) {
handleExpiredAccount(expiredAccount);
Expand All @@ -218,6 +220,9 @@ public void handleExpiredAccounts() {
for (IamAccount a : accountsScheduledForRemoval) {
removeAccount(a);
}

LOG.info("Expired accounts handler ... [END]");

}

@Override
Expand Down
Loading

0 comments on commit 30284b9

Please sign in to comment.