Skip to content

Commit

Permalink
[SECURITY-3349]
Browse files Browse the repository at this point in the history
  • Loading branch information
yaroslavafenkin authored and jenkinsci-cert-ci committed Jul 22, 2024
1 parent cec49ce commit efece77
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 2 deletions.
25 changes: 23 additions & 2 deletions core/src/main/java/hudson/model/MyViewsProperty.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.Extension;
import hudson.Util;
import hudson.model.Descriptor.FormException;
Expand All @@ -41,6 +42,7 @@
import java.util.concurrent.CopyOnWriteArrayList;
import javax.servlet.ServletException;
import jenkins.model.Jenkins;
import jenkins.util.SystemProperties;
import net.sf.json.JSONObject;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
Expand All @@ -50,6 +52,7 @@
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerFallback;
import org.kohsuke.stapler.StaplerProxy;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.kohsuke.stapler.verb.POST;
Expand All @@ -59,7 +62,14 @@
*
* @author Tom Huybrechts
*/
public class MyViewsProperty extends UserProperty implements ModifiableViewGroup, Action, StaplerFallback {
public class MyViewsProperty extends UserProperty implements ModifiableViewGroup, Action, StaplerFallback, StaplerProxy {

/**
* Escape hatch for StaplerProxy-based access control
*/
@Restricted(NoExternalUse.class)
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "for script console")
public static /* non-final */ boolean SKIP_PERMISSION_CHECK = SystemProperties.getBoolean(MyViewsProperty.class.getName() + ".skipPermissionCheck");

/**
* Name of the primary view defined by the user.
Expand Down Expand Up @@ -225,14 +235,25 @@ public String getDisplayName() {

@Override
public String getIconFileName() {
return "symbol-browsers";
if (SKIP_PERMISSION_CHECK || getACL().hasPermission(Jenkins.ADMINISTER))
return "symbol-browsers";
else
return null;
}

@Override
public String getUrlName() {
return "my-views";
}

@Override
public Object getTarget() {
if (!SKIP_PERMISSION_CHECK) {
checkPermission(Jenkins.ADMINISTER);
}
return this;
}

@Extension @Symbol("myView")
public static class DescriptorImpl extends UserPropertyDescriptor {

Expand Down
80 changes: 80 additions & 0 deletions test/src/test/java/hudson/model/Security3349Test.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package hudson.model;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import jenkins.model.Jenkins;
import org.htmlunit.html.HtmlPage;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.FlagRule;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.MockAuthorizationStrategy;

public class Security3349Test {

@Rule
public JenkinsRule rule = new JenkinsRule();

@Rule public FlagRule<Boolean> skipPermissionCheck = new FlagRule<>(() -> MyViewsProperty.SKIP_PERMISSION_CHECK, x -> MyViewsProperty.SKIP_PERMISSION_CHECK = x);

@Test
@Issue("SECURITY-3349")
public void usersCannotAccessOtherUsersViews() throws Exception {
User user = User.getOrCreateByIdOrFullName("user");
User admin = User.getOrCreateByIdOrFullName("admin");

rule.jenkins.setSecurityRealm(rule.createDummySecurityRealm());
MockAuthorizationStrategy mockAuthorizationStrategy = new MockAuthorizationStrategy();
mockAuthorizationStrategy.grant(Jenkins.READ, View.READ).everywhere().to("user");
mockAuthorizationStrategy.grant(Jenkins.ADMINISTER).everywhere().to("admin");
rule.jenkins.setAuthorizationStrategy(mockAuthorizationStrategy);

MyViewsProperty prop1 = new MyViewsProperty(null);
MyView usersView = new MyView("User's view", prop1);
user.addProperty(prop1);
prop1.setUser(user);
prop1.addView(usersView);

MyViewsProperty prop2 = new MyViewsProperty(null);
MyView adminsView = new MyView("Admin's view", prop2);
admin.addProperty(prop2);
prop2.setUser(admin);
prop2.addView(adminsView);

try (JenkinsRule.WebClient wc = rule.createWebClient()) {
wc.setThrowExceptionOnFailingStatusCode(false);
wc.login("user");

HtmlPage adminViews = wc.goTo("user/admin/my-views/view/all/");
assertEquals(403, adminViews.getWebResponse().getStatusCode());

HtmlPage adminUserPage = wc.goTo("user/admin/");
assertFalse(adminUserPage.getWebResponse().getContentAsString().contains("My Views"));

HtmlPage userViews = wc.goTo("user/user/my-views/view/all/");
assertEquals(200, userViews.getWebResponse().getStatusCode());

HtmlPage userUserPage = wc.goTo("user/user/");
assertTrue(userUserPage.getWebResponse().getContentAsString().contains("My Views"));

wc.login("admin");

adminViews = wc.goTo("user/admin/my-views/view/all/");
assertEquals(200, adminViews.getWebResponse().getStatusCode());
userViews = wc.goTo("user/user/my-views/view/all/");
assertEquals(200, userViews.getWebResponse().getStatusCode());

MyViewsProperty.SKIP_PERMISSION_CHECK = true;

wc.login("user");
adminViews = wc.goTo("user/admin/my-views/view/all/");
assertEquals(200, adminViews.getWebResponse().getStatusCode());
adminUserPage = wc.goTo("user/admin/");
assertTrue(adminUserPage.getWebResponse().getContentAsString().contains("My Views"));

}
}
}

0 comments on commit efece77

Please sign in to comment.