Commit 34bc9381 authored by Brian Major's avatar Brian Major
Browse files

nep110 - Fixed bugs to do with group deletion and membership

parent eb8c29bb
Loading
Loading
Loading
Loading
+36 −5
Original line number Diff line number Diff line
@@ -626,7 +626,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
        throws GroupNotFoundException, TransientException,
               AccessControlException
    {
        Group group = getGroup(groupDN, groupID, false);
        Group group = getGroup(groupDN, groupID, true);
        List<Modification> modifs = new ArrayList<Modification>();
        modifs.add(new Modification(ModificationType.ADD, "nsaccountlock", "true"));
        
@@ -707,6 +707,14 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
            groupDNs.addAll(getMemberGroups(user, userDN, groupID, true));
        }
        
        if (logger.isDebugEnabled())
        {
            for (DN dn : groupDNs)
            {
                logger.debug("Search adding DN: " + dn);
            }
        }
        
        Collection<Group> groups = new HashSet<Group>();
        try
        {
@@ -716,7 +724,17 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
                {
                    groupDN = new DN(groupDN.getRDNString() + "," + config.getGroupsDN());
                }
                try
                {
                    groups.add(getGroup(groupDN));
                    logger.debug("Search adding group: " + groupDN);
                }
                catch (GroupNotFoundException e)
                {
                    throw new IllegalStateException(
                        "BUG: group " + groupDN + " not found but " +
                        "membership exists (" + userID + ")");
                }
            }
        }
        catch (LDAPException e)
