Skip to content

Commit

Permalink
Add request.auth_domain property
Browse files Browse the repository at this point in the history
This removes some duplication.
  • Loading branch information
seanh committed Oct 20, 2015
1 parent c22efde commit f5d54b9
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 37 deletions.
15 changes: 11 additions & 4 deletions h/accounts/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ def make_staff(username):
raise NoSuchUserError


def auth_domain(request):
"""Return the value of the h.auth_domain config settings.
Falls back on returning request.domain if h.auth_domain isn't set.
"""
return request.registry.settings.get('h.auth_domain', request.domain)


def authenticated_user(request):
"""Return the authenticated user or None.
Expand All @@ -49,17 +58,15 @@ def authenticated_user(request):
except ValueError:
return None

auth_domain = request.registry.settings.get(
'h.auth_domain', request.domain)

if parts['domain'] != auth_domain:
if parts['domain'] != request.auth_domain:
return None

return models.User.get_by_username(parts['username'])


def includeme(config):
"""A local identity provider."""
config.add_request_method(auth_domain, name='auth_domain', reify=True)
config.add_request_method(
authenticated_user, name='authenticated_user', reify=True)

Expand Down
12 changes: 3 additions & 9 deletions h/accounts/test/__init___test.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,7 @@ def test_authenticated_user_returns_None_if_split_user_raises_ValueError(util):

@authenticated_user_fixtures
def test_authenticated_user_returns_None_if_domain_does_not_match(util):
request = mock.Mock(
registry=mock.Mock(settings={'h.auth_domain': 'hypothes.is'})
)
request = mock.Mock(auth_domain='hypothes.is')
util.split_user.return_value = {
'username': 'username', 'domain': 'other'}

Expand All @@ -130,9 +128,7 @@ def test_authenticated_user_returns_None_if_domain_does_not_match(util):
@authenticated_user_fixtures
def test_authenticated_user_calls_get_by_username(util, get_by_username):
"""It should call get_by_username() once with the username."""
request = mock.Mock(
registry=mock.Mock(settings={'h.auth_domain': 'hypothes.is'})
)
request = mock.Mock(auth_domain='hypothes.is')
util.split_user.return_value = {
'username': 'username', 'domain': 'hypothes.is'}

Expand All @@ -144,9 +140,7 @@ def test_authenticated_user_calls_get_by_username(util, get_by_username):
@authenticated_user_fixtures
def test_authenticated_user_returns_user(util, get_by_username):
"""It should return the result from get_by_username()."""
request = mock.Mock(
registry=mock.Mock(settings={'h.auth_domain': 'hypothes.is'})
)
request = mock.Mock(auth_domain='hypothes.is')
util.split_user.return_value = {
'username': 'username', 'domain': 'hypothes.is'}

Expand Down
9 changes: 5 additions & 4 deletions h/accounts/test/views_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ def test_login_no_event_when_validation_fails(loginevent,

@pytest.mark.usefixtures('routes_mapper')
def test_login_redirects_when_validation_succeeds(authn_policy):
request = DummyRequest()
request = DummyRequest(auth_domain='hypothes.is')
authn_policy.authenticated_userid.return_value = None # Logged out
controller = AuthController(request)
controller.form = form_validating_to({"user": FakeUser(username='cara')})
Expand All @@ -235,7 +235,8 @@ def test_login_redirects_when_validation_succeeds(authn_policy):

@pytest.mark.usefixtures('routes_mapper')
def test_login_redirects_to_next_param_when_validation_succeeds(authn_policy):
request = DummyRequest(params={'next': '/foo/bar'})
request = DummyRequest(
params={'next': '/foo/bar'}, auth_domain='hypothes.is')
authn_policy.authenticated_userid.return_value = None # Logged out
controller = AuthController(request)
controller.form = form_validating_to({"user": FakeUser(username='cara')})
Expand All @@ -251,7 +252,7 @@ def test_login_redirects_to_next_param_when_validation_succeeds(authn_policy):
def test_login_event_when_validation_succeeds(loginevent,
authn_policy,
notify):
request = DummyRequest()
request = DummyRequest(auth_domain='hypothes.is')
authn_policy.authenticated_userid.return_value = None # Logged out
elephant = FakeUser(username='avocado')
controller = AuthController(request)
Expand Down Expand Up @@ -316,7 +317,7 @@ def test_logout_response_has_forget_headers(authn_policy):

@pytest.mark.usefixtures('routes_mapper')
def test_login_ajax_returns_status_okay_when_validation_succeeds():
request = DummyRequest(json_body={})
request = DummyRequest(json_body={}, auth_domain='hypothes.is')
controller = AjaxAuthController(request)
controller.form = form_validating_to({'user': FakeUser(username='bob')})

Expand Down
6 changes: 3 additions & 3 deletions h/test/admin_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def test_nipsa_index_with_multiple_nipsad_users(nipsa):

@nipsa_add_fixtures
def test_nipsa_add_calls_nipsa_api_with_userid(nipsa):
request = DummyRequest(params={"add": "kiki"})
request = DummyRequest(params={"add": "kiki"}, auth_domain='example.com')

admin.nipsa_add(request)

Expand All @@ -116,7 +116,7 @@ def test_nipsa_add_calls_nipsa_api_with_userid(nipsa):

@nipsa_add_fixtures
def test_nipsa_add_returns_index(nipsa_index):
request = DummyRequest(params={"add": "kiki"})
request = DummyRequest(params={"add": "kiki"}, auth_domain='example.com')
nipsa_index.return_value = "Keine Bange!"

assert admin.nipsa_add(request) == "Keine Bange!"
Expand All @@ -130,7 +130,7 @@ def test_nipsa_add_returns_index(nipsa_index):
def test_nipsa_remove_calls_nipsa_api_with_userid(nipsa):
request = Mock(
params={"remove": "kiki"},
domain="hypothes.is",
auth_domain="hypothes.is",
registry=Mock(settings={})
)

Expand Down
17 changes: 3 additions & 14 deletions h/test/util_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,11 @@ def test_split_user_no_match():
parts = util.split_user("donkeys")


def test_userid_from_username_uses_auth_domain_setting():
def test_userid_from_username_uses_request_dot_auth_domain():
"""It should use the h.auth_domain setting if set."""
userid = util.userid_from_username(
'douglas',
mock.Mock(
domain='should_not_be_used.com',
registry=mock.Mock(
settings={'h.auth_domain': 'example.com'})))

assert userid == 'acct:douglas@example.com'


def test_userid_from_username_falls_back_on_request_domain():
"""It should use request.domain if there's no h.auth_domain setting."""
userid = util.userid_from_username(
'douglas',
mock.Mock(domain='example.com', registry=mock.Mock(settings={})))
mock.Mock(auth_domain='example.com')
)

assert userid == 'acct:douglas@example.com'
4 changes: 1 addition & 3 deletions h/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,5 @@ def userid_from_username(username, request):
(if we're at domain "hypothes.is").
"""
auth_domain = request.registry.settings.get('h.auth_domain',
request.domain)
return u"acct:{username}@{domain}".format(username=username,
domain=auth_domain)
domain=request.auth_domain)

0 comments on commit f5d54b9

Please sign in to comment.