Merge pull request #13612 from edx/saleem-latif/WL-611

WL-611: Fix "Access Denied" when downloading report

lms/djangoapps/instructor_task/models.py

@@ -194,10 +194,11 @@ class ReportStore(object):
         storage_type = config.get('STORAGE_TYPE', '').lower()
         if storage_type == 's3':
             return DjangoStorageReportStore(
-                storage_class='storages.backends.s3boto.S3BotoStorage',
+                storage_class='openedx.core.storage.S3ReportStorage',
                 storage_kwargs={
                     'bucket': config['BUCKET'],
                     'location': config['ROOT_PATH'],
+                    'custom_domain': config.get("CUSTOM_DOMAIN", None),
                     'querystring_expire': 300,
                     'gzip': True,
                 },

lms/djangoapps/instructor_task/tests/test_models.py

@@ -7,7 +7,7 @@ import time
 
 import boto
 from django.conf import settings
-from django.test import SimpleTestCase, override_settings
+from django.test import SimpleTestCase, override_settings, TestCase
 from mock import patch
 
 from common.test.utils import MockS3Mixin
@@ -103,7 +103,7 @@ class DjangoStorageReportStoreS3TestCase(MockS3Mixin, ReportStoreTestMixin, Test
         storage.
         """
         test_settings = copy.deepcopy(settings.GRADES_DOWNLOAD)
-        test_settings['STORAGE_CLASS'] = 'storages.backends.s3boto.S3BotoStorage'
+        test_settings['STORAGE_CLASS'] = 'openedx.core.storage.S3ReportStorage'
         test_settings['STORAGE_KWARGS'] = {
             'bucket': settings.GRADES_DOWNLOAD['BUCKET'],
             'location': settings.GRADES_DOWNLOAD['ROOT_PATH'],
@@ -112,3 +112,24 @@ class DjangoStorageReportStoreS3TestCase(MockS3Mixin, ReportStoreTestMixin, Test
             connection = boto.connect_s3()
             connection.create_bucket(settings.GRADES_DOWNLOAD['STORAGE_KWARGS']['bucket'])
             return ReportStore.from_config(config_name='GRADES_DOWNLOAD')
+
+
+class TestS3ReportStorage(MockS3Mixin, TestCase):
+    """
+    Test the S3ReportStorage to make sure that configuration overrides from settings.FINANCIAL_REPORTS
+    are used instead of default ones.
+    """
+    def test_financial_report_overrides(self):
+        """
+        Test that CUSTOM_DOMAIN from FINANCIAL_REPORTS is used to construct file url. instead of domain defined via
+        AWS_S3_CUSTOM_DOMAIN setting.
+        """
+        with override_settings(FINANCIAL_REPORTS={
+            'STORAGE_TYPE': 's3',
+            'BUCKET': 'edx-financial-reports',
+            'CUSTOM_DOMAIN': 'edx-financial-reports.s3.amazonaws.com',
+            'ROOT_PATH': 'production',
+        }):
+            report_store = ReportStore.from_config(config_name="FINANCIAL_REPORTS")
+            # Make sure CUSTOM_DOMAIN from FINANCIAL_REPORTS is used to construct file url
+            self.assertIn("edx-financial-reports.s3.amazonaws.com", report_store.storage.url(""))

lms/envs/common.py

@@ -2372,6 +2372,7 @@ GRADES_DOWNLOAD = {
 FINANCIAL_REPORTS = {
     'STORAGE_TYPE': 'localfs',
     'BUCKET': 'edx-financial-reports',
+    'CUSTOM_DOMAIN': 'edx-financial-reports.s3.amazonaws.com',
     'ROOT_PATH': '/tmp/edx-s3/financial_reports',
 }
 

openedx/core/storage.py

@@ -13,6 +13,7 @@ from openedx.core.djangoapps.theming.storage import (
     ThemeCachedFilesMixin,
     ThemePipelineMixin
 )
+from storages.backends.s3boto import S3BotoStorage
 
 
 class ProductionStorage(
@@ -44,6 +45,29 @@ class DevelopmentStorage(
     pass
 
 
+class S3ReportStorage(S3BotoStorage):  # pylint: disable=abstract-method
+    """
+    Storage for reports.
+    """
+    def __init__(self, acl=None, bucket=None, custom_domain=None, **settings):
+        """
+        init method for S3ReportStorage, Note that we have added an extra key-word
+        argument named "custom_domain" and this argument should not be passed to the superclass's init.
+
+        Args:
+            acl: content policy for the uploads i.e. private, public etc.
+            bucket: Name of S3 bucket to use for storing and/or retrieving content
+            custom_domain: custom domain to use for generating file urls
+            **settings: additional settings to be passed in to S3BotoStorage,
+
+        Returns:
+
+        """
+        if custom_domain:
+            self.custom_domain = custom_domain
+        super(S3ReportStorage, self).__init__(acl=acl, bucket=bucket, **settings)
+
+
 @lru_cache()
 def get_storage(storage_class=None, **kwargs):
     """