Skip to content

Commit

Permalink
Merge pull request #876 from terrestris/fix-role-permissions
Browse files Browse the repository at this point in the history
Fix role permissions
  • Loading branch information
dnlkoch authored Jun 17, 2024
2 parents 61b7714 + 9716cec commit 4b09a23
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,4 @@ public RealmResource getRealm(@Autowired Keycloak kc) {
return kc.realm(keycloakProperties.getRealm());
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
@RestController
@RequestMapping("/webhooks")
public class WebhookController {

@Autowired
private ApplicationEventPublisher applicationEventPublisher;

Expand Down Expand Up @@ -85,21 +86,6 @@ public void handleKeyCloakEvent(@RequestBody KeycloakEventDto event) {
));
}
}
case "REALM_ROLE" -> {
if (StringUtils.equals(eventType, "CREATE")) {
applicationEventPublisher.publishEvent(new KeycloakEvent(
this,
KeycloakEventType.REALM_ROLE_CREATED,
split[1]
));
} else if (StringUtils.equals(eventType, "DELETE")) {
applicationEventPublisher.publishEvent(new KeycloakEvent(
this,
KeycloakEventType.REALM_ROLE_DELETED,
split[1]
));
}
}
case "REALM_ROLE_MAPPING", "CLIENT_ROLE_MAPPING" -> {
if (split[0].equals("users")) {
applicationEventPublisher.publishEvent(new KeycloakEvent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,5 @@ public enum KeycloakEventType {
USER_GROUP_MEMBERSHIP_CHANGED,
GROUP_CREATED,
GROUP_DELETED,
GROUP_ROLES_CHANGED,
REALM_ROLE_CREATED,
REALM_ROLE_DELETED
GROUP_ROLES_CHANGED
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import de.terrestris.shogun.lib.event.OnRegistrationConfirmedEvent;
import de.terrestris.shogun.lib.service.security.permission.UserInstancePermissionService;
import de.terrestris.shogun.lib.service.security.provider.GroupProviderService;
import de.terrestris.shogun.lib.service.security.provider.RoleProviderService;
import de.terrestris.shogun.lib.service.security.provider.UserProviderService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.event.EventListener;
Expand All @@ -37,9 +36,6 @@ public class KeycloakEventListener {
@Autowired
private GroupProviderService groupProviderService;

@Autowired
private RoleProviderService roleProviderService;

@Autowired
protected UserInstancePermissionService userInstancePermissionService;

Expand All @@ -48,7 +44,6 @@ public void onKeycloakEvent(KeycloakEvent event) {
switch (event.getEventType()) {
case USER_CREATED -> userProviderService.findOrCreateByProviderId(event.getKeycloakId());
case GROUP_CREATED -> groupProviderService.findOrCreateByProviderId(event.getKeycloakId());
case REALM_ROLE_CREATED -> roleProviderService.findOrCreateByProviderId(event.getKeycloakId());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@

import de.terrestris.shogun.lib.model.Role;
import de.terrestris.shogun.lib.repository.RoleRepository;
import de.terrestris.shogun.lib.service.security.permission.RoleClassPermissionService;
import de.terrestris.shogun.lib.service.security.provider.RoleProviderService;
import lombok.extern.log4j.Log4j2;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.jpa.domain.Specification;
import org.springframework.security.access.prepost.PostAuthorize;
import org.springframework.security.access.prepost.PostFilter;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Isolation;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;
Expand All @@ -37,6 +40,9 @@ public class RoleService extends BaseService<RoleRepository, Role> {
@Autowired
RoleProviderService roleProviderService;

@Autowired
RoleClassPermissionService roleClassPermissionService;

@PostFilter("hasRole('ROLE_ADMIN') or hasPermission(filterObject, 'READ')")
@Transactional(readOnly = true)
@Override
Expand Down Expand Up @@ -88,4 +94,13 @@ public Optional<Role> findByKeyCloakId(String keycloakId) {
return role;
}

@PreAuthorize("hasRole('ROLE_ADMIN') or hasPermission(#role, 'DELETE')")
@Transactional(isolation = Isolation.SERIALIZABLE)
public void delete(Role role) {
roleClassPermissionService.deleteAllFor(role);
roleInstancePermissionService.deleteAllFor(role);

repository.delete(role);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,20 @@ public void deleteAllFor(BaseEntity persistedEntity) {
log.trace("Deleted entity: {}", persistedEntity);
}

/**
* Deletes all {@link RoleInstancePermission} for the given role.
*
* @param role The role to clear the permissions for.
*/
public void deleteAllFor(Role role) {
List<RoleInstancePermission> roleInstancePermissions = this.findFor(role);

repository.deleteAll(roleInstancePermissions);

log.info("Successfully deleted all role instance permissions for role with ID {}",
role.getId());
}

/**
* Deletes the {@link RoleInstancePermission} for the given entity and role.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ public void deleteAllFor(BaseEntity persistedEntity) {
/**
* Deletes all {@link UserInstancePermission} for the given user.
*
* @param user The entity to clear the permissions for.
* @param user The user to clear the permissions for.
*/
public void deleteAllFor(User user) {
List<UserInstancePermission> userInstancePermissions = this.findFor(user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ public List<Role> getRolesForUser(User<UserRepresentation> user) {
List<Role> roles = new ArrayList<>();

try {
List<RoleRepresentation> rolesA = keycloakUtil.getKeycloakUserRoles(user);
List<RoleRepresentation> roleRepresentations = keycloakUtil.getKeycloakUserRoles(user);

roles = rolesA.stream()
roles = roleRepresentations.stream()
.map(role -> roleRepository.findByAuthProviderId(role.getId()))
.filter(Optional::isPresent)
.map(Optional::get)
Expand All @@ -87,7 +87,7 @@ public Role<RoleRepresentation> findOrCreateByProviderId(String providerRoleId)
Role<RoleRepresentation> role = roleOptional.orElse(null);

if (role == null) {
role = new Role<RoleRepresentation>(providerRoleId, null);
role = new Role(providerRoleId, null);
roleRepository.save(role);

log.info("Role with Keycloak ID {} did not yet exist in the SHOGun DB and was therefore created.", providerRoleId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@
import de.terrestris.shogun.lib.model.Group;
import de.terrestris.shogun.lib.model.Role;
import de.terrestris.shogun.lib.model.User;
import de.terrestris.shogun.properties.KeycloakProperties;
import jakarta.ws.rs.WebApplicationException;
import jakarta.ws.rs.core.Response;
import lombok.extern.log4j.Log4j2;
import org.apache.commons.lang3.StringUtils;
import org.keycloak.admin.client.CreatedResponseUtil;
import org.keycloak.admin.client.resource.*;
import org.keycloak.representations.IDToken;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.GroupRepresentation;
import org.keycloak.representations.idm.RoleRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
Expand All @@ -45,6 +47,9 @@
@Component
public class KeycloakUtil {

@Autowired
private KeycloakProperties keycloakProperties;

@Autowired
protected RealmResource keycloakRealm;

Expand Down Expand Up @@ -211,7 +216,7 @@ public List<GroupRepresentation> getKeycloakUserGroups(User<UserRepresentation>
}

/**
* Get the Keycloak RoleRepresentations from a user instance.
* Get the Keycloak RoleRepresentations (for the specified client) from a user instance.
*
* @param user
* @return
Expand All @@ -221,7 +226,13 @@ public List<RoleRepresentation> getKeycloakUserRoles(User<UserRepresentation> us
List<RoleRepresentation> roles = new ArrayList<>();

try {
roles = userResource.roles().getAll().getRealmMappings();
ClientRepresentation clientRepresentation = getClientRepresentationFromClientId();

if (clientRepresentation == null) {
return roles;
}

roles = userResource.roles().clientLevel(clientRepresentation.getId()).listEffective();
} catch (Exception e) {
log.warn("Could not get the RoleMappingResource for the user with SHOGun ID {} and " +
"Keycloak ID {}. This may happen if the user is not available in Keycloak.",
Expand All @@ -232,6 +243,47 @@ public List<RoleRepresentation> getKeycloakUserRoles(User<UserRepresentation> us
return roles;
}

/**
* Returns the client representation for the client the shogun instance is configured with (see keycloak.clientId
* in properties).
*
* @return
*/
public ClientRepresentation getClientRepresentationFromClientId() {
List<ClientRepresentation> clientRepresentations = keycloakRealm.clients().findByClientId(keycloakProperties.getClientId());

if (clientRepresentations.size() != 1) {
log.error("Could not find client with clientId {} in Keycloak. " +
"Expected to find exactly one client with this clientId, but found {} clients.",
keycloakProperties.getClientId(), clientRepresentations.size());

return null;
}

return clientRepresentations.getFirst();
}

public RoleRepresentation getRoleByName(String roleName) {
ClientRepresentation clientRepresentation = getClientRepresentationFromClientId();

if (clientRepresentation == null) {
return null;
}

RoleResource roleResource = keycloakRealm.clients().get(clientRepresentation.getId()).roles().get(roleName);

RoleRepresentation roleRepresentation = null;

try {
roleRepresentation = roleResource.toRepresentation();
} catch (Exception e) {
log.error("Could not get the RoleRepresentation for role with name {}. This may happen if " +
"the role is not available in Keycloak.", roleName);
}

return roleRepresentation;
}

/**
* Return keycloak user id from {@link Authentication} object
* - from {@link IDToken}
Expand Down

0 comments on commit 4b09a23

Please sign in to comment.