Mailing-List: contact commits-help@directory.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@directory.apache.org
Message-ID: <20050829050723.927.qmail@minotaur.apache.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r264063 - in /directory:
 apacheds/trunk/main/src/test/org/apache/ldap/server/
 protocol-providers/ldap/trunk/src/main/java/org/apache/ldap/server/protocol/
Date: Mon, 29 Aug 2005 05:07:22 -0000
To: commits@directory.apache.org
From: akarasulu@apache.org

Author: akarasulu
Date: Sun Aug 28 22:07:15 2005
New Revision: 264063

URL: http://svn.apache.org/viewcvs?rev=264063&view=rev
Log:
Added some schema checks to prevent improper modify operations from succeeding.
Namely we added checks to satisfy the modify tests submitted by Stefan in the
following JIRA DIREVE-232 issue:

http://issues.apache.org/jira/browse/DIREVE-232

His test case was added as well as the changes to make sure the tests pass. 

NOTE: most of these fixes testing for schema correctness should be moved into
the schema checking interceptor however there may be some information loss 
without access to the request - this should be inspected before moving this
code into the interceptor.


Added:
    directory/apacheds/trunk/main/src/test/org/apache/ldap/server/ModifyRemoveTest.java   (with props)
Modified:
    directory/protocol-providers/ldap/trunk/src/main/java/org/apache/ldap/server/protocol/CompareHandler.java
    directory/protocol-providers/ldap/trunk/src/main/java/org/apache/ldap/server/protocol/ModifyHandler.java

