[Opensource] Bug in DBController.stateAllowed()

[Ken Chow]

I think I've found a bug related to a user that is a member of multiple
Security Groups. Here's the scenario:

 - I've defined two Security Groups:  Reader and Creator
 - the group Creator is a member of the group Reader
 - Reader has access to the states promptUserUpdate and processUserUpdate in
MemberController
 - Creator has access to the states promptUserSearch, processUserSearch in
MemberController
 - logged on user is in the Creator group

 - when DBController.stateAllowed() is invoked with nextState
promptUserSearch for the MemberController
   . for the group Creator with allowedStates="promptUserSearch,
processUserSearch", a match
     is found, and allowedStates="promptUserSearch, processUserSearch" is
saved in the cache
   . for the group Reader with allowedStates="promptUserUpdate and
processUserUpdate", NO match
     is found, and nothing is saved in the cache

 - when DBController.stateAllowed() is invoked again with nextState
promptUserUpdate for the MemberController
   . a match is found in the cache with allowedStates="promptUserSearch,
processUserSearch"
   . promptUserUpdate is not contained in the allowedStates retrieved from
the cache
   . stateAllowed() incorrectly returns false (because not all the allowed
states for the uid|MemberController
     are stored in the cache)

I'm currently working with Expresso 5.0.3, but have also reviewed the 5.0.5
and 5.3 RC5 code and they appear to have the same problem.

I've attached a revised version of stateAllowed() with the problem corrected
by accumulating all allowed states for the user (from all groups that the
user is a member of) before adding to the cache. I suspect that this may not
be the best way to fix the problem since there may be side-effects that I'm
nor aware of, but it appears to be working.

Ken Chow

-----------------------------------------------------------

    public synchronized boolean stateAllowed(String newState,
                                             ControllerRequest myRequest)
            throws ControllerException {
        try {

            /* if the security cache is available, use it */
            if
(CacheManager.getInstance().existsCache(myRequest.getDBName(), cacheName)) {
                ValidValue secVal = (ValidValue)
CacheManager.getInstance().getItem(
                        myRequest.getDBName(), cacheName,
                        myRequest.getUid() + "|" +
                        getClass().getName());

                /* if there is an entry for this user */
                if (secVal != null) {
                    String sec = secVal.getDescription();


                    if (containsAllowedState(sec, newState)) {
                        if (log.isDebugEnabled()) {
                            log.debug("User '" + myRequest.getUid() +
                                    "' allowed state '" + newState +
                                    "' via cache");
                        }

                        return true;
                    } else {

                        /* otherwise they are denied */
                        if (log.isDebugEnabled()) {
                            log.debug("User '" + myRequest.getUid() +
                                    "' denied state '" + newState +
                                    "' via cache in db '" +
                                    myRequest.getDBName() +
                                    "'. Cache entry was '" + sec + "'");
                        }

                        return false;
                    }
                }

                /* otherwise there was no entry, so check the DB directly */
            }

            /* if security cache is available */
            User thisUser = new User();
            thisUser.setDBName(myRequest.getDBName());
            thisUser.setUid(myRequest.getUid());
            thisUser.retrieve();

            ControllerSecurity cs = new ControllerSecurity();
            cs.setDBName(myRequest.getDBName());

            String oneGroupName = null;
            ControllerSecurity oneSecurityEntry = null;
            String allowedStates = null;
// start of inserted code
            FastStringBuffer accumulatedAllowedStates = new
FastStringBuffer();
// end of inserted code
            boolean allowed = false;

            for (Enumeration eg = thisUser.getGroups().elements();
                 eg.hasMoreElements();) {
                oneGroupName = (String) eg.nextElement();
                cs.clear();
                cs.setField("ControllerClass", getClass().getName());
                cs.setField("GroupName", oneGroupName);

                for (Iterator cse = cs.searchAndRetrieveList().iterator();
                     cse.hasNext();) {

                    oneSecurityEntry = (ControllerSecurity) cse.next();
                    allowedStates = oneSecurityEntry.getField("States");
// start of inserted code
                    if ((allowedStates != null) &&
(!allowedStates.equals(""))) {
                        if (accumulatedAllowedStates.length() > 0) {
                            accumulatedAllowedStates.append(",");
                        }
                        accumulatedAllowedStates.append(allowedStates);
                    }
// end of inserted code

                    if (containsAllowedState(allowedStates, newState)) {
                        allowed = true;
// start of deleted code
//                        addCachedSecurity(myRequest.getUid(),
//                                myRequest.getDBName(),
//                                getClass().getName(), allowedStates);
// end of deleted code
                    }
                } /* for */

            } /* for each user group */

// start of inserted code
            addCachedSecurity(myRequest.getUid(), myRequest.getDBName(),
                    getClass().getName(),
accumulatedAllowedStates.toString());
// end of inserted code

            if (!allowed) {
// start of deleted code
//                addCachedSecurity(myRequest.getUid(),
myRequest.getDBName(),
//                        getClass().getName(), "");
//                DelayThread.delay();
// end of deleted code

                if (log.isDebugEnabled()) {
                    log.debug("User " + myRequest.getUid() +
                            " denied all states in " + getClass().getName()
+
                            " in db '" + myRequest.getDBName() + "'");
                }
            }

            return allowed;
        } catch (DBException de) {
            throw new ControllerException("Unable to check Controller " +
                    "security", de);
        } catch (CacheException ce) {
            throw new ControllerException("Cache exception checking " +
                    "Controller security", ce);
        }
    } /* stateAllowed(String) */