HUE-701 [jb] Job browser should not show the jobs from other users

Review Request #2042 — Created April 23, 2012 and submitted

Information
romain
old-hue-rw
HUE-701
Reviewers
hue
bcwalrus 
commit 2544f91e2f2141ce4bf215c04206ba6f1411d2bd
Author: Romain Rigaux <romain@cloudera.com>
Date:   Mon Apr 23 17:48:54 2012 -0700

    HUE-701 [jb] Job browser should not show the jobs from other users

:000000 100644 0000000... 66409c5... A	apps/jobbrowser/src/jobbrowser/conf.py
:100644 100644 0995185... 9d9ad12... M	apps/jobbrowser/src/jobbrowser/tests.py
:100644 100644 e6b2aa6... 44879b1... M	apps/jobbrowser/src/jobbrowser/views.py
:100644 100644 ce6edd7... f275c96... M	desktop/conf/pseudo-distributed.ini.tmpl
hadoop tests are also disabled
manual testing
  • 2
  • 0
  • 4
  • 0
  • 6
Description From Last Updated
You can "and" them together. bcwalrus bcwalrus
Great catch romain romain
romain
romain
bcwalrus
bcwalrus 
  1. Looks good. Minor comments below.
  2. apps/jobbrowser/src/jobbrowser/conf.py (Diff revision 1) 
     
     
     
    "Share submitted job info ..."
  3. apps/jobbrowser/src/jobbrowser/views.py (Diff revision 1) 
     
     
    Show all issues 
    The name is not entirely correct. It's owner or admin. Also, it's not required if the sharing is enabled.

Perhaps call this "check_job_permission"?
  4. apps/jobbrowser/src/jobbrowser/views.py (Diff revision 1) 
     
     
    Show all issues 
    """Ensure that the user has access to the job. Assumes that the wrapped function takes a `jobid' param."""
  5. apps/jobbrowser/src/jobbrowser/views.py (Diff revision 1) 
     
     
     
    Show all issues 
    You can "and" them together.
    1. Yes but it won't fit and the first line tests for the config/super user permission. The second tests for the real condition (does this job belongs to the user). So cutting the tests in two seems simpler?
    2. changed to

    if not conf.SHARE_JOBS.get() and not request.user.is_superuser \
      and job.user != request.user.username:
  6. apps/jobbrowser/src/jobbrowser/views.py (Diff revision 1) 
     
     
     
    Nit: " the job: %s" -> " job %s."

(You don't need the `the' here.)
  7. apps/jobbrowser/src/jobbrowser/views.py (Diff revision 2) 
     
     
     
     
     
     
     
     
    Show all issues 
    Would make get_matching_jobs() understand and check job visibility, rather than fixing the caller here. Probably don't need `user_exact'.
    1. Indeed, it should be cleaner now.
  8. 
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    9. 




  

 
















 



 

  

   




romain




   

   

  

 



 















 



 

  

   




romain




   

   

  

 



 















 





 

  

   




bcwalrus




   

   









  

 



 

  


   

   

    
 
bcwalrus


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Looks good. Thanks!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      apps/jobbrowser/src/jobbrowser/views.py
      


       (Diff revision 4)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    if not check_permission or request.user.is_superuser or j.profile.user == request.user)
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




romain




   

   







  

 



 

  


   

   

    
 
romain


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      apps/jobbrowser/src/jobbrowser/views.py
      


       (Diff revision 4)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    Great catch
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




romain




   

   

  

 



 















 



 

  

   




romain




   

   

  

 



 















 



 

  

   




romain




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 







 


 










  

 









     

    

   

  

  

   
   Loading...