Skip to content

Commit

Permalink
Administrators can disable a client (#747)
Browse files Browse the repository at this point in the history
* Add API and service methods to change client status
* Add new columns for client status
* Set status as active for new client
* Add suspended label next to client name
* Add column status_changed_by to client_details
* Make client suspension details available to client owner
* Make fields available for limited client view
* Throw exception on user's update of suspended client

Co-authored-by: Manoj Garai <manoj.garai@stfc.ac.uk>
  • Loading branch information
enricovianello and garaimanoj committed May 24, 2024
1 parent b5c0486 commit b0fe8a1
Show file tree
Hide file tree
Showing 36 changed files with 1,363 additions and 817 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package it.infn.mw.iam.api.account.find;

import static it.infn.mw.iam.api.utils.FindUtils.responseFromPage;
import static it.infn.mw.iam.api.utils.FindUtils.responseFromOptional;

import java.util.Optional;
import java.util.function.Supplier;
Expand All @@ -25,11 +26,9 @@
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import it.infn.mw.iam.api.scim.converter.UserConverter;
import it.infn.mw.iam.api.scim.exception.IllegalArgumentException;
import it.infn.mw.iam.api.scim.model.ScimListResponse;
import it.infn.mw.iam.api.scim.model.ScimListResponse.ScimListResponseBuilder;
import it.infn.mw.iam.api.scim.model.ScimUser;
import it.infn.mw.iam.persistence.model.IamAccount;
import it.infn.mw.iam.persistence.model.IamGroup;
Expand Down Expand Up @@ -65,18 +64,13 @@ public ScimListResponse<ScimUser> findAccountByLabel(String labelName, String la
@Override
public ScimListResponse<ScimUser> findAccountByEmail(String emailAddress) {
Optional<IamAccount> account = repo.findByEmail(emailAddress);

ScimListResponseBuilder<ScimUser> builder = ScimListResponse.builder();
account.ifPresent(a -> builder.singleResource(converter.dtoFromEntity(a)));
return builder.build();
return responseFromOptional(account, converter);
}

@Override
public ScimListResponse<ScimUser> findAccountByUsername(String username) {
Optional<IamAccount> account = repo.findByUsername(username);
ScimListResponseBuilder<ScimUser> builder = ScimListResponse.builder();
account.ifPresent(a -> builder.singleResource(converter.dtoFromEntity(a)));
return builder.build();
return responseFromOptional(account, converter);
}

@Override
Expand Down Expand Up @@ -115,9 +109,7 @@ private Supplier<IllegalArgumentException> groupNotFoundError(String groupNameOr
@Override
public ScimListResponse<ScimUser> findAccountByCertificateSubject(String certSubject) {
Optional<IamAccount> account = repo.findByCertificateSubject(certSubject);
ScimListResponseBuilder<ScimUser> builder = ScimListResponse.builder();
account.ifPresent(a -> builder.singleResource(converter.dtoFromEntity(a)));
return builder.build();
return responseFromOptional(account, converter);
}

@Override
Expand All @@ -143,4 +135,9 @@ public ScimListResponse<ScimUser> findAccountByGroupUuidWithFilter(String groupU
Page<IamAccount> results = repo.findByGroupUuidWithFilter(group.getUuid(), filter, pageable);
return responseFromPage(results, converter, pageable);
}

public ScimListResponse<ScimUser> findAccountByUuid(String uuid) {
Optional<IamAccount> account = repo.findByUuid(uuid);
return responseFromOptional(account, converter);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.validation.BindingResult;
import org.springframework.validation.annotation.Validated;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.bind.annotation.*;

import it.infn.mw.iam.api.common.ListResponseDTO;
import it.infn.mw.iam.api.common.form.PaginatedRequestWithFilterForm;
Expand All @@ -44,6 +41,7 @@ public class FindAccountController {
public static final String FIND_BY_LABEL_RESOURCE = "/iam/account/find/bylabel";
public static final String FIND_BY_EMAIL_RESOURCE = "/iam/account/find/byemail";
public static final String FIND_BY_USERNAME_RESOURCE = "/iam/account/find/byusername";
public static final String FIND_BY_UUID_RESOURCE = "/iam/account/find/byuuid/{accountUuid}";
public static final String FIND_BY_CERT_SUBJECT_RESOURCE = "/iam/account/find/bycertsubject";
public static final String FIND_BY_GROUP_RESOURCE = "/iam/account/find/bygroup/{groupUuid}";
public static final String FIND_NOT_IN_GROUP_RESOURCE =
Expand Down Expand Up @@ -121,4 +119,9 @@ public ListResponseDTO<ScimUser> findNotInGroup(@PathVariable String groupUuid,
}
}

@GetMapping(value = FIND_BY_UUID_RESOURCE, produces = ScimConstants.SCIM_CONTENT_TYPE)
@PreAuthorize("#iam.hasScope('iam:admin.read') or #iam.hasDashboardRole('ROLE_ADMIN') or hasRole('USER')")
public ListResponseDTO<ScimUser> findByUuid(@PathVariable String accountUuid) {
return service.findAccountByUuid(accountUuid);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,6 @@ ScimListResponse<ScimUser> findAccountByGroupUuidWithFilter(String groupUuid, St

ScimListResponse<ScimUser> findAccountNotInGroupWithFilter(String groupUuid, String filter,
Pageable pageable);


ScimListResponse<ScimUser> findAccountByUuid(String uuid);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package it.infn.mw.iam.api.client.error;

public class ClientSuspended extends RuntimeException {

private static final long serialVersionUID = 1L;

public ClientSuspended(String message) {
super(message);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
Expand All @@ -44,6 +45,7 @@

import com.fasterxml.jackson.annotation.JsonView;

import it.infn.mw.iam.api.account.AccountUtils;
import it.infn.mw.iam.api.client.error.InvalidPaginationRequest;
import it.infn.mw.iam.api.client.error.NoSuchClient;
import it.infn.mw.iam.api.client.management.service.ClientManagementService;
Expand All @@ -53,6 +55,7 @@
import it.infn.mw.iam.api.common.PagingUtils;
import it.infn.mw.iam.api.common.client.RegisteredClientDTO;
import it.infn.mw.iam.api.scim.model.ScimUser;
import it.infn.mw.iam.persistence.model.IamAccount;

@RestController
@RequestMapping(ClientManagementAPIController.ENDPOINT)
Expand All @@ -61,9 +64,11 @@ public class ClientManagementAPIController {
public static final String ENDPOINT = "/iam/api/clients";

private final ClientManagementService managementService;
private final AccountUtils accountUtils;

public ClientManagementAPIController(ClientManagementService managementService) {
public ClientManagementAPIController(ClientManagementService managementService, AccountUtils accountUtils) {
this.managementService = managementService;
this.accountUtils = accountUtils;
}

@PostMapping
Expand Down Expand Up @@ -140,6 +145,20 @@ public RegisteredClientDTO updateClient(@PathVariable String clientId,
return managementService.updateClient(clientId, client);
}

@PatchMapping("/{clientId}/enable")
@PreAuthorize("#iam.hasScope('iam:admin.write') or #iam.hasDashboardRole('ROLE_ADMIN')")
public void enableClient(@PathVariable String clientId) {
Optional<IamAccount> account = accountUtils.getAuthenticatedUserAccount();
account.ifPresent(a -> managementService.updateClientStatus(clientId, true, a.getUuid()));
}

@PatchMapping("/{clientId}/disable")
@PreAuthorize("#iam.hasScope('iam:admin.write') or #iam.hasDashboardRole('ROLE_ADMIN')")
public void disableClient(@PathVariable String clientId) {
Optional<IamAccount> account = accountUtils.getAuthenticatedUserAccount();
account.ifPresent(a -> managementService.updateClientStatus(clientId, false, a.getUuid()));
}

@PostMapping("/{clientId}/secret")
@ResponseStatus(CREATED)
@PreAuthorize("#iam.hasScope('iam:admin.write') or #iam.hasDashboardRole('ROLE_ADMIN')")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ RegisteredClientDTO updateClient(@NotBlank String clientId,

void deleteClientByClientId(@NotBlank String clientId);

void updateClientStatus(String clientId, boolean status, String userId);

ListResponseDTO<ScimUser> getClientOwners(@NotBlank String clientId, @NotNull Pageable pageable);

void assignClientOwner(@NotBlank String clientId, @IamAccountId String accountId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import it.infn.mw.iam.audit.events.client.ClientRegistrationAccessTokenRotatedEvent;
import it.infn.mw.iam.audit.events.client.ClientRemovedEvent;
import it.infn.mw.iam.audit.events.client.ClientSecretUpdatedEvent;
import it.infn.mw.iam.audit.events.client.ClientStatusChangedEvent;
import it.infn.mw.iam.audit.events.client.ClientUpdatedEvent;
import it.infn.mw.iam.core.IamTokenService;
import it.infn.mw.iam.persistence.model.IamAccount;
Expand Down Expand Up @@ -116,6 +117,7 @@ public RegisteredClientDTO saveNewClient(RegisteredClientDTO client) throws Pars
ClientDetailsEntity entity = converter.entityFromClientManagementRequest(client);
entity.setDynamicallyRegistered(false);
entity.setCreatedAt(Date.from(clock.instant()));
entity.setActive(true);

defaultsService.setupClientDefaults(entity);
entity = clientService.saveNewClient(entity);
Expand All @@ -133,6 +135,16 @@ public void deleteClientByClientId(String clientId) {
eventPublisher.publishEvent(new ClientRemovedEvent(this, client));
}

@Override
public void updateClientStatus(String clientId, boolean status, String userId) {

ClientDetailsEntity client = clientService.findClientByClientId(clientId)
.orElseThrow(ClientSuppliers.clientNotFound(clientId));
client = clientService.updateClientStatus(client, status, userId);
String message = "Client " + (status?"enabled":"disabled");
eventPublisher.publishEvent(new ClientStatusChangedEvent(this, client, message));
}

@Validated(OnClientUpdate.class)
@Override
public RegisteredClientDTO updateClient(String clientId, RegisteredClientDTO client)
Expand All @@ -148,6 +160,7 @@ public RegisteredClientDTO updateClient(String clientId, RegisteredClientDTO cli
newClient.setClientId(oldClient.getClientId());
newClient.setAuthorities(oldClient.getAuthorities());
newClient.setDynamicallyRegistered(oldClient.isDynamicallyRegistered());
newClient.setActive(oldClient.isActive());

if (NONE.equals(newClient.getTokenEndpointAuthMethod())) {
newClient.setClientSecret(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

import com.fasterxml.jackson.annotation.JsonView;

import it.infn.mw.iam.api.client.error.ClientSuspended;
import it.infn.mw.iam.api.client.error.InvalidClientRegistrationRequest;
import it.infn.mw.iam.api.client.error.NoSuchClient;
import it.infn.mw.iam.api.client.registration.service.ClientRegistrationService;
Expand Down Expand Up @@ -119,6 +120,12 @@ public ErrorDTO noSuchClient(HttpServletRequest req, Exception ex) {
return ErrorDTO.fromString(ex.getMessage());
}

@ResponseStatus(value = HttpStatus.FORBIDDEN)
@ExceptionHandler(ClientSuspended.class)
public ErrorDTO clientSuspended(HttpServletRequest req, Exception ex) {
return ErrorDTO.fromString(ex.getMessage());
}

@ResponseStatus(value = HttpStatus.BAD_REQUEST)
@ExceptionHandler(InvalidClientRegistrationRequest.class)
public ErrorDTO invalidRequest(HttpServletRequest req, Exception ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@

import it.infn.mw.iam.api.account.AccountUtils;
import it.infn.mw.iam.api.client.error.InvalidClientRegistrationRequest;
import it.infn.mw.iam.api.client.error.ClientSuspended;
import it.infn.mw.iam.api.client.registration.validation.OnDynamicClientRegistration;
import it.infn.mw.iam.api.client.registration.validation.OnDynamicClientUpdate;
import it.infn.mw.iam.api.client.service.ClientConverter;
Expand Down Expand Up @@ -320,6 +321,15 @@ && registrationAccessTokenAuthenticationValidForClientId(client.getClientId(), a
return Optional.empty();
}

private void checkUserUpdatingSuspendedClient(Authentication authentication, ClientDetailsEntity oldClient) {
if (accountUtils.isAdmin(authentication)) {
return;
}
if(!oldClient.isActive()){
throw new ClientSuspended("Client " + oldClient.getClientId() + " is suspended!");
}
}

@Validated(OnDynamicClientRegistration.class)
@Override
public RegisteredClientDTO registerClient(RegisteredClientDTO request,
Expand All @@ -330,6 +340,7 @@ public RegisteredClientDTO registerClient(RegisteredClientDTO request,
ClientDetailsEntity client = converter.entityFromRegistrationRequest(request);
defaultsService.setupClientDefaults(client);
client.setDynamicallyRegistered(true);
client.setActive(true);

checkAllowedGrantTypes(request, authentication);
cleanupRequestedScopes(client, authentication);
Expand Down Expand Up @@ -395,9 +406,10 @@ public RegisteredClientDTO updateClient(String clientId, RegisteredClientDTO req
ClientDetailsEntity oldClient =
lookupClient(clientId, authentication).orElseThrow(clientNotFound(clientId));

checkUserUpdatingSuspendedClient(authentication, oldClient);
checkAllowedGrantTypesOnUpdate(request, authentication, oldClient);
cleanupRequestedScopesOnUpdate(request, authentication, oldClient);

ClientDetailsEntity newClient = converter.entityFromRegistrationRequest(request);
newClient.setId(oldClient.getId());
newClient.setClientSecret(oldClient.getClientSecret());
Expand All @@ -410,6 +422,7 @@ public RegisteredClientDTO updateClient(String clientId, RegisteredClientDTO req
newClient.setAuthorities(oldClient.getAuthorities());
newClient.setCreatedAt(oldClient.getCreatedAt());
newClient.setReuseRefreshToken(oldClient.isReuseRefreshToken());
newClient.setActive(oldClient.isActive());

ClientDetailsEntity savedClient = clientService.updateClient(newClient);

Expand All @@ -421,8 +434,7 @@ public RegisteredClientDTO updateClient(String clientId, RegisteredClientDTO req
eventPublisher.publishEvent(new ClientRegistrationAccessTokenRotatedEvent(this, savedClient));
response.setRegistrationAccessToken(t);
});

return response;
return response;
}

@Override
Expand Down
Loading

0 comments on commit b0fe8a1

Please sign in to comment.