Allow access to public Local Sites by anonymous users.

Review Request #5357 — Created Jan. 30, 2014 and submitted

Information

Review Board
release-1.7.x

Reviewers

Allow access to public Local Sites by anonymous users.

Local Sites were recently given a flag to control whether they could be
publicly accessed. This still required the user to be logged in, though.

This change allows access to public Local Sites by anonymous users, when
anonymous access is turned on for the installation.

A large part of this change is just cleaning up reviews/views.py. Nearly
all the functions either did their own LocalSite checks or assumed
_find_review_request would do it. We now consolidate the logic in a
decorator.

The new logic works like this:

  • If the user is anonymous, and the site is private, they will get
    redirected to the Login page.

  • If the user is anonymous, and the site is public, they will be able
    to access the page's view (which still may deny them for other
    reasons, of course).

  • If the user is logged in, and the site is private, and they don't
    have access, they will get a Permission Denied error.

  • If the user is logged in, and the site is public, they will be able
    to access the page's view.

There's also a better experience for the dashboard. Now, whether using
LocalSites or not, anonymous users will get redirected to the All Review
Requests page instead of the Dashboard by default. Either page will then
be able to do their own checks, showing a Permission Denied or login box
as appropriate, given the settings.

Tested each of the above scenarios.

Tested all affected pages and didn't see any problems.

Description From Last Updated

This could just be {% trans %} to save a few lines.

daviddavid
david
  1. I kind of wish you'd pull apart the check_local_site_access refactor and the anonymous user stuff into two changes.

    1. I'll redo it if you want me to.

    2. Not critical unless you want it for bisect purposes.

  2. reviewboard/templates/permission_denied.html (Diff revision 1)
     
     
     
     
    Show all issues

    This could just be {% trans %} to save a few lines.

    1. {% trans %} is best for text intended to just be one line. {% blocktrans %} is for more paragraph-level text. This is a paragraph, and even though it's currently only one line, I feel it's more appropriate to use {% blocktrans %} for it. It's also more consistent with all our other paragraphs.

    2. While it's true that {% blocktrans %} is often more convenient for longer paragraphs than {% trans %}, I don't think there's any semantic meaning to it (the way there might be between <span> and <p>, for instance). I think it's better to have a smaller template.

  3. 
      
chipx86
david
  1. Ship It!

  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Loading...