Skip to content

Commit

Permalink
Remove Role enum and GroupRoleCheck
Browse files Browse the repository at this point in the history 
[#150118440] https://www.pivotaltracker.com/story/show/150118440

Signed-off-by: Mikhail Vyshegorodtsev <medvedzver@inbox.ru>
  • Loading branch information
@6palace @medvedzver
6palace authored and medvedzver committed Aug 16, 2017
1 parent 4404f7b commit 06800b8
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 314 deletions.
51 changes: 13 additions & 38 deletions model/src/main/java/org/cloudfoundry/identity/uaa/scim/ScimGroupMember.java
View file
Expand Up @@ -16,9 +16,6 @@
import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonProperty;
import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.constants.OriginKeys;


import java.util.Arrays;
import java.util.List;

@JsonInclude(JsonInclude.Include.NON_NULL) @JsonInclude(JsonInclude.Include.NON_NULL)
public class ScimGroupMember<TEntity extends ScimCore> { public class ScimGroupMember<TEntity extends ScimCore> {


Expand All @@ -30,13 +27,21 @@ public void setEntity(TEntity entity) {
this.entity = entity; this.entity = entity;
} }


@JsonInclude(JsonInclude.Include.NON_NULL) public ScimGroupMember() {
public enum Role {
MEMBER, READER, WRITER
} }


public static final List<Role> GROUP_MEMBER = Arrays.asList(Role.MEMBER); public ScimGroupMember(String memberId) {
public static final List<Role> GROUP_ADMIN = Arrays.asList(Role.READER, Role.WRITER); this(memberId, Type.USER);
}

public ScimGroupMember(TEntity entity) {
this(entity.getId(), getEntityType(entity));
}

public ScimGroupMember(String memberId, Type type) {
this.memberId = memberId;
this.type = type;
}


@JsonProperty("value") @JsonProperty("value")
private String memberId; private String memberId;
Expand All @@ -54,16 +59,6 @@ public enum Type {


private TEntity entity; private TEntity entity;


// @JsonIgnore
// private List<Role> roles;
//
// public List<Role> getRoles() {
// return new LinkedList<>();
// }
//
// public void setRoles(List<Role> permissions) {
// }

public String getMemberId() { public String getMemberId() {
return memberId; return memberId;
} }
Expand Down Expand Up @@ -123,28 +118,8 @@ public int hashCode() {
return result; return result;
} }


public ScimGroupMember() {
}

public ScimGroupMember(String memberId) {
this(memberId, Type.USER);
}




public ScimGroupMember(String memberId, Type type) {
this.memberId = memberId;
this.type = type;
}

public ScimGroupMember(TEntity entity) {
this(entity, GROUP_MEMBER);
}

public ScimGroupMember(TEntity entity, List<Role> roles) {
this(entity.getId(), getEntityType(entity));
this.entity = entity;
}

private static Type getEntityType(ScimCore entity) { private static Type getEntityType(ScimCore entity) {
Type type = null; Type type = null;
if(entity instanceof ScimGroup) { type = Type.GROUP; } if(entity instanceof ScimGroup) { type = Type.GROUP; }
Expand Down
12 changes: 0 additions & 12 deletions server/src/main/java/org/cloudfoundry/identity/uaa/scim/ScimGroupMembershipManager.java
View file
Expand Up @@ -46,18 +46,6 @@ ScimGroupMember addMember(String groupId, ScimGroupMember member, final String z
List<ScimGroupMember> getMembers(String groupId, boolean includeEntities, String zoneId) List<ScimGroupMember> getMembers(String groupId, boolean includeEntities, String zoneId)
throws ScimResourceNotFoundException; throws ScimResourceNotFoundException;


/**
* Retrieve members that have the specified authority on the group
*
* @param groupId
* @param permission
* @param zoneId
* @return
* @throws ScimResourceNotFoundException
*/
List<ScimGroupMember> getMembers(String groupId, ScimGroupMember.Role permission, final String zoneId)
throws ScimResourceNotFoundException;

/** /**
* Retrieve all groups that the given member belongs to * Retrieve all groups that the given member belongs to
* *
Expand Down
38 changes: 8 additions & 30 deletions ...src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManager.java
View file
Expand Up @@ -67,8 +67,6 @@ public class JdbcScimGroupMembershipManager implements ScimGroupMembershipManage


public static final String GET_GROUPS_BY_MEMBER_SQL = String.format("select distinct(group_id) from %s where member_id=? and identity_zone_id=?", MEMBERSHIP_TABLE); public static final String GET_GROUPS_BY_MEMBER_SQL = String.format("select distinct(group_id) from %s where member_id=? and identity_zone_id=?", MEMBERSHIP_TABLE);


public static final String GET_MEMBERS_WITH_AUTHORITY_SQL = String.format("select %s from %s where group_id=? and lower(authorities) like ? and identity_zone_id=?", MEMBERSHIP_FIELDS,MEMBERSHIP_TABLE);

public static final String GET_MEMBERS_SQL = String.format("select %s from %s where group_id=? and identity_zone_id=?",MEMBERSHIP_FIELDS, MEMBERSHIP_TABLE); public static final String GET_MEMBERS_SQL = String.format("select %s from %s where group_id=? and identity_zone_id=?",MEMBERSHIP_FIELDS, MEMBERSHIP_TABLE);


public static final String GET_MEMBER_SQL = String.format("select %s from %s where member_id=? and group_id=? and identity_zone_id=?",MEMBERSHIP_FIELDS, MEMBERSHIP_TABLE); public static final String GET_MEMBER_SQL = String.format("select %s from %s where member_id=? and group_id=? and identity_zone_id=?",MEMBERSHIP_FIELDS, MEMBERSHIP_TABLE);
Expand Down Expand Up @@ -154,17 +152,14 @@ public ScimGroupMember addMember(final String groupId, final ScimGroupMember mem
final String type = (member.getType() == null ? ScimGroupMember.Type.USER : member.getType()).toString(); final String type = (member.getType() == null ? ScimGroupMember.Type.USER : member.getType()).toString();
try { try {
logger.debug("Associating group:"+groupId+" with member:"+member); logger.debug("Associating group:"+groupId+" with member:"+member);
jdbcTemplate.update(ADD_MEMBER_SQL, new PreparedStatementSetter() { jdbcTemplate.update(ADD_MEMBER_SQL, ps -> {
@Override ps.setString(1, groupId);
public void setValues(PreparedStatement ps) throws SQLException { ps.setString(2, member.getMemberId());
ps.setString(1, groupId); ps.setString(3, type);
ps.setString(2, member.getMemberId()); ps.setNull(4, Types.VARCHAR);
ps.setString(3, type); ps.setTimestamp(5, new Timestamp(new Date().getTime()));
ps.setNull(4, Types.NVARCHAR); ps.setString(6, member.getOrigin());
ps.setTimestamp(5, new Timestamp(new Date().getTime())); ps.setString(7, zoneId);
ps.setString(6, member.getOrigin());
ps.setString(7, zoneId);
}
}); });
} catch (DuplicateKeyException e) { } catch (DuplicateKeyException e) {
throw new MemberAlreadyExistsException(member.getMemberId() + " is already part of the group: " + groupId); throw new MemberAlreadyExistsException(member.getMemberId() + " is already part of the group: " + groupId);
Expand Down Expand Up @@ -260,23 +255,6 @@ public Set<ScimGroup> getGroupsWithExternalMember(final String memberId, final S
return new HashSet<>(results); return new HashSet<>(results);
} }


@Override
public List<ScimGroupMember> getMembers(final String groupId, final ScimGroupMember.Role permission, final String zoneId)
throws ScimResourceNotFoundException {
logger.debug("getting members of type: " + permission + " from group: " + groupId);
List<ScimGroupMember> members = new ArrayList<ScimGroupMember>();
members.addAll(jdbcTemplate.query(GET_MEMBERS_WITH_AUTHORITY_SQL, new PreparedStatementSetter() {
@Override
public void setValues(PreparedStatement ps) throws SQLException {
ps.setString(1, groupId);
ps.setString(2, "%" + permission.toString().toLowerCase() + "%");
ps.setString(3, zoneId);
}
}, rowMapper)
);
return members;
}

@Override @Override
public ScimGroupMember getMemberById(String groupId, String memberId, String zoneId) throws ScimResourceNotFoundException, public ScimGroupMember getMemberById(String groupId, String memberId, String zoneId) throws ScimResourceNotFoundException,
MemberNotFoundException { MemberNotFoundException {
Expand Down
64 changes: 0 additions & 64 deletions server/src/main/java/org/cloudfoundry/identity/uaa/scim/security/GroupRoleCheck.java
View file

This file was deleted.

64 changes: 20 additions & 44 deletions ...est/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManagerTests.java
View file
Expand Up @@ -59,6 +59,7 @@
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
Expand Down Expand Up @@ -469,58 +470,33 @@ public void canGetMembers_Fails_In_Other_Zone() throws Exception {
assertEquals(0, dao.getMembers("g1", false, IdentityZoneHolder.get().getId()).size()); assertEquals(0, dao.getMembers("g1", false, IdentityZoneHolder.get().getId()).size());
} }


// @Test
// public void testBackwardsCompatibilityToMemberAuthorities() {
// addMember("g1", "m1", "USER", "READ");
// addMember("g1", "g2", "GROUP", "member");
// addMember("g1", "m2", "USER", "READER,write");
//
// List<ScimGroupMember> members = dao.getMembers("g1", false, IdentityZoneHolder.get().getId());
// assertNotNull(members);
// assertEquals(3, members.size());
// List<ScimGroupMember> readers = new ArrayList<ScimGroupMember>(), writers = new ArrayList<ScimGroupMember>();
// for (ScimGroupMember member : members) {
// if (member.getRoles().contains(ScimGroupMember.Role.READER)) {
// readers.add(member);
// }
// if (member.getRoles().contains(ScimGroupMember.Role.WRITER)) {
// writers.add(member);
// }
// }
// assertEquals(2, readers.size());
// assertEquals(1, writers.size());
// }

@Test @Test
public void canGetDefaultGroupsUsingGetGroupsForMember() { public void canReadNullFromAuthoritiesColumn() {
Set<ScimGroup> groups = dao.getGroupsWithMember("m1", false, IdentityZoneHolder.get().getId()); String addNullAuthoritySQL =
assertNotNull(groups); "insert into group_membership (group_id, member_id, member_type, authorities, origin, identity_zone_id) values ('%s', '%s', '%s', NULL, '%s', '%s')";
assertEquals(1, groups.size()); jdbcTemplate.execute(String.format(addNullAuthoritySQL, "g1", "m1", "USER", "uaa", IdentityZoneHolder.get().getId()));

ScimGroupMember member = dao.getMemberById("g1", "m1", IdentityZoneHolder.get().getId());
assertNotNull(member);
assertEquals("m1", member.getMemberId());
} }


@Test @Test
public void canGetAdminMembers() { public void canReadNonNullFromAuthoritiesColumn() {
addMember("g1", "m3", "USER", "READER,WRITER"); String addNullAuthoritySQL =
addMember("g1", "g2", "GROUP", "READER"); "insert into group_membership (group_id, member_id, member_type, authorities, origin, identity_zone_id) values ('%s', '%s', '%s', '%s', '%s', '%s')";
jdbcTemplate.execute(String.format(addNullAuthoritySQL, "g1", "m1", "USER", "ANYTHING", "uaa", IdentityZoneHolder.get().getId()));


assertEquals(1, dao.getMembers("g1", ScimGroupMember.Role.WRITER, IdentityZoneHolder.get().getId()).size()); ScimGroupMember member = dao.getMemberById("g1", "m1", IdentityZoneHolder.get().getId());
assertTrue(dao.getMembers("g1", ScimGroupMember.Role.WRITER, IdentityZoneHolder.get().getId()).contains(new ScimGroupMember("m3"))); assertNotNull(member);

assertEquals("m1", member.getMemberId());
assertEquals(0, dao.getMembers("g2", ScimGroupMember.Role.WRITER, IdentityZoneHolder.get().getId()).size());
} }


@Test @Test
public void canGetMembersByAuthority() { public void canGetDefaultGroupsUsingGetGroupsForMember() {
addMember("g1", "m3", "USER", "READER,WRITER"); Set<ScimGroup> groups = dao.getGroupsWithMember("m1", false, IdentityZoneHolder.get().getId());
addMember("g1", "g2", "GROUP", "READER,MEMBER"); assertNotNull(groups);
addMember("g2", "g3", "GROUP", "MEMBER"); assertEquals(1, groups.size());

assertEquals(1, dao.getMembers("g1", ScimGroupMember.Role.MEMBER, IdentityZoneHolder.get().getId()).size());
assertEquals(2, dao.getMembers("g1", ScimGroupMember.Role.READER, IdentityZoneHolder.get().getId()).size());
assertEquals(1, dao.getMembers("g1", ScimGroupMember.Role.WRITER, IdentityZoneHolder.get().getId()).size());

assertEquals(1, dao.getMembers("g2", ScimGroupMember.Role.MEMBER, IdentityZoneHolder.get().getId()).size());
assertEquals(0, dao.getMembers("g2", ScimGroupMember.Role.WRITER, IdentityZoneHolder.get().getId()).size());
} }


@Test @Test
Expand Down

0 comments on commit 06800b8

Please sign in to comment.