Some tickets are viewable even if they aren't watched, authored or assigned

[estriv]

First, thanks for this plugin. It's a great addition to redmine which makes the software, even through the small change, much more powerfull.

There is a problem with the "allowed_to" and "visible" patch that leads to not expected behaviour: Some issues can be viewed even the viewing user isn't assigned to the issue, created the issue or is watching it. I'm not very familiar with the redmine code/structure or ruby itself but i investigated a bit and I think i found the problem or at least a hint.

In the patch for the "allowed_to" function true is returned when there is an issue with the same tracker id as the current viewed issue (?). So each issue with the same tracker id is visible for the user in this project because in the "visible" patch true is returned when the original "visible" function returns true, what happens for every issue with the same tracker id as one watched issue in this project. I replaced the current code in the issue patch with the following code:

        def visible_with_extwatch?(usr=nil)
          visible = visible_without_extwatch?(usr)
          #return true if visible

          if (usr || User.current).logged?
            uid = (usr || User.current).id
            visible = (self.watched_by?(usr || User.current) || (self.author_id == uid) || (self.assigned_to_id == uid))
            visible |= (usr || User.current).admin?
          end

          logger.error "visible_with_extwatch #{visible}"
          visible
        end

I would be really glad if you could investigate this further because i consider this as a dirty hack which works as a start but doesn't stand all use cases.

[maxrossello]

Hi, here are my comments:

1. you comment a line that preserves a former visibility rule given by visible_without_extwatch?(usr), then you try to reimplement the default implementation by extending the plugin rules here
2. This is btw detrimental for performances. In any case, by commenting the sentence you always loose the original result for every logged in user, because you don't use a "|=" in the first "visibile = ..." sentence within the if.

This makes me think that the problem you complain may depend on a false true given by visible_without_extwatch?(usr), which may be overridden in other plugins of yours as well, and that you try to fix in this plugin. I think that the original plugin code does what it is meant to do: extend visibility when the user is logged in and watches an issue. So it immediately returns true if older conditions do so, and adds a true condition when the user is watching the issue.

I suggest to try to make your tests while disabling all other plugins.

[estriv]

Hi, thanks for your answer.

The problem aren't other plugins, I looked over them and found no other patch which overrides the visibility of tickets.

I know that i completely overwrite the visibility the original function returns. I know that i "kill" the functionality especially from roles by this. Here is the condition of what i expect with the plugin enabled:

          if (usr || User.current).logged?
            if !visible
              return (self.watched_by?(usr || User.current))
            end
            uid = (usr || User.current).id
            roles = (usr || User.current).roles_for_project(self.project).map { |r| r.name }
            visible = self.watched_by?(usr || User.current)
            visible |= (roles.include? 'Manager')
            visible |= (self.author_id == uid) || (self.assigned_to_id == uid)
            visible |= (usr || User.current).admin?
          end

I think i found the line leading to the behaviour that all issues with the same tracker id in a project are visible to a user which isn't member in the project but is able to watch one issue inside the project. To clarify further, the issues are not shown in any list or table but can be viewed if the issue id is known.

Here the code in the user patch:

return true if Issue.where(:project_id => context).watched_by(self).joins(:project => :enabled_modules).where("#{EnabledModule.table_name}.name = 'issue_tracking'").any?

If I understand this correctly, "allowed_to?" returns true whenever an issue in a project exists which is watched by the current user and thus the "visible?" function returns true too, without testing wether the quested issue is really watched or not. I'm having a workaround in mind and will test it later and tell you if it works.

[estriv]

Ok it seems to work. Because the result of the user patch has to be checked further i use an integer instead of a boolean here:

def allowed_to_with_extwatch?(action, context, options={}, &block)
  is_allowed = allowed_to_without_extwatch?(action, context, options, &block)
  return true if is_allowed

  if (options[:watchers].nil? || options[:watchers]) && self.logged? && context && context.is_a?(Project)
    if action.is_a?(Hash)
      if action[:controller] == "issues" && action[:action] == "index"
        return 2 if Issue.where(:project_id => context).watched_by(self).joins(:project => :enabled_modules).where("#{EnabledModule.table_name}.name = 'issue_tracking'").any?
      end 
    elsif action == :view_issues
      return 2 if Issue.where(:project_id => context).watched_by(self).joins(:project => :enabled_modules).where("#{EnabledModule.table_name}.name = 'issue_tracking'").any?
    end 
  end 
  return false
end 

Then i can check for "1" explicitly (what should be true for "true", too) and will check for "watched_by?" in the issue controller. Because any value not 0 is translated to boolean true in general this will not impair other conditionals.

visible = visible_without_extwatch?(usr)
return true if visible == 1
if (usr || User.current).logged?
  visible = self.watched_by?(usr || User.current)
end

I did some tests and now everything seems to be working as expected, maybe you can confirm this? Why do you need the user patch in general (I didn't comprehend your complete plugin and redmine code)?

[maxrossello]

Your reasoning and possible solution makes sense, but I cannot replicate the problem :-)

In a project, I have a watched and a non-watched issue. When I enter "Issues", I can see only the watched issue. If I try to enter the issue via direct URL, using the issue id, then I get an error 403. I am using unmodified code. Any other setting aside?

[maxrossello]

Do you have any permission assigned to Non member role, either administratively and/or in the project? Because, if the user is not a member, then _allowedto should apply a restrictive policy.

Are you using truly original code, to exploit the problem, or did you already modified (usr | User.current) into self.watched_by?(usr)?

[estriv]

Hm, non member and anonymous don't have any permission and are not privileged to anything in any project. I configured redmine so that only managers are able to see issues created by others and developers or reporters can only see their own issues or issues they are assigned to or are watching. I am using redmine 3.3-stable from github.

I already modified (usr || User.current) because I wasn't able to view a watched issue when i didn't.

I will set up a clean redmine installation later, try to reproduce the issue again and report the result.

[maxrossello]

I think it can be something related with changes in redmine 3.3, which I haven't tested properly yet

[estriv]

Hi i tested the problem with a vanilla redmine installation (3.3 and master) with only your plugin installed and was able to recreate the problem. The suggested changes seem to work.

[maxrossello]

Hi, the plugin now explicitly supports Redmine 3.4.13. I needed to apply a workaround to something similar to what you observed, but I am not completely sure it's the same issue. I used an option to discriminate when the allowed_to? :view_issues came from issue.visible? I believe the problem is now solved, please reopen if you don't think so. Many thanks