diff --git a/physionet-django/project/cloud/s3.py b/physionet-django/project/cloud/s3.py index 215461ce2..1c99c1d7d 100644 --- a/physionet-django/project/cloud/s3.py +++ b/physionet-django/project/cloud/s3.py @@ -711,7 +711,7 @@ def get_access_point_name_for_user_and_project(current_user, aws): ).first() if access_point: return access_point.name - except Exception as e: + except Exception: return "No access point found for this user with the specified project details" return "No access point found for this user with the specified project details" @@ -879,7 +879,8 @@ def create_data_access_point_policy( "Principal": principal_value, "Action": ["s3:GetObject", "s3:ListBucket"], "Resource": [ - f"arn:aws:s3:us-east-1:{settings.AWS_ACCOUNT_ID}:accesspoint/{access_point_name}/object/{project_slug}/{project_version}/*", + f"arn:aws:s3:us-east-1:{settings.AWS_ACCOUNT_ID}:accesspoint/" + f"{access_point_name}/object/{project_slug}/{project_version}/*", f"arn:aws:s3:us-east-1:{settings.AWS_ACCOUNT_ID}:accesspoint/{access_point_name}", ], } @@ -912,7 +913,7 @@ def set_data_access_point_policy(data_access_point_name, data_access_point_polic if s3_control is None: return try: - response = s3_control.put_access_point_policy( + s3_control.put_access_point_policy( AccountId=settings.AWS_ACCOUNT_ID, Name=data_access_point_name, Policy=data_access_point_policy, @@ -1023,7 +1024,7 @@ def update_data_access_point_policy(project): data_access_point_version = str(i + 1).zfill(2) data_access_point_name = f"{project.slug}-v{project.version.replace('.', '-')}-{data_access_point_version}" subset_aws_ids = aws_ids[ - i * MAX_PRINCIPALS_PER_AP_POLICY : (i + 1) * MAX_PRINCIPALS_PER_AP_POLICY + i * MAX_PRINCIPALS_PER_AP_POLICY: (i + 1) * MAX_PRINCIPALS_PER_AP_POLICY ] access_point = AWSAccessPoint.objects.filter( name=data_access_point_name, aws__project=project @@ -1120,7 +1121,7 @@ def create_s3_access_point(project, access_point_name, bucket_name, account_id): """ s3 = create_s3_control_client() try: - response = s3.create_access_point( + s3.create_access_point( AccountId=account_id, Bucket=bucket_name, Name=access_point_name, @@ -1199,9 +1200,9 @@ def upload_project_to_S3(project): raise Exception(f"A bucket named {bucket_name} already exists.") except s3.exceptions.BucketAlreadyOwnedByYou: bucket_created = False - + # Set the bucket policy only if the bucket was newly created and has controlled access - if bucket_created and project.access_policy == AccessPolicy.CONTROLLED: + if bucket_created and project.access_policy == AccessPolicy.CREDENTIALED: controlled_policy = create_controlled_bucket_policy(bucket_name) s3.put_bucket_policy(Bucket=bucket_name, Policy=controlled_policy) diff --git a/physionet-django/project/modelcomponents/middleware.py b/physionet-django/project/modelcomponents/middleware.py index f31d23a89..4dfb5dd65 100644 --- a/physionet-django/project/modelcomponents/middleware.py +++ b/physionet-django/project/modelcomponents/middleware.py @@ -2,10 +2,12 @@ _request_local = threading.local() + def get_current_request(): """Retrieve the current request from thread-local storage.""" return getattr(_request_local, 'request', None) + class CurrentRequestMiddleware: """Middleware to store the current request in thread-local storage.""" def __init__(self, get_response): diff --git a/physionet-django/project/modelcomponents/storage.py b/physionet-django/project/modelcomponents/storage.py index 7a6eaef3b..5d349912f 100644 --- a/physionet-django/project/modelcomponents/storage.py +++ b/physionet-django/project/modelcomponents/storage.py @@ -63,7 +63,7 @@ class AWS(models.Model): class Meta: default_permissions = () - + def s3_uri(self): """ Construct the S3 URI for the project. @@ -81,7 +81,10 @@ def s3_uri(self): # Fetch access point name access_point_name = get_access_point_name_for_user_and_project(current_user, self) if access_point_name and "No " not in access_point_name: - return f's3://arn:aws:s3:us-east-1:{settings.AWS_ACCOUNT_ID}:accesspoint/{access_point_name}/{self.project.slug}/{self.project.version}/' + return ( + f's3://arn:aws:s3:us-east-1:{settings.AWS_ACCOUNT_ID}:accesspoint/' + f'{access_point_name}/{self.project.slug}/{self.project.version}/' + ) else: print(f"Error: {access_point_name}") return None @@ -105,12 +108,12 @@ class AWSAccessPoint(models.Model): class AWSAccessPointUser(models.Model): access_point = models.ForeignKey( - AWSAccessPoint, + AWSAccessPoint, related_name='linked_users', on_delete=models.CASCADE ) user = models.ForeignKey( - 'user.User', + 'user.User', related_name='aws_access_point_users', on_delete=models.CASCADE ) @@ -120,4 +123,3 @@ class Meta: def __str__(self): return f"User: {self.user}, Access Point: {self.access_point}" - diff --git a/physionet-django/project/test_s3.py b/physionet-django/project/test_s3.py index 572bf6898..7581b5986 100644 --- a/physionet-django/project/test_s3.py +++ b/physionet-django/project/test_s3.py @@ -22,6 +22,7 @@ AWS_ACCOUNT_ID='123456789012', S3_OPEN_ACCESS_BUCKET='datashare-public', S3_SERVER_ACCESS_LOG_BUCKET='datashare-logs', + S3_CONTROLLED_ACCESS_BUCKET='datashare-protected', ) class TestS3(TestMixin): """ diff --git a/physionet-django/project/views.py b/physionet-django/project/views.py index 6ccc36f0a..6be556561 100644 --- a/physionet-django/project/views.py +++ b/physionet-django/project/views.py @@ -1938,7 +1938,7 @@ def published_project(request, project_slug, version, subdir=''): s3_uri = project.aws.s3_uri() else: s3_uri = '--no-sign-request ' + project.aws.s3_uri() - + context = { 'project': project, 'authors': authors, @@ -2048,7 +2048,11 @@ def sign_dua(request, project_slug, version): if request.method == 'POST' and 'agree' in request.POST: DUASignature.objects.create(user=user, project=project) - if has_s3_credentials() and files_sent_to_S3(project) is not None and s3_bucket_has_credentialed_users(project): + if ( + has_s3_credentials() + and files_sent_to_S3(project) is not None + and s3_bucket_has_credentialed_users(project) + ): update_data_access_point_policy(project) return render(request, 'project/sign_dua_complete.html', { 'project':project})