S1651: merge

projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java

@@ -68,16 +68,16 @@
  */
 package ca.nrc.cadc.ac.server;
 
+import java.security.AccessControlException;
+import java.security.Principal;
+import java.util.Collection;
+
 import ca.nrc.cadc.ac.Group;
 import ca.nrc.cadc.ac.GroupAlreadyExistsException;
 import ca.nrc.cadc.ac.GroupNotFoundException;
-import ca.nrc.cadc.ac.IdentityType;
 import ca.nrc.cadc.ac.Role;
 import ca.nrc.cadc.ac.UserNotFoundException;
 import ca.nrc.cadc.net.TransientException;
-import java.security.AccessControlException;
-import java.security.Principal;
-import java.util.Collection;
 
 public abstract interface GroupPersistence<T extends Principal>
 {

projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/PluginFactory.java

@@ -125,8 +125,8 @@ public class PluginFactory
         {
             try
             {
-                Class c = Class.forName(cname);
-                ret = (GroupPersistence) c.newInstance();
+                Class<?> c = Class.forName(cname);
+                ret = (GroupPersistence<T>) c.newInstance();
             }
             catch (Exception ex)
             {
@@ -149,8 +149,8 @@ public class PluginFactory
         {
             try
             {
-                Class c = Class.forName(cname);
-                ret = (UserPersistence) c.newInstance();
+                Class<?> c = Class.forName(cname);
+                ret = (UserPersistence<T>) c.newInstance();
             }
             catch (Exception ex)
             {

projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapDAO.java

@@ -68,22 +68,26 @@
  */
 package ca.nrc.cadc.ac.server.ldap;
 
+import java.security.AccessControlException;
+import java.security.AccessController;
+import java.security.Principal;
+import java.util.Set;
+
+import javax.security.auth.Subject;
+import javax.security.auth.x500.X500Principal;
+
 import ca.nrc.cadc.auth.HttpPrincipal;
 import ca.nrc.cadc.auth.NumericPrincipal;
 import ca.nrc.cadc.auth.OpenIdPrincipal;
+import ca.nrc.cadc.net.TransientException;
+
 import com.unboundid.ldap.sdk.DN;
 import com.unboundid.ldap.sdk.LDAPConnection;
 import com.unboundid.ldap.sdk.LDAPException;
+import com.unboundid.ldap.sdk.ResultCode;
 import com.unboundid.ldap.sdk.SearchResult;
 import com.unboundid.ldap.sdk.SearchResultEntry;
 import com.unboundid.ldap.sdk.SearchScope;
-import java.security.AccessControlException;
-import java.security.AccessController;
-import java.security.Principal;
-import java.util.List;
-import java.util.Set;
-import javax.security.auth.Subject;
-import javax.security.auth.x500.X500Principal;
 
 public abstract class LdapDAO
 {
@@ -116,7 +120,15 @@ public abstract class LdapDAO
         {
             conn = new LDAPConnection(config.getServer(), config.getPort());
             conn.bind(config.getAdminUserDN(), config.getAdminPasswd());
+        }
 
+        return conn;
+    }
+
+    protected DN getSubjectDN() throws LDAPException
+    {
+        if (subjDN == null)
+        {
             Subject callerSubject = 
                     Subject.getSubject(AccessController.getContext());
             if (callerSubject == null)
@@ -161,7 +173,7 @@ public abstract class LdapDAO
             }
 
             SearchResult searchResult = 
-                    conn.search(config.getUsersDN(), SearchScope.ONE, 
+                    getConnection().search(config.getUsersDN(), SearchScope.ONE, 
                                 ldapField, new String[] {"entrydn"});
 
             if (searchResult.getEntryCount() < 1)
@@ -173,17 +185,50 @@ public abstract class LdapDAO
             subjDN = ((SearchResultEntry) searchResult.getSearchEntries()
                     .get(0)).getAttributeValueAsDN("entrydn");
         }
-
-        return conn;
+        return subjDN;
     }
-
-    protected DN getSubjectDN() throws LDAPException
+    
+    /**
+     * Checks the Ldap result code, and if the result is not SUCCESS,
+     * throws an appropriate exception. This is the place to decide on 
+     * mapping between ldap errors and exception types
+     * @param code
+     * @param errorMsg
+     * @throws TransientException 
+     */
+    protected static void checkLdapResult(ResultCode code, String errorMsg) 
+            throws TransientException
     {
-        if (subjDN == null)
+        String msg = "";
+        if (errorMsg != null)
         {
-            getConnection();
+            msg = "(" + errorMsg + ")";
+        }
+        if (code == ResultCode.INSUFFICIENT_ACCESS_RIGHTS)
+        {
+            throw new AccessControlException("Not authorized " + msg);
+        }
+        else if (code == ResultCode.INVALID_CREDENTIALS)
+        {
+            throw new AccessControlException("Invalid credentials " + msg);
+        }
+        else if (code == ResultCode.SUCCESS)
+        {
+            // all good. nothing to do
+        }
+        else if (code == ResultCode.PARAM_ERROR)
+        {
+            throw new IllegalArgumentException("Error in Ldap parameters " + msg);
+        }
+        else if (code == ResultCode.BUSY ||
+                 code == ResultCode.CONNECT_ERROR )
+        {
+            throw new TransientException("Connection problems " + msg );
+        }
+        else
+        {
+            throw new RuntimeException("Ldap error" + msg);
         }
-        return subjDN;
     }
 
 }

projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java

@@ -84,7 +84,6 @@ import ca.nrc.cadc.ac.PersonalDetails;
 import ca.nrc.cadc.ac.User;
 import ca.nrc.cadc.ac.UserNotFoundException;
 import ca.nrc.cadc.auth.HttpPrincipal;
-import ca.nrc.cadc.auth.NumericPrincipal;
 import ca.nrc.cadc.net.TransientException;
 
 import com.unboundid.ldap.sdk.CompareRequest;
@@ -167,7 +166,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
         }
         catch (LDAPException e)
         {
-            e.printStackTrace();
+            LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
         }
 
         if (searchResult == null)
@@ -203,6 +202,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
     public Collection<DN> getUserGroups(final T userID, final boolean isAdmin)
         throws UserNotFoundException, TransientException, AccessControlException
     {
+        Collection<DN> groupDNs = new HashSet<DN>();
         try
         {
             String searchField = (String) userLdapAttrib.get(userID.getClass());
@@ -239,7 +239,6 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
                 parentDN = new DN(config.getGroupsDN());
             }
             
-            Collection<DN> groupDNs = new HashSet<DN>();
             if (searchResult != null)
             {
                 String[] members = searchResult.getAttributeValues("memberOf");
@@ -254,17 +253,13 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
                         }
                     }
                 }
-            }
-            return groupDNs;
+            } 
         }
         catch (LDAPException e)
         {
-            e.printStackTrace();
-            // TODO check which LDAP exceptions are transient and which
-            // ones are
-            // access control
-            throw new TransientException("Error getting user groups", e);
+            LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
         }