Added: directory/apacheds/trunk/main/src/test/org/apache/ldap/server/ModifyRemoveTest.java
URL: http://svn.apache.org/viewcvs/directory/apacheds/trunk/main/src/test/org/apache/ldap/server/ModifyRemoveTest.java?rev=264063&view=auto
==============================================================================
--- directory/apacheds/trunk/main/src/test/org/apache/ldap/server/ModifyRemoveTest.java (added)
+++ directory/apacheds/trunk/main/src/test/org/apache/ldap/server/ModifyRemoveTest.java Sun Aug 28 22:07:15 2005
@@ -0,0 +1,292 @@
+/*
+ * Copyright (c) 2004 Solarsis Group LLC.
+ *
+ * Licensed under the Open Software License, Version 2.1 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://opensource.org/licenses/osl-2.1.php
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ldap.server;
+
+import java.util.Hashtable;
+
+import javax.naming.NamingException;
+import javax.naming.directory.Attribute;
+import javax.naming.directory.Attributes;
+import javax.naming.directory.BasicAttribute;
+import javax.naming.directory.BasicAttributes;
+import javax.naming.directory.DirContext;
+import javax.naming.directory.InvalidAttributeIdentifierException;
+import javax.naming.directory.NoSuchAttributeException;
+import javax.naming.directory.SchemaViolationException;
+import javax.naming.ldap.InitialLdapContext;
+import javax.naming.ldap.LdapContext;
+
+import junit.framework.TestCase;
+
+/**
+ * Testcase with different modify operations on a person entry. Each includes a
+ * single removal op only.
+ * 
+ * @author <a href="mailto:dev@directory.apache.org">Apache Directory Project</a>
+ * @version $Rev$
+ */
+public class ModifyRemoveTest extends AbstractServerTest
+{
+
+    private LdapContext ctx = null;
+
+    public static final String RDN = "cn=Tori Amos";
+
+    /**
+     * Creation of required attributes of a person entry.
+     */
+    protected Attributes getPersonAttributes(String sn, String cn)
+    {
+        Attributes attributes = new BasicAttributes();
+        Attribute attribute = new BasicAttribute("objectClass");
+        attribute.add("top");
+        attribute.add("person");
+        attributes.put(attribute);
+        attributes.put("cn", cn);
+        attributes.put("sn", sn);
+
+        return attributes;
+    }
+
+    /**
+     * Create context and a person entry.
+     */
+    public void setUp() throws Exception
+    {
+        super.setUp();
+
+        Hashtable env = new Hashtable();
+        env.put("java.naming.factory.initial", "com.sun.jndi.ldap.LdapCtxFactory");
+        env.put("java.naming.provider.url", "ldap://localhost:" + port + "/ou=system");
+        env.put("java.naming.security.principal", "uid=admin,ou=system");
+        env.put("java.naming.security.credentials", "secret");
+        env.put("java.naming.security.authentication", "simple");
+
+        ctx = new InitialLdapContext(env, null);
+        assertNotNull(ctx);
+
+        // Create a person with description
+        Attributes attributes = this.getPersonAttributes("Amos", "Tori Amos");
+        attributes.put("description", "an American singer-songwriter");
+        ctx.createSubcontext(RDN, attributes);
+
+    }
+
+    /**
+     * Remove person entry and close context.
+     */
+    public void tearDown() throws Exception
+    {
+        ctx.unbind(RDN);
+        ctx.close();
+
+        ctx.close();
+        ctx = null;
+
+        super.tearDown();
+    }
+
+    /**
+     * Just a little test to check wether opening the connection and creation of
+     * the person succeeds succeeds.
+     */
+    public void testSetUpTearDown() throws NamingException
+    {
+        assertNotNull(ctx);
+        DirContext tori = (DirContext) ctx.lookup(RDN);
+        assertNotNull(tori);
+    }
+
+    /**
+     * Remove an attribute, which is not required.
+     * 
+     * Expected result: After successful deletion, attribute is not present in
+     * entry.
+     * 
+     * @throws NamingException
+     */
+    public void testRemoveNotRequiredAttribute() throws NamingException
+    {
+        // Remove description Attribute
+        Attribute attr = new BasicAttribute("description");
+        Attributes attrs = new BasicAttributes();
+        attrs.put(attr);
+        ctx.modifyAttributes(RDN, DirContext.REMOVE_ATTRIBUTE, attrs);
+
+        // Verify, that attribute is deleted
+        attrs = ctx.getAttributes(RDN);
+        attr = attrs.get("description");
+        assertNull(attr);
+    }
+
+    /**
+     * Remove two not required attributes.
+     * 
+     * Expected result: After successful deletion, both attributes ar not
+     * present in entry.
+     * 
+     * @throws NamingException
+     */
+    public void testRemoveTwoNotRequiredAttributes() throws NamingException
+    {
+
+        // add telephoneNumber to entry
+        Attributes tn = new BasicAttributes("telephoneNumber", "12345678");
+        ctx.modifyAttributes(RDN, DirContext.ADD_ATTRIBUTE, tn);
+
+        // Remove description and telephoneNumber to Attribute
+        Attributes attrs = new BasicAttributes();
+        attrs.put(new BasicAttribute("description"));
+        attrs.put(new BasicAttribute("telephoneNumber"));
+        ctx.modifyAttributes(RDN, DirContext.REMOVE_ATTRIBUTE, attrs);
+
+        // Verify, that attributes are deleted
+        attrs = ctx.getAttributes(RDN);
+        assertNull(attrs.get("description"));
+        assertNull(attrs.get("telephoneNumber"));
+        assertNotNull(attrs.get("cn"));
+        assertNotNull(attrs.get("sn"));
+    }
+
+    /**
+     * Remove a required attribute. The sn attribute of the person entry is used
+     * here.
+     * 
+     * Expected Result: Deletion fails with NamingException (Schema Violation).
+     * 
+     * @throws NamingException
+     */
+    public void testRemoveRequiredAttribute() throws NamingException
+    {
+
+        // Remove sn attribute
+        Attribute attr = new BasicAttribute("sn");
+        Attributes attrs = new BasicAttributes();
+        attrs.put(attr);
+
+        try {
+            ctx.modifyAttributes(RDN, DirContext.REMOVE_ATTRIBUTE, attrs);
+            fail("Deletion of required attribute should fail.");
+        } catch (SchemaViolationException e) {
+            // expected behaviour
+        }
+    }
+
+    /**
+     * Remove a required attribute from RDN.
+     * 
+     * Expected Result: Deletion fails with SchemaViolationException.
+     * 
+     * @throws NamingException
+     */
+    public void testRemovePartOfRdn() throws NamingException
+    {
+
+        // Remove sn attribute
+        Attribute attr = new BasicAttribute("cn");
+        Attributes attrs = new BasicAttributes();
+        attrs.put(attr);
+
+        try {
+            ctx.modifyAttributes(RDN, DirContext.REMOVE_ATTRIBUTE, attrs);
+            fail("Deletion of RDN attribute should fail.");
+        } catch (SchemaViolationException e) {
+            // expected behaviour
+        }
+    }
+
+    /**
+     * Remove a not required attribute from RDN.
+     * 
+     * Expected Result: Deletion fails with SchemaViolationException.
+     * 
+     * @throws NamingException
+     */
+    public void testRemovePartOfRdnNotRequired() throws NamingException
+    {
+
+        // Change RDN to another attribute
+        String newRdn = "description=an American singer-songwriter";
+        ctx.addToEnvironment("java.naming.ldap.deleteRDN", "false");
+        ctx.rename(RDN, newRdn);
+
+        // Remove description, which is now RDN attribute
+        Attribute attr = new BasicAttribute("description");
+        Attributes attrs = new BasicAttributes();
+        attrs.put(attr);
+
+        try {
+            ctx.modifyAttributes(newRdn, DirContext.REMOVE_ATTRIBUTE, attrs);
+            fail("Deletion of RDN attribute should fail.");
+        } catch (SchemaViolationException e) {
+            // expected behaviour
+        }
+
+        // Change RDN back to original
+        ctx.addToEnvironment("java.naming.ldap.deleteRDN", "false");
+        ctx.rename(newRdn, RDN);
+    }
+
+    /**
+     * Remove a an attribute which is not present on the entry, but in the
+     * schema.
+     * 
+     * Expected result: Deletion fails with NoSuchAttributeException
+     * 
+     * @throws NamingException
+     */
+    public void testRemoveAttributeNotPresent() throws NamingException
+    {
+
+        // Remove telephoneNumber Attribute
+        Attribute attr = new BasicAttribute("telephoneNumber");
+        Attributes attrs = new BasicAttributes();
+        attrs.put(attr);
+
+        try {
+            ctx.modifyAttributes(RDN, DirContext.REMOVE_ATTRIBUTE, attrs);
+            fail("Deletion of attribute, which is not present in the entry, should fail.");
+        } catch (NoSuchAttributeException e) {
+            // expected behaviour
+        }
+    }
+
+    /**
+     * Remove a an attribute which is not present in the schema.
+     * 
+     * Expected result: Deletion fails with NoSuchAttributeException
+     * 
+     * @throws NamingException
+     */
+    public void testRemoveAttributeNotValid() throws NamingException
+    {
+
+        // Remove phantasy attribute
+        Attribute attr = new BasicAttribute("XXX");
+        Attributes attrs = new BasicAttributes();
+        attrs.put(attr);
+
+        try {
+            ctx.modifyAttributes(RDN, DirContext.REMOVE_ATTRIBUTE, attrs);
+            fail("Deletion of an invalid attribute should fail.");
+        } catch (NoSuchAttributeException e) {
+            // expected behaviour
+        } catch (InvalidAttributeIdentifierException e) {
+            // expected behaviour
+        }
+    }
+
+}

