From 5e5b357c7e1df1be48af371636c719165d9a7da8 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Aug 2018 13:15:01 -0400 Subject: [PATCH 01/19] add mechanism for initial modal display; fix financial verification modal display --- atst/app.py | 2 +- atst/routes/requests/requests_form.py | 5 ++- js/index.js | 8 ++++ templates/base.html | 5 +++ templates/requests.html | 53 ++++++++++++--------------- tests/routes/test_request_submit.py | 6 +-- 6 files changed, 44 insertions(+), 35 deletions(-) diff --git a/atst/app.py b/atst/app.py index d095a8bf..4a0f80c4 100644 --- a/atst/app.py +++ b/atst/app.py @@ -68,7 +68,7 @@ def make_flask_callbacks(app): ) g.dev = os.getenv("FLASK_ENV", "dev") == "dev" g.matchesPath = lambda href: re.match("^" + href, request.path) - g.modalOpen = request.args.get("modal", False) + g.modal = request.args.get("modal", None) g.current_user = { "id": "cce17030-4109-4719-b958-ed109dbb87c8", "first_name": "Amanda", diff --git a/atst/routes/requests/requests_form.py b/atst/routes/requests/requests_form.py index d384abdf..15033d47 100644 --- a/atst/routes/requests/requests_form.py +++ b/atst/routes/requests/requests_form.py @@ -4,6 +4,7 @@ from . import requests_bp from atst.domain.requests import Requests from atst.routes.requests.jedi_request_flow import JEDIRequestFlow from atst.models.permissions import Permissions +from atst.models.request_status_event import RequestStatus from atst.domain.exceptions import UnauthorizedError @@ -99,8 +100,8 @@ def requests_submit(request_id=None): request = Requests.get(request_id) Requests.submit(request) - if request.status == "approved": - return redirect("/requests?modal=True") + if request.status == RequestStatus.PENDING_FINANCIAL_VERIFICATION: + return redirect("/requests?modal=pendingFinancialVerification") else: return redirect("/requests") diff --git a/js/index.js b/js/index.js index 59883c65..07da8d42 100644 --- a/js/index.js +++ b/js/index.js @@ -20,7 +20,15 @@ const app = new Vue({ return { modals: { styleguideModal: false, + pendingFinancialVerification: false, } } + }, + mounted: function() { + const modalOpen = document.querySelector("#modalOpen"); + if (modalOpen) { + const modal = modalOpen.getAttribute("data-modal"); + this.modals[modal] = true; + } } }) diff --git a/templates/base.html b/templates/base.html index 8416032f..a29a6c04 100644 --- a/templates/base.html +++ b/templates/base.html @@ -35,6 +35,11 @@ {% include 'footer.html' %} {% block modal %}{% endblock %} + + {% if g.modal %} +
+
+ {% endif %} {% assets "js_all" %} diff --git a/templates/requests.html b/templates/requests.html index 25520ac2..4bb7509b 100644 --- a/templates/requests.html +++ b/templates/requests.html @@ -4,37 +4,32 @@ {% from "components/modal.html" import Modal %} {% from "components/empty_state.html" import EmptyState %} -{% block modal %} - {% if g.modalOpen %} - {% call Modal() %} -

Your request is now approved!

- -

- Your next step is to create a Task Order (T.O.) associated with - JEDI Cloud. Please consult a Contracting Officer (KO) or - Contracting Officer Representative (COR) to help with this step. -

- -

- Once the Task Order (T.O.) has been created, we will need the following - details to create your account. These details will help keep your cloud - usage in sync with your budget. -

