Remove some code duplication

h/accounts/__init__.py

@@ -44,26 +44,37 @@ def auth_domain(request):
     return request.registry.settings.get('h.auth_domain', request.domain)
 
 
-def authenticated_user(request):
-    """Return the authenticated user or None.
+def get_user(userid, request):
+    """Return the User object for the given userid, or None.
 
-    :rtype: h.accounts.models.User or None
+    This will also return None if the given userid is None, if it isn't a valid
+    userid, if its domain doesn't match the site's domain, or if there's just
+    no user with that userid.
 
     """
-    if not request.authenticated_userid:
+    if userid is None:
         return None
 
     try:
-        parts = util.split_user(request.authenticated_userid)
+        parts = util.split_user(userid)
     except ValueError:
-        return None
+        return
 
     if parts['domain'] != request.auth_domain:
         return None
 
     return models.User.get_by_username(parts['username'])
 
 
+def authenticated_user(request):
+    """Return the authenticated user or None.
+
+    :rtype: h.accounts.models.User or None
+
+    """
+    return get_user(request.authenticated_userid, request)
+
+
 def includeme(config):
     """A local identity provider."""
     config.add_request_method(auth_domain, name='auth_domain', reify=True)

h/auth.py

@@ -42,6 +42,7 @@
 
 from .interfaces import IClientFactory
 from .oauth import JWT_BEARER
+from h import accounts
 from h import util
 from h.accounts import models
 from h.api import groups
@@ -137,12 +138,7 @@ def groupfinder(userid, request):
     """
     principals = set()
 
-    try:
-        username = util.split_user(userid)['username']
-    except ValueError:
-        return
-
-    user = models.User.get_by_username(username)
+    user = accounts.get_user(userid, request)
     if user is None:
         return
 

h/test/auth_test.py

@@ -155,47 +155,43 @@ def test_get_client_bad_secret(config):
 
 
 # The fixtures required to mock all of groupfinder()'s dependencies.
-groupfinder_fixtures = pytest.mark.usefixtures('models', 'groups')
+groupfinder_fixtures = pytest.mark.usefixtures('accounts', 'groups')
 
 
 @groupfinder_fixtures
-def test_groupfinder_returns_no_principals(models):
+def test_groupfinder_returns_no_principals(accounts):
     """It should return only [] by default.
 
     If the request has no client and the user is not an admin or staff member
     nor a member of any group, it should return no additional principals.
 
     """
-    models.User.get_by_username.return_value = MagicMock(
-        admin=False, staff=False)
+    accounts.get_user.return_value = MagicMock(admin=False, staff=False)
 
     assert auth.groupfinder("acct:jiji@hypothes.is", Mock()) == []
 
 
 @groupfinder_fixtures
-def test_groupfinder_with_admin_user(models):
+def test_groupfinder_with_admin_user(accounts):
     """If the user is an admin it should return "group:__admin__"."""
-    models.User.get_by_username.return_value = MagicMock(
-        admin=True, staff=False)
+    accounts.get_user.return_value = MagicMock(admin=True, staff=False)
 
     assert "group:__admin__" in auth.groupfinder(
         "acct:jiji@hypothes.is", Mock())
 
 
 @groupfinder_fixtures
-def test_groupfinder_with_staff_user(models):
+def test_groupfinder_with_staff_user(accounts):
     """If the user is staff it should return a "group:__staff__" principal."""
-    models.User.get_by_username.return_value = MagicMock(
-        admin=False, staff=True)
+    accounts.get_user.return_value = MagicMock(admin=False, staff=True)
 
     assert "group:__staff__" in auth.groupfinder(
         "acct:jiji@hypothes.is", Mock())
 
 
 @groupfinder_fixtures
-def test_groupfinder_admin_and_staff(models):
-    models.User.get_by_username.return_value = MagicMock(
-        admin=True, staff=True)
+def test_groupfinder_admin_and_staff(accounts):
+    accounts.get_user.return_value = MagicMock(admin=True, staff=True)
 
     principals = auth.groupfinder("acct:jiji@hypothes.is", Mock())
 
@@ -204,11 +200,11 @@ def test_groupfinder_admin_and_staff(models):
 
 
 @groupfinder_fixtures
-def test_groupfinder_calls_group_principals(models, groups):
+def test_groupfinder_calls_group_principals(accounts, groups):
     auth.groupfinder("acct:jiji@hypothes.is", Mock())
 
     groups.group_principals.assert_called_once_with(
-        models.User.get_by_username.return_value)
+        accounts.get_user.return_value)
 
 
 @groupfinder_fixtures
@@ -300,8 +296,8 @@ def test_effective_principals_calls_groupfinder_with_userid_and_request():
 
 
 @pytest.fixture
-def models(request):
-    patcher = patch('h.auth.models', autospec=True)
+def accounts(request):
+    patcher = patch('h.auth.accounts', autospec=True)
     request.addfinalizer(patcher.stop)
     return patcher.start()