• 
      

    Prevent non-superusers from modifying site settings

    Review Request #8330 — Created Aug. 15, 2016 and submitted

    Information

    Review Board
    release-2.5.x
    e999957...

    Reviewers

    Previously, any staff member (superuser or non-superuser) could change
    any of the site settings. We now prevent all non-superuser staff members
    from accessing site settings views so that they cannot. A new decorator
    (similar to Django's staff_member_required) has been added to
    accomplish this.

    • Manually verified that superusers can still change settings.
    • Manually verified that non-superusers are shown a permission denied
      page.
    • Manually verified that unauthenticated users are shown a login form.

    Description From Last Updated

    This is no longer correct. It should probably also mention that users should use @login_required first.

    daviddavid

    I think it's a little confusing to redirect to the login page if a user is already logged in. Can …

    daviddavid

    It's only luck that's making flake8 not complain about this. Can we put all the arguments on the next line …

    daviddavid
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/admin/views.py
          reviewboard/admin/decorators.py
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/admin/views.py
          reviewboard/admin/decorators.py
      
      
    2. 
        
    david
    1. 
        
    2. reviewboard/admin/decorators.py (Diff revision 1)
       
       
      Show all issues

      I think it's a little confusing to redirect to the login page if a user is already logged in. Can we instead show a "permission denied" error?

    3. reviewboard/admin/decorators.py (Diff revision 1)
       
       
       
      Show all issues

      It's only luck that's making flake8 not complain about this. Can we put all the arguments on the next line and indented 4 spaces from the return?

    4. 
        
    brennie
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/admin/views.py
          reviewboard/admin/decorators.py
      
      Ignored Files:
          reviewboard/templates/admin/permission_denied.html
          reviewboard/static/rb/css/pages/admin.less
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/admin/views.py
          reviewboard/admin/decorators.py
      
      Ignored Files:
          reviewboard/templates/admin/permission_denied.html
          reviewboard/static/rb/css/pages/admin.less
      
      
    2. 
        
    david
    1. 
        
    2. reviewboard/admin/decorators.py (Diff revisions 1 - 2)
       
       
       
      Show all issues

      This is no longer correct.

      It should probably also mention that users should use @login_required first.

    3. 
        
    brennie
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/admin/views.py
          reviewboard/admin/decorators.py
      
      Ignored Files:
          reviewboard/templates/admin/permission_denied.html
          reviewboard/static/rb/css/pages/admin.less
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/admin/views.py
          reviewboard/admin/decorators.py
      
      Ignored Files:
          reviewboard/templates/admin/permission_denied.html
          reviewboard/static/rb/css/pages/admin.less
      
      
    2. 
        
    david
    1. "Testing done" has an extra "Testing done"

    2. 
        
    brennie
    brennie
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-3.0.x (5480b16)