Edgewall Software

Opened 6 years ago

Last modified 5 years ago

#11166 closed defect

TracLinks to non-existent and forbidden reports should not be followable — at Version 4

Reported by: Ryan J Ollos <ryan.j.ollos@…> Owned by:
Priority: normal Milestone: 1.0.2
Component: report system Version: 1.0-stable
Severity: normal Keywords: permissions, traclinks
Cc: Branch:
Release Notes:
API Changes:

Description (last modified by Ryan J Ollos <ryan.j.ollos@…>)

For example, the non-existent report {10000} is rendered as a followable link. The HTML is:

<a class="report" href="/report/10000">{10000}</a>

but should be:

<a class="missing report">{10000}</a>

resulting in:

{10000}

Also, when the user doesn't have permission to view a report the TracLink is rendered as a link that can be followed, but the behavior should be similar to when the report doesn't exist.

Change History (4)

comment:1 by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

  • b27a1cfd (unrelated refactoring) The Milestone class was being indirectly imported from the roadmap module.
  • 6f0e3cea Add a get_report method and use it to determine how the report link should be formatted. The report link is formatted with the missing class and without an href attribute in the case that the report doesn't exist.
  • 9b6497b6 Utilize get_report method to eliminate code duplication.

The quality of the Report test cases in wiki_syntax.py have been diminished somewhat because every report TracLink is of the "missing" type. It looks like we'll need to add some reports in the report_setup method. I was thinking of adding methods report_create and report_update in order to facilitate that, but wanted to ask for feedback before doing so.

comment:2 by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

  • bb78cb89 (refactoring) Move the return statement out of the try and into an else clause of the try...except.

in reply to:  1 comment:3 by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

Replying to Ryan J Ollos <ryan.j.ollos@…>:

The quality of the Report test cases in wiki_syntax.py have been diminished somewhat because every report TracLink is of the "missing" type. It looks like we'll need to add some reports in the report_setup method. I was thinking of adding methods report_create and report_update in order to facilitate that, but wanted to ask for feedback before doing so.

Instead of adding methods to the ReportModule class, I added a function in the test fixture setup method for creating reports, following along with what Jun did for a test case in the th:BookmarkPlugin.

  • a3725d73 Added test cases for 'non-missing' reports.

So now I have 5 commits pending for this ticket, however commit (4) and (5) could be squashed into commit (2). Commits (1) and (3) should probably continue to stand alone. Should I rebase the changes on a new branch, or just leave them as they are?

comment:4 by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

Description: modified (diff)
Keywords: permissions added
Summary: TracLinks to non-existent reports should have the "missing" class and should not have an href attributeTracLinks to non-existent and forbidden reports should not be followable

I'm expanding scope of this ticket to deal with users not having REPORT_VIEW.

Currently the wiki TracLink is the only TracLink to utilize the forbidden class, but it looks like we could use it here as well.

Note: See TracTickets for help on using tickets.