Skip to content

Commit

Permalink
Make sure organization groups can not be managed but when managing an…
Browse files Browse the repository at this point in the history 
… organization

Closes keycloak#29431

Signed-off-by: Pedro Igor <[email protected]>
  • Loading branch information
@pedroigor
pedroigor committed May 10, 2024
1 parent 401d58a commit e280e15
Show file tree
Hide file tree
Showing 18 changed files with 580 additions and 339 deletions.
13 changes: 13 additions & 0 deletions model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public String getName() {
@Override
public void setName(String name) {
group.setName(name);
fireGroupUpdatedEvent();
}

@Override
Expand Down Expand Up @@ -105,6 +106,7 @@ public void setParent(GroupModel parent) {
GroupEntity parentEntity = toEntity(parent, em);
group.setParentId(parentEntity.getId());
}
fireGroupUpdatedEvent();
}

@Override
Expand All @@ -113,6 +115,7 @@ public void addChild(GroupModel subGroup) {
return;
}
subGroup.setParent(this);
fireGroupUpdatedEvent();
}

@Override
Expand All @@ -121,6 +124,7 @@ public void removeChild(GroupModel subGroup) {
return;
}
subGroup.setParent(null);
fireGroupUpdatedEvent();
}

@Override
Expand Down Expand Up @@ -176,10 +180,12 @@ public void setSingleAttribute(String name, String value) {
}

if (found) {
fireGroupUpdatedEvent();
return;
}

persistAttributeValue(name, value);
fireGroupUpdatedEvent();
}

@Override
Expand Down Expand Up @@ -213,6 +219,7 @@ public void removeAttribute(String name) {
em.remove(attr);
}
}
fireGroupUpdatedEvent();
}

@Override
Expand Down Expand Up @@ -264,6 +271,7 @@ public void grantRole(RoleModel role) {
em.persist(entity);
em.flush();
em.detach(entity);
fireGroupUpdatedEvent();
}

@Override
Expand Down Expand Up @@ -293,6 +301,7 @@ public void deleteRoleMapping(RoleModel role) {
em.remove(entity);
}
em.flush();
fireGroupUpdatedEvent();
}

@Override
Expand All @@ -318,4 +327,8 @@ public int hashCode() {
public boolean escapeSlashesInGroupPath() {
return KeycloakModelUtils.escapeSlashesInGroupPath(session);
}

private void fireGroupUpdatedEvent() {
GroupUpdatedEvent.fire(this, session);
}
}
43 changes: 19 additions & 24 deletions model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@
import org.keycloak.models.ClientScopeProvider;
import org.keycloak.models.DeploymentStateProvider;
import org.keycloak.models.GroupModel;
import org.keycloak.models.GroupModel.GroupCreatedEvent;
import org.keycloak.models.GroupModel.GroupPathChangeEvent;
import org.keycloak.models.GroupModel.GroupUpdatedEvent;
import org.keycloak.models.GroupProvider;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelDuplicateException;
Expand Down Expand Up @@ -524,29 +527,8 @@ public void moveGroup(RealmModel realm, GroupModel group, GroupModel toParent) {
String newPath = KeycloakModelUtils.buildGroupPath(group);
String previousPath = KeycloakModelUtils.buildGroupPath(group, previousParent);

GroupModel.GroupPathChangeEvent event =
new GroupModel.GroupPathChangeEvent() {
@Override
public RealmModel getRealm() {
return realm;
}

@Override
public String getNewPath() {
return newPath;
}

@Override
public String getPreviousPath() {
return previousPath;
}

@Override
public KeycloakSession getKeycloakSession() {
return session;
}
};
session.getKeycloakSessionFactory().publish(event);
GroupPathChangeEvent.fire(group, newPath, previousPath, session);
fireGroupUpdatedEvent(group);
}

@Override
Expand Down Expand Up @@ -733,12 +715,17 @@ public GroupModel createGroup(RealmModel realm, String id, String name, GroupMod
em.persist(groupEntity);
em.flush();

return new GroupAdapter(session, realm, em, groupEntity);
GroupAdapter group = new GroupAdapter(session, realm, em, groupEntity);

fireGroupCreatedEvent(group);

return group;
}

@Override
public void addTopLevelGroup(RealmModel realm, GroupModel subGroup) {
subGroup.setParent(null);
fireGroupUpdatedEvent(subGroup);
}

