[Pulp-list] Cleaning up user authentication

[Nick Coghlan]

I'm currently looking into patching Pulp to enable Kerberos logins to
the REST API, and noticed that the current authentication handling
mechanism is a little less than elegant (the comments in the code don't
put it quite so politely).

I figured I would share some thoughts on one possible way to clean it up.

First, some design goals:

1. Don't check authentication methods that aren't configured (i.e. if
the oauth fields aren't set, don't even check for oauth authentication)
2. Make it easy to add new authentication methods (like web server
preauthentication).
3. Ensure each authenticator can customise the auth failure message when
appropriate

The approach that comes to mind is to replace the current if/elif chain
in pulp.server.webservices.controllers.decorators.auth_required with:

    try:
        user = get_authenticated_user()
    except AuthenticationFailed as ex: # AuthenticationFailed, ex to
support 2.4
        return self.unauthorised(str(ex))

get_authenticated_user() would just be a search loop that did the following:

    def get_authenticated_user():
        for authenticate_user in <registered authentication functions>:
            user = authenticate_user()
            if user is not None:
                return user
        raise AuthenticationFailed('Authentication Required')

And an initial list of authentication functions:

    def password_authentication():
        username, password = http.username_password()
        if username is None:
            return None
        user = check_username_password(username, password)
        if user is None:
            raise AuthenticationFailed('Invalid username or password')
        return user

    def user_cert_authentication(cert_checker):
        cert_pem = http.ssl_client_cert()
        if cert_pem is None:
            return None
        return check_user_cert(cert_pem)

    def consumer_cert_authentication(cert_checker):
        cert_pem = http.ssl_client_cert()
        if cert_pem is None:
            return None
        return check_consumer_cert(cert_pem)

    # Only install this one if oauth is configured
    def oauth_authentication():
        username = http.request_info('HTTP_PULP_USER')
        auth = http.http_authorization()
        cert_pem = http.ssl_client_cert()
        if username is None or auth is None:
            if cert_pem is not None:
                raise AuthenticationFailure('Invalid SSL Certificate')
            return None
        meth = http.request_info('REQUEST_METHOD')
        url = http.request_url()
        query = http.request_info('QUERY_STRING')
        user = check_oauth(username, meth, url, auth, query)
        if user is None:
            raise AuthenticationFailure('Invalid OAuth Credentials')
        return user

With such a setup, it would also be possible to change the way LDAP is
handled to install a different password authenticator when it is
enabled, so the check for whether or not to fallback to an LDAP lookup
doesn't need to be repeated on each login attempt.

I haven't gone into detail on how the list of authentication functions
would get populated, since I don't know enough about the startup and
configuration sequence for the server to sensibly comment on that aspect.

Anyway, hopefully these thoughts might prove useful if anyone decides to
bite the bullet and actually clean up the authentication handling code :)

Cheers,
Nick.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=831937

-- 
Nick Coghlan
Red Hat Infrastructure Engineering & Development, Brisbane