mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-08-24 00:07:02 -04:00
return 200 to js instead of 302 when logging in
the js fetch function is really not designed to work with 302s. when it receives a 302, it automatically follows it and fetches the next page. This is awkward because I don't want js to do all this in ajax, I want the browser to get the new URL so it can load the page. A better approach is to view the admin endpoint as a more pure API: the js sends a request for authentication to the admin app, and the admin app responds with a 200 indicating success, and then a payload of relevant data with that. The relevant data in this case is "Which URL should I redirect to", it might be the user's list of services page, or it might be a page telling them that their email needs revalidating.
This commit is contained in:
@@ -33,21 +33,22 @@
|
|||||||
});
|
});
|
||||||
})
|
})
|
||||||
.then(response => {
|
.then(response => {
|
||||||
if (response.status === 403){
|
if (response.status === 403) {
|
||||||
// flask will have `flash`ed an error message up
|
// flask will have `flash`ed an error message up
|
||||||
window.location.reload();
|
window.location.reload();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!response.ok) {
|
return response.arrayBuffer()
|
||||||
// probably an internal server error
|
.then(cbor => {
|
||||||
throw Error(response.statusText);
|
return Promise.resolve(window.CBOR.decode(cbor));
|
||||||
}
|
})
|
||||||
|
.catch(() => {
|
||||||
// fetch will already have done the login redirect dance and will at this point be
|
throw Error(response.statusText);
|
||||||
// referring to the final 200 - hopefully to the `/accounts` url or similar. Set the location
|
})
|
||||||
// to trigger a browser navigate to that URL.
|
.then(data => {
|
||||||
window.location.href = response.url;
|
window.location = data.redirect_url;
|
||||||
|
});
|
||||||
})
|
})
|
||||||
.catch(error => {
|
.catch(error => {
|
||||||
console.error(error);
|
console.error(error);
|
||||||
|
|||||||
@@ -97,7 +97,8 @@ def webauthn_complete_authentication():
|
|||||||
|
|
||||||
_complete_webauthn_authentication(user_to_login)
|
_complete_webauthn_authentication(user_to_login)
|
||||||
|
|
||||||
return _verify_webauthn_login(user_to_login)
|
redirect = _verify_webauthn_login(user_to_login)
|
||||||
|
return cbor.encode({'redirect_url': redirect.location}), 200
|
||||||
|
|
||||||
|
|
||||||
def _complete_webauthn_authentication(user):
|
def _complete_webauthn_authentication(user):
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import base64
|
import base64
|
||||||
from unittest.mock import ANY
|
from unittest.mock import ANY, Mock
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from fido2 import cbor
|
from fido2 import cbor
|
||||||
@@ -260,13 +260,12 @@ def test_complete_authentication_checks_credentials(
|
|||||||
platform_admin_user['auth_type'] = 'webauthn_auth'
|
platform_admin_user['auth_type'] = 'webauthn_auth'
|
||||||
mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
|
mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
|
||||||
mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=[webauthn_credential])
|
mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=[webauthn_credential])
|
||||||
# fake returning a 200 just to keep flask happy, normally this'll redirect
|
mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_login', return_value=Mock(location='/foo'))
|
||||||
mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_login', return_value=('ok', 200))
|
|
||||||
|
|
||||||
response = client.post(url_for('main.webauthn_complete_authentication'), data=webauthn_authentication_post_data)
|
response = client.post(url_for('main.webauthn_complete_authentication'), data=webauthn_authentication_post_data)
|
||||||
|
|
||||||
# matches response of verify_webauthn_login
|
assert response.status_code == 200
|
||||||
assert response.data == b'ok'
|
assert cbor.decode(response.data) == {'redirect_url': '/foo'}
|
||||||
|
|
||||||
|
|
||||||
def test_complete_authentication_403s_if_key_isnt_in_users_credentials(
|
def test_complete_authentication_403s_if_key_isnt_in_users_credentials(
|
||||||
@@ -311,8 +310,7 @@ def test_complete_authentication_clears_session(
|
|||||||
platform_admin_user['auth_type'] = 'webauthn_auth'
|
platform_admin_user['auth_type'] = 'webauthn_auth'
|
||||||
mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
|
mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
|
||||||
mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=[webauthn_credential])
|
mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=[webauthn_credential])
|
||||||
# fake returning a 200 just to keep flask happy, normally this'll redirect
|
mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_login', return_value=Mock(location='/foo'))
|
||||||
mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_login', return_value=('ok', 200))
|
|
||||||
|
|
||||||
client.post(url_for('main.webauthn_complete_authentication'), data=webauthn_authentication_post_data)
|
client.post(url_for('main.webauthn_complete_authentication'), data=webauthn_authentication_post_data)
|
||||||
|
|
||||||
@@ -337,8 +335,8 @@ def test_verify_webauthn_login_signs_user_in_signs_user_in(client, mocker, mock_
|
|||||||
|
|
||||||
resp = client.post(url_for('main.webauthn_complete_authentication'))
|
resp = client.post(url_for('main.webauthn_complete_authentication'))
|
||||||
|
|
||||||
assert resp.status_code == 302
|
assert resp.status_code == 200
|
||||||
assert resp.location == url_for('main.show_accounts_or_dashboard', _external=True)
|
assert cbor.decode(resp.data)['redirect_url'] == url_for('main.show_accounts_or_dashboard')
|
||||||
# removes stuff from session
|
# removes stuff from session
|
||||||
with client.session_transaction() as session:
|
with client.session_transaction() as session:
|
||||||
assert 'user_details' not in session
|
assert 'user_details' not in session
|
||||||
@@ -390,8 +388,8 @@ def test_verify_webauthn_login_signs_user_in_sends_revalidation_email_if_needed(
|
|||||||
|
|
||||||
resp = client.post(url_for('main.webauthn_complete_authentication'))
|
resp = client.post(url_for('main.webauthn_complete_authentication'))
|
||||||
|
|
||||||
assert resp.status_code == 302
|
assert resp.status_code == 200
|
||||||
assert resp.location == url_for('main.revalidate_email_sent', _external=True)
|
assert cbor.decode(resp.data)['redirect_url'] == url_for('main.revalidate_email_sent')
|
||||||
|
|
||||||
with client.session_transaction() as session:
|
with client.session_transaction() as session:
|
||||||
# stuff stays in session so we can log them in later when they validate their email
|
# stuff stays in session so we can log them in later when they validate their email
|
||||||
|
|||||||
Reference in New Issue
Block a user