• 
      

    Issue 379: Shouldn't access to unpublished review requests.

    Review Request #366 — Created April 23, 2008 and submitted

    Information

    Review Board SVN (deprecated)
    trunk
    379

    Reviewers

    This fix limits access to unpublished review requests. 
    Only the submitter and people who has permission should be able to access the review request. 
    
    I define a decorator maker at reviewboad/accounts/decorators.py, and  decorate review_request views.
    
    Current code uses
     raise HttpResponseForbidden()
    But it should be
     return HttpResponseForbidden("Error Message HTML")
    or
     from django.core.exceptions import PermissionDenied
     raise PermissionDenied
    
    I use HttpResponseForbidden, but error message is hard coded and very cheep. Maybe we can refactor it later to something like "templates/404.html" way of django.http.Http404.
    
    ------------------------------------
    According to David's review, 
     * moved decorators.py to reviewboard/reviews/
     * changed a parameter name from only_unpublic to only_nonpublic
     * changed from returning HttpForbidden to raising Http404 when permission doesn't match
    Thanks for reviwing.
    Tested on my local machine. 
    Post new review request but not publish. Access the urls (/r/<id>/, /r/<id>/diff, and so on) as another user. 
    david
    1. 
        
    2. This is introducing a really ugly cross-dependency between modules.  Maybe create a new reviews/decorators.py?
    3. It might be nice to come up with a shorter name.  Maybe just owner_required?
    4. /trunk/reviewboard/accounts/decorators.py (Diff revision 1)
       
       
       
       
       
       
       
      This should document the usage of perms and only_unpublic
    5. Don't need this blank line.
    6. Or this one.
    7. /trunk/reviewboard/accounts/decorators.py (Diff revision 1)
       
       
       
       
      I think I'd prefer "only_nonpublic" instead of "only_unpublic"
    8. I think this makes a little more sense as 404 instead of forbidden.
    9. 
        
    david
    1. Looks good now.  Committed as r1337.  Thanks!
    2.