+        return groupDNs;
     }
     
     /**
@@ -315,15 +310,48 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
             }
             return true;
         }
-        catch (LDAPException e1)
+        catch (LDAPException e)
         {
-            // TODO check which LDAP exceptions are transient and which
-            // ones are
-            // access control
-            throw new TransientException("Error getting the user", e1);
+            LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
         }
+        return false;
     }
     
+//    public boolean isMember(T userID, String groupID)
+//        throws UserNotFoundException, TransientException,
+//               AccessControlException
+//    {
+//        try
+//        {
+//            String searchField = (String) userLdapAttrib.get(userID.getClass());
+//            if (searchField == null)
+//            {
+//                throw new IllegalArgumentException(
+//                        "Unsupported principal type " + userID.getClass());
+//            }
+//
+//            User<T> user = getUser(userID);
+//            DN userDN = getUserDN(user);
+//
+//            CompareRequest compareRequest = 
+//                    new CompareRequest(userDN.toNormalizedString(), 
+//                                      "memberOf", groupID);
+//            
+//            compareRequest.addControl(
+//                    new ProxiedAuthorizationV2RequestControl("dn:" + 
+//                            getSubjectDN().toNormalizedString()));
+//            
+//            CompareResult compareResult = 
+//                    getConnection().compare(compareRequest);
+//            return compareResult.compareMatched();
+//        }
+//        catch (LDAPException e)
+//        {
+//            LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
+//            throw new RuntimeException("Unexpected LDAP exception", e);
+//        }
+//    }
+    
     /**
      * Returns a member user identified by the X500Principal only. The
      * returned object has the fields required by the GMS.
@@ -371,7 +399,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
     
 
     DN getUserDN(User<? extends Principal> user)
-        throws LDAPException, UserNotFoundException
+        throws UserNotFoundException, TransientException
     {
         String searchField = (String) userLdapAttrib.get(user.getUserID().getClass());
         if (searchField == null)
@@ -383,17 +411,22 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO
         searchField = "(" + searchField + "=" + 
                       user.getUserID().getName() + ")";
 
-        SearchRequest searchRequest = 
-                new SearchRequest(this.config.getUsersDN(), SearchScope.SUB, 
-                                 searchField, new String[] {"entrydn"});
+        SearchResultEntry searchResult = null;
+        try
+        {
+            SearchRequest searchRequest = new SearchRequest(this.config.getUsersDN(), SearchScope.SUB, 
+                             searchField, new String[] {"entrydn"});
         
-//        searchRequest.addControl(
-//                    new ProxiedAuthorizationV2RequestControl("dn:" + 
-//                            getSubjectDN().toNormalizedString()));
 
-        SearchResultEntry searchResult = 
+            searchResult = 
                 getConnection().searchForEntry(searchRequest);
 
+        } catch (LDAPException e)
+        {
+            LdapDAO.checkLdapResult(e.getResultCode(), e.getDiagnosticMessage());
+        }
+        
+
         if (searchResult == null)
         {
             String msg = "User not found " + user.getUserID().toString();

projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAOTest.java

@@ -719,6 +719,32 @@ public class LdapGroupDAOTest
                 return null;
             }
         });
+        
+        
+        // change the user
+        Subject.doAs(daoTestUser2Subject, new PrivilegedExceptionAction<Object>()
+        {
+            
+            public Object run() throws Exception
+            {
+
+                
+                try
+                {
+                    Group group = getGroupDAO().getGroup(groupID);
+                    assertTrue(group == null);
+                    
+                    fail("searchGroups with unknown user should throw " + 
+                         "GroupNotFoundException");
+                }
+                catch (GroupNotFoundException ignore) 
+                {
+
+                }
+
+                return null;
+            }
+        });
     }
 
     private void assertGroupsEqual(Group gr1, Group gr2)