- - {{ Alert("You'll need these details: ", - message="

Task Order Number

Contracting Officer: Name, E-mail and Office

" - ) }} - - -
- Close -
- {% endcall %} - {% endif %} -{% endblock %} - {% block content %} + {% call Modal(name='pendingFinancialVerification', dismissable=True) %} +

Your request is now approved!

+ +

+ Your next step is to create a Task Order (T.O.) associated with + JEDI Cloud. Please consult a Contracting Officer (KO) or + Contracting Officer Representative (COR) to help with this step. +

+ +

+ Once the Task Order (T.O.) has been created, we will need the following + details to create your account. These details will help keep your cloud + usage in sync with your budget. +

+ + {{ Alert("You'll need these details: ", + message="

Task Order Number

Contracting Officer: Name, E-mail and Office

" + ) }} + +
+ Close +
+ {% endcall %} + {% if not requests %} {{ EmptyState( diff --git a/tests/routes/test_request_submit.py b/tests/routes/test_request_submit.py index 428e056e..f8e61400 100644 --- a/tests/routes/test_request_submit.py +++ b/tests/routes/test_request_submit.py @@ -1,6 +1,7 @@ import pytest from tests.mocks import MOCK_USER from tests.factories import RequestFactory +from atst.models.request_status_event import RequestStatus def _mock_func(*args, **kwargs): @@ -27,12 +28,11 @@ def test_submit_autoapproved_reviewed_request(monkeypatch, client, user_session) user_session() monkeypatch.setattr("atst.domain.requests.Requests.get", _mock_func) monkeypatch.setattr("atst.domain.requests.Requests.submit", _mock_func) - monkeypatch.setattr("atst.models.request.Request.status", "approved") - # this just needs to send a known invalid form value + monkeypatch.setattr("atst.models.request.Request.status", RequestStatus.PENDING_FINANCIAL_VERIFICATION) response = client.post( "/requests/submit/1", headers={"Content-Type": "application/x-www-form-urlencoded"}, data="", follow_redirects=False, ) - assert "/requests?modal=True" in response.headers["Location"] + assert "/requests?modal=" in response.headers["Location"] From a9dfa005a919a8f96fb15dc1ec3c66afcd7e5162 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Aug 2018 13:53:45 -0400 Subject: [PATCH 02/19] update financial verification copy --- templates/requests.html | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/templates/requests.html b/templates/requests.html index 4bb7509b..879560a2 100644 --- a/templates/requests.html +++ b/templates/requests.html @@ -7,23 +7,22 @@ {% block content %} {% call Modal(name='pendingFinancialVerification', dismissable=True) %} -

Your request is now approved!

+

Request submitted!

- Your next step is to create a Task Order (T.O.) associated with - JEDI Cloud. Please consult a Contracting Officer (KO) or - Contracting Officer Representative (COR) to help with this step. + The next step is to create a Task Order associated with JEDI Cloud. + Please contact a Contracting Officer (KO), Contracting Officer + Representative (COR), or a Financial Manager to help with this step.

- Once the Task Order (T.O.) has been created, we will need the following - details to create your account. These details will help keep your cloud - usage in sync with your budget. + Once the Task Order has been created, you will be asked to provide + details about the task order in the Financial Verification step.

- {{ Alert("You'll need these details: ", - message="

Task Order Number

Contracting Officer: Name, E-mail and Office

" - ) }} +

+ Learn more about the JEDI Task Order and the Financial Verification process. +

Close @@ -42,7 +41,20 @@ {% else %} {{ Alert('Pending Financial Verification', - message="

Your next step is to create a Task Order (T.O.) associated with JEDI Cloud. Please consult a Contracting Officer (KO) or Contracting Officer Representative (COR) to help with this step.

" + message=" +

+ The next step is to create a Task Order associated with JEDI Cloud. + Please contact a Contracting Officer (KO), Contracting Officer + Representative (COR), or a Financial Manager to help with this step. +

+

+ Once the Task Order has been created, you will be asked to provide + details about the task order in the Financial Verification step. +

+

+ Learn more about the JEDI Task Order and the Financial Verification process. +

+ " ) }}
From 5f9b58946576cc37b313bbb2abbaa5dc95c19ac7 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Aug 2018 14:10:17 -0400 Subject: [PATCH 03/19] only show financial verification alert on requests index page if relevant --- atst/routes/requests/index.py | 5 ++++- templates/requests.html | 36 +++++++++++++++++++---------------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/atst/routes/requests/index.py b/atst/routes/requests/index.py index 5e9d26ca..dc1d137e 100644 --- a/atst/routes/requests/index.py +++ b/atst/routes/requests/index.py @@ -3,6 +3,7 @@ from flask import render_template, g, url_for from . import requests_bp from atst.domain.requests import Requests +from atst.models.request_status_event import RequestStatus def map_request(request): @@ -33,4 +34,6 @@ def requests_index(): mapped_requests = [map_request(r) for r in requests] - return render_template("requests.html", requests=mapped_requests) + pending_fv = any(r["status"] == RequestStatus.PENDING_FINANCIAL_VERIFICATION.value for r in mapped_requests) + + return render_template("requests.html", requests=mapped_requests, pending_financial_verification=pending_fv) diff --git a/templates/requests.html b/templates/requests.html index 879560a2..4dc0e865 100644 --- a/templates/requests.html +++ b/templates/requests.html @@ -40,22 +40,26 @@ {% else %} - {{ Alert('Pending Financial Verification', - message=" -

- The next step is to create a Task Order associated with JEDI Cloud. - Please contact a Contracting Officer (KO), Contracting Officer - Representative (COR), or a Financial Manager to help with this step. -

-

- Once the Task Order has been created, you will be asked to provide - details about the task order in the Financial Verification step. -

-

- Learn more about the JEDI Task Order and the Financial Verification process. -

- " - ) }} + {% if pending_financial_verification %} + + {{ Alert('Pending Financial Verification', + message=" +

+ The next step is to create a Task Order associated with JEDI Cloud. + Please contact a Contracting Officer (KO), Contracting Officer + Representative (COR), or a Financial Manager to help with this step. +

+

+ Once the Task Order has been created, you will be asked to provide + details about the task order in the Financial Verification step. +

+

+ Learn more about the JEDI Task Order and the Financial Verification process. +

+ " + ) }} + + {% endif %}
From c0d72cd0d624fd7c087beb9f5d6345b8e9ef6e51 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 8 Aug 2018 10:49:34 -0400 Subject: [PATCH 04/19] utility function for getting user email from x509 certificate --- atst/domain/authnid/utils.py | 20 ++++++++++++++- tests/domain/authnid/test_utils.py | 34 ++++++++++++++++++++++--- tests/fixtures/artgarfunkel@uso.mil.crt | 29 +++++++++++++++++++++ tests/fixtures/no-email.crt | 29 +++++++++++++++++++++ tests/fixtures/no-san.crt | 31 ++++++++++++++++++++++ 5 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 tests/fixtures/artgarfunkel@uso.mil.crt create mode 100644 tests/fixtures/no-email.crt create mode 100644 tests/fixtures/no-san.crt diff --git a/atst/domain/authnid/utils.py b/atst/domain/authnid/utils.py index 763820ce..58f04ce4 100644 --- a/atst/domain/authnid/utils.py +++ b/atst/domain/authnid/utils.py @@ -1,7 +1,9 @@ import re +import cryptography.x509 as x509 +from cryptography.hazmat.backends import default_backend + -# TODO: our sample SDN does not have an email address def parse_sdn(sdn): try: parts = sdn.split(",") @@ -9,5 +11,21 @@ def parse_sdn(sdn): cn = cn_string.split("=")[-1] info = cn.split(".") return {"last_name": info[0], "first_name": info[1], "dod_id": info[-1]} + except (IndexError, AttributeError): raise ValueError("'{}' is not a valid SDN".format(sdn)) + + +def email_from_certificate(cert_file): + cert = x509.load_pem_x509_certificate(cert_file, default_backend()) + try: + ext = cert.extensions.get_extension_for_class(x509.SubjectAlternativeName) + email = ext.value.get_values_for_type(x509.RFC822Name) + if email: + return email[0] + + else: + raise ValueError("No email available for certificate with serial {}".format(cert.serial_number)) + + except x509.extensions.ExtensionNotFound: + raise ValueError("No subjectAltName available for certificate with serial {}".format(cert.serial_number)) diff --git a/tests/domain/authnid/test_utils.py b/tests/domain/authnid/test_utils.py index caaa29f9..e72f095b 100644 --- a/tests/domain/authnid/test_utils.py +++ b/tests/domain/authnid/test_utils.py @@ -5,12 +5,38 @@ from tests.mocks import DOD_SDN def test_parse_sdn(): parsed = utils.parse_sdn(DOD_SDN) - assert parsed.get('first_name') == 'ART' - assert parsed.get('last_name') == 'GARFUNKEL' - assert parsed.get('dod_id') == '5892460358' + assert parsed.get("first_name") == "ART" + assert parsed.get("last_name") == "GARFUNKEL" + assert parsed.get("dod_id") == "5892460358" + def test_parse_bad_sdn(): with pytest.raises(ValueError): - utils.parse_sdn('this has nothing to do with anything') + utils.parse_sdn("this has nothing to do with anything") with pytest.raises(ValueError): utils.parse_sdn(None) + + +FIXTURE_EMAIL_ADDRESS = "artgarfunkel@uso.mil" + + +def test_parse_email_cert(): + cert_file = open("tests/fixtures/{}.crt".format(FIXTURE_EMAIL_ADDRESS), "rb").read() + email = utils.email_from_certificate(cert_file) + assert email == FIXTURE_EMAIL_ADDRESS + + +def test_parse_cert_with_no_email(): + cert_file = open("tests/fixtures/no-email.crt", "rb").read() + with pytest.raises(ValueError) as excinfo: + email = utils.email_from_certificate(cert_file) + (message,) = excinfo.value.args + assert "email" in message + + +def test_parse_cert_with_no_san(): + cert_file = open("tests/fixtures/no-san.crt", "rb").read() + with pytest.raises(ValueError) as excinfo: + email = utils.email_from_certificate(cert_file) + (message,) = excinfo.value.args + assert "subjectAltName" in message diff --git a/tests/fixtures/artgarfunkel@uso.mil.crt b/tests/fixtures/artgarfunkel@uso.mil.crt new file mode 100644 index 00000000..ab9d0c92 --- /dev/null +++ b/tests/fixtures/artgarfunkel@uso.mil.crt @@ -0,0 +1,29 @@ +-----BEGIN CERTIFICATE----- +MIIE8DCCAtigAwIBAgIJALTstfJQuulmMA0GCSqGSIb3DQEBCwUAMCUxIzAhBgNV +BAMMGkdBUkZVTktFTC5BUlQuRy41ODkyNDYwMzU4MB4XDTE4MDgwODE0MDI0N1oX +DTI4MDgwNTE0MDI0N1owJTEjMCEGA1UEAwwaR0FSRlVOS0VMLkFSVC5HLjU4OTI0 +NjAzNTgwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQD1OuSSniuiUP3Q +JqVJOS2LE+kmK4Y5TexTCCDhBebarg+dEYipdA4AwZMKSDL/6D+lZJCM1MTsUgaN +X/8lRv2obVGnWuEL5Rbcwhlf3yTaohhlPk/qFyQoQaxcLZgwlwUn47i5jKG1cFqA +l4TignN3n6cwbpjFfkP9Oepiffu4ThrOsrOWTN56IB7TrHElFIdjVuUWbIuK9CET +8UWixUecrLr64AKyDndaVyzGJBwhtyn7AanVYld9la0FSxu8ZcYMikSOvOEqqOMA +Nu2NapInrb+g1JEPycXTpGxMiLbFscmAkgmAqkxzeFBW0UHCQsbxG6Ep1Km3QfYw +QqvEfNRPuGq2bGtpbMUF9K4DSsI2yErtc8QvKVQ86xEuwoiFxiRtO+WQKJrq8CqU +sZxcz6ZAw2pERIYtGCi573rxb8g7skEvlIPIYWqljEwFOIrgoRav0x3dHdfA5Ubh +M0fx38icinVmL0Xd7G0JFY2RFQ13/r/zaxmmm546tH9tSjn1bwaO/6OcX9g5kCUH +p2cWklug3/bDQyKre9UZBjI7bUMWtL1w6uhdRm5yq4lX+o8G/tbUYVPER75z+AKO +p/eizAKCKSHRXDKIJr3zZG54jyd+VzTcjBSNQN/liclEBzlnZqZUgPPUR8kQ0S3E +n8jQ/Jk9MS/DUuNvEzBgZS5e3KtpZwIDAQABoyMwITAfBgNVHREEGDAWgRRhcnRn +YXJmdW5rZWxAdXNvLm1pbDANBgkqhkiG9w0BAQsFAAOCAgEAQzAA7aweU7ZHDK3l +pjcpfXruVOqceGst/avMHZp3ZS9YOkd+K3jnLVBObfBGwZkJjsyqvs0AMVi3mTYY +WeEkhTk50G2xA2UydsOQcuH/qOT6duj54a0TCB4/2kMBq6IhCT3xR4rbfxA+5ArD +yCConiy1FUX5nofYGNC7VPUgjQb64LtTr1+wO6nTwdpALeOX2GZXoBWVQO2W+2Ul +buIGV5TnpjoJGJmuO/A76qwMi5+e6EYAKmomjGCaTKyvbb2WAlCoHzdDd+nQMFYm +gBBMVOkiTZ2udIbQMFGdqAZjDEP484rsCVrth4PKAZ9/3LAe6XddLZZbqq5cap2l +u6jDinFIeV2aldRh285qwvX7+R3KQK7k5wNDbf8DlaPUhnF+CliYDBKFCoKE60AY +mp40YME0NE3XSGuIemJUazxFAJ8zUu8yEP3K/mzAwtRHiy+yQwKyPK4Wl+skXYHs +XbouRkWK7jleVKXLiE0Uw0EbWkfAVBM8IgGWp70UivCTlAdokwdKBxsLhsn57mJ5 +GP+9YTpwVQKWTBp06z0ZHaRI91d9Ke7YUSfDmLZ6VE9txqd9P2X2B2HbXFaYzGJh +gWtvqRh94ttaVsGr9iK7ANS9gXvn7Vb1ElyyF2wzP64WJtew7tywFq+Xhbm4/WPr +wM+BoGmfKP7uq0GBfu/HengJEGk= +-----END CERTIFICATE----- diff --git a/tests/fixtures/no-email.crt b/tests/fixtures/no-email.crt new file mode 100644 index 00000000..645ea845 --- /dev/null +++ b/tests/fixtures/no-email.crt @@ -0,0 +1,29 @@ +-----BEGIN CERTIFICATE----- +MIIE6TCCAtGgAwIBAgIJAKlkkD2Xt+vWMA0GCSqGSIb3DQEBCwUAMCUxIzAhBgNV +BAMMGkdBUkZVTktFTC5BUlQuRy41ODkyNDYwMzU4MB4XDTE4MDgwODE0MjI0MFoX +DTI4MDgwNTE0MjI0MFowJTEjMCEGA1UEAwwaR0FSRlVOS0VMLkFSVC5HLjU4OTI0 +NjAzNTgwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQDi81dB+2WcfgoD +ls0A7q/lefu+rOEDp90o22MO/D4uAkqztI/O/JUzGs3MG6YWEREwanlgS67Cnhki +NFHWKh0QUXyqGqYgxmyNMXemawFI5ilpCXhToSoi3aKP8Da6YO1FbhF+X9NpEgpC +cNHwKnfzOreQ4s01q8TdKL6X9wQvtX1ILNjPpCRMrfaBkiD7VbAC+Ds8SW9V10MD +1jQkZyaPtZgNU9nou9OCwHpiva1HIckNy0E8UAuSGHWmwkK62rTUvZfKHrtWWaWY +G/njwSdotAZvH4xdFW+/wJdcpj1IHACtzkctLjub78RmuvPsNHcEy6x77efSJKvb +oBGvEzOFYqoXDhvLOpxQfsZNFO1suJlcXynzVx9hmVrUfw7l8Z/yUhuNKhuRQ7fw ++9YMuXrYrcTCsZx73eTsQCX7A6QSq5//N9GNSHl5/adZXcmSwFed6OOUrMRs73HY +IH35yiyGS4BbulyKUeGHdeiD00Crb2/DSxrH5M2BqFQw993clkhdbr8AT/B/lhh8 +Bysc3fHxwXGN65k1vfgrMm3aULUHLDH9RWjMra8OF2dZndQfmFSIxVOmDmmVjfME +lBg1TXY+JyKdkZrMb8IOpd08F+g10s+OnImldjsoSW0qkxDzUIbDRSvPK5dxukDc +ygecXqeKB7Bm2lceAurcARZiDdGvRwIDAQABoxwwGjAYBgNVHREEETAPgg1nYXJm +dW5rZWwuY29tMA0GCSqGSIb3DQEBCwUAA4ICAQCdaxkg4ZmmFqGqQ5bkjOucEowI +UpFIlgn3ORX/NjeAFpRlXr+kAyrezOfe3DzffFM63GVyqCR3swfwu0DdgpaGI++z +wMjXdDKDWfCSdFeFQczt/UyOQg7lkgKAgP6AgWrS9iOUwWY2Ecd+IhLjEAJ8ESgO +udi60tx9fDSlmpc3BlXBNkZUPGQW8abv+E2hV9dhNwCLVOxgK655E+9Lv3qRFFG5 +HczGP8UcKL/0e1CIV8JfiPNG3lI9LJKE0fik7jN1nvPuM9ubKwKuxWgxDH4iP4aw +qa76rGYRT4VDcU89bRRX6fVCOK7iFd4db32zsAaFcOnztpMWAyIaTSZ4RuJivpqn +rTl0+ZOVHLierhFAH96prWcUBtyaprRCx5y/bIme+KBdEuge+s6+H4fYjMeryenQ +6kK8yqqAngDxxD400U1uP5TERu+E/JiP1AaiyPyh5j1bOjzM8/aohwTLK4pSeUHC +2AITpHPjXumTYMVLJliJ1/B+ZW8wS7kg1ICL6x9hrt/SbdDqQPZa/pE8NHuzMNSr +TaTDjaBEz50awlMYv4b3u+YQbVhGabw+2sYDG6VhiMakyuY2FCIi5Tc/ybBvXta8 +lh8Xo8hSVlwvPumqLLITl17+KXHNL1KnTgWfXntFL6t/2OQrSbDfVXmThtW+FEmm +7ZFG54OsGWYdg8uNNg== +-----END CERTIFICATE----- diff --git a/tests/fixtures/no-san.crt b/tests/fixtures/no-san.crt new file mode 100644 index 00000000..584e1135 --- /dev/null +++ b/tests/fixtures/no-san.crt @@ -0,0 +1,31 @@ +-----BEGIN CERTIFICATE----- +MIIFVTCCAz2gAwIBAgIJAJRtzRX0VhJJMA0GCSqGSIb3DQEBCwUAMCUxIzAhBgNV +BAMTGkdBUkZVTktFTC5BUlQuRy41ODkyNDYwMzU4MB4XDTE4MDgwODE0MjQ1NFoX +DTI4MDgwNTE0MjQ1NFowJTEjMCEGA1UEAxMaR0FSRlVOS0VMLkFSVC5HLjU4OTI0 +NjAzNTgwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQCak8upRyMLNUKK +vP6Ly50KGXDAktTBOFHDDsTRIIBeEiRImcuQ3nrqgHPKxlYdPG1k88VSDnrCDZry +DTm58NGCrtB6tJPqlZag8vpNffk9pEPOBKvUN9v5xqGgSN3sIdv0aMtMRJUXS61R +gzKJ76D+QM/7sKFhtPmETcfkBN99On7Zxw33TcwIlpv8t8tPB6F/r8jw07oWFBza +Z1Ui2+mFs6rZlxFOP8qRo82iencrMuW3/Tvqjl0N/AHPkdT7PbqAyg1aDkHYIBvc +euk/23Rgp1BQCX/Dia412/mMW0l6wYrw3pMBQ0j9LPSKTWx6rf7xa5TTweqcoKhB +zaeOV90wQk7gd+13u12ZqtPDI2Lgzi9PiIIDyDOGe4yX+O4YGTOV1pX2RyYCx9Hi +D6Pz9LoABz7TYq7A+LjKx5T5Q4XXiyUiQHTHQ5dC8v1rcUdZBB47eyAE0ZtVcCVI +tcG6eJgbM907AAabwca5sy0ogfYABMSUz6YWA1SMeDclwtRBlSWMFa2OCDJl7wBU +5Iyj/5a4MJ834IJh++gxpeijTktU1RyCDRUgXlAQNdqFxPmgwPbTo4KPDOw/YUnt +PSZfO2jiqhXgSRxlG5+2CAMiUVo2kelJxemDkJ30Yk3ebjx6qyEYizE0Mmh3xFYf +cOr7h1dxhjvAUtu3/ekNZWdz4WUcMQIDAQABo4GHMIGEMB0GA1UdDgQWBBRGIuCr +zBlH956853iOtEt/RF1wkzBVBgNVHSMETjBMgBRGIuCrzBlH956853iOtEt/RF1w +k6EppCcwJTEjMCEGA1UEAxMaR0FSRlVOS0VMLkFSVC5HLjU4OTI0NjAzNTiCCQCU +bc0V9FYSSTAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBCwUAA4ICAQAHVurBIQJS +makSWkuIFYuhKI5GDU9R1xeFe56zDVKE6Xoqki6CxUlHcIY/QN2nJ02GVN12GMAi +p4jewaiomr0LIlmzk63jn380okRjOFNoieIyQiXL0rH2oV4DESbWLuLoFnWFHGI7 +8VsyURDe00H58t3MsEEOzrbjSV7KeyifjIND6yrDuzoLY2FquTOq3Q41XRJxIOuk +0p0Cd9E07YzAb9kzODO5ZPvXfkAIqZIrAYb9bjcMs6gb8CbzA/STdSEPp2NjgAsc +fjI0VtUPyTX2fKE9nrHeSNsT7WFPslbzvXVtlmUvlyDgnHglKjsgSLTgFaAERUSz +WkJG0+lysAPga/qpD22C3OB/igT/S+KJjw8oubX6iAAxIDM1Oa+YStft5IXX2KSm +5FT2HIMtXBG9pkgmJ9O+xrDrJwSz+sezXYuV88T4fDYdXAUqgBudmml/h+OGEB4C +k3Mc0ibe5Np4SyDg9qWDa+u6GojQCkTA0ygxcXR0M/t204MXqV7g4zCt624BB+nH +TYLeq49SQsl2XmPLsjwWIToly1F6tizP0gWYFamGD2bqZNDIEl/5a/CLwpOlSWc8 +K6tfqAlNnM56/vMXDeo/na7XLRHPkLisUZCxBYVuSFu77gZsawVxcZlO3Hwn1L7a +Pdu9qr067Y/6AAogCQANMXWfywkc+TZMlQ== +-----END CERTIFICATE----- From 05de0665d48a0c11f3ba7a7f5255b46bf08edc0b Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 8 Aug 2018 13:52:16 -0400 Subject: [PATCH 05/19] basic implementation of email parsing for CAC user login --- atst/routes/__init__.py | 15 +++++-- tests/domain/authnid/test_utils.py | 5 +-- tests/mocks.py | 15 ++----- tests/test_auth.py | 64 +++++++++++++++++++++++++++--- 4 files changed, 75 insertions(+), 24 deletions(-) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 965b4b37..4d767d05 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -4,8 +4,8 @@ import pendulum from atst.domain.requests import Requests from atst.domain.users import Users -from atst.domain.authnid.utils import parse_sdn -from atst.domain.exceptions import UnauthenticatedError +from atst.domain.authnid.utils import parse_sdn, email_from_certificate +from atst.domain.exceptions import UnauthenticatedError, NotFoundError bp = Blueprint("atst", __name__) @@ -35,10 +35,19 @@ def catch_all(path): # or raises the UnauthenticatedError @bp.route('/login-redirect') def login_redirect(): + # raise S_DN parse errors if request.environ.get('HTTP_X_SSL_CLIENT_VERIFY') == 'SUCCESS' and _is_valid_certificate(request): sdn = request.environ.get('HTTP_X_SSL_CLIENT_S_DN') sdn_parts = parse_sdn(sdn) - user = Users.get_or_create_by_dod_id(**sdn_parts) + try: + user = Users.get_by_dod_id(sdn_parts["dod_id"]) + except NotFoundError: + try: + email = email_from_certificate(request.environ.get('HTTP_X_SSL_CLIENT_CERT').encode()) + sdn_parts["email"] = email + except ValueError: + pass + user = Users.create(**sdn_parts) session["user_id"] = user.id return redirect(url_for("atst.home")) diff --git a/tests/domain/authnid/test_utils.py b/tests/domain/authnid/test_utils.py index e72f095b..54e8f13d 100644 --- a/tests/domain/authnid/test_utils.py +++ b/tests/domain/authnid/test_utils.py @@ -1,6 +1,6 @@ import pytest import atst.domain.authnid.utils as utils -from tests.mocks import DOD_SDN +from tests.mocks import DOD_SDN, FIXTURE_EMAIL_ADDRESS def test_parse_sdn(): @@ -17,9 +17,6 @@ def test_parse_bad_sdn(): utils.parse_sdn(None) -FIXTURE_EMAIL_ADDRESS = "artgarfunkel@uso.mil" - - def test_parse_email_cert(): cert_file = open("tests/fixtures/{}.crt".format(FIXTURE_EMAIL_ADDRESS), "rb").read() email = utils.email_from_certificate(cert_file) diff --git a/tests/mocks.py b/tests/mocks.py index 0307997e..2a2f0fab 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -3,18 +3,11 @@ from tests.factories import RequestFactory, UserFactory MOCK_USER = UserFactory.build() MOCK_REQUEST = RequestFactory.build( - creator=MOCK_USER.id, - body={ - "financial_verification": { - "pe_id": "0203752A", - }, - } + creator=MOCK_USER.id, body={"financial_verification": {"pe_id": "0203752A"}} ) -DOD_SDN_INFO = { - 'first_name': 'ART', - 'last_name': 'GARFUNKEL', - 'dod_id': '5892460358' - } +DOD_SDN_INFO = {"first_name": "ART", "last_name": "GARFUNKEL", "dod_id": "5892460358"} DOD_SDN = f"CN={DOD_SDN_INFO['last_name']}.{DOD_SDN_INFO['first_name']}.G.{DOD_SDN_INFO['dod_id']},OU=OTHER,OU=PKI,OU=DoD,O=U.S. Government,C=US" MOCK_VALID_PE_ID = "8675309U" + +FIXTURE_EMAIL_ADDRESS = "artgarfunkel@uso.mil" diff --git a/tests/test_auth.py b/tests/test_auth.py index 7e2b483d..60183451 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,5 +1,8 @@ +import pytest from flask import session, url_for -from .mocks import DOD_SDN +from .mocks import DOD_SDN_INFO, DOD_SDN, FIXTURE_EMAIL_ADDRESS +from atst.domain.users import Users +from atst.domain.exceptions import NotFoundError MOCK_USER = {"id": "438567dd-25fa-4d83-a8cc-8aa8366cb24a"} @@ -11,11 +14,14 @@ def _fetch_user_info(c, t): def test_successful_login_redirect(client, monkeypatch): monkeypatch.setattr("atst.routes._is_valid_certificate", lambda *args: True) + monkeypatch.setattr("atst.routes.email_from_certificate", lambda *args: None) resp = client.get( "/login-redirect", environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN + "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", + "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, + "HTTP_X_SSL_CLIENT_CERT": "", }, ) @@ -58,8 +64,8 @@ UNPROTECTED_ROUTES = ["/", "/login-dev", "/login-redirect", "/unauthorized"] def test_crl_validation_on_login(client): - good_cert = open("ssl/client-certs/atat.mil.crt", "rb").read() - bad_cert = open("ssl/client-certs/bad-atat.mil.crt", "rb").read() + good_cert = open("ssl/client-certs/atat.mil.crt").read() + bad_cert = open("ssl/client-certs/bad-atat.mil.crt").read() # bad cert is on the test CRL resp = client.get( @@ -67,7 +73,7 @@ def test_crl_validation_on_login(client): environ_base={ "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, - "HTTP_X_SSL_CLIENT_CERT": bad_cert.decode(), + "HTTP_X_SSL_CLIENT_CERT": bad_cert, }, ) assert resp.status_code == 401 @@ -79,9 +85,55 @@ def test_crl_validation_on_login(client): environ_base={ "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, - "HTTP_X_SSL_CLIENT_CERT": good_cert.decode(), + "HTTP_X_SSL_CLIENT_CERT": good_cert, }, ) assert resp.status_code == 302 assert "home" in resp.headers["Location"] assert session["user_id"] + + +def test_creates_new_user_on_login(monkeypatch, client): + monkeypatch.setattr("atst.routes._is_valid_certificate", lambda *args: True) + cert_file = open("tests/fixtures/{}.crt".format(FIXTURE_EMAIL_ADDRESS)).read() + + # ensure user does not exist + with pytest.raises(NotFoundError): + Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) + + resp = client.get( + "/login-redirect", + environ_base={ + "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", + "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, + "HTTP_X_SSL_CLIENT_CERT": cert_file, + }, + ) + + user = Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) + assert user.first_name == DOD_SDN_INFO["first_name"] + assert user.last_name == DOD_SDN_INFO["last_name"] + assert user.email == FIXTURE_EMAIL_ADDRESS + + +def test_creates_new_user_without_email_on_login(monkeypatch, client): + monkeypatch.setattr("atst.routes._is_valid_certificate", lambda *args: True) + cert_file = open("ssl/client-certs/atat.mil.crt").read() + + # ensure user does not exist + with pytest.raises(NotFoundError): + Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) + + resp = client.get( + "/login-redirect", + environ_base={ + "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", + "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, + "HTTP_X_SSL_CLIENT_CERT": cert_file, + }, + ) + + user = Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) + assert user.first_name == DOD_SDN_INFO["first_name"] + assert user.last_name == DOD_SDN_INFO["last_name"] + assert user.email == None From c25fa2f5d85c93c443fe427b8c6cc080b5a64032 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 8 Aug 2018 14:42:52 -0400 Subject: [PATCH 06/19] autopopulate the appropriate request form fields with current user info --- atst/routes/requests/jedi_request_flow.py | 20 ++++++++++++++++-- atst/routes/requests/requests_form.py | 2 +- tests/routes/test_request_new.py | 25 +++++++++++++++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/atst/routes/requests/jedi_request_flow.py b/atst/routes/requests/jedi_request_flow.py index 1a3a7163..2b8bc312 100644 --- a/atst/routes/requests/jedi_request_flow.py +++ b/atst/routes/requests/jedi_request_flow.py @@ -11,13 +11,15 @@ class JEDIRequestFlow(object): def __init__( self, current_step, + current_user=None, request=None, post_data=None, request_id=None, - current_user=None, existing_request=None, ): self.current_step = current_step + + self.current_user = current_user self.request = request self.post_data = post_data @@ -26,7 +28,6 @@ class JEDIRequestFlow(object): self.request_id = request_id self.form = self._form() - self.current_user = current_user self.existing_request = existing_request def _form(self): @@ -59,6 +60,18 @@ class JEDIRequestFlow(object): def form_class(self): return self.current_screen["form"] + # maps user data to fields in OrgForm; this should be moved into the + # request initialization process when we have a request schema + def map_current_user(self): + if self.current_user.id == self.request.creator: + return { + "fname_request": self.current_user.first_name, + "lname_request": self.current_user.last_name, + "email_request": self.current_user.email + } + else: + return {} + @property def current_step_data(self): data = {} @@ -69,6 +82,9 @@ class JEDIRequestFlow(object): if self.request: if self.form_section == "review_submit": data = self.request.body + if self.form_section == "information_about_you": + form_data = self.request.body.get(self.form_section, {}) + data = { **form_data, **self.map_current_user() } else: data = self.request.body.get(self.form_section, {}) diff --git a/atst/routes/requests/requests_form.py b/atst/routes/requests/requests_form.py index d384abdf..1c6a7c26 100644 --- a/atst/routes/requests/requests_form.py +++ b/atst/routes/requests/requests_form.py @@ -31,7 +31,7 @@ def requests_form_update(screen=1, request_id=None): _check_can_view_request(request_id) request = Requests.get(request_id) if request_id is not None else None - jedi_flow = JEDIRequestFlow(screen, request, request_id=request_id) + jedi_flow = JEDIRequestFlow(screen, request=request, request_id=request_id, current_user=g.current_user) return render_template( "requests/screen-%d.html" % int(screen), diff --git a/tests/routes/test_request_new.py b/tests/routes/test_request_new.py index e31aae79..565f7886 100644 --- a/tests/routes/test_request_new.py +++ b/tests/routes/test_request_new.py @@ -66,3 +66,28 @@ def test_nonexistent_request(client, user_session): response = client.get("/requests/new/1/foo", follow_redirects=True) assert response.status_code == 404 + + +def test_creator_info_is_autopopulated(monkeypatch, client, user_session): + user = UserFactory.create() + user_session(user) + request = RequestFactory.create(creator=user.id, body={"information_about_you": {}}) + + response = client.get("/requests/new/2/{}".format(request.id)) + body = response.data.decode() + assert 'value="{}"'.format(user.first_name) in body + assert 'value="{}"'.format(user.last_name) in body + assert 'value="{}"'.format(user.email) in body + + +def test_non_creator_info_is_not_autopopulated(monkeypatch, client, user_session): + user = UserFactory.create() + creator = UserFactory.create() + user_session(user) + request = RequestFactory.create(creator=creator.id, body={"information_about_you": {}}) + + response = client.get("/requests/new/2/{}".format(request.id)) + body = response.data.decode() + assert not user.first_name in body + assert not user.last_name in body + assert not user.email in body From 3a41d9f81c7f19918b9e3835b3baca498b5f4e0c Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 8 Aug 2018 15:35:22 -0400 Subject: [PATCH 07/19] pass logger in to CRL validator --- atst/app.py | 4 +--- atst/domain/authnid/crl/validator.py | 12 ++++++++---- atst/routes/__init__.py | 2 -- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/atst/app.py b/atst/app.py index d095a8bf..bdaadcb7 100644 --- a/atst/app.py +++ b/atst/app.py @@ -142,8 +142,6 @@ def make_crl_validator(app): for filename in pathlib.Path(app.config["CRL_DIRECTORY"]).glob("*"): crl_locations.append(filename.absolute()) app.crl_validator = Validator( - roots=[app.config["CA_CHAIN"]], crl_locations=crl_locations + roots=[app.config["CA_CHAIN"]], crl_locations=crl_locations, logger=app.logger ) - for e in app.crl_validator.errors: - app.logger.error(e) diff --git a/atst/domain/authnid/crl/validator.py b/atst/domain/authnid/crl/validator.py index 409a8bf7..0beb9830 100644 --- a/atst/domain/authnid/crl/validator.py +++ b/atst/domain/authnid/crl/validator.py @@ -20,11 +20,11 @@ class Validator: re.DOTALL, ) - def __init__(self, crl_locations=[], roots=[], base_store=crypto.X509Store): - self.errors = [] + def __init__(self, crl_locations=[], roots=[], base_store=crypto.X509Store, logger=None): self.crl_locations = crl_locations self.roots = roots self.base_store = base_store + self.logger = logger self._reset() def _reset(self): @@ -34,12 +34,16 @@ class Validator: self._add_roots(self.roots) self.store.set_flags(crypto.X509StoreFlags.CRL_CHECK) + def log(self, message): + if self.logger: + self.logger.error(message) + def _add_crls(self, locations): for filename in locations: try: self._add_crl(filename) except crypto.Error as err: - self.errors.append( + self.log( "CRL could not be parsed. Filename: {}, Error: {}, args: {}".format( filename, type(err), err.args ) @@ -116,7 +120,7 @@ class Validator: return True except crypto.X509StoreContextError as err: - self.errors.append( + self.log( "Certificate revoked or errored. Error: {}. Args: {}".format( type(err), err.args ) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 4d767d05..dde0299c 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -59,8 +59,6 @@ def _is_valid_certificate(request): cert = request.environ.get('HTTP_X_SSL_CLIENT_CERT') if cert: result = app.crl_validator.validate(cert.encode()) - if not result: - app.logger.info(app.crl_validator.errors[-1]) return result else: return False From 07ce940650c60f8047530119286a66b7e6c3545b Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Aug 2018 09:26:57 -0400 Subject: [PATCH 08/19] create AuthenticationContext to consolidate auth logic --- atst/domain/authnid/__init__.py | 62 +++++++++++++++++++++++++++++++++ atst/domain/exceptions.py | 4 +++ atst/routes/__init__.py | 38 +++++++++----------- tests/test_auth.py | 9 ++--- 4 files changed, 87 insertions(+), 26 deletions(-) diff --git a/atst/domain/authnid/__init__.py b/atst/domain/authnid/__init__.py index e69de29b..93ca7f99 100644 --- a/atst/domain/authnid/__init__.py +++ b/atst/domain/authnid/__init__.py @@ -0,0 +1,62 @@ +from atst.domain.exceptions import UnauthenticatedError, NotFoundError +from atst.domain.users import Users +from .utils import parse_sdn, email_from_certificate + + +class AuthenticationContext(): + + def __init__(self, crl_validator, auth_status, sdn, cert): + if None in locals().values(): + raise UnauthenticatedError("Missing required authentication context components") + + self.crl_validator = crl_validator + self.auth_status = auth_status + self.sdn = sdn + self.cert = cert.encode() + self._parsed_sdn = None + + + def authenticate(self): + if not self.auth_status == "SUCCESS": + raise UnauthenticatedError("SSL/TLS client authentication failed") + + elif not self._crl_check(): + raise UnauthenticatedError("Client certificate failed CRL check") + + return True + + + def get_user(self): + try: + return Users.get_by_dod_id(self.parsed_sdn["dod_id"]) + + except NotFoundError: + email = self._get_user_email() + return Users.create(**{"email": email, **self.parsed_sdn}) + + def _get_user_email(self): + try: + return email_from_certificate(self.cert) + + # this just means it is not an email certificate; we might choose to + # log in that case + except ValueError: + return None + + def _crl_check(self): + if self.cert: + result = self.crl_validator.validate(self.cert) + return result + + else: + return False + + @property + def parsed_sdn(self): + if not self._parsed_sdn: + try: + self._parsed_sdn = parse_sdn(self.sdn) + except ValueError as exc: + raise UnauthenticatedError(str(exc)) + + return self._parsed_sdn diff --git a/atst/domain/exceptions.py b/atst/domain/exceptions.py index ec574232..53c83548 100644 --- a/atst/domain/exceptions.py +++ b/atst/domain/exceptions.py @@ -30,3 +30,7 @@ class UnauthenticatedError(Exception): @property def message(self): return str(self) + + +class CRLValidationError(Exception): + pass diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index dde0299c..f8c4199c 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -4,8 +4,8 @@ import pendulum from atst.domain.requests import Requests from atst.domain.users import Users -from atst.domain.authnid.utils import parse_sdn, email_from_certificate -from atst.domain.exceptions import UnauthenticatedError, NotFoundError +from atst.domain.authnid import AuthenticationContext + bp = Blueprint("atst", __name__) @@ -30,29 +30,23 @@ def catch_all(path): return render_template("{}.html".format(path)) -# TODO: this should be partly consolidated into a domain function that takes -# all the necessary UWSGI environment values as args and either returns a user -# or raises the UnauthenticatedError +def _make_authentication_context(): + return AuthenticationContext( + crl_validator=app.crl_validator, + auth_status=request.environ.get("HTTP_X_SSL_CLIENT_VERIFY"), + sdn=request.environ.get("HTTP_X_SSL_CLIENT_S_DN"), + cert=request.environ.get("HTTP_X_SSL_CLIENT_CERT") + ) + + @bp.route('/login-redirect') def login_redirect(): - # raise S_DN parse errors - if request.environ.get('HTTP_X_SSL_CLIENT_VERIFY') == 'SUCCESS' and _is_valid_certificate(request): - sdn = request.environ.get('HTTP_X_SSL_CLIENT_S_DN') - sdn_parts = parse_sdn(sdn) - try: - user = Users.get_by_dod_id(sdn_parts["dod_id"]) - except NotFoundError: - try: - email = email_from_certificate(request.environ.get('HTTP_X_SSL_CLIENT_CERT').encode()) - sdn_parts["email"] = email - except ValueError: - pass - user = Users.create(**sdn_parts) - session["user_id"] = user.id + auth_context = _make_authentication_context() + auth_context.authenticate() + user = auth_context.get_user() + session["user_id"] = user.id - return redirect(url_for("atst.home")) - else: - raise UnauthenticatedError() + return redirect(url_for("atst.home")) def _is_valid_certificate(request): diff --git a/tests/test_auth.py b/tests/test_auth.py index 60183451..4aa5bc1d 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -3,6 +3,7 @@ from flask import session, url_for from .mocks import DOD_SDN_INFO, DOD_SDN, FIXTURE_EMAIL_ADDRESS from atst.domain.users import Users from atst.domain.exceptions import NotFoundError +from .factories import UserFactory MOCK_USER = {"id": "438567dd-25fa-4d83-a8cc-8aa8366cb24a"} @@ -13,14 +14,14 @@ def _fetch_user_info(c, t): def test_successful_login_redirect(client, monkeypatch): - monkeypatch.setattr("atst.routes._is_valid_certificate", lambda *args: True) - monkeypatch.setattr("atst.routes.email_from_certificate", lambda *args: None) + monkeypatch.setattr("atst.domain.authnid.AuthenticationContext.authenticate", lambda *args: True) + monkeypatch.setattr("atst.domain.authnid.AuthenticationContext.get_user", lambda *args: UserFactory.create()) resp = client.get( "/login-redirect", environ_base={ "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, + "HTTP_X_SSL_CLIENT_S_DN": "", "HTTP_X_SSL_CLIENT_CERT": "", }, ) @@ -94,7 +95,7 @@ def test_crl_validation_on_login(client): def test_creates_new_user_on_login(monkeypatch, client): - monkeypatch.setattr("atst.routes._is_valid_certificate", lambda *args: True) + monkeypatch.setattr("atst.domain.authnid.AuthenticationContext.authenticate", lambda *args: True) cert_file = open("tests/fixtures/{}.crt".format(FIXTURE_EMAIL_ADDRESS)).read() # ensure user does not exist From efee79a566ef03afc37f7d97af75ffda47dbf4d0 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Aug 2018 09:32:17 -0400 Subject: [PATCH 09/19] update request autopopulate to rely on creator --- atst/routes/requests/jedi_request_flow.py | 8 ++++---- tests/routes/test_request_new.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/atst/routes/requests/jedi_request_flow.py b/atst/routes/requests/jedi_request_flow.py index 2b8bc312..02e10e2b 100644 --- a/atst/routes/requests/jedi_request_flow.py +++ b/atst/routes/requests/jedi_request_flow.py @@ -63,11 +63,11 @@ class JEDIRequestFlow(object): # maps user data to fields in OrgForm; this should be moved into the # request initialization process when we have a request schema def map_current_user(self): - if self.current_user.id == self.request.creator: + if self.request: return { - "fname_request": self.current_user.first_name, - "lname_request": self.current_user.last_name, - "email_request": self.current_user.email + "fname_request": self.request.creator.first_name, + "lname_request": self.request.creator.last_name, + "email_request": self.request.creator.email } else: return {} diff --git a/tests/routes/test_request_new.py b/tests/routes/test_request_new.py index 565f7886..06cd47da 100644 --- a/tests/routes/test_request_new.py +++ b/tests/routes/test_request_new.py @@ -71,7 +71,7 @@ def test_nonexistent_request(client, user_session): def test_creator_info_is_autopopulated(monkeypatch, client, user_session): user = UserFactory.create() user_session(user) - request = RequestFactory.create(creator=user.id, body={"information_about_you": {}}) + request = RequestFactory.create(creator=user, body={"information_about_you": {}}) response = client.get("/requests/new/2/{}".format(request.id)) body = response.data.decode() @@ -84,7 +84,7 @@ def test_non_creator_info_is_not_autopopulated(monkeypatch, client, user_session user = UserFactory.create() creator = UserFactory.create() user_session(user) - request = RequestFactory.create(creator=creator.id, body={"information_about_you": {}}) + request = RequestFactory.create(creator=creator, body={"information_about_you": {}}) response = client.get("/requests/new/2/{}".format(request.id)) body = response.data.decode() From 4da814aaf48a42dc82bbd52927856a46037d61f6 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Aug 2018 09:42:13 -0400 Subject: [PATCH 10/19] move crl validator --- atst/app.py | 2 +- atst/domain/authnid/crl/{validator.py => __init__.py} | 0 atst/domain/authnid/crl/util.py | 1 - atst/routes/requests/jedi_request_flow.py | 3 ++- tests/domain/authnid/test_crl.py | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename atst/domain/authnid/crl/{validator.py => __init__.py} (100%) diff --git a/atst/app.py b/atst/app.py index bdaadcb7..2c2b0cba 100644 --- a/atst/app.py +++ b/atst/app.py @@ -16,7 +16,7 @@ from atst.routes.workspaces import bp as workspace_routes from atst.routes.requests import requests_bp from atst.routes.dev import bp as dev_routes from atst.routes.errors import make_error_pages -from atst.domain.authnid.crl.validator import Validator +from atst.domain.authnid.crl import Validator from atst.domain.auth import apply_authentication diff --git a/atst/domain/authnid/crl/validator.py b/atst/domain/authnid/crl/__init__.py similarity index 100% rename from atst/domain/authnid/crl/validator.py rename to atst/domain/authnid/crl/__init__.py diff --git a/atst/domain/authnid/crl/util.py b/atst/domain/authnid/crl/util.py index 13e8106d..7e9948e1 100644 --- a/atst/domain/authnid/crl/util.py +++ b/atst/domain/authnid/crl/util.py @@ -56,7 +56,6 @@ def refresh_crls(out_dir, logger=None): if __name__ == "__main__": import sys - import datetime import logging logging.basicConfig( diff --git a/atst/routes/requests/jedi_request_flow.py b/atst/routes/requests/jedi_request_flow.py index 02e10e2b..34f3508f 100644 --- a/atst/routes/requests/jedi_request_flow.py +++ b/atst/routes/requests/jedi_request_flow.py @@ -61,7 +61,8 @@ class JEDIRequestFlow(object): return self.current_screen["form"] # maps user data to fields in OrgForm; this should be moved into the - # request initialization process when we have a request schema + # request initialization process when we have a request schema, or we just + # shouldn't record this data on the request def map_current_user(self): if self.request: return { diff --git a/tests/domain/authnid/test_crl.py b/tests/domain/authnid/test_crl.py index 6bfdd99e..5593a865 100644 --- a/tests/domain/authnid/test_crl.py +++ b/tests/domain/authnid/test_crl.py @@ -4,7 +4,7 @@ import re import os import shutil from OpenSSL import crypto, SSL -from atst.domain.authnid.crl.validator import Validator +from atst.domain.authnid.crl import Validator import atst.domain.authnid.crl.util as util From 855c0bc3c4878a50977fd4da768ba6437a81d259 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Aug 2018 10:09:30 -0400 Subject: [PATCH 11/19] tests for AuthenticationContext --- atst/domain/authnid/__init__.py | 6 +- .../authnid/test_authentication_context.py | 92 +++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 tests/domain/authnid/test_authentication_context.py diff --git a/atst/domain/authnid/__init__.py b/atst/domain/authnid/__init__.py index 93ca7f99..80d645b8 100644 --- a/atst/domain/authnid/__init__.py +++ b/atst/domain/authnid/__init__.py @@ -7,7 +7,9 @@ class AuthenticationContext(): def __init__(self, crl_validator, auth_status, sdn, cert): if None in locals().values(): - raise UnauthenticatedError("Missing required authentication context components") + raise UnauthenticatedError( + "Missing required authentication context components" + ) self.crl_validator = crl_validator self.auth_status = auth_status @@ -15,7 +17,6 @@ class AuthenticationContext(): self.cert = cert.encode() self._parsed_sdn = None - def authenticate(self): if not self.auth_status == "SUCCESS": raise UnauthenticatedError("SSL/TLS client authentication failed") @@ -25,7 +26,6 @@ class AuthenticationContext(): return True - def get_user(self): try: return Users.get_by_dod_id(self.parsed_sdn["dod_id"]) diff --git a/tests/domain/authnid/test_authentication_context.py b/tests/domain/authnid/test_authentication_context.py new file mode 100644 index 00000000..f2a359af --- /dev/null +++ b/tests/domain/authnid/test_authentication_context.py @@ -0,0 +1,92 @@ +import pytest + +from atst.domain.authnid import AuthenticationContext +from atst.domain.exceptions import UnauthenticatedError, NotFoundError +from atst.domain.users import Users + +from tests.mocks import DOD_SDN_INFO, DOD_SDN, FIXTURE_EMAIL_ADDRESS +from tests.factories import UserFactory + +CERT = open("tests/fixtures/{}.crt".format(FIXTURE_EMAIL_ADDRESS)).read() + + +class MockCRLValidator(): + + def __init__(self, value): + self.value = value + + def validate(self, cert): + return self.value + + +def test_can_authenticate(): + auth_context = AuthenticationContext( + MockCRLValidator(True), "SUCCESS", DOD_SDN, CERT + ) + assert auth_context.authenticate() + + +def test_unsuccessful_status(): + auth_context = AuthenticationContext( + MockCRLValidator(True), "FAILURE", DOD_SDN, CERT + ) + with pytest.raises(UnauthenticatedError) as excinfo: + assert auth_context.authenticate() + + (message,) = excinfo.value.args + assert "client authentication" in message + + +def test_crl_check_fails(): + auth_context = AuthenticationContext( + MockCRLValidator(False), "SUCCESS", DOD_SDN, CERT + ) + with pytest.raises(UnauthenticatedError) as excinfo: + assert auth_context.authenticate() + + (message,) = excinfo.value.args + assert "CRL check" in message + + +def test_bad_sdn(): + auth_context = AuthenticationContext( + MockCRLValidator(True), "SUCCESS", "abc123", CERT + ) + with pytest.raises(UnauthenticatedError) as excinfo: + auth_context.get_user() + + (message,) = excinfo.value.args + assert "SDN" in message + + +def test_user_exists(): + user = UserFactory.create(**DOD_SDN_INFO) + auth_context = AuthenticationContext( + MockCRLValidator(True), "SUCCESS", DOD_SDN, CERT + ) + auth_user = auth_context.get_user() + + assert auth_user == user + + +def test_creates_user(): + # check user does not exist + with pytest.raises(NotFoundError): + Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) + + auth_context = AuthenticationContext( + MockCRLValidator(True), "SUCCESS", DOD_SDN, CERT + ) + user = auth_context.get_user() + assert user.dod_id == DOD_SDN_INFO["dod_id"] + assert user.email == FIXTURE_EMAIL_ADDRESS + + +def test_user_cert_has_no_email(): + cert = open("ssl/client-certs/atat.mil.crt").read() + auth_context = AuthenticationContext( + MockCRLValidator(True), "SUCCESS", DOD_SDN, cert + ) + user = auth_context.get_user() + + assert user.email == None From b61f97880e47688670521afa279af17720acab88 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Aug 2018 10:21:59 -0400 Subject: [PATCH 12/19] remove unused exception --- atst/domain/exceptions.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/atst/domain/exceptions.py b/atst/domain/exceptions.py index 53c83548..ec574232 100644 --- a/atst/domain/exceptions.py +++ b/atst/domain/exceptions.py @@ -30,7 +30,3 @@ class UnauthenticatedError(Exception): @property def message(self): return str(self) - - -class CRLValidationError(Exception): - pass From c4ddba1d72ab6d1d4d6823257962844099242d4d Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Aug 2018 14:52:59 -0400 Subject: [PATCH 13/19] better name for CRL validator log method --- atst/domain/authnid/crl/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/atst/domain/authnid/crl/__init__.py b/atst/domain/authnid/crl/__init__.py index 0beb9830..68358e37 100644 --- a/atst/domain/authnid/crl/__init__.py +++ b/atst/domain/authnid/crl/__init__.py @@ -34,7 +34,7 @@ class Validator: self._add_roots(self.roots) self.store.set_flags(crypto.X509StoreFlags.CRL_CHECK) - def log(self, message): + def log_error(self, message): if self.logger: self.logger.error(message) @@ -43,7 +43,7 @@ class Validator: try: self._add_crl(filename) except crypto.Error as err: - self.log( + self.log_error( "CRL could not be parsed. Filename: {}, Error: {}, args: {}".format( filename, type(err), err.args ) @@ -120,7 +120,7 @@ class Validator: return True except crypto.X509StoreContextError as err: - self.log( + self.log_error( "Certificate revoked or errored. Error: {}. Args: {}".format( type(err), err.args ) From cd3be9c7c0f741a11e1ce66eac6a589b080dd443 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Aug 2018 14:53:27 -0400 Subject: [PATCH 14/19] give preference to pre-existing user data on the request --- atst/routes/requests/jedi_request_flow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/routes/requests/jedi_request_flow.py b/atst/routes/requests/jedi_request_flow.py index 34f3508f..fd642478 100644 --- a/atst/routes/requests/jedi_request_flow.py +++ b/atst/routes/requests/jedi_request_flow.py @@ -85,7 +85,7 @@ class JEDIRequestFlow(object): data = self.request.body if self.form_section == "information_about_you": form_data = self.request.body.get(self.form_section, {}) - data = { **form_data, **self.map_current_user() } + data = { **self.map_current_user(), **form_data } else: data = self.request.body.get(self.form_section, {}) From 52fc5a941459f5d780c2bff9136b98669c725990 Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Aug 2018 14:53:42 -0400 Subject: [PATCH 15/19] readme for regenerating client cert fixtures --- tests/fixtures/README.md | 46 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 tests/fixtures/README.md diff --git a/tests/fixtures/README.md b/tests/fixtures/README.md new file mode 100644 index 00000000..564e4553 --- /dev/null +++ b/tests/fixtures/README.md @@ -0,0 +1,46 @@ +# Regenerating Fixture Certificates + +You don't need to keep the key file generated by this process. + +1. Certificate with an email as subjectAltName: + +``` +openssl req -x509 \ + -newkey rsa:4096 \ + -sha256 \ + -nodes \ + -days 3650 \ + -keyout _foo.key \ + -out artgarfunkel@uso.mil.crt \ + -subj "/CN=GARFUNKEL.ART.G.5892460358" \ + -extensions SAN \ + -config <(cat /etc/ssl/openssl.cnf; echo '[SAN]'; echo 'subjectAltName=email:artgarfunkel@uso.mil') +``` + +2. Certificate with a DNS name as subjectAltName: + +``` +openssl req -x509 \ + -newkey rsa:4096 \ + -sha256 \ + -nodes \ + -days 3650 \ + -keyout _foo.key \ + -out no-email.crt \ + -subj "/CN=GARFUNKEL.ART.G.5892460358" \ + -extensions SAN \ + -config <(cat /etc/ssl/openssl.cnf; echo '[SAN]'; echo 'subjectAltName=DNS:artgarfunkel.com') +``` + +3. Certificate with no subjectAltName: + +``` +openssl req -x509 \ + -newkey rsa:4096 \ + -sha256 \ + -nodes \ + -days 3650 \ + -keyout _foo.key \ + -out no-san.crt \ + -subj "/CN=GARFUNKEL.ART.G.5892460358" +``` From f68e5a5ed2a316a02c1ce722ccc1857ab822ee3d Mon Sep 17 00:00:00 2001 From: dandds Date: Thu, 9 Aug 2018 15:04:19 -0400 Subject: [PATCH 16/19] no logger for tests --- tests/conftest.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index ab912679..12e7c320 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,11 +2,19 @@ import os import pytest import alembic.config import alembic.command +from logging.config import dictConfig from atst.app import make_app, make_config from atst.database import db as _db import tests.factories as factories +dictConfig({ + 'version': 1, + 'handlers': {'wsgi': { + 'class': 'logging.NullHandler', + }} +}) + @pytest.fixture(scope="session") def app(request): From 239dcb90a13a8b461d1df5ee69f6782ec1a123a2 Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 10 Aug 2018 11:16:27 -0400 Subject: [PATCH 17/19] autofill user data even if request does not exist yet --- atst/routes/requests/jedi_request_flow.py | 23 ++++++++++------------- atst/routes/requests/requests_form.py | 2 +- tests/routes/test_request_new.py | 11 +++++++++++ 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/atst/routes/requests/jedi_request_flow.py b/atst/routes/requests/jedi_request_flow.py index fd642478..67a59cc0 100644 --- a/atst/routes/requests/jedi_request_flow.py +++ b/atst/routes/requests/jedi_request_flow.py @@ -33,10 +33,8 @@ class JEDIRequestFlow(object): def _form(self): if self.is_post: return self.form_class()(self.post_data) - elif self.request: - return self.form_class()(data=self.current_step_data) else: - return self.form_class()() + return self.form_class()(data=self.current_step_data) def validate(self): return self.form.validate() @@ -63,15 +61,12 @@ class JEDIRequestFlow(object): # maps user data to fields in OrgForm; this should be moved into the # request initialization process when we have a request schema, or we just # shouldn't record this data on the request - def map_current_user(self): - if self.request: - return { - "fname_request": self.request.creator.first_name, - "lname_request": self.request.creator.last_name, - "email_request": self.request.creator.email - } - else: - return {} + def map_user_data(self, user): + return { + "fname_request": user.first_name, + "lname_request": user.last_name, + "email_request": user.email + } @property def current_step_data(self): @@ -85,9 +80,11 @@ class JEDIRequestFlow(object): data = self.request.body if self.form_section == "information_about_you": form_data = self.request.body.get(self.form_section, {}) - data = { **self.map_current_user(), **form_data } + data = { **self.map_user_data(self.request.creator), **form_data } else: data = self.request.body.get(self.form_section, {}) + elif self.form_section == "information_about_you": + data = self.map_user_data(self.current_user) return defaultdict(lambda: defaultdict(lambda: "Input required"), data) diff --git a/atst/routes/requests/requests_form.py b/atst/routes/requests/requests_form.py index 1c6a7c26..50a47217 100644 --- a/atst/routes/requests/requests_form.py +++ b/atst/routes/requests/requests_form.py @@ -9,7 +9,7 @@ from atst.domain.exceptions import UnauthorizedError @requests_bp.route("/requests/new/", methods=["GET"]) def requests_form_new(screen): - jedi_flow = JEDIRequestFlow(screen, request=None) + jedi_flow = JEDIRequestFlow(screen, request=None, current_user=g.current_user) return render_template( "requests/screen-%d.html" % int(screen), diff --git a/tests/routes/test_request_new.py b/tests/routes/test_request_new.py index 06cd47da..7d770a28 100644 --- a/tests/routes/test_request_new.py +++ b/tests/routes/test_request_new.py @@ -80,6 +80,17 @@ def test_creator_info_is_autopopulated(monkeypatch, client, user_session): assert 'value="{}"'.format(user.email) in body +def test_creator_info_is_autopopulated_for_new_request(monkeypatch, client, user_session): + user = UserFactory.create() + user_session(user) + + response = client.get("/requests/new/2") + body = response.data.decode() + assert 'value="{}"'.format(user.first_name) in body + assert 'value="{}"'.format(user.last_name) in body + assert 'value="{}"'.format(user.email) in body + + def test_non_creator_info_is_not_autopopulated(monkeypatch, client, user_session): user = UserFactory.create() creator = UserFactory.create() From 1c101a3c4f0f7a426156198fc32aa60e2c4f84cb Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 10 Aug 2018 12:05:01 -0400 Subject: [PATCH 18/19] use Requests method to check in pending financial verification --- atst/routes/requests/index.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/atst/routes/requests/index.py b/atst/routes/requests/index.py index dc1d137e..f756836f 100644 --- a/atst/routes/requests/index.py +++ b/atst/routes/requests/index.py @@ -3,7 +3,6 @@ from flask import render_template, g, url_for from . import requests_bp from atst.domain.requests import Requests -from atst.models.request_status_event import RequestStatus def map_request(request): @@ -34,6 +33,6 @@ def requests_index(): mapped_requests = [map_request(r) for r in requests] - pending_fv = any(r["status"] == RequestStatus.PENDING_FINANCIAL_VERIFICATION.value for r in mapped_requests) + pending_fv = any(Requests.is_pending_financial_verification(r) for r in requests) return render_template("requests.html", requests=mapped_requests, pending_financial_verification=pending_fv) From 251bef6bb88585ca06b43ff13d20640ece1a34e0 Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 10 Aug 2018 12:12:02 -0400 Subject: [PATCH 19/19] move pending financial review text --- templates/components/alert.html | 8 ++++- .../pending_financial_verification.html | 12 +++++++ templates/requests.html | 32 ++----------------- 3 files changed, 21 insertions(+), 31 deletions(-) create mode 100644 templates/fragments/pending_financial_verification.html diff --git a/templates/components/alert.html b/templates/components/alert.html index d3385d1d..1fd30167 100644 --- a/templates/components/alert.html +++ b/templates/components/alert.html @@ -1,6 +1,6 @@ {% from "components/icon.html" import Icon %} -{% macro Alert(title, message=None, actions=None, level='info') -%} +{% macro Alert(title, message=None, actions=None, level='info', fragment=None) -%} {% set role = 'alertdialog' if actions else 'alert' %} {% set levels = { 'warning': { @@ -31,6 +31,12 @@
{{ message | safe }}
{% endif %} + {% if fragment %} +
+ {% include fragment %} +
+ {% endif %} + {% if actions %}
{{ actions | safe }}
{% endif %} diff --git a/templates/fragments/pending_financial_verification.html b/templates/fragments/pending_financial_verification.html new file mode 100644 index 00000000..858b3e26 --- /dev/null +++ b/templates/fragments/pending_financial_verification.html @@ -0,0 +1,12 @@ +

+ The next step is to create a Task Order associated with JEDI Cloud. + Please contact a Contracting Officer (KO), Contracting Officer + Representative (COR), or a Financial Manager to help with this step. +

+

+ Once the Task Order has been created, you will be asked to provide + details about the task order in the Financial Verification step. +

+

+ Learn more about the JEDI Task Order and the Financial Verification process. +

diff --git a/templates/requests.html b/templates/requests.html index 4dc0e865..df6999ca 100644 --- a/templates/requests.html +++ b/templates/requests.html @@ -9,20 +9,7 @@ {% call Modal(name='pendingFinancialVerification', dismissable=True) %}

Request submitted!

-

- The next step is to create a Task Order associated with JEDI Cloud. - Please contact a Contracting Officer (KO), Contracting Officer - Representative (COR), or a Financial Manager to help with this step. -

- -

- Once the Task Order has been created, you will be asked to provide - details about the task order in the Financial Verification step. -

- -

- Learn more about the JEDI Task Order and the Financial Verification process. -

+ {% include 'fragments/pending_financial_verification.html' %}
Close @@ -42,22 +29,7 @@ {% if pending_financial_verification %} - {{ Alert('Pending Financial Verification', - message=" -

- The next step is to create a Task Order associated with JEDI Cloud. - Please contact a Contracting Officer (KO), Contracting Officer - Representative (COR), or a Financial Manager to help with this step. -

-

- Once the Task Order has been created, you will be asked to provide - details about the task order in the Financial Verification step. -

-

- Learn more about the JEDI Task Order and the Financial Verification process. -

- " - ) }} + {{ Alert('Pending Financial Verification', fragment="fragments/pending_financial_verification.html") }} {% endif %}