@@ -745,7 +763,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
            }
            
            SearchRequest searchRequest =  new SearchRequest(
                    config.getGroupsDN(), SearchScope.SUB, filter, "entrydn");
                    config.getGroupsDN(), SearchScope.SUB, filter, "entrydn", "nsaccountlock");
            
            searchRequest.addControl(
                    new ProxiedAuthorizationV2RequestControl("dn:" + 
@@ -755,8 +773,13 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
            for (SearchResultEntry result : results.getSearchEntries())
            {
                String entryDN = result.getAttributeValue("entrydn");
                // make sure the group isn't deleted
                if (result.getAttribute("nsaccountlock") == null)
                {
                    groupDNs.add(new DN(entryDN));
                }
                
            }
        }
        catch (LDAPException e1)
        {
@@ -816,7 +839,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
        
        SearchRequest searchRequest =  new SearchRequest(
                    config.getGroupsDN(), SearchScope.SUB, filter, 
                    "cn", "description", "owner");
                    "cn", "description", "owner", "nsaccountlock");
            
        searchRequest.addControl(
                    new ProxiedAuthorizationV2RequestControl("dn:" + 
@@ -832,6 +855,14 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO
            throw new GroupNotFoundException(groupDN.toNormalizedString());
        }
        
        if (searchResult.getAttribute("nsaccountlock") != null)
        {
            // deleted group
            String msg = "Group not found " + groupDN;
            logger.debug(msg);
            throw new GroupNotFoundException(groupDN.toNormalizedString());
        }

        Group group = new Group(searchResult.getAttributeValue("cn"),
                                userPersist.getMember(
                                        new DN(searchResult.getAttributeValue(
+46 −0
Original line number Diff line number Diff line
@@ -68,6 +68,7 @@
 */
package ca.nrc.cadc.ac.server.web;

import java.io.IOException;
import java.security.AccessControlContext;
import java.security.AccessControlException;
import java.security.AccessController;
@@ -240,6 +241,15 @@ public class ACSearchRunner implements JobRunner
            log.error("FAIL", t);
            
            syncOut.setResponseCode(503);
            syncOut.setHeader("Content-Type", "text/plan");
            try
            {
                syncOut.getOutputStream().write(t.getMessage().getBytes());
            }
            catch (IOException e)
            {
                log.warn("Could not write response to output stream", e);
            }
            
//            ErrorSummary errorSummary =
//                new ErrorSummary(t.getMessage(), ErrorType.FATAL);
@@ -261,6 +271,15 @@ public class ACSearchRunner implements JobRunner
            log.debug("FAIL", t);
            
            syncOut.setResponseCode(404);
            syncOut.setHeader("Content-Type", "text/plan");
            try
            {
                syncOut.getOutputStream().write(t.getMessage().getBytes());
            }
            catch (IOException e)
            {
                log.warn("Could not write response to output stream", e);
            }
            
//            ErrorSummary errorSummary =
//                new ErrorSummary(t.getMessage(), ErrorType.FATAL);
@@ -282,6 +301,15 @@ public class ACSearchRunner implements JobRunner
            log.debug("FAIL", t);
            
            syncOut.setResponseCode(404);
            syncOut.setHeader("Content-Type", "text/plan");
            try
            {
                syncOut.getOutputStream().write(t.getMessage().getBytes());
            }
            catch (IOException e)
            {
                log.warn("Could not write response to output stream", e);
            }
            
//            ErrorSummary errorSummary =
//                new ErrorSummary(t.getMessage(), ErrorType.FATAL);
@@ -303,6 +331,15 @@ public class ACSearchRunner implements JobRunner
            log.debug("FAIL", t);
            
            syncOut.setResponseCode(403);
            syncOut.setHeader("Content-Type", "text/plan");
            try
            {
                syncOut.getOutputStream().write(t.getMessage().getBytes());
            }
            catch (IOException e)
            {
                log.warn("Could not write response to output stream", e);
            }
            
//            ErrorSummary errorSummary =
//                new ErrorSummary(t.getMessage(), ErrorType.FATAL);
@@ -324,6 +361,15 @@ public class ACSearchRunner implements JobRunner
            log.error("FAIL", t);
            
            syncOut.setResponseCode(500);
            syncOut.setHeader("Content-Type", "text/plan");
            try
            {
                syncOut.getOutputStream().write(t.getMessage().getBytes());
            }
            catch (IOException e)
            {
                log.warn("Could not write response to output stream", e);
            }
            
//            ErrorSummary errorSummary =
//                new ErrorSummary(t.getMessage(), ErrorType.FATAL);
+39 −4
Original line number Diff line number Diff line
@@ -86,7 +86,6 @@ import java.util.Map;
import java.util.Set;

import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.SSLSocketFactory;
import javax.security.auth.Subject;

@@ -186,6 +185,9 @@ public class GMSClient
        URL createGroupURL = new URL(this.baseURL + "/groups");
        log.debug("createGroupURL request to " + createGroupURL.toString());
        
        // reset the state of the cache
        clearCache();

        StringBuilder groupXML = new StringBuilder();
        GroupWriter.write(group, groupXML);
        log.debug("createGroup: " + groupXML);
@@ -309,6 +311,9 @@ public class GMSClient
        URL updateGroupURL = new URL(this.baseURL + "/groups/" + group.getID());
        log.debug("updateGroup request to " + updateGroupURL.toString());
        
        // reset the state of the cache
        clearCache();

        StringBuilder groupXML = new StringBuilder();
        GroupWriter.write(group, groupXML);
        log.debug("updateGroup: " + groupXML);
@@ -371,6 +376,10 @@ public class GMSClient
    {
        URL deleteGroupURL = new URL(this.baseURL + "/groups/" + groupName);
        log.debug("deleteGroup request to " + deleteGroupURL.toString());
        
        // reset the state of the cache
        clearCache();
        
        HttpURLConnection conn = 
                (HttpURLConnection) deleteGroupURL.openConnection();
        conn.setRequestMethod("DELETE");
@@ -379,14 +388,14 @@ public class GMSClient
        if ((sf != null) && ((conn instanceof HttpsURLConnection)))
        {
            ((HttpsURLConnection) conn)
                    .setSSLSocketFactory(getSSLSocketFactory());
                    .setSSLSocketFactory(sf);
        }
        int responseCode = -1;
        try
        {
            responseCode = conn.getResponseCode();
        }
        catch(SSLHandshakeException e)
        catch(Exception e)
        {
            throw new AccessControlException(e.getMessage());
        }
@@ -433,6 +442,9 @@ public class GMSClient
                                        groupMemberName);
        log.debug("addGroupMember request to " + addGroupMemberURL.toString());
        
        // reset the state of the cache
        clearCache();

        HttpURLConnection conn = 
                (HttpURLConnection) addGroupMemberURL.openConnection();
        conn.setRequestMethod("PUT");
@@ -496,6 +508,9 @@ public class GMSClient

        log.debug("addUserMember request to " + addUserMemberURL.toString());
        
        // reset the state of the cache
        clearCache();

        HttpURLConnection conn = 
                (HttpURLConnection) addUserMemberURL.openConnection();
        conn.setRequestMethod("PUT");
@@ -558,6 +573,9 @@ public class GMSClient
        log.debug("removeGroupMember request to " + 
                  removeGroupMemberURL.toString());
        
        // reset the state of the cache
        clearCache();

        HttpURLConnection conn = 
                (HttpURLConnection) removeGroupMemberURL.openConnection();
        conn.setRequestMethod("DELETE");
@@ -623,6 +641,9 @@ public class GMSClient
        log.debug("removeUserMember request to " + 
                  removeUserMemberURL.toString());
        
        // reset the state of the cache
        clearCache();

        HttpURLConnection conn = 
                (HttpURLConnection) removeUserMemberURL.openConnection();
        conn.setRequestMethod("DELETE");
@@ -928,10 +949,23 @@ public class GMSClient
            AccessControlContext ac = AccessController.getContext();
            Subject s = Subject.getSubject(ac);
            this.sslSocketFactory = SSLUtil.getSocketFactory(s);
            log.debug("Socket Factory: " + this.sslSocketFactory);
        }
        return this.sslSocketFactory;
    }
    
    protected void clearCache()
    {
        AccessControlContext acContext = AccessController.getContext();
        Subject subject = Subject.getSubject(acContext);
        
        if (subject != null)
        {
            log.debug("Clearing cache");
            subject.getPrivateCredentials().clear();
        }
    }

    protected List<Group> getCachedGroups(Principal userID, Role role)
    {
        AccessControlContext acContext = AccessController.getContext();
@@ -940,7 +974,6 @@ public class GMSClient
        // only consult cache if the userID is of the calling subject
        if (userIsSubject(userID, subject))
        {
            
            Set groupCredentialSet = subject.getPrivateCredentials(GroupMemberships.class);
            if ((groupCredentialSet != null) && 
                (groupCredentialSet.size() == 1))
@@ -961,6 +994,8 @@ public class GMSClient
        // only save to cache if the userID is of the calling subject
        if (userIsSubject(userID, subject))
        {
            log.debug("Caching groups for " + userID + ", role " + role);
            
            GroupMemberships groupCredentials = null;
            Set groupCredentialSet = subject.getPrivateCredentials(GroupMemberships.class);
            if ((groupCredentialSet != null) && 
+1 −1
Original line number Diff line number Diff line
@@ -96,7 +96,7 @@ public class GMSClientTest
    
    public GMSClientTest()
    {
        Log4jInit.setLevel("ca.nrc.cadc.ac", Level.INFO);
        Log4jInit.setLevel("ca.nrc.cadc.ac", Level.DEBUG);
    }
    
    @Test