Add mozsvc.user module for access to auth backends

VERIFIED FIXED

Status

Cloud Services
Server: Core
VERIFIED FIXED
7 years ago
6 years ago

People

(Reporter: rfkelly, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
Created attachment 578960 [details]
python file implementing mozsvc.user module

This adds a module "mozsvc.user" which provides some conveniences for using the server-core auth backends:

    * user database backend loaded as a plugin from "auth" config section
    * user database backend available as request.registry["auth"]
    * function mozsvc.user.authenticate() to easily auth against the configured backend
    * authenticated user's data as a dict at request.user

The idea being that you can include this into your config, and just use "request.user" to access the current user's data in the same way as in server-core (and in the accountportal refactor)

I'm tempted to have this install a default AuthenticationPolicy and AuthorizationPolicy as well, but let's start small.  Thoughts?
Attachment #578960 - Flags: review?(telliott)
(Reporter)

Comment 1

7 years ago
Created attachment 578961 [details]
python file with tests for mozsvc.user

(Since these are new files, submitting them as a patch was just ugly)
Comment on attachment 578960 [details]
python file implementing mozsvc.user module

Looks good. My one concern would be this section:

    # Store the user dict on the request, and return it for conveience.
    if getattr(request, "user", None) is None:
        request.user = user

If there's already a user object, wouldn't we usually want to replace it with this one, which is more likely to have the information we need? I'm trying to imagine a scenario where this is behavior that doesn't get us into trouble. Seems like we should either replace it, or be throwing an error.
Attachment #578960 - Flags: review?(telliott) → review+
(Reporter)

Comment 3

7 years ago
The idea is that if there's already a user object, we update it in place rather than replacing it with a new one.  Here are the relevant three bits, with the in-between code removed:

    # Update an existing user object if one exists on the request.
    user = getattr(request, "user", None)
    if user is None:
        user = {}

    # Authenticate against the configured backend.
    if not auth.authenticate_user(user, credentials, attrs):
        return False

    # Store the user dict on the request, and return it for conveience.
    if getattr(request, "user", None) is None:
        request.user = user

I suppose I could avoid re-checking for the existence of request.user, and just do "request.user = user" at the in either case.  This should be safe since is it would be just replacing request.user with the same object, updated in-place.
yeah, you're right. I missed that you'd already grabbed the request user object and thought you were starting from scratch. All good.
(Reporter)

Comment 5

7 years ago
Committed in https://github.com/mozilla-services/mozservices/commit/a4c7825d06f77c6244f391212830d2cead13539f
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Replaced TokenServer code.
Dev to consider ripping out the code.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.