[417483] [Security] Issues in invalidation when missing write Permission
https://bugs.eclipse.org/bugs/show_bug.cgi?id=417483
- Modify CDORevisionImpl to avoid throwing NoPermissionExceptions when
copying a new revision out of an existing one
- Modify CDOSessionImpl to avoid throwing NoPermissionExceptions when
revising revisisons
diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/CDORevisionImpl.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/CDORevisionImpl.java
index 2475856..2b7c718 100644
--- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/CDORevisionImpl.java
+++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/CDORevisionImpl.java
@@ -31,6 +31,11 @@
{
private Object[] values;
+ /**
+ * Indicates if the revision is being copied from another revision. In that case isWritable() will allways return true.
+ */
+ private boolean isCopying;
+
public CDORevisionImpl(EClass eClass)
{
super(eClass);
@@ -42,23 +47,31 @@
EStructuralFeature[] features = clearValues();
int length = features.length;
- for (int i = 0; i < length; i++)
+ isCopying = true;
+ try
{
- EStructuralFeature feature = features[i];
- if (feature.isMany())
+ for (int i = 0; i < length; i++)
{
- InternalCDOList sourceList = (InternalCDOList)source.values[i];
- if (sourceList != null)
+ EStructuralFeature feature = features[i];
+ if (feature.isMany())
{
- EClassifier classifier = feature.getEType();
- setValue(i, sourceList.clone(classifier));
+ InternalCDOList sourceList = (InternalCDOList)source.values[i];
+ if (sourceList != null)
+ {
+ EClassifier classifier = feature.getEType();
+ setValue(i, sourceList.clone(classifier));
+ }
+ }
+ else
+ {
+ CDOType type = CDOModelUtil.getType(feature);
+ setValue(i, type.copyValue(source.values[i]));
}
}
- else
- {
- CDOType type = CDOModelUtil.getType(feature);
- setValue(i, type.copyValue(source.values[i]));
- }
+ }
+ finally
+ {
+ isCopying = false;
}
}
@@ -84,4 +97,22 @@
{
values[featureIndex] = value;
}
+
+ /**
+ *
+ * {@inheritDoc}
+ *
+ * @see org.eclipse.emf.cdo.spi.common.revision.AbstractCDORevision#isWritable()
+ */
+ @Override
+ public boolean isWritable()
+ {
+ // If we are copying another revision (
+ if (isCopying)
+ {
+ return true;
+ }
+ return super.isWritable();
+ }
+
}
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_417483_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_417483_Test.java
new file mode 100644
index 0000000..b75a8a3
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_417483_Test.java
@@ -0,0 +1,218 @@
+/*
+ * Copyright (c) 2004-2013 Eike Stepper (Berlin, Germany) and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ * Eike Stepper - initial API and implementation
+ */
+package org.eclipse.emf.cdo.tests.bugzilla;
+
+import org.eclipse.emf.cdo.common.CDOCommonSession.Options.PassiveUpdateMode;
+import org.eclipse.emf.cdo.eresource.CDOResource;
+import org.eclipse.emf.cdo.security.Access;
+import org.eclipse.emf.cdo.security.Group;
+import org.eclipse.emf.cdo.security.Realm;
+import org.eclipse.emf.cdo.security.ResourcePermission;
+import org.eclipse.emf.cdo.security.Role;
+import org.eclipse.emf.cdo.security.SecurityFactory;
+import org.eclipse.emf.cdo.security.User;
+import org.eclipse.emf.cdo.server.security.ISecurityManager;
+import org.eclipse.emf.cdo.server.security.SecurityManagerUtil;
+import org.eclipse.emf.cdo.session.CDOSession;
+import org.eclipse.emf.cdo.session.CDOSessionInvalidationEvent;
+import org.eclipse.emf.cdo.tests.AbstractCDOTest;
+import org.eclipse.emf.cdo.tests.config.impl.ConfigTest.CleanRepositoriesAfter;
+import org.eclipse.emf.cdo.tests.config.impl.ConfigTest.CleanRepositoriesBefore;
+import org.eclipse.emf.cdo.tests.config.impl.RepositoryConfig;
+import org.eclipse.emf.cdo.tests.config.impl.SessionConfig;
+import org.eclipse.emf.cdo.tests.model1.Category;
+import org.eclipse.emf.cdo.transaction.CDOTransaction;
+import org.eclipse.emf.cdo.util.CommitException;
+import org.eclipse.emf.cdo.util.ConcurrentAccessException;
+
+import org.eclipse.net4j.util.event.IEvent;
+import org.eclipse.net4j.util.event.IListener;
+import org.eclipse.net4j.util.security.IPasswordCredentials;
+import org.eclipse.net4j.util.security.IPasswordCredentialsProvider;
+import org.eclipse.net4j.util.security.PasswordCredentials;
+import org.eclipse.net4j.util.security.PasswordCredentialsProvider;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Tests ensuring that the permissions are correctly computed, no matter what passive update mode is chosen.
+ * @author Alex Lagarde <alex.lagarde@obeo.fr>
+ */
+@CleanRepositoriesBefore
+@CleanRepositoriesAfter
+public class Bugzilla_417483_Test extends AbstractCDOTest
+{
+
+ /**
+ * Login for use with write rights.
+ */
+ private static final String USER_WITH_WRITE_RIGHTS_ID = "Stepper";
+
+ /**
+ * Password for use with write rights.
+ */
+ private static final String USER_WITH_WRITE_RIGHTS_PASSWORD = "12345";
+
+ /**
+ * Login for use without write rights.
+ */
+ private static final String USER_WITHOUT_WRITE_RIGHTS_ID = "Lagarde";
+
+ /**
+ * Password for use without write rights.
+ */
+ private static final String USER_WITHOUT_WRITE_RIGHTS_PASSWORD = "54321";
+
+ /**
+ * Boolean used by the
+ */
+ private boolean loginWithUserWithoutRights;
+
+ @Override
+ protected void doSetUp() throws Exception
+ {
+ super.doSetUp();
+
+ // Define 2 users:
+ // - one with all rights
+ // - one without write rights (but all read rights)
+ ISecurityManager securityManager = startRepository();
+ securityManager.modify(new ISecurityManager.RealmOperation()
+ {
+ public void execute(Realm realm)
+ {
+ // User with write rights
+ User userwithWriteRights = realm.addUser(USER_WITH_WRITE_RIGHTS_ID, USER_WITH_WRITE_RIGHTS_PASSWORD);
+ userwithWriteRights.getGroups().add(realm.getGroup("Users"));
+ userwithWriteRights.getRoles().add(realm.getRole("All Objects Writer"));
+
+ // User with only read rights
+ ResourcePermission readlAllPermision = SecurityFactory.eINSTANCE.createResourcePermission();
+ readlAllPermision.setPattern(".*");
+ readlAllPermision.setAccess(Access.READ);
+ Role roleWithoutWriteRights = realm.addRole("User without Write - Role");
+ roleWithoutWriteRights.getPermissions().add(readlAllPermision);
+ Group groupWithoutWriteRights = realm.addGroup("User without Write - Group");
+ groupWithoutWriteRights.getRoles().add(roleWithoutWriteRights);
+ User userwithoutWriteRights = realm.addUser(USER_WITHOUT_WRITE_RIGHTS_ID, USER_WITHOUT_WRITE_RIGHTS_PASSWORD);
+ userwithoutWriteRights.getGroups().add(groupWithoutWriteRights);
+ }
+ });
+ }
+
+ /**
+ * Ensures that the permissions are correctly computed when integrating changes with the {@link PassiveUpdateMode#CHANGES} update mode.
+ */
+ public void testPermissionWithChangesPassiveUpdateMode() throws Exception
+ {
+ doTestPermissionWithPassiveUpdates(PassiveUpdateMode.CHANGES);
+ }
+
+ /**
+ * Ensures that the permissions are correctly computed when integrating changes with the {@link PassiveUpdateMode#ADDITIONS} update mode.
+ */
+ public void testPermissionWithAdditionPassiveUpdateMode() throws Exception
+ {
+ doTestPermissionWithPassiveUpdates(PassiveUpdateMode.ADDITIONS);
+ }
+
+ /**
+ * Ensures that the permissions are correctly computed when integrating changes with the {@link PassiveUpdateMode#INVALIDATIONS} update mode.
+ */
+ public void testPermissionWithInvalidationPassiveUpdateMode() throws Exception
+ {
+ doTestPermissionWithPassiveUpdates(PassiveUpdateMode.INVALIDATIONS);
+ }
+
+ /**
+ * Ensures that the permissions are correctly computed, with the given {@link PassiveUpdateMode}.
+ */
+ protected void doTestPermissionWithPassiveUpdates(PassiveUpdateMode passiveUpdateMode)
+ throws ConcurrentAccessException, CommitException, InterruptedException
+ {
+ // Step 1: Both users open transaction on the repository
+ CDOSession userWithWriteRightSession = openSession();
+ CDOTransaction userWithWriteRightTransaction = userWithWriteRightSession.openTransaction();
+ loginWithUserWithoutRights = true;
+ CDOSession userWithoutWriteRightSession = openSession();
+ userWithoutWriteRightSession.options().setPassiveUpdateMode(passiveUpdateMode);
+
+ CDOTransaction userWithoutWriteRightTransaction = userWithoutWriteRightSession.openTransaction();
+ assertEquals(USER_WITH_WRITE_RIGHTS_ID, userWithWriteRightSession.getUserID());
+ assertEquals(USER_WITHOUT_WRITE_RIGHTS_ID, userWithoutWriteRightSession.getUserID());
+
+ // Step 2: User with write rights creates a resource and commits
+ CDOResource resource = userWithWriteRightTransaction.createResource(getResourcePath("/res"));
+ assertEquals(true, resource.cdoRevision().isWritable());
+ Category resourceRoot = getModel1Factory().createCategory();
+ resource.getContents().add(resourceRoot);
+ userWithWriteRightTransaction.commit();
+
+ // => User without rights should be able to integrate changes without permission issues
+ CDOResource resourceWithoutWrite = userWithoutWriteRightTransaction.getResource(getResourcePath("/res"));
+ assertEquals(1, resourceWithoutWrite.getContents().size());
+ // => Trigger loading of resource root so that invalidation are sent
+ resourceWithoutWrite.getContents().iterator().next();
+
+ // Step 3: User with write rights modifies the resource root and commits
+ resourceRoot.setName("RENAMMED");
+ final CountDownLatch latch = new CountDownLatch(1);
+ userWithoutWriteRightSession.addListener(new IListener()
+ {
+ public void notifyEvent(IEvent event)
+ {
+ if (event instanceof CDOSessionInvalidationEvent)
+ {
+ latch.countDown();
+ }
+ }
+ });
+ userWithWriteRightTransaction.commit();
+
+ // => User without rights should be able to integrate changes without permission issues
+ boolean notified = latch.await(DEFAULT_TIMEOUT, TimeUnit.MILLISECONDS);
+ assertEquals("Timeout: user without Write Rights should have been notified of changes", true, notified);
+ }
+
+ /**
+ * Starts a repository with security enabled.
+ * @return the {@link ISecurityManager} associated to the started repository
+ */
+ private ISecurityManager startRepository()
+ {
+ // Create & register security manager
+ ISecurityManager securityManager = SecurityManagerUtil.createSecurityManager("/security", getServerContainer());
+ getTestProperties().put(RepositoryConfig.PROP_TEST_SECURITY_MANAGER, securityManager);
+
+ // Create & register a credential provider allowing to logging with one user or another
+ IPasswordCredentialsProvider credentialsProvider = new PasswordCredentialsProvider(USER_WITH_WRITE_RIGHTS_ID,
+ USER_WITH_WRITE_RIGHTS_PASSWORD)
+ {
+ @Override
+ public IPasswordCredentials getCredentials()
+ {
+ if (loginWithUserWithoutRights)
+ {
+ return new PasswordCredentials(USER_WITHOUT_WRITE_RIGHTS_ID, USER_WITHOUT_WRITE_RIGHTS_PASSWORD.toCharArray());
+ }
+ return super.getCredentials();
+ }
+ };
+ getTestProperties().put(SessionConfig.PROP_TEST_CREDENTIALS_PROVIDER, credentialsProvider);
+
+ // Start repository
+ getRepository();
+
+ return securityManager;
+ }
+
+}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/CDOSessionImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/CDOSessionImpl.java
index 1385fab..35dbb60 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/CDOSessionImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/session/CDOSessionImpl.java
@@ -47,6 +47,7 @@
import org.eclipse.emf.cdo.common.revision.delta.CDORemoveFeatureDelta;
import org.eclipse.emf.cdo.common.revision.delta.CDORevisionDelta;
import org.eclipse.emf.cdo.common.revision.delta.CDOSetFeatureDelta;
+import org.eclipse.emf.cdo.common.security.CDOPermission;
import org.eclipse.emf.cdo.common.util.CDOCommonUtil;
import org.eclipse.emf.cdo.common.util.CDOException;
import org.eclipse.emf.cdo.common.util.RepositoryStateChangedEvent;
@@ -938,6 +939,12 @@
addOldValuesToDelta(oldRevision, revisionDelta);
InternalCDORevision newRevision = oldRevision.copy();
+ // If revision as READ permission, temporarily provide WRITE permission to be able to integrate remote changes
+ CDOPermission oldPermission = newRevision.getPermission();
+ if (CDOPermission.READ.equals(oldPermission))
+ {
+ newRevision.setPermission(CDOPermission.WRITE);
+ }
newRevision.adjustForCommit(commitInfo.getBranch(), commitInfo.getTimeStamp());
CDORevisable target = revisionDelta.getTarget();
@@ -948,7 +955,7 @@
revisionDelta.apply(newRevision);
newRevision.freeze();
-
+ newRevision.setPermission(oldPermission);
revisionManager.addRevision(newRevision);
if (oldRevisions == null)
{
