HUE-7624 [core] Support multi-authentication with AllowFirstUserDjangoBackend and LdapBackend

Review Request #12028 - Created Nov. 15, 2017 and submitted

Ying Chen
enricoberti, jgauthier, johan, ranade, romain, weixia
commit 86bb58527d3009844ab0ceec2a91716b2856e7f1 
Author: Ying Chen <>
Date:   Tue Nov 14 18:55:12 2017 -0800

    HUE-7624 [core] Support multi-authentication with AllowFirstUserDjangoBackend and LdapBackend

:100644 100644 d29cf06928... b1747a5faf... M    desktop/core/src/desktop/auth/
:100644 100644 0ba80148cf... 06659f224a... M    desktop/core/src/desktop/auth/
:100644 100644 80c0318ee2... d38cfcf346... M    desktop/core/src/desktop/auth/

  • 0
  • 0
  • 11
  • 0
  • 11
Description From Last Updated
  1. Better commit message?


    [core] Support multi-authentication with AllowFirstUserDjangoBackend and LdapBackend

  2. desktop/core/src/desktop/auth/ (Diff revision 2)

    Could we create a new variable to explain more what we are checking for?

    What are we trying to do here? (any change to only require a request.POST.get('server') == 'LDAP')


    is_ldap_multi_backend = 'server' not in request.POST or request.POST['server'] == 'LDAP'):

    What are the possible values for request.POST['server']?

    1. When the very first time loading login page 'server' is not in request.POST. And LDAP is selected by default.
  3. desktop/core/src/desktop/auth/ (Diff revision 2)

    Is there a way to move the logic to get_server_choices() ?

  1. Nice, getting much cleaner!

  2. desktop/core/src/desktop/auth/ (Diff revision 3)

    Don't we already have a get_backend_names() function cf. the other file?

    1. Can't import from auth.views because of mutual imports. Move it from auth.views to auth.forms
  3. desktop/core/src/desktop/auth/ (Diff revision 3)

    list is a builtin Python, use 'auth_choice'?

  4. desktop/core/src/desktop/auth/ (Diff revision 3)

    Create a helper methode so that we don't duplicate the logic in the other file?

  1. Really nice!!

    Do you think we could add a new test class to test the multi auth?

    e.g. def test_login(self)
    with ('AllowFirstUserDjangoBackend', 'LdapBackend')

    That way we know if we regress later.

  2. desktop/core/src/desktop/auth/ (Diff revision 5)

    nit: newline before?

  1. Really nice!! Just some nits and one question

  2. Is this new tests or can be removed?

  3. 'mb' or just 'm' or 'multiple'? (clearer)

  4. desktop/core/src/desktop/auth/ (Diff revision 6)
  1. Really nice!

  2. Nit: space after ,

Review request changed

Status: Closed (submitted)