Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix role permissions #876

Merged
merged 2 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading