Skip to content

Commit

Permalink
Merge branch 'master' into JENKINS-34616
Browse files Browse the repository at this point in the history
  • Loading branch information
daniel-beck committed Sep 19, 2017
2 parents d09a2af + 67dffd1 commit 76e5974
Show file tree
Hide file tree
Showing 18 changed files with 97 additions and 87 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -6,7 +6,7 @@
<version>2.33</version>
</parent>
<artifactId>matrix-auth</artifactId>
<version>2.0-beta-2-SNAPSHOT</version>
<version>2.0-beta-3-SNAPSHOT</version>
<packaging>hpi</packaging>
<name>Matrix Authorization Strategy Plugin</name>
<description>Offers matrix-based security authorization strategies (global and per-project).</description>
Expand Down
Expand Up @@ -52,6 +52,7 @@
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.verb.GET;

import java.io.IOException;
import java.util.Collections;
Expand All @@ -60,6 +61,8 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* Holds ACL for {@link ProjectMatrixAuthorizationStrategy}.
Expand All @@ -82,6 +85,7 @@ public class AuthorizationMatrixProperty extends AbstractFolderProperty<Abstract
* @deprecated unused, use {@link #setInheritanceStrategy(InheritanceStrategy)} instead.
*/
@Deprecated
@SuppressWarnings("unused")
private transient Boolean blocksInheritance;

private InheritanceStrategy inheritanceStrategy = new InheritParentStrategy();
Expand Down Expand Up @@ -143,10 +147,12 @@ public AuthorizationMatrixProperty newInstance(StaplerRequest req, JSONObject fo
}

@Override
@SuppressWarnings("rawtypes")
public boolean isApplicable(Class<? extends AbstractFolder> folder) {
return isApplicable();
}

@GET
public FormValidation doCheckName(@AncestorInPath AbstractFolder<?> folder, @QueryParameter String value) {
return doCheckName_(value, folder, Item.CONFIGURE);
}
Expand Down Expand Up @@ -180,6 +186,7 @@ public InheritanceStrategy getInheritanceStrategy() {
*/
@Restricted(DoNotUse.class)
public static final class ConverterImpl extends AbstractAuthorizationPropertyConverter<AuthorizationMatrixProperty> {
@SuppressWarnings("rawtypes")
public boolean canConvert(Class type) {
return type == AuthorizationMatrixProperty.class;
}
Expand Down Expand Up @@ -222,11 +229,13 @@ public void onCreated(Item item) {
try {
folder.addProperty(prop);
} catch (IOException ex) {
// TODO LOGGER
LOGGER.log(Level.WARNING, "Failed to grant creator permissions on folder " + item.getFullName(), ex);
}
}
}
}
}
}

private static final Logger LOGGER = Logger.getLogger(AuthorizationMatrixProperty.class.getName());
}
13 changes: 11 additions & 2 deletions src/main/java/hudson/security/AuthorizationMatrixProperty.java
Expand Up @@ -46,6 +46,8 @@
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;

import jenkins.model.Jenkins;
Expand All @@ -66,6 +68,7 @@
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.verb.GET;

/**
* {@link JobProperty} to associate ACL for each project.
Expand All @@ -91,6 +94,7 @@ public class AuthorizationMatrixProperty extends JobProperty<Job<?, ?>> implemen
* @deprecated unused, use {@link #setInheritanceStrategy(InheritanceStrategy)} instead.
*/
@Deprecated
@SuppressWarnings("unused")
private transient Boolean blocksInheritance;

private InheritanceStrategy inheritanceStrategy = new InheritParentStrategy();
Expand Down Expand Up @@ -173,11 +177,13 @@ public JobProperty<?> newInstance(StaplerRequest req, JSONObject formData) throw
}

@Override
@SuppressWarnings("rawtypes")
public boolean isApplicable(Class<? extends Job> jobType) {
return isApplicable();
}

public FormValidation doCheckName(@AncestorInPath Job project, @QueryParameter String value) {
@GET
public FormValidation doCheckName(@AncestorInPath Job<?, ?> project, @QueryParameter String value) {
return doCheckName_(value, project, Item.CONFIGURE);
}
}
Expand Down Expand Up @@ -213,6 +219,7 @@ public InheritanceStrategy getInheritanceStrategy() {
*/
@Restricted(DoNotUse.class)
public static final class ConverterImpl extends AbstractAuthorizationPropertyConverter<AuthorizationMatrixProperty> {
@SuppressWarnings("rawtypes")
public boolean canConvert(Class type) {
return type == AuthorizationMatrixProperty.class;
}
Expand Down Expand Up @@ -254,11 +261,13 @@ public void onCreated(Item item) {
try {
job.addProperty(prop);
} catch (IOException ex) {
// TODO LOGGER
LOGGER.log(Level.WARNING, "Failed to grant creator permissions on job " + item.getFullName(), ex);
}
}
}
}
}
}

