Merge pull request #1010 from dod-ccpo/upload-cleanup

Clean up defunct upload and CRL logic.

atst/app.py

@@ -30,7 +30,6 @@ from atst.utils.json import CustomJSONEncoder
 from atst.queue import queue
 from atst.utils.notification_sender import NotificationSender
 from atst.utils.session_limiter import SessionLimiter
-from atst.domain.csp.file_uploads import build_uploader
 
 from logging.config import dictConfig
 from atst.utils.logging import JsonFormatter, RequestContextFilter
@@ -61,7 +60,7 @@ def make_app(config):
 
     make_flask_callbacks(app)
     register_filters(app)
-    make_csp_provider(app)
+    make_csp_provider(app, config.get("CSP", "mock"))
     make_crl_validator(app)
     make_mailer(app)
     make_notification_sender(app)
@@ -79,7 +78,6 @@ def make_app(config):
     app.register_blueprint(task_orders_bp)
     app.register_blueprint(applications_bp)
     app.register_blueprint(user_routes)
-    app.uploader = build_uploader(app.config)
 
     if ENV != "prod":
         app.register_blueprint(dev_routes)
@@ -228,10 +226,7 @@ def make_crl_validator(app):
         for filename in pathlib.Path(app.config["CRL_STORAGE_CONTAINER"]).glob("*.crl"):
             crl_locations.append(filename.absolute())
         app.crl_cache = CRLCache(
-            app.config["CA_CHAIN"],
+            app.config["CA_CHAIN"], crl_locations, logger=app.logger
-            crl_locations,
-            logger=app.logger,
-            crl_update_func=app.csp.crls.sync_crls,
         )
 
 

atst/domain/csp/__init__.py

@@ -1,15 +1,33 @@
 from .cloud import MockCloudProvider
-from .files import RackspaceFileProvider, RackspaceCRLProvider
+from .file_uploads import AwsUploader, AzureUploader, MockUploader
 from .reports import MockReportingProvider
 
 
 class MockCSP:
     def __init__(self, app):
         self.cloud = MockCloudProvider()
-        self.files = RackspaceFileProvider(app)
+        self.files = MockUploader(app)
         self.reports = MockReportingProvider()
-        self.crls = RackspaceCRLProvider(app)
 
 
-def make_csp_provider(app):
+class AzureCSP:
-    app.csp = MockCSP(app)
+    def __init__(self, app):
+        self.cloud = MockCloudProvider()
+        self.files = AzureUploader(app.config)
+        self.reports = MockReportingProvider()
+
+
+class AwsCSP:
+    def __init__(self, app):
+        self.cloud = MockCloudProvider()
+        self.files = AwsUploader(app.config)
+        self.reports = MockReportingProvider()
+
+
+def make_csp_provider(app, csp=None):
+    if csp == "aws":
+        app.csp = AwsCSP(app)
+    elif csp == "azure":
+        app.csp = AzureCSP(app)
+    else:
+        app.csp = MockCSP(app)

atst/domain/csp/file_uploads.py

@@ -2,16 +2,6 @@ from datetime import datetime, timedelta
 from uuid import uuid4
 
 
-def build_uploader(config):
-    csp = config.get("CSP")
-    if csp == "aws":
-        return AwsUploader(config)
-    elif csp == "azure":
-        return AzureUploader(config)
-    else:
-        return MockUploader(config)
-
-
 class Uploader:
     def generate_token(self):
         pass

atst/domain/csp/files.py

