FA does not properly support Flag module's flags on comments

Sorry for the ambiguous report. I've got Flag, Flag Note, and Advanced Forum. I create a flag for comments and see a flag link appear on comments, but a user can flag each comment multiple times - the action never changes to unflag.

The flag is set up as non-global and can flag any content but their own content.

Thanks,

Diving deep into the flag module, I found that $GLOBALS['user']->uid was 1 when the comment links were being rendered! This stumped me majorly ...

I dove further with the help of debug_backtrace, and eventually found this nugget in the forum_access module (forum_access_link_alter() line 398):

    if (!empty($link_is_missing)) {
      // One of the $required_links should be present, because the current
      // user has the corresponding permission, but it isn't.
      // We temporarily switch to UID 1 to 'harvest' all comment links.
      if (!isset($user1)) {
        $user1 = user_load(1);
      }
      $saved_user = $user;
      session_save_session(FALSE);
      $user = $user1;

      // With UID 1 we call hook_link(). This should give us the full set of
      // links that the site admin sees.
      $admin_links = module_invoke_all('link', 'comment', $comment, array_key_exists('comment_parent', $links));

      $user = $saved_user;
      session_save_session(TRUE);

      // Remove the links from $admin_links that are not in the reduced
      // set of $required_links AND not available to the current user anyway.
      // Afterwards, $admin_links should have the same content as $links, plus
      // one or more additional links from the original set in $required_links.
      foreach ($admin_links as $key => $target) {
        if (!in_array($key, $required_keys) && !array_key_exists($key, $links)) {
          unset($admin_links[$key]);
        }
      }

      // As the real current user, call hook_link_alter on the admin links.
      // First we set a static variable so that the next time this function is
      // called it will store a copy of the links at their current state. Then
      // we pull those links which have not been link_alter'ed by any modules
      // that come after forum_access.
      $recursing = TRUE;
      drupal_alter('link', $admin_links, $node, $comment);
      $recursing = FALSE;
      // Now return the stored links.
      $links = $stored_links;
    }

This was triggered by a link_alter in one of our modules:

   if (isset($comment)) {
    unset($links['comment_reply']);
  }

So ... not a bug in Flag.

Over to the forum_access module - what's the justification for switching to User 1 in the middle of a request? Can this be re-architected?

Hi salvis,

Indeed, it is a tricky problem you are trying to solve.

The reason why using UID1 when generating the links breaks Flag is this: Flags can be configured to be per-person per-item, not just per-item ... think bookmarking a comment, or marking it as a favourite. The forum_access module replaces these per-person flags with uid1's flags.

So, for example, User 1 marks a comment as favourite. Suddenly, that comment is now visually favourited by all users on the site. Additionally, users can no longer favourite that comment, because clicking the link takes them to the unflag action. Further, all users get administrator access to unflag an item because the access control check is done by drupal_get_token and drupal_valid_token, both of which use the UID from the session.

RE fixing this in D7 first: There's no report of users having this problem in D7 in the flag issue queue, and if we're not swapping to UID1 when generating the links then I don't see why it would be a problem.

On my D6 site, I just got bitten by this again on a different site implementing "like this" functionality. I was crawling through the source and I had this strange feeling of deja-vou. then I remembered this issue ...

I've edited for clarity