private static final Logger LOGGER = Logger.getLogger(AuthorizationMatrixProperty.class.getName());
}
Expand Up @@ -139,6 +139,7 @@ protected Boolean hasPermission(Sid p, Permission permission) {
*/
@Restricted(NoExternalUse.class)
public static class ConverterImpl extends AbstractAuthorizationContainerConverter<GlobalMatrixAuthorizationStrategy> {
@SuppressWarnings("rawtypes")
public boolean canConvert(Class type) {
return type == GlobalMatrixAuthorizationStrategy.class;
}
Expand Down Expand Up @@ -168,7 +169,7 @@ public String getDisplayName() {

@Override
public AuthorizationStrategy newInstance(StaplerRequest req, @Nonnull JSONObject formData) throws FormException {
// TODO parent impl
// TODO Is there a way to pull this up into AuthorizationContainerDescriptor and share code with AuthorizationPropertyDescriptor?
GlobalMatrixAuthorizationStrategy gmas = create();
Map<String,Object> data = formData.getJSONObject("data");

Expand Down
Expand Up @@ -76,7 +76,7 @@ public boolean hasPermission(@Nonnull Authentication a, @Nonnull Permission perm
};
}

public ACL getACL(ItemGroup g) {
public ACL getACL(ItemGroup<?> g) {
if (g instanceof Item) {
Item item = (Item) g;
return item.getACL();
Expand All @@ -99,7 +99,7 @@ public ACL getACL(@Nonnull Node node) {
public ACL getACL(@Nonnull AbstractItem item) {
if (Jenkins.getInstance().getPlugin("cloudbees-folder") != null) { // optional dependency
if (item instanceof AbstractFolder) {
com.cloudbees.hudson.plugins.folder.properties.AuthorizationMatrixProperty p = (com.cloudbees.hudson.plugins.folder.properties.AuthorizationMatrixProperty) ((AbstractFolder) item).getProperties().get(com.cloudbees.hudson.plugins.folder.properties.AuthorizationMatrixProperty.class);
com.cloudbees.hudson.plugins.folder.properties.AuthorizationMatrixProperty p = ((AbstractFolder<?>) item).getProperties().get(com.cloudbees.hudson.plugins.folder.properties.AuthorizationMatrixProperty.class);
if (p != null) {
return p.getInheritanceStrategy().getEffectiveACL(p.getACL(), item);
}
Expand Down Expand Up @@ -155,6 +155,7 @@ public GlobalMatrixAuthorizationStrategy create() {
}

@Override
@SuppressWarnings("rawtypes")
public boolean canConvert(Class type) {
return type==ProjectMatrixAuthorizationStrategy.class;
}
Expand Down
Expand Up @@ -44,7 +44,7 @@

@Restricted(NoExternalUse.class)
public abstract class AbstractAuthorizationContainerConverter<T extends AuthorizationContainer> implements Converter {

@SuppressWarnings("rawtypes")
abstract public boolean canConvert(Class type);

abstract public T create();
Expand Down Expand Up @@ -81,9 +81,8 @@ protected void unmarshalContainer(T container, HierarchicalStreamReader reader,
try {
container.add(reader.getValue());
} catch (IllegalArgumentException ex) {
// TODO Logger field
Logger.getLogger(GlobalMatrixAuthorizationStrategy.class.getName())
.log(Level.WARNING,"Skipping a non-existent permission",ex);
Logger.getLogger(AbstractAuthorizationContainerConverter.class.getName())
.log(Level.WARNING,"Skipping a non-existent permission", ex);
RobustReflectionConverter.addErrorInContext(context, ex);
}
reader.moveUp();
Expand Down
Expand Up @@ -37,6 +37,7 @@

@Restricted(NoExternalUse.class)
public abstract class AbstractAuthorizationPropertyConverter<T extends AuthorizationProperty> extends AbstractAuthorizationContainerConverter<T> {
@SuppressWarnings("rawtypes")
abstract public boolean canConvert(Class type);

abstract public T create();
Expand Down Expand Up @@ -89,5 +90,5 @@ protected void unmarshalContainer(T container, HierarchicalStreamReader reader,
super.unmarshalContainer(container, reader, context);
}

public static final Logger LOGGER = Logger.getLogger(AbstractAuthorizationPropertyConverter.class.getName());
private static final Logger LOGGER = Logger.getLogger(AbstractAuthorizationPropertyConverter.class.getName());
}
Expand Up @@ -69,7 +69,6 @@ default String getDescription(Permission p) {

@Restricted(DoNotUse.class) // Called from Jelly view
default List<PermissionGroup> getAllGroups() {
// TODO parent impl
List<PermissionGroup> groups = new ArrayList<>();
for (PermissionGroup group : PermissionGroup.getAll()) {
if (group == PermissionGroup.get(Permission.class)) {
Expand Down
Expand Up @@ -58,6 +58,8 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;

public class AuthorizationMatrixNodeProperty extends NodeProperty<Node> implements AuthorizationProperty {

Expand All @@ -71,6 +73,7 @@ public class AuthorizationMatrixNodeProperty extends NodeProperty<Node> implemen
* @deprecated unused, use {@link #setInheritanceStrategy(InheritanceStrategy)} instead.
*/
@Deprecated
@SuppressWarnings("unused")
private transient Boolean blocksInheritance;

private InheritanceStrategy inheritanceStrategy = new InheritGlobalStrategy();
Expand Down Expand Up @@ -143,6 +146,7 @@ public SidACL getACL() {
*/
@Restricted(NoExternalUse.class)
public static final class ConverterImpl extends AbstractAuthorizationPropertyConverter<AuthorizationMatrixNodeProperty> {
@SuppressWarnings("rawtypes")
public boolean canConvert(Class type) {
return type == AuthorizationMatrixNodeProperty.class;
}
Expand Down Expand Up @@ -220,10 +224,12 @@ protected void onCreated(@Nonnull Node node) {
try {
node.getNodeProperties().replace(prop);
} catch (IOException ex) {
// TODO LOGGER
LOGGER.log(Level.WARNING, "Failed to grant creator permissions on node " + node.getDisplayName(), ex);
}
}
}
}
}

private static final Logger LOGGER = Logger.getLogger(AuthorizationMatrixNodeProperty.class.getName());
}
Expand Up @@ -34,11 +34,11 @@ private ValidationUtil() {
// do not use
}

public static String formatNonExistentUserGroupValidationResponse(String user, String tooltip) {
static String formatNonExistentUserGroupValidationResponse(String user, String tooltip) {
return formatUserGroupValidationResponse("user-disabled.png", "<span style='text-decoration: line-through; color: grey;'>" + user + "</span>", tooltip, true);
}

public static String formatUserGroupValidationResponse(String img, String user, String tooltip, boolean inPlugin) {
static String formatUserGroupValidationResponse(String img, String user, String tooltip, boolean inPlugin) {
if (inPlugin) {
return String.format("<span title='%s'><img src='%s/plugin/matrix-auth/images/%s' style='margin-right:0.2em'>%s</span>", tooltip, Stapler.getCurrentRequest().getContextPath(), img, user);
} else {
Expand Down
Expand Up @@ -51,7 +51,7 @@ public InheritParentStrategy() {
public ACL getEffectiveACL(ACL acl, AccessControlled subject) {
if (subject instanceof AbstractItem) {
AbstractItem item = (AbstractItem) subject;
ItemGroup parent = item.getParent();
ItemGroup<?> parent = item.getParent();
final ACL parentACL;
if (parent instanceof AbstractItem) {
parentACL = Jenkins.getInstance().getAuthorizationStrategy().getACL((AbstractItem) parent);
Expand Down
Expand Up @@ -49,10 +49,4 @@ public static List<InheritanceStrategyDescriptor> getApplicableDescriptors(Class
}

public abstract boolean isApplicable(Class<?> clazz);

@Nonnull
@Override
public String getDisplayName() {
return super.getDisplayName();
}
}
Expand Up @@ -34,7 +34,6 @@
import hudson.security.ACLContext;
import hudson.security.HudsonPrivateSecurityRealm;
import hudson.security.ProjectMatrixAuthorizationStrategy;
import java.util.concurrent.Callable;
import java.util.logging.Level;

import jenkins.model.Jenkins;
Expand Down Expand Up @@ -69,7 +68,7 @@ public void ensureCreatorHasPermissions() throws Exception {
r.jenkins.setAuthorizationStrategy(authorizationStrategy);

Folder job;
try (ACLContext _ = ACL.as(User.get("alice"))) {
try (ACLContext ignored = ACL.as(User.get("alice"))) {
job = r.createProject(Folder.class);
}

Expand Down Expand Up @@ -112,17 +111,15 @@ public void ensureCreatorHasPermissions() throws Exception {
wc.getPage(foo); // this should succeed

// and build permission should be set, too
wc.executeOnServer(new Callable<Object>() {
public Object call() throws Exception {
foo.checkPermission(Item.BUILD);
try {
foo.checkPermission(Item.DELETE);
fail("acecss should be denied");
} catch (AccessDeniedException e) {
// expected
}
return null;
wc.executeOnServer(() -> {
foo.checkPermission(Item.BUILD);
try {
foo.checkPermission(Item.DELETE);
fail("access should be denied");
} catch (AccessDeniedException e) {
// expected
}
return null;
});
}

Expand Down

0 comments on commit 76e5974

Please sign in to comment.