public void preRemove(RealmModel realm, RoleModel role) {
Expand Down Expand Up @@ -1262,4 +1249,12 @@ public boolean deleteLocalizationText(RealmModel realm, String locale, String ke
public Set<String> getClientSearchableAttributes() {
return clientSearchableAttributes;
}

private void fireGroupUpdatedEvent(GroupModel group) {
GroupUpdatedEvent.fire(group, session);
}

private void fireGroupCreatedEvent(GroupAdapter group) {
GroupCreatedEvent.fire(group, session);
}
}
6 changes: 4 additions & 2 deletions model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.keycloak.component.ComponentFactory;
import org.keycloak.component.ComponentModel;
import org.keycloak.models.*;
import org.keycloak.models.GroupModel.GroupUpdatedEvent;
import org.keycloak.models.jpa.entities.*;
import org.keycloak.models.utils.ComponentUtil;
import org.keycloak.models.utils.KeycloakModelUtils;
Expand Down Expand Up @@ -722,6 +723,7 @@ public void addDefaultGroup(GroupModel group) {

groupsIds.add(group.getId());
em.flush();
GroupUpdatedEvent.fire(group, session);
}

@Override
Expand Down Expand Up @@ -851,12 +853,12 @@ public boolean removeRole(RoleModel role) {
public Stream<RoleModel> getRolesStream() {
return session.roles().getRealmRolesStream(this);
}

@Override
public Stream<RoleModel> getRolesStream(Integer first, Integer max) {
return session.roles().getRealmRolesStream(this, first, max);
}

@Override
public Stream<RoleModel> searchForRolesStream(String search, Integer first, Integer max) {
return session.roles().searchForRolesStream(this, search, first, max);
Expand Down
7 changes: 4 additions & 3 deletions model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.keycloak.credential.UserCredentialManager;
import org.keycloak.models.ClientModel;
import org.keycloak.models.GroupModel;
import org.keycloak.models.GroupModel.GroupMemberJoinEvent;
import org.keycloak.models.GroupModel.GroupMemberLeaveEvent;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel;
Expand Down Expand Up @@ -412,6 +414,7 @@ protected void joinGroupImpl(GroupModel group) {
em.persist(entity);
em.flush();
em.detach(entity);
GroupMemberJoinEvent.fire(group, session);

}

Expand All @@ -427,7 +430,7 @@ public void leaveGroup(GroupModel group) {
em.remove(entity);
}
em.flush();

GroupMemberLeaveEvent.fire(group, session);
}

@Override
Expand Down Expand Up @@ -544,6 +547,4 @@ public boolean equals(Object o) {
public int hashCode() {
return getId().hashCode();
}


}
94 changes: 63 additions & 31 deletions model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,10 @@ public class JpaOrganizationProvider implements OrganizationProvider {
private final GroupProvider groupProvider;
private final UserProvider userProvider;
private final RealmModel realm;
private final KeycloakSession session;

public JpaOrganizationProvider(KeycloakSession session) {
this.session = session;
em = session.getProvider(JpaConnectionProvider.class).getEntityManager();
groupProvider = session.groups();
userProvider = session.users();
Expand All @@ -85,39 +87,45 @@ public OrganizationModel create(String name, Set<String> domains) {
throw new ModelDuplicateException("A organization with the same name already exists.");
}

OrganizationEntity entity = new OrganizationEntity();
entity.setId(KeycloakModelUtils.generateId());
OrganizationAdapter adapter = new OrganizationAdapter(realm, this);

GroupModel group = createOrganizationGroup(entity.getId());

entity.setGroupId(group.getId());
entity.setRealmId(realm.getId());
entity.setName(name);
entity.setEnabled(true);
try {
session.setAttribute(OrganizationModel.class.getName(), adapter);
GroupModel group = createOrganizationGroup(adapter.getId());

em.persist(entity);
adapter.setGroupId(group.getId());
adapter.setName(name);
adapter.setEnabled(true);

OrganizationAdapter adapter = new OrganizationAdapter(realm, entity, this);
em.persist(adapter.getEntity());

adapter.setDomains(domains.stream().map(OrganizationDomainModel::new).collect(Collectors.toSet()));
adapter.setDomains(domains.stream().map(OrganizationDomainModel::new).collect(Collectors.toSet()));
} finally {
session.removeAttribute(OrganizationModel.class.getName());
}

return adapter;
}

@Override
public boolean remove(OrganizationModel organization) {
OrganizationEntity entity = getEntity(organization.getId());
GroupModel group = getOrganizationGroup(entity);

if (group != null) {
//TODO: won't scale, requires a better mechanism for bulk deleting users
userProvider.getGroupMembersStream(realm, group).forEach(userModel -> removeMember(organization, userModel));
groupProvider.removeGroup(realm, group);
organization.getIdentityProviders().forEach((model) -> removeIdentityProvider(organization, model));
try {
session.setAttribute(OrganizationModel.class.getName(), organization);
GroupModel group = getOrganizationGroup(entity);

if (group != null) {
//TODO: won't scale, requires a better mechanism for bulk deleting users
userProvider.getGroupMembersStream(realm, group).forEach(userModel -> removeMember(organization, userModel));
groupProvider.removeGroup(realm, group);
organization.getIdentityProviders().forEach((model) -> removeIdentityProvider(organization, model));
}
em.remove(entity);
} finally {
session.removeAttribute(OrganizationModel.class.getName());
}

em.remove(entity);

return true;
}

Expand All @@ -133,18 +141,30 @@ public boolean addMember(OrganizationModel organization, UserModel user) {
throwExceptionIfObjectIsNull(user, "User");

OrganizationEntity entity = getEntity(organization.getId());
GroupModel group = getOrganizationGroup(entity);
OrganizationModel current = (OrganizationModel) session.getAttribute(OrganizationModel.class.getName());

if (user.isMemberOf(group)) {
return false;
if (current == null) {
session.setAttribute(OrganizationModel.class.getName(), organization);
}

if (user.getFirstAttribute(ORGANIZATION_ATTRIBUTE) != null) {
throw new ModelException("User [" + user.getId() + "] is a member of a different organization");
}
try {
GroupModel group = getOrganizationGroup(entity);

user.joinGroup(group);
user.setSingleAttribute(ORGANIZATION_ATTRIBUTE, entity.getId());
if (user.isMemberOf(group)) {
return false;
}

if (user.getFirstAttribute(ORGANIZATION_ATTRIBUTE) != null) {
throw new ModelException("User [" + user.getId() + "] is a member of a different organization");
}

user.joinGroup(group);
user.setSingleAttribute(ORGANIZATION_ATTRIBUTE, entity.getId());
} finally {
if (current == null) {
session.removeAttribute(OrganizationModel.class.getName());
}
}

return true;
}
Expand Down Expand Up @@ -333,10 +353,22 @@ public boolean removeMember(OrganizationModel organization, UserModel member) {
if (isManagedMember(organization, member)) {
userProvider.removeUser(realm, member);
} else {
List<String> organizations = member.getAttributes().get(ORGANIZATION_ATTRIBUTE);
organizations.remove(organization.getId());
member.setAttribute(ORGANIZATION_ATTRIBUTE, organizations);
member.leaveGroup(getOrganizationGroup(organization));
OrganizationModel current = (OrganizationModel) session.getAttribute(OrganizationModel.class.getName());

if (current == null) {
session.setAttribute(OrganizationModel.class.getName(), organization);
}

try {
List<String> organizations = member.getAttributes().get(ORGANIZATION_ATTRIBUTE);
organizations.remove(organization.getId());
member.setAttribute(ORGANIZATION_ATTRIBUTE, organizations);
member.leaveGroup(getOrganizationGroup(organization));
} finally {
if (current == null) {
session.removeAttribute(OrganizationModel.class.getName());
}
}
}

return true;
Expand Down
12 changes: 12 additions & 0 deletions model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProviderFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
package org.keycloak.organization.jpa;

import org.keycloak.Config.Scope;
import org.keycloak.models.GroupModel;
import org.keycloak.models.GroupModel.GroupEvent;
import org.keycloak.models.ModelValidationException;
import org.keycloak.organization.authentication.authenticators.broker.IdpOrganizationAuthenticatorFactory;
import org.keycloak.organization.authentication.authenticators.browser.OrganizationAuthenticatorFactory;
import org.keycloak.models.AuthenticationExecutionModel;
Expand All @@ -29,6 +32,7 @@
import org.keycloak.models.RealmModel.RealmRemovedEvent;
import org.keycloak.organization.OrganizationProvider;
import org.keycloak.organization.OrganizationProviderFactory;
import org.keycloak.organization.utils.Organizations;
import org.keycloak.provider.ProviderEvent;

public class JpaOrganizationProviderFactory implements OrganizationProviderFactory {
Expand Down Expand Up @@ -68,6 +72,14 @@ private void handleEvents(ProviderEvent event) {
OrganizationProvider provider = session.getProvider(OrganizationProvider.class);
provider.removeAll();
}
if (event instanceof GroupEvent) {
GroupEvent groupEvent = (GroupEvent) event;
KeycloakSession session = groupEvent.getKeycloakSession();
GroupModel group = groupEvent.getGroup();
if (!Organizations.canManageOrganizationGroup(session, group)) {
throw new ModelValidationException("Can not update organization group");
}
}
}

private void configureAuthenticationFlows(RealmModel realm) {
Expand Down
Loading

0 comments on commit e280e15

Please sign in to comment.