Skip to content
Permalink
Browse files

#957, #1592 - CCTray permission bug fix

* If no global superadmins are setup, every user is a super admin.
  Irrespective of anything else. So, every user, in such a scenario,
  should be able to see all pipelines in CCTray. This is consistent with
  how the dashboard works too.
  • Loading branch information...
arvindsv committed May 15, 2017
1 parent bf9b665 commit e89fdb669f0663d48fde2510248b7d525d5e9808
@@ -43,11 +43,12 @@ public CcTrayViewAuthority(GoConfigService goConfigService) {
SecurityConfig security = goConfigService.security();
final Map<String, Collection<String>> rolesToUsers = rolesToUsers(security);
final Set<String> superAdmins = namesOf(security.adminsConfig(), rolesToUsers);
final boolean noSuperAdminsAreDefined = superAdmins.size() == 0;

goConfigService.groups().accept(new PipelineGroupVisitor() {
@Override
public void visit(PipelineConfigs pipelineConfigs) {
if (!pipelineConfigs.hasAuthorizationDefined()) {
if (noSuperAdminsAreDefined || !pipelineConfigs.hasAuthorizationDefined()) {
pipelinesAndViewers.put(pipelineConfigs.getGroup(), Everyone.INSTANCE);
return;
}
@@ -76,7 +77,7 @@ public void visit(PipelineConfigs pipelineConfigs) {
}

for (AdminRole superAdminRole : adminsConfig.getRoles()) {
superAdminNames.addAll(rolesToUsers.get(superAdminRole.getName().toLower()));
superAdminNames.addAll(emptyIfNull(rolesToUsers.get(superAdminRole.getName().toLower())));
}

return superAdminNames;
@@ -89,4 +90,8 @@ public void visit(PipelineConfigs pipelineConfigs) {
}
return rolesToUsers;
}

private Collection<String> emptyIfNull(Collection<String> collection) {
return collection == null ? Collections.emptyList() : collection;
}
}
@@ -15,13 +15,14 @@
*************************GO-LICENSE-END***********************************/
package com.thoughtworks.go.domain.cctray;

import com.thoughtworks.go.config.Authorization;
import com.thoughtworks.go.config.CruiseConfig;
import com.thoughtworks.go.config.PipelineConfigs;
import com.thoughtworks.go.config.*;
import com.thoughtworks.go.domain.cctray.viewers.AllowedViewers;
import com.thoughtworks.go.domain.cctray.viewers.Viewers;
import com.thoughtworks.go.helper.GoConfigMother;
import com.thoughtworks.go.server.service.GoConfigService;
import com.thoughtworks.go.util.ConfigElementImplementationRegistryMother;
import com.thoughtworks.go.util.XmlUtils;
import com.thoughtworks.studios.shine.io.StringOutputStream;
import org.junit.Before;
import org.junit.Test;

@@ -96,6 +97,7 @@ public void shouldCreateAUniqueSetOfNamesWhenSameUserIsPartOfBothSuperAdminUsers

@Test
public void shouldConsiderPipelineGroupAdminsAsViewersOfTheirPipelineGroup() throws Exception {
configMother.addUserAsSuperAdmin(config, "superadmin1");
configMother.addPipelineWithGroup(config, "group1", "pipeline1", "stage1A", "job1A1", "job1A2");
configMother.addPipelineWithGroup(config, "group2", "pipeline1", "stage1A", "job1A1", "job1A2");
configMother.addUserAsViewerOfPipelineGroup(config, "viewer1", "group2");
@@ -106,12 +108,13 @@ public void shouldConsiderPipelineGroupAdminsAsViewersOfTheirPipelineGroup() thr
Map<String, Viewers> pipelinesAndTheirViewers = getGroupsAndTheirViewers();

assertThat(pipelinesAndTheirViewers.size(), is(2));
assertThat(pipelinesAndTheirViewers.get("group1"), is(viewers("groupadmin1", "groupadmin2")));
assertThat(pipelinesAndTheirViewers.get("group2"), is(viewers("viewer1")));
assertThat(pipelinesAndTheirViewers.get("group1"), is(viewers("superadmin1", "groupadmin1", "groupadmin2")));
assertThat(pipelinesAndTheirViewers.get("group2"), is(viewers("superadmin1", "viewer1")));
}

@Test
public void shouldConsiderAllUsersInPipelineGroupAdminRolesAsViewersOfTheirPipelineGroup() throws Exception {
configMother.addUserAsSuperAdmin(config, "superadmin1");
configMother.addPipelineWithGroup(config, "group1", "pipeline1", "stage1A", "job1A1", "job1A2");
configMother.addPipelineWithGroup(config, "group2", "pipeline1", "stage1A", "job1A1", "job1A2");
configMother.addUserAsViewerOfPipelineGroup(config, "viewer1", "group2");
@@ -124,12 +127,13 @@ public void shouldConsiderAllUsersInPipelineGroupAdminRolesAsViewersOfTheirPipel
Map<String, Viewers> pipelinesAndTheirViewers = getGroupsAndTheirViewers();

assertThat(pipelinesAndTheirViewers.size(), is(2));
assertThat(pipelinesAndTheirViewers.get("group1"), is(viewers("groupadmin1", "groupadmin2", "groupadmin3")));
assertThat(pipelinesAndTheirViewers.get("group2"), is(viewers("viewer1")));
assertThat(pipelinesAndTheirViewers.get("group1"), is(viewers("superadmin1", "groupadmin1", "groupadmin2", "groupadmin3")));
assertThat(pipelinesAndTheirViewers.get("group2"), is(viewers("superadmin1", "viewer1")));
}

@Test
public void shouldCreateAUniqueSetOfNamesWhenSameUserIsPartOfBothGroupAdminUsersAndRolesConfigurations() throws Exception {
configMother.addUserAsSuperAdmin(config, "superadmin1");
configMother.addPipelineWithGroup(config, "group1", "pipeline1", "stage1A", "job1A1", "job1A2");

configMother.addRole(config, configMother.createRole("group1_admin_role1", "groupadmin1", "groupadmin2"));
@@ -139,11 +143,12 @@ public void shouldCreateAUniqueSetOfNamesWhenSameUserIsPartOfBothGroupAdminUsers
Map<String, Viewers> pipelinesAndTheirViewers = getGroupsAndTheirViewers();

assertThat(pipelinesAndTheirViewers.size(), is(1));
assertThat(pipelinesAndTheirViewers.get("group1"), is(viewers("groupadmin1", "groupadmin2")));
assertThat(pipelinesAndTheirViewers.get("group1"), is(viewers("superadmin1", "groupadmin1", "groupadmin2")));
}

@Test
public void shouldConsiderUsersWithViewPermissionsAsViewersOfTheirPipelineGroup() throws Exception {
configMother.addUserAsSuperAdmin(config, "superadmin1");
configMother.addPipelineWithGroup(config, "group1", "pipeline1", "stage1A", "job1A1", "job1A2");
configMother.addPipelineWithGroup(config, "group2", "pipeline1", "stage1A", "job1A1", "job1A2");

@@ -154,12 +159,13 @@ public void shouldConsiderUsersWithViewPermissionsAsViewersOfTheirPipelineGroup(
Map<String, Viewers> pipelinesAndTheirViewers = getGroupsAndTheirViewers();

assertThat(pipelinesAndTheirViewers.size(), is(2));
assertThat(pipelinesAndTheirViewers.get("group1"), is(viewers("viewer1", "viewer2")));
assertThat(pipelinesAndTheirViewers.get("group2"), is(viewers("viewer3")));
assertThat(pipelinesAndTheirViewers.get("group1"), is(viewers("superadmin1", "viewer1", "viewer2")));
assertThat(pipelinesAndTheirViewers.get("group2"), is(viewers("superadmin1", "viewer3")));
}

@Test
public void shouldConsiderUsersOfRolesWithViewPermissionsAsViewersOfTheirPipelineGroup() throws Exception {
configMother.addUserAsSuperAdmin(config, "superadmin1");
configMother.addPipelineWithGroup(config, "group1", "pipeline1", "stage1A", "job1A1", "job1A2");
configMother.addPipelineWithGroup(config, "group2", "pipeline1", "stage1A", "job1A1", "job1A2");
configMother.addUserAsViewerOfPipelineGroup(config, "viewer1", "group2");
@@ -172,12 +178,13 @@ public void shouldConsiderUsersOfRolesWithViewPermissionsAsViewersOfTheirPipelin
Map<String, Viewers> pipelinesAndTheirViewers = getGroupsAndTheirViewers();

assertThat(pipelinesAndTheirViewers.size(), is(2));
assertThat(pipelinesAndTheirViewers.get("group1"), is(viewers("groupviewer1", "groupviewer2", "groupviewer3")));
assertThat(pipelinesAndTheirViewers.get("group2"), is(viewers("viewer1")));
assertThat(pipelinesAndTheirViewers.get("group1"), is(viewers("superadmin1", "groupviewer1", "groupviewer2", "groupviewer3")));
assertThat(pipelinesAndTheirViewers.get("group2"), is(viewers("superadmin1", "viewer1")));
}

@Test
public void shouldCreateAUniqueSetOfNamesWhenSameUserIsPartOfBothViewUsersAndViewRolesConfigurations() throws Exception {
configMother.addUserAsSuperAdmin(config, "superadmin1");
configMother.addPipelineWithGroup(config, "group1", "pipeline1", "stage1A", "job1A1", "job1A2");

configMother.addRole(config, configMother.createRole("group1_view_role1", "viewer1", "groupviewer2"));
@@ -187,7 +194,7 @@ public void shouldCreateAUniqueSetOfNamesWhenSameUserIsPartOfBothViewUsersAndVie
Map<String, Viewers> pipelinesAndTheirViewers = getGroupsAndTheirViewers();

assertThat(pipelinesAndTheirViewers.size(), is(1));
assertThat(pipelinesAndTheirViewers.get("group1"), is(viewers("viewer1", "groupviewer2")));
assertThat(pipelinesAndTheirViewers.get("group1"), is(viewers("superadmin1", "viewer1", "groupviewer2")));
}

@Test
@@ -269,7 +276,8 @@ public void shouldConsiderAllUsersAsViewersOfAGroupWithNoAuthorizationConfigurat
}

@Test
public void shouldNotConsiderAllUsersAsViewersOfAGroup_WhenExplicitGroupAdminIsSetup() throws Exception {
public void shouldConsiderAllUsersAsViewersOfAGroup_EvenIfExplicitGroupAdminIsSetup_AND_NoGlobalSuperAdminsExist() throws Exception {
/* No superuser */
configMother.addPipelineWithGroup(config, "group1", "pipeline1", "stage1A", "job1A1", "job1A2");
configMother.addAdminUserForPipelineGroup(config, "groupadmin1", "group1");

@@ -279,8 +287,8 @@ public void shouldNotConsiderAllUsersAsViewersOfAGroup_WhenExplicitGroupAdminIsS
Viewers viewersOfGroup1 = getGroupsAndTheirViewers().get("group1");

assertThat(viewersOfGroup1.contains("groupadmin1"), is(true));
assertThat(viewersOfGroup1.contains("some-user"), is(false));
assertThat(viewersOfGroup1.contains("some-other-user"), is(false));
assertThat(viewersOfGroup1.contains("some-user"), is(true));
assertThat(viewersOfGroup1.contains("some-other-user"), is(true));
}

@Test

0 comments on commit e89fdb6

Please sign in to comment.
You can’t perform that action at this time.