HUE-878: [desktop] Add a remote user backend to better support running Hue behind a proxy server

Review Request #2401 - Created Oct. 22, 2012 and submitted

Joey Echeverria
old-hue
HUE-878
hue
Created a simple middleware to fix a bug in Django's built-in remote user middleware that properly sets the header name to have the HTTP_ prefix and added a backend that authenticates or creates a user given nothing but the header. I also had to modify the settings.py to load the new middleware class.
I did some testing on a one node virtual cluster running CDH4.1. I applied the patch, set the backend to desktop.auth.backend.RemoteUserDjangoBackend and then used a Firefox add on to set the REMOTE_USER header as if the request came through a proxy. I then changed the backend back to the default one and verified that setting the header had no effect.
  • 2
  • 0
  • 5
  • 0
  • 7
Description From Last Updated
trailing space Romain Rigaux
Need a space before. Romain Rigaux
  1. Looks great to me. Just wondering if there is a workaround without a new middleware?
    1. I don't think so. You need something that can read the request header and call the backend with the username pulled from there. Here's Django's documentation for it:
      
      https://docs.djangoproject.com/en/1.4/howto/auth-remote-user/
  2. is_super = User.objects.exist() 
    
    ?
    1. There is no exists() method in the earlier version of Hue I'm applying my patches to for testing. Is it ok to leave it like this and I'll file a JIRA to fix all of the places where exists() could be used in trunk?
    2. Exists() is 1.2 so it should work in CDH3. If really not just do it in one line: is_super = User.objects.count() == 0
      
      build/env/bin/hue shell
      from django.contrib.auth.models import User
      
      In [7]: User.objects.exists()
      Out[7]: True
      
  3. desktop/core/src/desktop/middleware.py (Diff revision 1)
     
     
    Can you point to the Django bug?
    1. I haven't been able to find a bug report, but here's the issue. All of the HTTP headers end up in the Request.META in a normalized fashion by being made all upper case and having a HTTP_ prefix added:
      
      https://docs.djangoproject.com/en/1.4/ref/request-response/#django.http.HttpRequest.META
      
      For some reason, the default header for the RemoteUserMiddleware is set to REMOTE_USER, with no HTTP_ prefix. Another nice thing about this trivial middleware class is we could pull the header name from a configuration object so that you could change it if needed.
    2. Ok, can we put a little node about the HTTP prefix problem?
      
      I was wondering also if we should disable it if the auth backend property does not contain RemoteUserDjangoBackend'?
      https://docs.djangoproject.com/en/1.2/topics/http/middleware/#marking-middleware-as-unused
  4. 
      
  1. 
      
  2. Could we add a note that it requires 'HueRemoteUserMiddleware'?
  3. 
      
  1. Sorry I missed the second review, I think that we are good now?
    1. I've got one quick addition. Posting now.
  2. 
      
  1. Small comments:
    
    
    Could we add them as commented in the Hue ini?
    desktop/conf.dist/hue.ini
    desktop/conf/pseudo-distributed.ini.tmpl
    
    e.g.
    [[auth]]
    # ... PamBackend
    # .... HueRemoteUserMiddleware
    ##
    
    And add the same for
    https://issues.cloudera.org//browse/HUE-892 too?
    1. Done. Should I include the one for HUE-892 in this patch, or file a separate JIRA?
    2. I think it is fine to include both of them in this patch.
    3. I'm doing one last round of testing before uploading the latest patch.
  2. desktop/core/src/desktop/middleware.py (Diff revisions 2 - 3)
     
     
    A class comment about the purpose of the Middleware?
  3. desktop/core/src/desktop/middleware.py (Diff revisions 2 - 3)
     
     
    Is the correct name 'HueRemoteUserMiddleware'?
  4. desktop/core/src/desktop/conf.py (Diff revision 3)
     
     
    Probably can say in by the middleware?
    1. I'm not sure I understand your comment. Currently, I'm requiring that when you configure the header, you do the normalization (e.g. Remote-User -> HTTP_REMOTE_USER or Auth-User -> HTTP_AUTH_USER). Are you saying I should change the code to do the normalization and let you configure it with the actual header?
    2. Missing some words, sorry. I think I just wanted to say that maybe we could add that this options is used by the 'HueRemoteUserMiddleware'?
    3. That makes sense. Adding it now.
  5. 
      
  1. Thanks, great stuff!
    
    I am going to push the patch for you. I will take care of the comments below.
  2. trailing space
  3. desktop/core/src/desktop/conf.py (Diff revision 4)
     
     
    Need a space before.
  4. 
      
Review request changed

Status: Closed (submitted)

Loading...