Propchange: directory/apacheds/trunk/main/src/test/org/apache/ldap/server/ModifyRemoveTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: directory/protocol-providers/ldap/trunk/src/main/java/org/apache/ldap/server/protocol/CompareHandler.java
URL: http://svn.apache.org/viewcvs/directory/protocol-providers/ldap/trunk/src/main/java/org/apache/ldap/server/protocol/CompareHandler.java?rev=264063&r1=264062&r2=264063&view=diff
==============================================================================
--- directory/protocol-providers/ldap/trunk/src/main/java/org/apache/ldap/server/protocol/CompareHandler.java (original)
+++ directory/protocol-providers/ldap/trunk/src/main/java/org/apache/ldap/server/protocol/CompareHandler.java Sun Aug 28 22:07:15 2005
@@ -28,7 +28,6 @@
 import org.apache.ldap.common.message.LdapResultImpl;
 import org.apache.ldap.common.message.ResultCodeEnum;
 import org.apache.ldap.common.util.ExceptionUtils;
-import org.apache.ldap.common.schema.AttributeType;
 import org.apache.ldap.server.jndi.ContextFactoryService;
 import org.apache.ldap.server.schema.AttributeTypeRegistry;
 import org.apache.mina.protocol.ProtocolSession;

Modified: directory/protocol-providers/ldap/trunk/src/main/java/org/apache/ldap/server/protocol/ModifyHandler.java
URL: http://svn.apache.org/viewcvs/directory/protocol-providers/ldap/trunk/src/main/java/org/apache/ldap/server/protocol/ModifyHandler.java?rev=264063&r1=264062&r2=264063&view=diff
==============================================================================
--- directory/protocol-providers/ldap/trunk/src/main/java/org/apache/ldap/server/protocol/ModifyHandler.java (original)
+++ directory/protocol-providers/ldap/trunk/src/main/java/org/apache/ldap/server/protocol/ModifyHandler.java Sun Aug 28 22:07:15 2005
@@ -19,6 +19,9 @@
 
 import javax.naming.NamingException;
 import javax.naming.directory.ModificationItem;