@@ -1,125 +0,0 @@
-import os
-import tarfile
-from tempfile import NamedTemporaryFile, TemporaryDirectory
-from uuid import uuid4
-
-from libcloud.storage.types import Provider
-from libcloud.storage.providers import get_driver
-
-from atst.domain.exceptions import UploadError
-
-
-class CSPFileError(Exception):
-    pass
-
-
-class FileProviderInterface:
-    _PERMITTED_MIMETYPES = ["application/pdf"]
-
-    def _enforce_mimetype(self, fyle):
-        # TODO: for hardening, we should probably use a better library for
-        # determining mimetype and not rely on FileUpload's determination
-        # TODO: we should set MAX_CONTENT_LENGTH in the config to prevent large
-        # uploads
-        if not fyle.mimetype in self._PERMITTED_MIMETYPES:
-            raise UploadError(
-                "could not upload {} with mimetype {}".format(
-                    fyle.filename, fyle.mimetype
-                )
-            )
-
-    def upload(self, fyle):  # pragma: no cover
-        """Store the file object `fyle` in the CSP. This method returns the
-        object name that can be used to later look up the file."""
-        raise NotImplementedError()
-
-    def download(self, object_name):  # pragma: no cover
-        """Retrieve the stored file represented by `object_name`. Returns a
-        file object.
-        """
-        raise NotImplementedError()
-
-
-def get_rackspace_container(provider, container=None, **kwargs):
-    if provider == "LOCAL":  # pragma: no branch
-        kwargs["key"] = container
-        if not os.path.exists(container):
-            os.mkdir(container)
-        container = ""
-
-    driver = get_driver(getattr(Provider, provider))(**kwargs)
-    return driver.get_container(container)
-
-
-class RackspaceFileProvider(FileProviderInterface):
-    def __init__(self, app):
-        self.container = get_rackspace_container(
-            provider=app.config.get("STORAGE_PROVIDER"),
-            container=app.config.get("STORAGE_CONTAINER"),
-            key=app.config.get("STORAGE_KEY"),
-            secret=app.config.get("STORAGE_SECRET"),
-        )
-
-    def upload(self, fyle):
-        self._enforce_mimetype(fyle)
-
-        object_name = uuid4().hex
-        with NamedTemporaryFile() as tempfile:
-            tempfile.write(fyle.stream.read())
-            tempfile.seek(0)
-            self.container.upload_object(
-                file_path=tempfile.name,
-                object_name=object_name,
-                extra={"acl": "private"},
-            )
-        return object_name
-
-    def download(self, object_name):
-        obj = self.container.get_object(object_name=object_name)
-        with NamedTemporaryFile() as tempfile:
-            obj.download(tempfile.name, overwrite_existing=True)
-            return open(tempfile.name, "rb")
-
-
-class CRLProviderInterface:
-    def sync_crls(self):  # pragma: no cover
-        """
-        Retrieve copies of the CRLs and unpack them to disk.
-        """
-        raise NotImplementedError()
-
-
-class RackspaceCRLProvider(CRLProviderInterface):
-    def __init__(self, app):
-        provider = app.config.get("CRL_STORAGE_PROVIDER") or app.config.get(
-            "STORAGE_PROVIDER"
-        )
-        self.container = get_rackspace_container(
-            provider=provider,
-            container=app.config.get("CRL_STORAGE_CONTAINER"),
-            key=app.config.get("STORAGE_KEY"),
-            secret=app.config.get("STORAGE_SECRET"),
-            region=app.config.get("CRL_STORAGE_REGION"),
-        )
-        self._crl_dir = app.config.get("CRL_STORAGE_CONTAINER")
-        self._object_name = app.config.get("STORAGE_CRL_ARCHIVE_NAME")
-        self._object = None
-
-    @property
-    def object(self):
-        if self._object is None:
-            self._object = self.container.get_object(object_name=self._object_name)
-
-        return self._object
-
-    def sync_crls(self):
-        if not os.path.exists(self._crl_dir):
-            os.mkdir(self._crl_dir)
-
-        with TemporaryDirectory() as tempdir:
-            dl_path = os.path.join(tempdir, self._object_name)
-            success = self.object.download(dl_path, overwrite_existing=True)
-            if not success:
-                raise CSPFileError("The CRL package was not downloaded")
-            archive = tarfile.open(dl_path, "r:bz2")
-            archive.extractall(self._crl_dir)

atst/models/attachment.py

@@ -1,11 +1,10 @@
 from sqlalchemy import Column, String
 from sqlalchemy.dialects.postgresql import UUID
 from sqlalchemy.orm.exc import NoResultFound
-from flask import current_app as app
 
 from atst.models import Base, types, mixins
 from atst.database import db
-from atst.domain.exceptions import NotFoundError, UploadError
+from atst.domain.exceptions import NotFoundError
 
 
 class AttachmentError(Exception):
@@ -21,25 +20,6 @@ class Attachment(Base, mixins.TimestampsMixin):
     resource = Column(String)
     resource_id = Column(UUID(as_uuid=True), index=True)
 
-    @classmethod
-    def attach(cls, fyle, resource=None, resource_id=None):
-        try:
-            object_name = app.csp.files.upload(fyle)
-        except UploadError as e:
-            raise AttachmentError("Could not add attachment. " + str(e))
-
-        attachment = Attachment(
-            filename=fyle.filename,
-            object_name=object_name,
-            resource=resource,
-            resource_id=resource_id,
-        )
-
-        db.session.add(attachment)
-        db.session.commit()
-
-        return attachment
-
     @classmethod
     def get_or_create(cls, object_name, params):
         try:

atst/models/task_order.py

@@ -4,7 +4,6 @@ from enum import Enum
 from sqlalchemy import Column, DateTime, ForeignKey, String
 from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.orm import relationship
-from werkzeug.datastructures import FileStorage
 
 from atst.models import Attachment, Base, mixins, types
 from atst.models.clin import JEDICLINType
@@ -57,8 +56,6 @@ class TaskOrder(Base, mixins.TimestampsMixin):
     def _set_attachment(self, new_attachment, attribute):
         if isinstance(new_attachment, Attachment):
             return new_attachment
-        elif isinstance(new_attachment, FileStorage):
-            return Attachment.attach(new_attachment, "task_order", self.id)
         elif isinstance(new_attachment, dict):
             if new_attachment["filename"] and new_attachment["object_name"]:
                 attachment = Attachment.get_or_create(

atst/routes/task_orders/new.py

@@ -17,7 +17,7 @@ from atst.utils.flash import formatted_flash as flash
 
 
 def render_task_orders_edit(template, portfolio_id=None, task_order_id=None, form=None):
-    (token, object_name) = current_app.uploader.get_token()
+    (token, object_name) = current_app.csp.files.get_token()
     render_args = {"token": token, "object_name": object_name}
 
     if task_order_id:

config/base.ini

@@ -3,9 +3,6 @@ CAC_URL = http://localhost:8000/login-redirect
 CA_CHAIN = ssl/server-certs/ca-chain.pem
 CLASSIFIED = false
 COOKIE_SECRET = some-secret-please-replace
-CRL_STORAGE_CONTAINER = crls
-CRL_STORAGE_PROVIDER = LOCAL
-CRL_STORAGE_REGION = iad
 DISABLE_CRL_CHECK = false
 CRL_FAIL_OPEN = false
 DEBUG = true
@@ -28,10 +25,5 @@ SESSION_COOKIE_NAME=atat
 SESSION_TYPE = redis
 SESSION_USE_SIGNER = True
 SQLALCHEMY_ECHO = False
-STORAGE_CONTAINER=uploads
-STORAGE_KEY=''
-STORAGE_SECRET=''
-STORAGE_PROVIDER=LOCAL
-STORAGE_CRL_ARCHIVE_NAME = dod_crls.tar.bz
 WTF_CSRF_ENABLED = true
 LIMIT_CONCURRENT_SESSIONS = false

tests/domain/csp/test_files.py

@@ -1,75 +0,0 @@
-import os
-import pytest
-from werkzeug.datastructures import FileStorage
-from unittest.mock import Mock
-
-from atst.domain.csp.files import (
-    CSPFileError,
-    RackspaceFileProvider,
-    RackspaceCRLProvider,
-)
-from atst.domain.exceptions import UploadError
-
-from tests.mocks import PDF_FILENAME
-
-
-@pytest.fixture
-def uploader(app):
-    return RackspaceFileProvider(app)
-
-
-NONPDF_FILENAME = "tests/fixtures/disa-pki.html"
-
-
-def test_upload(app, uploader, pdf_upload):
-    object_name = uploader.upload(pdf_upload)
-    upload_dir = app.config["STORAGE_CONTAINER"]
-    assert os.path.isfile(os.path.join(upload_dir, object_name))
-
-
-def test_upload_fails_for_non_pdfs(uploader):
-    with open(NONPDF_FILENAME, "rb") as fp:
-        fs = FileStorage(fp, content_type="text/plain")
-        with pytest.raises(UploadError):
-            uploader.upload(fs)
-
-
-def test_download(app, uploader, pdf_upload):
-    # write pdf content to upload file storage and make sure it is flushed to
-    # disk
-    pdf_upload.seek(0)
-    pdf_content = pdf_upload.read()
-    pdf_upload.close()
-    upload_dir = app.config["STORAGE_CONTAINER"]
-    full_path = os.path.join(upload_dir, "abc")
-    with open(full_path, "wb") as output_file:
-        output_file.write(pdf_content)
-        output_file.flush()
-
-    stream = uploader.download("abc")
-    stream_content = b"".join([b for b in stream])
-    assert pdf_content == stream_content
-
-
-def test_downloading_uploaded_object(uploader, pdf_upload):
-    object_name = uploader.upload(pdf_upload)
-    stream = uploader.download(object_name)
-    stream_content = b"".join([b for b in stream])
-
-    pdf_upload.seek(0)
-    pdf_content = pdf_upload.read()
-
-    assert stream_content == pdf_content
-
-
-def test_crl_download_fails(app, monkeypatch):
-    mock_object = Mock()
-    mock_object.download.return_value = False
-    monkeypatch.setattr(
-        "atst.domain.csp.files.RackspaceCRLProvider.object", mock_object
-    )
-
-    rs_crl_provider = RackspaceCRLProvider(app)
-
-    with pytest.raises(CSPFileError):
-        rs_crl_provider.sync_crls()

tests/domain/test_task_orders.py

@@ -75,7 +75,7 @@ def test_task_order_sorting():
     assert TaskOrders.sort(task_orders) == task_orders
 
 
-def test_create_adds_clins(pdf_upload):
+def test_create_adds_clins():
     portfolio = PortfolioFactory.create()
     clins = [
         {
@@ -100,12 +100,12 @@ def test_create_adds_clins(pdf_upload):
         portfolio_id=portfolio.id,
         number="0123456789",
         clins=clins,
-        pdf=pdf_upload,
+        pdf={"filename": "sample.pdf", "object_name": "1234567"},
     )
     assert len(task_order.clins) == 2
 
 
-def test_update_adds_clins(pdf_upload):
+def test_update_adds_clins():
     task_order = TaskOrderFactory.create(number="1231231234")
     to_number = task_order.number
     clins = [
@@ -131,13 +131,13 @@ def test_update_adds_clins(pdf_upload):
         portfolio_id=task_order.portfolio_id,
         number="0000000000",
         clins=clins,
-        pdf=pdf_upload,
+        pdf={"filename": "sample.pdf", "object_name": "1234567"},
     )
     assert task_order.number != to_number
     assert len(task_order.clins) == 2
 
 
-def test_update_does_not_duplicate_clins(pdf_upload):
+def test_update_does_not_duplicate_clins():
     task_order = TaskOrderFactory.create(
         number="3453453456", create_clins=["123", "456"]
     )
@@ -160,7 +160,10 @@ def test_update_does_not_duplicate_clins(pdf_upload):
         },
     ]
     task_order = TaskOrders.update(
-        task_order_id=task_order.id, number="0000000000", clins=clins, pdf=pdf_upload
+        task_order_id=task_order.id,
+        number="0000000000",
+        clins=clins,
+        pdf={"filename": "sample.pdf", "object_name": "1234567"},
     )
     assert len(task_order.clins) == 2
     for clin in task_order.clins:

tests/models/test_attachment.py

@@ -1,25 +0,0 @@
-import pytest
-from werkzeug.datastructures import FileStorage
-
-from atst.models.attachment import Attachment, AttachmentError
-
-from tests.mocks import PDF_FILENAME
-
-
-def test_attach(pdf_upload):
-    attachment = Attachment.attach(pdf_upload)
-    assert attachment.filename == PDF_FILENAME
-    assert attachment.object_name is not None
-
-
-def test_attach_raises():
-    with open(PDF_FILENAME, "rb") as fp:
-        fs = FileStorage(fp, content_type="something/else")
-        with pytest.raises(AttachmentError):
-            Attachment.attach(fs)
-
-
-def test_repr(pdf_upload):
-    attachment = Attachment.attach(pdf_upload)
-    assert attachment.filename in str(attachment)
-    assert str(attachment.id) in str(attachment)

tests/models/test_task_order.py

@@ -157,12 +157,9 @@ class TestPDF:
 
         assert to.pdf_attachment_id == attachment.id
 
-    def test_setting_pdf_with_file_storage(self):
+    def test_setting_pdf_with_dictionary(self):
         to = TaskOrder()
-        with open(PDF_FILENAME, "rb") as fp:
+        to.pdf = {"filename": PDF_FILENAME, "object_name": "123456"}
-            fs = FileStorage(fp, content_type="application/pdf")
-            to.pdf = fs
-
         assert to.pdf is not None
         assert to.pdf.filename == PDF_FILENAME
 

tests/routes/task_orders/test_new.py

@@ -87,7 +87,7 @@ def test_task_orders_submit_form_step_one_add_pdf_existing_to(client, user_sessi
 
 
 def test_task_orders_submit_form_step_one_add_pdf_delete_pdf(
-    client, user_session, portfolio, pdf_upload
+    client, user_session, portfolio
 ):
     user_session(portfolio.owner)
     task_order = TaskOrderFactory.create(portfolio=portfolio)