From da0c5fb4982e699bfd410ff2798f4bd91a8823ec Mon Sep 17 00:00:00 2001 From: Tomas Westling Date: Wed, 5 Jul 2023 06:31:25 +0200 Subject: [PATCH] [JENKINS-71368] Obtain groups from all matrix properties using SYSTEM identity (#141) * Using SYSTEM2 ACLContext to find out which group permissions has been set The getGroups method of ProjectMatrixAuthorizationStrategy uses the users ACLContext to iterate through all projects/folders/nodes and find which group permissions are set on each. In conjunction with the active-directory plugin, with the option "remove irrelevant groups" this causes a problem, since we save the relevant groups upon logging in, when the user is still anonymous. This is done for performance reasons, since going through each AD group everytime a user tries to access a resource is not doable for systems with a lot of AD groups. This means that for everything on the master where Anonymous doesn't have read access, the groups defined are deemed irrelevant, since we can't access them at the time of calculation. This in turn means that you can't set up a system where access is given through AD groups and anonymous can't already read everything. * Add test Also rename the unused try-with-resources holder for IntelliJ --------- Co-authored-by: Daniel Beck --- .../ProjectMatrixAuthorizationStrategy.java | 33 ++++++++++--------- ...rojectMatrixAuthorizationStrategyTest.java | 30 +++++++++++++++++ 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/main/java/hudson/security/ProjectMatrixAuthorizationStrategy.java b/src/main/java/hudson/security/ProjectMatrixAuthorizationStrategy.java index 5e5dea72..f37b6e94 100644 --- a/src/main/java/hudson/security/ProjectMatrixAuthorizationStrategy.java +++ b/src/main/java/hudson/security/ProjectMatrixAuthorizationStrategy.java @@ -99,21 +99,24 @@ public ACL getACL(@NonNull AbstractItem item) { @NonNull public Set getGroups() { Set r = new TreeSet<>(new IdStrategyComparator()); - r.addAll(super.getGroups()); - for (Job j : Jenkins.get().getAllItems(Job.class)) { - AuthorizationMatrixProperty jobProperty = j.getProperty(AuthorizationMatrixProperty.class); - if (jobProperty != null) r.addAll(jobProperty.getGroups()); - } - for (AbstractFolder j : Jenkins.get().getAllItems(AbstractFolder.class)) { - com.cloudbees.hudson.plugins.folder.properties.AuthorizationMatrixProperty folderProperty = - j.getProperties() - .get(com.cloudbees.hudson.plugins.folder.properties.AuthorizationMatrixProperty.class); - if (folderProperty != null) r.addAll(folderProperty.getGroups()); - } - for (Node node : Jenkins.get().getNodes()) { - AuthorizationMatrixNodeProperty nodeProperty = node.getNodeProperty(AuthorizationMatrixNodeProperty.class); - if (nodeProperty != null) { - r.addAll(nodeProperty.getGroups()); + try (ACLContext ignored = ACL.as2(ACL.SYSTEM2)) { + r.addAll(super.getGroups()); + for (Job j : Jenkins.get().getAllItems(Job.class)) { + AuthorizationMatrixProperty jobProperty = j.getProperty(AuthorizationMatrixProperty.class); + if (jobProperty != null) r.addAll(jobProperty.getGroups()); + } + for (AbstractFolder j : Jenkins.get().getAllItems(AbstractFolder.class)) { + com.cloudbees.hudson.plugins.folder.properties.AuthorizationMatrixProperty folderProperty = + j.getProperties() + .get(com.cloudbees.hudson.plugins.folder.properties.AuthorizationMatrixProperty.class); + if (folderProperty != null) r.addAll(folderProperty.getGroups()); + } + for (Node node : Jenkins.get().getNodes()) { + AuthorizationMatrixNodeProperty nodeProperty = + node.getNodeProperty(AuthorizationMatrixNodeProperty.class); + if (nodeProperty != null) { + r.addAll(nodeProperty.getGroups()); + } } } return r; diff --git a/src/test/java/hudson/security/ProjectMatrixAuthorizationStrategyTest.java b/src/test/java/hudson/security/ProjectMatrixAuthorizationStrategyTest.java index e33f69d2..2202c047 100644 --- a/src/test/java/hudson/security/ProjectMatrixAuthorizationStrategyTest.java +++ b/src/test/java/hudson/security/ProjectMatrixAuthorizationStrategyTest.java @@ -1,18 +1,27 @@ package hudson.security; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; + import com.cloudbees.hudson.plugins.folder.Folder; +import hudson.model.FreeStyleProject; import hudson.model.Item; import hudson.model.Job; import hudson.model.User; import hudson.util.VersionNumber; +import java.io.IOException; import java.util.Collections; +import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; import jenkins.model.Jenkins; import org.htmlunit.html.HtmlElement; import org.htmlunit.html.HtmlForm; import org.htmlunit.html.HtmlOption; import org.htmlunit.html.HtmlSelect; +import org.jenkinsci.plugins.matrixauth.PermissionEntry; +import org.jenkinsci.plugins.matrixauth.inheritance.InheritParentStrategy; import org.jenkinsci.plugins.matrixauth.inheritance.NonInheritingStrategy; import org.junit.Assert; import org.junit.Rule; @@ -267,4 +276,25 @@ public void subdirectoriesCanExcludeOtherNonAdminUsers() throws Exception { // expected } } + + @Test + public void getGroupsAlwaysEverything() throws IOException { + HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(false, false, null); + r.jenkins.setSecurityRealm(securityRealm); + + ProjectMatrixAuthorizationStrategy authorizationStrategy = new ProjectMatrixAuthorizationStrategy(); + authorizationStrategy.add(Jenkins.READ, PermissionEntry.group("group1")); + r.jenkins.setAuthorizationStrategy(authorizationStrategy); + + final Folder f = r.jenkins.createProject(Folder.class, "F"); + final FreeStyleProject job = f.createProject(FreeStyleProject.class, "job"); + job.addProperty(new AuthorizationMatrixProperty( + Map.of(Item.READ, Set.of(PermissionEntry.group("group2"))), new InheritParentStrategy())); + + assertThat(authorizationStrategy.getGroups(), containsInAnyOrder("group1", "group2")); + + try (ACLContext ignored = ACL.as2(Jenkins.ANONYMOUS2)) { + assertThat(authorizationStrategy.getGroups(), containsInAnyOrder("group1", "group2")); + } + } }