+import javax.naming.directory.DirContext;
+import javax.naming.directory.Attributes;
+import javax.naming.directory.Attribute;
 import javax.naming.ldap.LdapContext;
 
 import org.apache.ldap.common.exception.LdapException;
@@ -28,6 +31,12 @@
 import org.apache.ldap.common.message.ModifyResponseImpl;
 import org.apache.ldap.common.message.ResultCodeEnum;
 import org.apache.ldap.common.util.ExceptionUtils;
+import org.apache.ldap.common.schema.ObjectClass;
+import org.apache.ldap.common.schema.AttributeType;
+import org.apache.ldap.server.schema.AttributeTypeRegistry;
+import org.apache.ldap.server.schema.ObjectClassRegistry;
+import org.apache.ldap.server.schema.OidRegistry;
+import org.apache.ldap.server.jndi.ContextFactoryService;
 import org.apache.mina.protocol.ProtocolSession;
 import org.apache.mina.protocol.handler.MessageHandler;
 
@@ -47,6 +56,31 @@
     private static final ModificationItem[] EMPTY = new ModificationItem[0];
 
 
+    public boolean isRequired( String attrId, Attribute objectClass ) throws NamingException
+    {
+        OidRegistry oidRegistry = ContextFactoryService.getInstance().
+                getConfiguration().getGlobalRegistries().getOidRegistry();
+        ObjectClassRegistry registry = ContextFactoryService.getInstance().
+                getConfiguration().getGlobalRegistries().getObjectClassRegistry();
+
+        String attrOid = oidRegistry.getOid( attrId );
+        for ( int ii = 0; ii < objectClass.size(); ii++ )
+        {
+            ObjectClass ocSpec = registry.lookup( ( String ) objectClass.get( ii ) );
+            AttributeType[] mustList = ocSpec.getMustList();
+            for ( int jj = 0; jj < mustList.length; jj++ )
+            {
+                if ( mustList[jj].getOid().equals( attrOid ) )
+                {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
+
+
     public void messageReceived( ProtocolSession session, Object request )
     {
         ModifyRequest req = ( ModifyRequest ) request;
@@ -56,7 +90,54 @@
         try
         {
             LdapContext ctx = SessionRegistry.getSingleton().getLdapContext( session, null, true );
+            Attributes entry = ctx.getAttributes( req.getName() );
             Object[] mods = req.getModificationItems().toArray( EMPTY );
+
+            /*
+             * Check first to see if the modification request is valid by looking
+             * at the attributes being modified. Some modification operations are
+             * invalid if the attributes they modify do not exist or are undefined
+             */
+            AttributeTypeRegistry registry = ContextFactoryService.getInstance().
+                    getConfiguration().getGlobalRegistries().getAttributeTypeRegistry();
+
+            // As I write this code I am begining to realize that this should really
+            // be located within the schema interceptor for schema checking however
+            // resolution of what kind of operation is being performed is often lost
+            // @todo figure out how to best integrate this code into the schema interceptor
+
+            for ( int ii = 0; ii < mods.length; ii++ )
+            {
+                ModificationItem item = ( ModificationItem ) mods[ii];
+                String id = item.getAttribute().getID();
+                Attribute attr = entry.get( id );
+
+                if ( ! registry.hasAttributeType( id ) )
+                {
+                    resp.getLdapResult().setResultCode( ResultCodeEnum.UNDEFINEDATTRIBUTETYPE );
+                    resp.getLdapResult().setMatchedDn( req.getName() );
+                    session.write( resp );
+                    return;
+                }
+
+
+                if ( item.getModificationOp() == DirContext.REMOVE_ATTRIBUTE && attr == null )
+                {
+                    resp.getLdapResult().setResultCode( ResultCodeEnum.NOSUCHATTRIBUTE );
+                    resp.getLdapResult().setMatchedDn( req.getName() );
+                    session.write( resp );
+                    return;
+                }
+
+                if ( item.getModificationOp() == DirContext.REMOVE_ATTRIBUTE && isRequired( id, entry.get( "objectClass" ) ) )
+                {
+                    resp.getLdapResult().setResultCode( ResultCodeEnum.OBJECTCLASSVIOLATION );
+                    resp.getLdapResult().setMatchedDn( req.getName() );
+                    session.write( resp );
+                    return;
+                }
+            }
+
             ctx.modifyAttributes( req.getName(), ( ModificationItem[] ) mods );
         }
         catch ( NamingException e )