Project edit access not revoked if user has cvs privilages disabled

For some time now, users who are granted CVS access for a project can also edit the project node and create releases. This is controlled through the project_check_admin_access() function in project.module. If a somewhat unusual string of events takes place, however, this might not be ideal.

Here's the string of events:

1. User 'maint' creates ProjectA.
2. User 'comaint' applies for CVS access to help co-maintain ProjectA and is granted access by the d.o admins.
3. User 'maint' grants user 'comaint' CVS access to ProjectA. User 'comaint' now has permission to edit the ProjectA node and create releases.
4. User 'comaint' does something crazy that gets his CVS account disabled by the d.o admins.
5. User 'comaint' can still act as a co-maintainer for the project because the default value for $cvs_account in project_check_admin_access() is $cvs_account=TRUE, even though that's not actually checked.

On the other hand, if user 'maint' tried to give user 'comaint' CVS access to the project but user 'comaint' had his CVS access disabled, an error would be returned.

I'm testing this on my system, so it's possible that things are set up slightly different on d.o and so this doesn't happen.

I'm not sure if this is undesired behavior or not, since the project maintainer (user 'maint') could just revoke access to user 'comaint' for the project, but maybe it would be best to have this be automatic.

Feel free to mark this as by design if you think that's appropriate. If you do think it's a bug, I've patched the project_check_admin_access() function as so and this seems to keep this from happening. I can roll a patch if you want me to:

function project_check_admin_access($project, $cvs_access = NULL) {
  global $user;
  if (empty($user->uid)) {
    return FALSE;
  }
  
  if (!isset($cvs_access)) {
      if (db_num_rows(db_query("SELECT * FROM {cvs_accounts} WHERE uid = %d AND status = %d", $user->uid, 1))) {
        $cvs_access = TRUE;
      }
      else {
        $cvs_access = FALSE;
      }  
  }
  
  $project_obj = is_numeric($project) ? node_load($project) : $project;
  if (!isset($project_obj) || $project_obj->type != 'project_project') {
    return FALSE;
  }
  if (user_access('administer projects')) {
     return TRUE;
  }
  if (user_access('maintain projects')) {
    if ($user->uid == $project_obj->uid) {
       return TRUE;
    }
    if (project_use_cvs($project) && $cvs_access) {
      if (db_num_rows(db_query("SELECT * FROM {cvs_project_maintainers} WHERE uid = %d AND nid = %d", $user->uid, $project_obj->nid))) {
        return TRUE;
      }
    }
  }
  return FALSE;
}

I haven't looked everywhere else that project_check_admin_access() is called, however, so this might cause problems that I haven't thought of.

AC