details: https://code.openbravo.com/erp/devel/pi/rev/5438c3739fa2 changeset: 29364:5438c3739fa2 user: Asier Lostalé <asier.lostale <at> openbravo.com> date: Fri Mar 04 14:59:30 2016 +0100 summary: fixes 32398: Organization role can't see data in System/Client tables
A new preference has been created in order to bypass check that compares role's user level with entity's access level to completely prevent accessing latter one. details: https://code.openbravo.com/erp/devel/pi/rev/e47bc61fb8bd changeset: 29365:e47bc61fb8bd user: Asier Lostalé <asier.lostale <at> openbravo.com> date: Thu Apr 28 14:03:21 2016 +0200 summary: related to issue 32398: fixes test case diffstat: src-db/database/sourcedata/AD_REF_LIST.xml | 12 + src-test/src/org/openbravo/test/AllAntTaskTests.java | 2 + src-test/src/org/openbravo/test/security/BypassAccessLevelCheck.java | 93 ++++++++++ src-test/src/org/openbravo/test/security/CrossOrganizationReference.java | 1 + src-test/src/org/openbravo/test/security/ExplicitCrossOrganizationReference.java | 42 ++- src/org/openbravo/base/secureApp/HttpSecureAppServlet.java | 4 + src/org/openbravo/base/secureApp/LoginUtils.java | 13 + src/org/openbravo/dal/core/OBContext.java | 15 + src/org/openbravo/dal/security/EntityAccessChecker.java | 4 + 9 files changed, 169 insertions(+), 17 deletions(-) diffs (truncated from 312 to 300 lines): diff -r e8c0df5664de -r e47bc61fb8bd src-db/database/sourcedata/AD_REF_LIST.xml --- a/src-db/database/sourcedata/AD_REF_LIST.xml Thu Apr 28 13:52:52 2016 +0200 +++ b/src-db/database/sourcedata/AD_REF_LIST.xml Thu Apr 28 14:03:21 2016 +0200 @@ -12383,6 +12383,18 @@ <!--D9FFDD5DD51D434FB45628FCBAD10A4E--> <AD_MODULE_ID><![CDATA[0]]></AD_MODULE_ID> <!--D9FFDD5DD51D434FB45628FCBAD10A4E--></AD_REF_LIST> +<!--DA98D956E0BF4DCA887932756C327D73--><AD_REF_LIST> +<!--DA98D956E0BF4DCA887932756C327D73--> <AD_REF_LIST_ID><![CDATA[DA98D956E0BF4DCA887932756C327D73]]></AD_REF_LIST_ID> +<!--DA98D956E0BF4DCA887932756C327D73--> <AD_CLIENT_ID><![CDATA[0]]></AD_CLIENT_ID> +<!--DA98D956E0BF4DCA887932756C327D73--> <AD_ORG_ID><![CDATA[0]]></AD_ORG_ID> +<!--DA98D956E0BF4DCA887932756C327D73--> <ISACTIVE><![CDATA[Y]]></ISACTIVE> +<!--DA98D956E0BF4DCA887932756C327D73--> <VALUE><![CDATA[BypassAccessLevelEntityCheck]]></VALUE> +<!--DA98D956E0BF4DCA887932756C327D73--> <NAME><![CDATA[Bypass Access Level Entity Check]]></NAME> +<!--DA98D956E0BF4DCA887932756C327D73--> <DESCRIPTION><![CDATA[When its value is Y, entity access level check will not be performed.]]></DESCRIPTION> +<!--DA98D956E0BF4DCA887932756C327D73--> <AD_REFERENCE_ID><![CDATA[A26BA480E2014707B47257024C3CBFF7]]></AD_REFERENCE_ID> +<!--DA98D956E0BF4DCA887932756C327D73--> <AD_MODULE_ID><![CDATA[0]]></AD_MODULE_ID> +<!--DA98D956E0BF4DCA887932756C327D73--></AD_REF_LIST> + <!--DAC02370F50D485BAA4F79957AA1A7A4--><AD_REF_LIST> <!--DAC02370F50D485BAA4F79957AA1A7A4--> <AD_REF_LIST_ID><![CDATA[DAC02370F50D485BAA4F79957AA1A7A4]]></AD_REF_LIST_ID> <!--DAC02370F50D485BAA4F79957AA1A7A4--> <AD_CLIENT_ID><![CDATA[0]]></AD_CLIENT_ID> diff -r e8c0df5664de -r e47bc61fb8bd src-test/src/org/openbravo/test/AllAntTaskTests.java --- a/src-test/src/org/openbravo/test/AllAntTaskTests.java Thu Apr 28 13:52:52 2016 +0200 +++ b/src-test/src/org/openbravo/test/AllAntTaskTests.java Thu Apr 28 14:03:21 2016 +0200 @@ -77,6 +77,7 @@ import org.openbravo.test.scheduling.ProcessSchedulingTest; import org.openbravo.test.security.AccessLevelTest; import org.openbravo.test.security.AllowedOrganizationsTest; +import org.openbravo.test.security.BypassAccessLevelCheck; import org.openbravo.test.security.EntityAccessTest; import org.openbravo.test.security.StandardCrossOrganizationReference; import org.openbravo.test.security.WritableReadableOrganizationClientTest; @@ -159,6 +160,7 @@ EntityAccessTest.class, // WritableReadableOrganizationClientTest.class, // StandardCrossOrganizationReference.class, // + BypassAccessLevelCheck.class, // // system SystemServiceTest.class, // diff -r e8c0df5664de -r e47bc61fb8bd src-test/src/org/openbravo/test/security/BypassAccessLevelCheck.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src-test/src/org/openbravo/test/security/BypassAccessLevelCheck.java Thu Apr 28 14:03:21 2016 +0200 @@ -0,0 +1,93 @@ +/* + ************************************************************************* + * The contents of this file are subject to the Openbravo Public License + * Version 1.1 (the "License"), being the Mozilla Public License + * Version 1.1 with a permitted attribution clause; you may not use this + * file except in compliance with the License. You may obtain a copy of + * the License at http://www.openbravo.com/legal/license.html + * Software distributed under the License is distributed on an "AS IS" + * basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the + * License for the specific language governing rights and limitations + * under the License. + * The Original Code is Openbravo ERP. + * The Initial Developer of the Original Code is Openbravo SLU + * All portions are Copyright (C) 2016 Openbravo SLU + * All Rights Reserved. + * Contributor(s): ______________________________________. + ************************************************************************ + */ + +package org.openbravo.test.security; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.greaterThan; +import static org.junit.Assert.assertThat; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.openbravo.base.exception.OBSecurityException; +import org.openbravo.dal.core.OBContext; +import org.openbravo.dal.service.OBCriteria; +import org.openbravo.dal.service.OBDal; +import org.openbravo.model.ad.access.Role; +import org.openbravo.model.common.currency.Currency; +import org.openbravo.test.base.OBBaseTest; + +/** + * By default access level in entity and role is checked preventing reading data if role's user + * level is Organization and the entity trying to be accessed is Client or System. These checks can + * be bypassed. + * + * @author alostale + * + */ +public class BypassAccessLevelCheck extends OBBaseTest { + private static String ORG_LEVEL_ROLE; + private static final String CURRENCY_WINDOW = "115"; + private static final String SPAIN_ORG = "357947E87C284935AD1D783CF6F099A1"; + + @Rule + public ExpectedException exception = ExpectedException.none(); + + @BeforeClass + public static void createOrgLevelRole() { + Role role = ExplicitCrossOrganizationReference.createOrgUserLevelRole(); + ORG_LEVEL_ROLE = role.getId(); + ExplicitCrossOrganizationReference.grantWindowAccess(role, CURRENCY_WINDOW); + + OBDal.getInstance().commitAndClose(); + } + + /** By default Org level roles cannot see data in System level entities */ + @Test + public void orgLevelShouldntGrantAccessToSystemEntity() { + OBContext.setOBContext("100", ORG_LEVEL_ROLE, QA_TEST_CLIENT_ID, SPAIN_ORG); + + assertThat("doOrgClientAccessCheck", OBContext.getOBContext().doOrgClientAccessCheck(), + is(true)); + + exception.expect(OBSecurityException.class); + exception.expectMessage(containsString("Entity Currency is not readable")); + + OBDal.getInstance().createCriteria(Currency.class); + } + + /** Default behavior of for access level check can be bypassed */ + @Test + public void orgLevelCanAccessEntityAccessIfEnabled() { + OBContext.setOBContext("100", ORG_LEVEL_ROLE, QA_TEST_CLIENT_ID, SPAIN_ORG); + OBContext.getOBContext().setCheckAccessLevel(false); + OBCriteria<Currency> q = OBDal.getInstance().createCriteria(Currency.class); + + assertThat("Visible currencies", q.count(), is(greaterThan(0))); + } + + @AfterClass + public static void cleanUpCreatedObjects() { + CrossOrganizationReference.removeCreatedObjects(); + } +} diff -r e8c0df5664de -r e47bc61fb8bd src-test/src/org/openbravo/test/security/CrossOrganizationReference.java --- a/src-test/src/org/openbravo/test/security/CrossOrganizationReference.java Thu Apr 28 13:52:52 2016 +0200 +++ b/src-test/src/org/openbravo/test/security/CrossOrganizationReference.java Thu Apr 28 14:03:21 2016 +0200 @@ -182,6 +182,7 @@ OBDal.getInstance().flush(); } OBDal.getInstance().commitAndClose(); + createdObjects.clear(); } catch (Exception ignore) { } finally { OBContext.restorePreviousMode(); diff -r e8c0df5664de -r e47bc61fb8bd src-test/src/org/openbravo/test/security/ExplicitCrossOrganizationReference.java --- a/src-test/src/org/openbravo/test/security/ExplicitCrossOrganizationReference.java Thu Apr 28 13:52:52 2016 +0200 +++ b/src-test/src/org/openbravo/test/security/ExplicitCrossOrganizationReference.java Thu Apr 28 14:03:21 2016 +0200 @@ -64,6 +64,7 @@ * */ public class ExplicitCrossOrganizationReference extends CrossOrganizationReference { + private static final String SALES_ORDER_WINDOW = "143"; private static String QA_ONLY_SPAIN_ROLE; private static final String CORE = "0"; private static final String ORDER_WAREHOUSE_COLUMN = "2202"; @@ -489,7 +490,15 @@ DalLayerInitializer.getInstance().setInitialized(false); setDalUp(); - // create a role with access only to Spain org + Role qaRole = createOrgUserLevelRole(); + QA_ONLY_SPAIN_ROLE = qaRole.getId(); + grantWindowAccess(qaRole, SALES_ORDER_WINDOW); + + OBDal.getInstance().commitAndClose(); + } + + /** Creates a role with Org user level, and access to Spain Org */ + static Role createOrgUserLevelRole() { setQAAdminRole(); Role spainRole = OBProvider.getInstance().get(Role.class); spainRole.setName("QA Only Spain - " + System.currentTimeMillis()); // some randomness @@ -500,21 +509,6 @@ spainRole.setUserLevel(" O"); OBDal.getInstance().save(spainRole); createdObjects.add(spainRole); - QA_ONLY_SPAIN_ROLE = spainRole.getId(); - - // create a window access for previous role - Role role = OBDal.getInstance().get(Role.class, QA_ONLY_SPAIN_ROLE); - WindowAccess windowAccess = OBProvider.getInstance().get(WindowAccess.class); - final OBCriteria<Window> obCriteria = OBDal.getInstance().createCriteria(Window.class); - obCriteria.add(Restrictions.eq(Window.PROPERTY_ID, "143")); - obCriteria.setMaxResults(1); - windowAccess.setClient(role.getClient()); - windowAccess.setOrganization(role.getOrganization()); - windowAccess.setRole(role); - windowAccess.setWindow((Window) obCriteria.uniqueResult()); - windowAccess.setEditableField(true); - OBDal.getInstance().save(windowAccess); - createdObjects.add(windowAccess); RoleOrganization orgAccess = OBProvider.getInstance().get(RoleOrganization.class); orgAccess.setOrganization(OBDal.getInstance().getProxy(Organization.class, SPAIN_ORG)); @@ -529,7 +523,21 @@ OBDal.getInstance().save(userRole); createdObjects.add(userRole); - OBDal.getInstance().commitAndClose(); + return spainRole; + } + + static void grantWindowAccess(Role role, String windowId) { + WindowAccess windowAccess = OBProvider.getInstance().get(WindowAccess.class); + final OBCriteria<Window> obCriteria = OBDal.getInstance().createCriteria(Window.class); + obCriteria.add(Restrictions.eq(Window.PROPERTY_ID, windowId)); + obCriteria.setMaxResults(1); + windowAccess.setClient(role.getClient()); + windowAccess.setOrganization(role.getOrganization()); + windowAccess.setRole(role); + windowAccess.setWindow((Window) obCriteria.uniqueResult()); + windowAccess.setEditableField(true); + OBDal.getInstance().save(windowAccess); + createdObjects.add(windowAccess); } @AfterClass diff -r e8c0df5664de -r e47bc61fb8bd src/org/openbravo/base/secureApp/HttpSecureAppServlet.java --- a/src/org/openbravo/base/secureApp/HttpSecureAppServlet.java Thu Apr 28 13:52:52 2016 +0200 +++ b/src/org/openbravo/base/secureApp/HttpSecureAppServlet.java Thu Apr 28 14:03:21 2016 +0200 @@ -528,6 +528,10 @@ * */ private boolean hasLevelAccess(VariablesSecureApp vars, String accessLevel) { + if (!OBContext.getOBContext().doAccessLevelCheck()) { + return true; + } + final String userLevel = vars.getSessionValue("#User_Level"); boolean retValue = true; diff -r e8c0df5664de -r e47bc61fb8bd src/org/openbravo/base/secureApp/LoginUtils.java --- a/src/org/openbravo/base/secureApp/LoginUtils.java Thu Apr 28 13:52:52 2016 +0200 +++ b/src/org/openbravo/base/secureApp/LoginUtils.java Thu Apr 28 14:03:21 2016 +0200 @@ -32,6 +32,7 @@ import org.openbravo.erpCommon.businessUtility.Preferences; import org.openbravo.erpCommon.security.SessionLogin; import org.openbravo.erpCommon.utility.DimensionDisplayUtility; +import org.openbravo.erpCommon.utility.PropertyException; import org.openbravo.erpCommon.utility.Utility; import org.openbravo.model.ad.access.RoleOrganization; import org.openbravo.model.ad.domain.Preference; @@ -209,6 +210,18 @@ return false; } + boolean shouldCheckAccessLevel = true; + + try { + shouldCheckAccessLevel = "N".equals(Preferences.getPreferenceValue( + "BypassAccessLevelEntityCheck", true, OBContext.getOBContext().getCurrentClient(), + OBContext.getOBContext().getCurrentOrganization(), OBContext.getOBContext().getUser(), + OBContext.getOBContext().getRole(), null)); + } catch (PropertyException prefNotDefined) { + } + + OBContext.getOBContext().setCheckAccessLevel(shouldCheckAccessLevel); + // Set session vars vars.setSessionValue("#AD_User_ID", strUserAuth); vars.setSessionValue("#SalesRep_ID", strUserAuth); diff -r e8c0df5664de -r e47bc61fb8bd src/org/openbravo/dal/core/OBContext.java --- a/src/org/openbravo/dal/core/OBContext.java Thu Apr 28 13:52:52 2016 +0200 +++ b/src/org/openbravo/dal/core/OBContext.java Thu Apr 28 14:03:21 2016 +0200 @@ -573,6 +573,8 @@ // check whether using new or old UI private boolean newUI = false; + private boolean checkAccessLevel = true; + public String getUserLevel() { return userLevel; } @@ -1111,6 +1113,19 @@ return !(adminModeSet.get() != null || isAdministrator); } + /** + * Defines whether entity check should or not compare entity's access level with role's user level + * in order to completely prevent access to that entity. + */ + public boolean doAccessLevelCheck() { + return checkAccessLevel; + } + + /** @see OBContext#doAccessLevelCheck() */ + public void setCheckAccessLevel(boolean checkAccessLevel) { + this.checkAccessLevel = checkAccessLevel; + } + public boolean isAdminContext() { return this == adminContext; } diff -r e8c0df5664de -r e47bc61fb8bd src/org/openbravo/dal/security/EntityAccessChecker.java --- a/src/org/openbravo/dal/security/EntityAccessChecker.java Thu Apr 28 13:52:52 2016 +0200 ------------------------------------------------------------------------------ Find and fix application performance issues faster with Applications Manager Applications Manager provides deep performance insights into multiple tiers of your business applications. It resolves application problems quickly and reduces your MTTR. Get your free trial! https://ad.doubleclick.net/ddm/clk/302982198;130105516;z _______________________________________________ Openbravo-commits mailing list Openbravo-commits@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbravo-commits
