mirror of
https://github.com/cisagov/manage.get.gov.git
synced 2025-05-15 09:07:02 +02:00
oidc error on init logs error; oidc re-inits on request if in error state
This commit is contained in:
parent
d4c47cc365
commit
1df7dc48df
2 changed files with 46 additions and 24 deletions
|
@ -82,14 +82,14 @@ class ViewsTest(TestCase):
|
||||||
# mock
|
# mock
|
||||||
mock_client.callback.side_effect = self.user_info
|
mock_client.callback.side_effect = self.user_info
|
||||||
# test
|
# test
|
||||||
with patch("djangooidc.views.requires_step_up_auth", return_value=False), less_console_noise():
|
with patch("djangooidc.views._requires_step_up_auth", return_value=False), less_console_noise():
|
||||||
response = self.client.get(reverse("openid_login_callback"))
|
response = self.client.get(reverse("openid_login_callback"))
|
||||||
# assert
|
# assert
|
||||||
self.assertEqual(response.status_code, 302)
|
self.assertEqual(response.status_code, 302)
|
||||||
self.assertEqual(response.url, reverse("logout"))
|
self.assertEqual(response.url, reverse("logout"))
|
||||||
|
|
||||||
def test_login_callback_no_step_up_auth(self, mock_client):
|
def test_login_callback_no_step_up_auth(self, mock_client):
|
||||||
"""Walk through login_callback when requires_step_up_auth returns False
|
"""Walk through login_callback when _requires_step_up_auth returns False
|
||||||
and assert that we have a redirect to /"""
|
and assert that we have a redirect to /"""
|
||||||
with less_console_noise():
|
with less_console_noise():
|
||||||
# setup
|
# setup
|
||||||
|
@ -98,14 +98,14 @@ class ViewsTest(TestCase):
|
||||||
# mock
|
# mock
|
||||||
mock_client.callback.side_effect = self.user_info
|
mock_client.callback.side_effect = self.user_info
|
||||||
# test
|
# test
|
||||||
with patch("djangooidc.views.requires_step_up_auth", return_value=False), less_console_noise():
|
with patch("djangooidc.views._requires_step_up_auth", return_value=False), less_console_noise():
|
||||||
response = self.client.get(reverse("openid_login_callback"))
|
response = self.client.get(reverse("openid_login_callback"))
|
||||||
# assert
|
# assert
|
||||||
self.assertEqual(response.status_code, 302)
|
self.assertEqual(response.status_code, 302)
|
||||||
self.assertEqual(response.url, "/")
|
self.assertEqual(response.url, "/")
|
||||||
|
|
||||||
def test_requires_step_up_auth(self, mock_client):
|
def test_requires_step_up_auth(self, mock_client):
|
||||||
"""Invoke login_callback passing it a request when requires_step_up_auth returns True
|
"""Invoke login_callback passing it a request when _requires_step_up_auth returns True
|
||||||
and assert that session is updated and create_authn_request (mock) is called."""
|
and assert that session is updated and create_authn_request (mock) is called."""
|
||||||
with less_console_noise():
|
with less_console_noise():
|
||||||
# Configure the mock to return an expected value for get_step_up_acr_value
|
# Configure the mock to return an expected value for get_step_up_acr_value
|
||||||
|
@ -114,12 +114,12 @@ class ViewsTest(TestCase):
|
||||||
request = self.factory.get("/some-url")
|
request = self.factory.get("/some-url")
|
||||||
request.session = {"acr_value": ""}
|
request.session = {"acr_value": ""}
|
||||||
# Ensure that the CLIENT instance used in login_callback is the mock
|
# Ensure that the CLIENT instance used in login_callback is the mock
|
||||||
# patch requires_step_up_auth to return True
|
# patch _requires_step_up_auth to return True
|
||||||
with patch("djangooidc.views.requires_step_up_auth", return_value=True), patch(
|
with patch("djangooidc.views._requires_step_up_auth", return_value=True), patch(
|
||||||
"djangooidc.views.CLIENT.create_authn_request", return_value=MagicMock()
|
"djangooidc.views.CLIENT.create_authn_request", return_value=MagicMock()
|
||||||
) as mock_create_authn_request:
|
) as mock_create_authn_request:
|
||||||
login_callback(request)
|
login_callback(request)
|
||||||
# create_authn_request only gets called when requires_step_up_auth is True
|
# create_authn_request only gets called when _requires_step_up_auth is True
|
||||||
# and it changes this acr_value in request.session
|
# and it changes this acr_value in request.session
|
||||||
# Assert that acr_value is no longer empty string
|
# Assert that acr_value is no longer empty string
|
||||||
self.assertNotEqual(request.session["acr_value"], "")
|
self.assertNotEqual(request.session["acr_value"], "")
|
||||||
|
@ -127,7 +127,7 @@ class ViewsTest(TestCase):
|
||||||
mock_create_authn_request.assert_called_once()
|
mock_create_authn_request.assert_called_once()
|
||||||
|
|
||||||
def test_does_not_requires_step_up_auth(self, mock_client):
|
def test_does_not_requires_step_up_auth(self, mock_client):
|
||||||
"""Invoke login_callback passing it a request when requires_step_up_auth returns False
|
"""Invoke login_callback passing it a request when _requires_step_up_auth returns False
|
||||||
and assert that session is not updated and create_authn_request (mock) is not called.
|
and assert that session is not updated and create_authn_request (mock) is not called.
|
||||||
|
|
||||||
Possibly redundant with test_login_callback_requires_step_up_auth"""
|
Possibly redundant with test_login_callback_requires_step_up_auth"""
|
||||||
|
@ -136,12 +136,12 @@ class ViewsTest(TestCase):
|
||||||
request = self.factory.get("/some-url")
|
request = self.factory.get("/some-url")
|
||||||
request.session = {"acr_value": ""}
|
request.session = {"acr_value": ""}
|
||||||
# Ensure that the CLIENT instance used in login_callback is the mock
|
# Ensure that the CLIENT instance used in login_callback is the mock
|
||||||
# patch requires_step_up_auth to return False
|
# patch _requires_step_up_auth to return False
|
||||||
with patch("djangooidc.views.requires_step_up_auth", return_value=False), patch(
|
with patch("djangooidc.views._requires_step_up_auth", return_value=False), patch(
|
||||||
"djangooidc.views.CLIENT.create_authn_request", return_value=MagicMock()
|
"djangooidc.views.CLIENT.create_authn_request", return_value=MagicMock()
|
||||||
) as mock_create_authn_request:
|
) as mock_create_authn_request:
|
||||||
login_callback(request)
|
login_callback(request)
|
||||||
# create_authn_request only gets called when requires_step_up_auth is True
|
# create_authn_request only gets called when _requires_step_up_auth is True
|
||||||
# and it changes this acr_value in request.session
|
# and it changes this acr_value in request.session
|
||||||
# Assert that acr_value is NOT updated by testing that it is still an empty string
|
# Assert that acr_value is NOT updated by testing that it is still an empty string
|
||||||
self.assertEqual(request.session["acr_value"], "")
|
self.assertEqual(request.session["acr_value"], "")
|
||||||
|
@ -155,7 +155,7 @@ class ViewsTest(TestCase):
|
||||||
mock_client.callback.side_effect = self.user_info
|
mock_client.callback.side_effect = self.user_info
|
||||||
mock_auth.return_value = None
|
mock_auth.return_value = None
|
||||||
# test
|
# test
|
||||||
with patch("djangooidc.views.requires_step_up_auth", return_value=False), less_console_noise():
|
with patch("djangooidc.views._requires_step_up_auth", return_value=False), less_console_noise():
|
||||||
response = self.client.get(reverse("openid_login_callback"))
|
response = self.client.get(reverse("openid_login_callback"))
|
||||||
# assert
|
# assert
|
||||||
self.assertEqual(response.status_code, 401)
|
self.assertEqual(response.status_code, 401)
|
||||||
|
|
|
@ -15,15 +15,28 @@ from registrar.models import User
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
try:
|
CLIENT = None
|
||||||
|
|
||||||
|
|
||||||
|
def _initialize_client():
|
||||||
|
"""Initialize the OIDC client. Exceptions are allowed to raise
|
||||||
|
and will need to be caught."""
|
||||||
|
global CLIENT
|
||||||
# Initialize provider using pyOICD
|
# Initialize provider using pyOICD
|
||||||
OP = getattr(settings, "OIDC_ACTIVE_PROVIDER")
|
OP = getattr(settings, "OIDC_ACTIVE_PROVIDER")
|
||||||
CLIENT = Client(OP)
|
CLIENT = Client(OP)
|
||||||
logger.debug("client initialized %s" % CLIENT)
|
logger.debug("Client initialized: %s" % CLIENT)
|
||||||
|
|
||||||
|
|
||||||
|
# Initialize CLIENT
|
||||||
|
try:
|
||||||
|
_initialize_client()
|
||||||
except Exception as err:
|
except Exception as err:
|
||||||
CLIENT = None # type: ignore
|
# In the event of an exception, log the error and allow the app load to continue
|
||||||
logger.warning(err)
|
# without the OIDC Client. Subsequent login attempts will attempt to initialize
|
||||||
logger.warning("Unable to configure OpenID Connect provider. Users cannot log in.")
|
# again if Client is None
|
||||||
|
logger.error(err)
|
||||||
|
logger.error("Unable to configure OpenID Connect provider. Users cannot log in.")
|
||||||
|
|
||||||
|
|
||||||
def error_page(request, error):
|
def error_page(request, error):
|
||||||
|
@ -55,12 +68,14 @@ def error_page(request, error):
|
||||||
|
|
||||||
def openid(request):
|
def openid(request):
|
||||||
"""Redirect the user to an authentication provider (OP)."""
|
"""Redirect the user to an authentication provider (OP)."""
|
||||||
# If the session reset because of a server restart, attempt to login again
|
global CLIENT
|
||||||
request.session["acr_value"] = CLIENT.get_default_acr_value()
|
|
||||||
|
|
||||||
request.session["next"] = request.GET.get("next", "/")
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
# If the CLIENT is none, attempt to reinitialize before handling the request
|
||||||
|
if CLIENT is None:
|
||||||
|
_initialize_client()
|
||||||
|
request.session["acr_value"] = CLIENT.get_default_acr_value()
|
||||||
|
request.session["next"] = request.GET.get("next", "/")
|
||||||
|
# Create the authentication request
|
||||||
return CLIENT.create_authn_request(request.session)
|
return CLIENT.create_authn_request(request.session)
|
||||||
except Exception as err:
|
except Exception as err:
|
||||||
return error_page(request, err)
|
return error_page(request, err)
|
||||||
|
@ -68,12 +83,16 @@ def openid(request):
|
||||||
|
|
||||||
def login_callback(request):
|
def login_callback(request):
|
||||||
"""Analyze the token returned by the authentication provider (OP)."""
|
"""Analyze the token returned by the authentication provider (OP)."""
|
||||||
|
global CLIENT
|
||||||
try:
|
try:
|
||||||
|
# If the CLIENT is none, attempt to reinitialize before handling the request
|
||||||
|
if CLIENT is None:
|
||||||
|
_initialize_client()
|
||||||
query = parse_qs(request.GET.urlencode())
|
query = parse_qs(request.GET.urlencode())
|
||||||
userinfo = CLIENT.callback(query, request.session)
|
userinfo = CLIENT.callback(query, request.session)
|
||||||
# test for need for identity verification and if it is satisfied
|
# test for need for identity verification and if it is satisfied
|
||||||
# if not satisfied, redirect user to login with stepped up acr_value
|
# if not satisfied, redirect user to login with stepped up acr_value
|
||||||
if requires_step_up_auth(userinfo):
|
if _requires_step_up_auth(userinfo):
|
||||||
# add acr_value to request.session
|
# add acr_value to request.session
|
||||||
request.session["acr_value"] = CLIENT.get_step_up_acr_value()
|
request.session["acr_value"] = CLIENT.get_step_up_acr_value()
|
||||||
return CLIENT.create_authn_request(request.session)
|
return CLIENT.create_authn_request(request.session)
|
||||||
|
@ -86,13 +105,16 @@ def login_callback(request):
|
||||||
else:
|
else:
|
||||||
raise o_e.BannedUser()
|
raise o_e.BannedUser()
|
||||||
except o_e.NoStateDefined as nsd_err:
|
except o_e.NoStateDefined as nsd_err:
|
||||||
|
# In the event that a user is in the middle of a login when the app is restarted,
|
||||||
|
# their session state will no longer be available, so redirect the user to the
|
||||||
|
# beginning of login process without raising an error to the user.
|
||||||
logger.warning(f"No State Defined: {nsd_err}")
|
logger.warning(f"No State Defined: {nsd_err}")
|
||||||
return redirect(request.session.get("next", "/"))
|
return redirect(request.session.get("next", "/"))
|
||||||
except Exception as err:
|
except Exception as err:
|
||||||
return error_page(request, err)
|
return error_page(request, err)
|
||||||
|
|
||||||
|
|
||||||
def requires_step_up_auth(userinfo):
|
def _requires_step_up_auth(userinfo):
|
||||||
"""if User.needs_identity_verification and step_up_acr_value not in
|
"""if User.needs_identity_verification and step_up_acr_value not in
|
||||||
ial returned from callback, return True"""
|
ial returned from callback, return True"""
|
||||||
step_up_acr_value = CLIENT.get_step_up_acr_value()
|
step_up_acr_value = CLIENT.get_step_up_acr_value()
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue