From: Jean-Baptiste Lievremont Date: Thu, 30 Apr 2015 09:15:17 +0000 (+0200) Subject: SONAR-6427 SONAR-6428 Add permission checks on plugin pages X-Git-Tag: 5.2-RC1~2050 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=3445f73e4cb2f908a512354468367a2b1311f27f;p=sonarqube.git SONAR-6427 SONAR-6428 Add permission checks on plugin pages --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/ui/ViewProxy.java b/server/sonar-server/src/main/java/org/sonar/server/ui/ViewProxy.java index a36b8792e41..de4cc75d909 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ui/ViewProxy.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ui/ViewProxy.java @@ -43,6 +43,8 @@ import org.sonar.api.web.WidgetLayoutType; import org.sonar.api.web.WidgetProperties; import org.sonar.api.web.WidgetProperty; import org.sonar.api.web.WidgetScope; +import org.sonar.core.component.ComponentDto; +import org.sonar.server.user.UserSession; import java.util.Collection; import java.util.Map; @@ -272,6 +274,24 @@ public class ViewProxy implements Comparable { } } + public boolean isUserAuthorized() { + boolean authorized = userRoles.length == 0; + for (String userRole : getUserRoles()) { + authorized |= UserSession.get().hasGlobalPermission(userRole); + } + return authorized; + } + + public boolean isUserAuthorized(ComponentDto component) { + boolean authorized = userRoles.length == 0; + for (String userRole : getUserRoles()) { + authorized |= (UserRole.VIEWER.equals(userRole) + || UserRole.USER.equals(userRole) + || UserSession.get().hasProjectPermissionByUuid(userRole, component.uuid())); + } + return authorized; + } + public boolean isWidget() { return isWidget; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/ui/ws/ComponentNavigationAction.java b/server/sonar-server/src/main/java/org/sonar/server/ui/ws/ComponentNavigationAction.java index f9b272ec9f4..48189880e0a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ui/ws/ComponentNavigationAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ui/ws/ComponentNavigationAction.java @@ -165,7 +165,9 @@ public class ComponentNavigationAction implements NavigationAction { private void writeExtensions(JsonWriter json, ComponentDto component, List> pages, Locale locale) { json.name("extensions").beginArray(); for (ViewProxy page: pages) { - writePage(json, getPageUrl(page, component), i18n.message(locale, page.getId() + ".page", page.getTitle())); + if (page.isUserAuthorized(component)) { + writePage(json, getPageUrl(page, component), i18n.message(locale, page.getId() + ".page", page.getTitle())); + } } json.endArray(); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/ui/ws/GlobalNavigationAction.java b/server/sonar-server/src/main/java/org/sonar/server/ui/ws/GlobalNavigationAction.java index 3647510ccb0..45907e1538c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ui/ws/GlobalNavigationAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ui/ws/GlobalNavigationAction.java @@ -93,10 +93,12 @@ public class GlobalNavigationAction implements NavigationAction { private void writePages(JsonWriter json) { json.name("globalPages").beginArray(); for (ViewProxy page : views.getPages(NavigationSection.HOME)) { - json.beginObject() - .prop("name", page.getTitle()) - .prop("url", page.isController() ? page.getId() : String.format("/plugins/home/%s", page.getId())) - .endObject(); + if (page.isUserAuthorized()) { + json.beginObject() + .prop("name", page.getTitle()) + .prop("url", page.isController() ? page.getId() : String.format("/plugins/home/%s", page.getId())) + .endObject(); + } } json.endArray(); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/ui/ViewProxyTest.java b/server/sonar-server/src/test/java/org/sonar/server/ui/ViewProxyTest.java index b1f9351f3c2..ab93684bfc0 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ui/ViewProxyTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ui/ViewProxyTest.java @@ -33,16 +33,13 @@ import org.sonar.api.web.WidgetProperties; import org.sonar.api.web.WidgetProperty; import org.sonar.api.web.WidgetPropertyType; import org.sonar.api.web.WidgetScope; +import org.sonar.server.user.MockUserSession; import java.util.List; import static com.google.common.collect.Iterables.getOnlyElement; import static org.assertj.core.api.Assertions.assertThat; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.lessThan; -import static org.hamcrest.Matchers.greaterThan; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertThat; +import static org.sonar.server.component.ComponentTesting.newProjectDto; public class ViewProxyTest { @@ -51,9 +48,9 @@ public class ViewProxyTest { @Test public void compareTo() { - assertThat(new ViewProxy(new FakeView("aaa")).compareTo(new ViewProxy(new FakeView("bbb"))), lessThan(0)); - assertThat(new ViewProxy(new FakeView("aaa")).compareTo(new ViewProxy(new FakeView("aaa"))), is(0)); - assertThat(new ViewProxy(new FakeView("bbb")).compareTo(new ViewProxy(new FakeView("aaa"))), greaterThan(0)); + assertThat(new ViewProxy(new FakeView("aaa")).compareTo(new ViewProxy(new FakeView("bbb")))).isLessThan(0); + assertThat(new ViewProxy(new FakeView("aaa")).compareTo(new ViewProxy(new FakeView("aaa")))).isZero(); + assertThat(new ViewProxy(new FakeView("bbb")).compareTo(new ViewProxy(new FakeView("aaa")))).isGreaterThan(0); } @Test @@ -70,9 +67,9 @@ public class ViewProxyTest { View view = new MyView(); ViewProxy proxy = new ViewProxy(view); - assertThat(proxy.getTarget(), is(view)); - assertArrayEquals(proxy.getSections(), new String[]{NavigationSection.RESOURCE}); - assertArrayEquals(proxy.getUserRoles(), new String[]{UserRole.ADMIN}); + assertThat(proxy.getTarget()).isEqualTo(view); + assertThat(proxy.getSections()).isEqualTo(new String[] {NavigationSection.RESOURCE}); + assertThat(proxy.getUserRoles()).isEqualTo(new String[] {UserRole.ADMIN}); } @Test @@ -86,9 +83,9 @@ public class ViewProxyTest { View view = new MyView(); ViewProxy proxy = new ViewProxy(view); - assertThat(proxy.getTarget(), is(view)); - assertArrayEquals(proxy.getSections(), new String[]{NavigationSection.HOME}); - assertThat(proxy.getUserRoles().length, org.hamcrest.Matchers.is(0)); + assertThat(proxy.getTarget()).isEqualTo(view); + assertThat(proxy.getSections()).isEqualTo(new String[] {NavigationSection.HOME}); + assertThat(proxy.getUserRoles()).isEmpty(); } @Test @@ -102,8 +99,8 @@ public class ViewProxyTest { ViewProxy proxy = new ViewProxy(new MyView()); - assertThat(proxy.isDefaultTab(), is(true)); - assertThat(proxy.getDefaultTabForMetrics().length, is(0)); + assertThat(proxy.isDefaultTab()).isTrue(); + assertThat(proxy.getDefaultTabForMetrics()).isEmpty(); } @Test @@ -115,8 +112,8 @@ public class ViewProxyTest { } ViewProxy proxy = new ViewProxy(new MyView()); - assertThat(proxy.isDefaultTab(), is(false)); - assertThat(proxy.getDefaultTabForMetrics().length, is(0)); + assertThat(proxy.isDefaultTab()).isFalse(); + assertThat(proxy.getDefaultTabForMetrics()).isEmpty(); } @Test @@ -129,8 +126,8 @@ public class ViewProxyTest { } ViewProxy proxy = new ViewProxy(new MyView()); - assertThat(proxy.isDefaultTab(), is(false)); - assertThat(proxy.getDefaultTabForMetrics(), is(new String[]{"ncloc", "coverage"})); + assertThat(proxy.isDefaultTab()).isFalse(); + assertThat(proxy.getDefaultTabForMetrics()).isEqualTo(new String[] {"ncloc", "coverage"}); } @Test @@ -185,13 +182,13 @@ public class ViewProxyTest { @Test public void widgetShouldRequireMandatoryProperties() { ViewProxy proxy = new ViewProxy(new EditableWidget()); - assertThat(proxy.hasRequiredProperties(), is(true)); + assertThat(proxy.hasRequiredProperties()).isTrue(); } @Test public void widgetShouldDefineOnlyOptionalProperties() { ViewProxy proxy = new ViewProxy(new WidgetWithOptionalProperties()); - assertThat(proxy.hasRequiredProperties(), is(false)); + assertThat(proxy.hasRequiredProperties()).isFalse(); } @Test @@ -203,7 +200,7 @@ public class ViewProxyTest { } ViewProxy proxy = new ViewProxy(new MyView()); - assertThat(proxy.acceptsAvailableMeasures(new String[]{"lines", "ncloc", "coverage"}), is(true)); + assertThat(proxy.acceptsAvailableMeasures(new String[] {"lines", "ncloc", "coverage"})).isTrue(); } @Test @@ -216,8 +213,8 @@ public class ViewProxyTest { } ViewProxy proxy = new ViewProxy(new MyView()); - assertThat(proxy.acceptsAvailableMeasures(new String[]{"lines", "ncloc", "coverage"}), is(true)); - assertThat(proxy.acceptsAvailableMeasures(new String[]{"lines", "coverage"}), is(false)); + assertThat(proxy.acceptsAvailableMeasures(new String[] {"lines", "ncloc", "coverage"})).isTrue(); + assertThat(proxy.acceptsAvailableMeasures(new String[] {"lines", "coverage"})).isFalse(); } @Test @@ -230,8 +227,8 @@ public class ViewProxyTest { } ViewProxy proxy = new ViewProxy(new MyView()); - assertThat(proxy.acceptsAvailableMeasures(new String[]{"lines", "coverage"}), is(true)); - assertThat(proxy.acceptsAvailableMeasures(new String[]{"complexity", "coverage"}), is(false)); + assertThat(proxy.acceptsAvailableMeasures(new String[] {"lines", "coverage"})).isTrue(); + assertThat(proxy.acceptsAvailableMeasures(new String[] {"complexity", "coverage"})).isFalse(); } @Test @@ -245,13 +242,146 @@ public class ViewProxyTest { ViewProxy proxy = new ViewProxy(new MyView()); // ok, mandatory measures and 1 needed measure - assertThat(proxy.acceptsAvailableMeasures(new String[]{"lines", "ncloc", "coverage", "duplications"}), is(true)); + assertThat(proxy.acceptsAvailableMeasures(new String[] {"lines", "ncloc", "coverage", "duplications"})).isTrue(); // ko, one of the needed measures but not all of the mandatory ones - assertThat(proxy.acceptsAvailableMeasures(new String[]{"lines", "coverage", "duplications"}), is(false)); + assertThat(proxy.acceptsAvailableMeasures(new String[] {"lines", "coverage", "duplications"})).isFalse(); // ko, mandatory measures but no one of the needed measures - assertThat(proxy.acceptsAvailableMeasures(new String[]{"lines", "nloc", "coverage", "dsm"}), is(false)); + assertThat(proxy.acceptsAvailableMeasures(new String[] {"lines", "nloc", "coverage", "dsm"})).isFalse(); } + @Test + public void is_authorized_by_default() { + + @NavigationSection(NavigationSection.HOME) + class MyView extends FakeView { + MyView() { + super("fake"); + } + } + + ViewProxy proxy = new ViewProxy(new MyView()); + + MockUserSession.set(); + assertThat(proxy.isUserAuthorized()).isTrue(); + } + + @Test + public void is_authorized_on_any_permission() { + + @NavigationSection(NavigationSection.HOME) + @UserRole({"polop", "palap"}) + class MyView extends FakeView { + MyView() { + super("fake"); + } + } + + ViewProxy proxy = new ViewProxy(new MyView()); + + MockUserSession.set().setGlobalPermissions("palap"); + assertThat(proxy.isUserAuthorized()).isTrue(); + } + + @Test + public void is_not_authorized() { + + @NavigationSection(NavigationSection.HOME) + @UserRole({"polop", "palap"}) + class MyView extends FakeView { + MyView() { + super("fake"); + } + } + + ViewProxy proxy = new ViewProxy(new MyView()); + + MockUserSession.set().setGlobalPermissions("pilip"); + assertThat(proxy.isUserAuthorized()).isFalse(); + } + + @Test + public void is_authorized_by_default_on_component() { + + @NavigationSection(NavigationSection.RESOURCE) + class MyView extends FakeView { + MyView() { + super("fake"); + } + } + + ViewProxy proxy = new ViewProxy(new MyView()); + + MockUserSession.set(); + assertThat(proxy.isUserAuthorized(newProjectDto("abcd"))).isTrue(); + } + + @Test + public void is_authorized_on_any_permission_on_component() { + + @NavigationSection(NavigationSection.RESOURCE) + @UserRole({"polop", "palap"}) + class MyView extends FakeView { + MyView() { + super("fake"); + } + } + + ViewProxy proxy = new ViewProxy(new MyView()); + + MockUserSession.set().addProjectUuidPermissions("palap", "abcd"); + assertThat(proxy.isUserAuthorized(newProjectDto("abcd"))).isTrue(); + } + + @Test + public void is_not_authorized_on_component() { + + @NavigationSection(NavigationSection.RESOURCE) + @UserRole({"polop", "palap"}) + class MyView extends FakeView { + MyView() { + super("fake"); + } + } + + ViewProxy proxy = new ViewProxy(new MyView()); + + MockUserSession.set().addProjectUuidPermissions("pilip", "abcd"); + assertThat(proxy.isUserAuthorized(newProjectDto("abcd"))).isFalse(); + } + + @Test + public void is_authorized_on_component_viewer_bypass() { + + @NavigationSection(NavigationSection.RESOURCE) + @UserRole(UserRole.VIEWER) + class MyView extends FakeView { + MyView() { + super("fake"); + } + } + + ViewProxy proxy = new ViewProxy(new MyView()); + + MockUserSession.set(); + assertThat(proxy.isUserAuthorized(newProjectDto("abcd"))).isTrue(); + } + + @Test + public void is_authorized_on_component_user_bypass() { + + @NavigationSection(NavigationSection.RESOURCE) + @UserRole(UserRole.USER) + class MyView extends FakeView { + MyView() { + super("fake"); + } + } + + ViewProxy proxy = new ViewProxy(new MyView()); + + MockUserSession.set(); + assertThat(proxy.isUserAuthorized(newProjectDto("abcd"))).isTrue(); + } } class FakeView implements View { diff --git a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentNavigationActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentNavigationActionTest.java index caa29df721d..53dcad171ca 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentNavigationActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentNavigationActionTest.java @@ -230,19 +230,47 @@ public class ComponentNavigationActionTest { @Test public void with_extensions() throws Exception { - final String language = "xoo"; ComponentDto project = dbClient.componentDao().insert(session, ComponentTesting.newProjectDto("abcd") - .setKey("polop").setName("Polop").setLanguage(language)); + .setKey("polop").setName("Polop").setLanguage("xoo")); dbClient.snapshotDao().insert(session, new SnapshotDto() .setLast(true).setQualifier(project.qualifier()).setResourceId(project.getId()).setRootProjectId(project.getId()).setScope(project.scope())); session.commit(); MockUserSession.set().addProjectUuidPermissions(UserRole.USER, "abcd"); + Views views = createViews(); + + wsTester = new WsTester(new NavigationWs(new ComponentNavigationAction(dbClient, activeDashboardDao, + views, i18n, resourceTypes))); + + wsTester.newGetRequest("api/navigation", "component").setParam("componentKey", "polop").execute().assertJson(getClass(), "with_extensions.json"); + } + + @Test + public void admin_with_extensions() throws Exception { + ComponentDto project = dbClient.componentDao().insert(session, ComponentTesting.newProjectDto("abcd") + .setKey("polop").setName("Polop").setLanguage("xoo")); + dbClient.snapshotDao().insert(session, new SnapshotDto() + .setLast(true).setQualifier(project.qualifier()).setResourceId(project.getId()).setRootProjectId(project.getId()).setScope(project.scope())); + session.commit(); + + MockUserSession.set() + .addProjectUuidPermissions(UserRole.USER, "abcd") + .addProjectUuidPermissions(UserRole.ADMIN, "abcd"); + + Views views = createViews(); + + wsTester = new WsTester(new NavigationWs(new ComponentNavigationAction(dbClient, activeDashboardDao, + views, i18n, resourceTypes))); + + wsTester.newGetRequest("api/navigation", "component").setParam("componentKey", "polop").execute().assertJson(getClass(), "admin_with_extensions.json"); + } + + private Views createViews() { @NavigationSection(NavigationSection.RESOURCE) @ResourceScope(Scopes.PROJECT) @ResourceQualifier(Qualifiers.PROJECT) - @ResourceLanguage(language) + @ResourceLanguage("xoo") class FirstPage implements Page { @Override public String getTitle() { @@ -259,7 +287,7 @@ public class ComponentNavigationActionTest { @NavigationSection(NavigationSection.RESOURCE) @ResourceScope(Scopes.PROJECT) @ResourceQualifier(Qualifiers.PROJECT) - @ResourceLanguage(language) + @ResourceLanguage("xoo") class SecondPage implements Page { @Override public String getTitle() { @@ -273,10 +301,25 @@ public class ComponentNavigationActionTest { } Page page2 = new SecondPage(); - wsTester = new WsTester(new NavigationWs(new ComponentNavigationAction(dbClient, activeDashboardDao, - new Views(new Page[] {page1, page2}), i18n, resourceTypes))); + @NavigationSection(NavigationSection.RESOURCE) + @ResourceScope(Scopes.PROJECT) + @ResourceQualifier(Qualifiers.PROJECT) + @ResourceLanguage("xoo") + @UserRole(UserRole.ADMIN) + class AdminPage implements Page { + @Override + public String getTitle() { + return "Admin Page"; + } - wsTester.newGetRequest("api/navigation", "component").setParam("componentKey", "polop").execute().assertJson(getClass(), "with_extensions.json"); + @Override + public String getId() { + return "/admin/page"; + } + } + Page adminPage = new AdminPage(); + Views views = new Views(new Page[] {page1, page2, adminPage}); + return views; } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/GlobalNavigationActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/GlobalNavigationActionTest.java index b7ae528083c..18b74d183c4 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/GlobalNavigationActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/GlobalNavigationActionTest.java @@ -28,12 +28,15 @@ import org.sonar.api.resources.ResourceType; import org.sonar.api.resources.ResourceTypeTree; import org.sonar.api.resources.ResourceTypes; import org.sonar.api.utils.System2; +import org.sonar.api.web.NavigationSection; import org.sonar.api.web.Page; +import org.sonar.api.web.UserRole; import org.sonar.api.web.View; import org.sonar.core.dashboard.ActiveDashboardDao; import org.sonar.core.dashboard.ActiveDashboardDto; import org.sonar.core.dashboard.DashboardDao; import org.sonar.core.dashboard.DashboardDto; +import org.sonar.core.permission.GlobalPermissions; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.DbTester; import org.sonar.core.user.UserDto; @@ -140,6 +143,15 @@ public class GlobalNavigationActionTest { wsTester.newGetRequest("api/navigation", "global").execute().assertJson(getClass(), "user.json"); } + @Test + public void nominal_call_for_admin() throws Exception { + nominalSetup(); + + MockUserSession.set().setLogin("obiwan").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); + + wsTester.newGetRequest("api/navigation", "global").execute().assertJson(getClass(), "admin.json"); + } + @Test public void nominal_call_for_user_without_configured_dashboards() throws Exception { nominalSetup(); @@ -208,6 +220,7 @@ public class GlobalNavigationActionTest { return "my_plugin_page"; } }; + Page controller = new Page() { @Override public String getTitle() { @@ -218,6 +231,20 @@ public class GlobalNavigationActionTest { return "/my_rails_app"; } }; - return new Views(new View[] {page, controller}); + + @NavigationSection(NavigationSection.HOME) + @UserRole(GlobalPermissions.SYSTEM_ADMIN) + class AdminPage implements Page { + @Override + public String getTitle() { + return "Admin Page"; + } + + @Override + public String getId() { + return "admin_page"; + } + } + return new Views(new View[] {page, controller, new AdminPage()}); } } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/ui/ws/ComponentNavigationActionTest/admin_with_extensions.json b/server/sonar-server/src/test/resources/org/sonar/server/ui/ws/ComponentNavigationActionTest/admin_with_extensions.json new file mode 100644 index 00000000000..cddceb00943 --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/ui/ws/ComponentNavigationActionTest/admin_with_extensions.json @@ -0,0 +1,30 @@ +{ + "key": "polop", + "uuid": "abcd", + "name": "Polop", + "isComparable": false, + "canBeFavorite": false, + "isFavorite": false, + "dashboards": [], + "extensions": [ + { + "name": "First Page", + "url": "/plugins/resource/polop?page=first_page" + }, + { + "name": "Second Page", + "url": "/second/page?id=polop" + }, + { + "name": "Admin Page", + "url": "/admin/page?id=polop" + } + ], + "breadcrumbs": [ + { + "key": "polop", + "name": "Polop", + "qualifier": "TRK" + } + ] +} diff --git a/server/sonar-server/src/test/resources/org/sonar/server/ui/ws/GlobalNavigationActionTest/admin.json b/server/sonar-server/src/test/resources/org/sonar/server/ui/ws/GlobalNavigationActionTest/admin.json new file mode 100644 index 00000000000..abd48e7adac --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/ui/ws/GlobalNavigationActionTest/admin.json @@ -0,0 +1,23 @@ +{ + "globalDashboards": [ + { + "name": "Default Dashboard for User" + } + ], + "globalPages": [ + { + "name": "My Plugin Page", + "url": "/plugins/home/my_plugin_page" + }, + { + "name": "My Rails App", + "url": "/my_rails_app" + }, + { + "name": "Admin Page", + "url": "/plugins/home/admin_page" + } + ], + "logoUrl": "http://some-server.tld/logo.png", + "logoWidth": "123" +}