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

[JENKINS-55803] - Attempt to solve issue:removing the intermediate collections created each time getRolesHavingPermission() is invoked #54

Merged
merged 26 commits into from
Feb 11, 2019
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
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 @@ -92,9 +92,9 @@ public class RoleBasedAuthorizationStrategy extends AuthorizationStrategy {
public final static String SLAVE = "slaveRoles";
public final static String MACRO_ROLE = "roleMacros";
public final static String MACRO_USER = "userMacros";

private static final Logger LOGGER = Logger.getLogger(RoleBasedAuthorizationStrategy.class.getName());

/** {@link RoleMap}s associated to each {@link AccessControlled} class */
private final Map <String, RoleMap> grantedRoles;

Expand All @@ -116,9 +116,9 @@ public SidACL getRootACL() {
return root.getACL(RoleType.Global, null);
}


/**
* Universal function for getting ACL for different
* Universal function for getting ACL for different
* @param roleMapName Name of the role map section
* @param itemName Name of the item for patterns
* @return ACL
Expand All @@ -134,9 +134,9 @@ private ACL getACL(String roleMapName, String itemName, RoleType roleType, Acces
// Create a sub-RoleMap matching the project name, and create an inheriting from root ACL
acl = roleMap.newMatchingRoleMap(itemName).getACL(roleType, item).newInheritingACL(getRootACL());
}
return acl;
return acl;
}

/**
* Get the specific ACL for projects.
* @param project The access-controlled project
Expand All @@ -156,7 +156,7 @@ public ACL getACL(AbstractItem project) {
public ACL getACL(Computer computer) {
return getACL(SLAVE, computer.getName(), RoleType.Slave, computer);
}

/**
* Used by the container realm.
* @return All the sids referenced by the strategy
Expand Down Expand Up @@ -200,7 +200,7 @@ public Set<String> getSIDs(String type) {
}
return null;
}

/**
* Get the {@link RoleMap} associated to the given class.
* @param type The object type controlled by the {@link RoleMap}
Expand Down Expand Up @@ -258,7 +258,7 @@ private void assignRole(String type, Role role, String sid) {
roleMap.assignRole(role, sid);
}
}

/**
* API method to add roles
* <p>
Expand All @@ -269,7 +269,7 @@ private void assignRole(String type, Role role, String sid) {
* @param roleName Name of role
* @param permissionIds Comma separated list of IDs for given roleName
* @param overwrite Overwrite existing role
* @param pattern Role pattern
* @param pattern Role pattern
* @throws IOException In case saving changes fails
* @since 2.5.0
*/
Expand Down Expand Up @@ -515,7 +515,7 @@ public void doGetMatchingJobs(@QueryParameter(required = true) String pattern,
responseJson.write(writer);
writer.close();
}

@Extension
public static final DescriptorImpl DESCRIPTOR = new DescriptorImpl();

Expand All @@ -535,7 +535,7 @@ public boolean canConvert(Class type) {

public void marshal(Object source, HierarchicalStreamWriter writer, MarshallingContext context) {
RoleBasedAuthorizationStrategy strategy = (RoleBasedAuthorizationStrategy)source;

// Role maps
Map<String, RoleMap> maps = strategy.getRoleMaps();
for (Map.Entry<String, RoleMap> map : maps.entrySet()) {
Expand Down Expand Up @@ -576,7 +576,7 @@ public void marshal(Object source, HierarchicalStreamWriter writer, MarshallingC
public Object unmarshal(HierarchicalStreamReader reader, final UnmarshallingContext context) {
final RoleBasedAuthorizationStrategy strategy = create();
boolean showDangerousPermissionsDefined = false;

while(reader.hasMoreChildren()) {
reader.moveDown();

Expand Down Expand Up @@ -623,15 +623,15 @@ public Object unmarshal(HierarchicalStreamReader reader, final UnmarshallingCont
}
reader.moveUp();
}

return strategy;
}

protected RoleBasedAuthorizationStrategy create() {
return new RoleBasedAuthorizationStrategy();
}
}
}

/**
* Retrieves instance of the strategy.
* @return Strategy instance or {@code null} if it is disabled.
Expand All @@ -643,7 +643,7 @@ public static RoleBasedAuthorizationStrategy getInstance() {
if (authStrategy instanceof RoleBasedAuthorizationStrategy) {
return (RoleBasedAuthorizationStrategy)authStrategy;
}

// Nothing to do here, not a Role strategy
return null;
}
Expand All @@ -655,7 +655,7 @@ public static RoleBasedAuthorizationStrategy getInstance() {
void renewMacroRoles()
{
//TODO: add mandatory roles

// Check role extensions
for (UserMacroExtension userExt : UserMacroExtension.all())
{
Expand Down Expand Up @@ -686,7 +686,7 @@ public String getDisplayName() {
return Messages.RoleBasedAuthorizationStrategy_DisplayName();
}

/**
/**
* Called on role management form's submission.
*/
@RequirePOST
Expand All @@ -713,20 +713,20 @@ public void doAssignSubmit(StaplerRequest req, StaplerResponse rsp) throws Unsup
req.setCharacterEncoding("UTF-8");
JSONObject json = req.getSubmittedForm();
AuthorizationStrategy oldStrategy = instance().getAuthorizationStrategy();

if (json.has(GLOBAL) && json.has(PROJECT) && oldStrategy instanceof RoleBasedAuthorizationStrategy) {
RoleBasedAuthorizationStrategy strategy = (RoleBasedAuthorizationStrategy) oldStrategy;
Map<String, RoleMap> maps = strategy.getRoleMaps();

for (Map.Entry<String, RoleMap> map : maps.entrySet()) {
for (Map.Entry<String, RoleMap> map : maps.entrySet()) {
// Get roles and skip non-existent role entries (backward-comp)
RoleMap roleMap = map.getValue();
roleMap.clearSids();
JSONObject roles = json.getJSONObject(map.getKey());
if (roles.isNullObject()) {
continue;
}

for (Map.Entry<String,JSONObject> r : (Set<Map.Entry<String,JSONObject>>)roles.getJSONObject("data").entrySet()) {
String sid = r.getKey();
for (Map.Entry<String,Boolean> e : (Set<Map.Entry<String,Boolean>>)r.getValue().entrySet()) {
Expand All @@ -753,7 +753,7 @@ public AuthorizationStrategy newInstance(StaplerRequest req, JSONObject formData
AuthorizationStrategy oldStrategy = instance().getAuthorizationStrategy();
RoleBasedAuthorizationStrategy strategy;


// If the form contains data, it means the method has been called by plugin
// specifics forms, and we need to handle it.
if (formData.has(GLOBAL) && formData.has(PROJECT) && formData.has(SLAVE) && oldStrategy instanceof RoleBasedAuthorizationStrategy) {
Expand Down Expand Up @@ -800,14 +800,14 @@ else if (oldStrategy instanceof RoleBasedAuthorizationStrategy) {
strategy.addRole(GLOBAL, adminRole);
strategy.assignRole(GLOBAL, adminRole, getCurrentUser());
}

strategy.renewMacroRoles();
return strategy;
}

private void ReadRoles(JSONObject formData, String roleType,
RoleBasedAuthorizationStrategy targetStrategy, RoleBasedAuthorizationStrategy oldStrategy)
{
{
if (!formData.has(roleType)) {
assert false : "Unexistent Role type " + roleType;
return;
Expand All @@ -817,7 +817,7 @@ private void ReadRoles(JSONObject formData, String roleType,
assert false : "No data at role description";
return;
}

for (Map.Entry<String,JSONObject> r : (Set<Map.Entry<String,JSONObject>>)projectRoles.getJSONObject("data").entrySet()) {
String roleName = r.getKey();
Set<Permission> permissions = new HashSet<>();
Expand Down Expand Up @@ -849,7 +849,7 @@ private void ReadRoles(JSONObject formData, String roleType,
}
}
}

/**
* Create an admin role.
*/
Expand All @@ -875,7 +875,7 @@ private String getCurrentUser() {

/**
* Get the needed permissions groups.
*
*
* @param type Role type
* @return Groups, which should be displayed for a specific role type.
* {@code null} if an unsupported type is defined.
Expand All @@ -899,8 +899,8 @@ else if (type.equals(SLAVE)) {
groups.remove(PermissionGroup.get(Permission.class));
groups.remove(PermissionGroup.get(Hudson.class));
groups.remove(PermissionGroup.get(View.class));
// Project, SCM and Run permissions

// Project, SCM and Run permissions
groups.remove(PermissionGroup.get(Item.class));
groups.remove(PermissionGroup.get(SCM.class));
groups.remove(PermissionGroup.get(Run.class));
Expand All @@ -920,12 +920,12 @@ public boolean hasDangerousPermissions() {
}
return PermissionHelper.hasDangerousPermissions(instance);
}

@Restricted(NoExternalUse.class)
public boolean showPermission(String type, Permission p) {
return showPermission(type, p, false);
}

/**
* Check if the permission should be displayed.
* For Stapler only.
Expand All @@ -940,7 +940,7 @@ public boolean showPermission(String type, Permission p, boolean showDangerous)
// Should never happen
return false;
}

// When disabled, never show the permissions
return showDangerous && DangerousPermissionHandlingMode.getCurrent() != DangerousPermissionHandlingMode.DISABLED;
}
Expand Down
Loading