From 9660a56093a2594a4b4788030ae21f58d64251df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20G=C3=B6bel?= <dgoebel@techfak.uni-bielefeld.de>
Date: Tue, 3 Jan 2023 11:49:48 +0100
Subject: [PATCH] Add write-only buckets to the 'list buckets' response
#34
---
.gitlab-ci.yml | 4 ++--
CHANGELOG.md | 8 ++++++++
app/api/dependencies.py | 2 +-
app/crud/crud_bucket.py | 10 +---------
app/main.py | 2 +-
app/schemas/bucket_permission.py | 5 +++--
app/tests/crud/test_bucket.py | 3 ++-
app/tests/unit/test_bucket_permission_scheme.py | 14 +++++++++++---
8 files changed, 29 insertions(+), 19 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 94e00b9..ad7d19f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -73,7 +73,7 @@ e2e-test-job: # Runs e2e tests on the API endpoints
OIDC_CLIENT_SECRET: "$TEST_OIDC_CLIENT_SECRET"
OIDC_CLIENT_ID: "$TEST_OIDC_CLIENT_ID"
OIDC_BASE_URI: "http://mock-oidc-server"
- CLIENTS_CONFIGURATION_INLINE: "$TEST_OIDC_CLIENT_CONFIG"
+ CLIENTS_CONFIGURATION_PATH: "$TEST_OIDC_CLIENT_CONFIG"
services:
- name: mysql:8
alias: e2e-test-db
@@ -82,7 +82,7 @@ e2e-test-job: # Runs e2e tests on the API endpoints
MYSQL_DATABASE: "$DB_DATABASE"
MYSQL_USER: "$DB_USER"
MYSQL_PASSWORD: "$DB_PASSWORD"
- - name: ghcr.io/soluto/oidc-server-mock:latest
+ - name: ghcr.io/soluto/oidc-server-mock:0.8.1
alias: mock-oidc-server
variables:
ASPNETCORE_ENVIRONMENT: "Development"
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 81c327a..db4ef25 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,11 @@
+## Version 1.1.2
+
+### General
+* When requesting a list of buckets, all buckets, including WRITE-only buckets, are returned #34
+
+### Fixes
+* User with WRITE/READWRITE permission can now delete multiple Objects with a single request to the S3 endpoint #34
+
## Version 1.1.1
### General
diff --git a/app/api/dependencies.py b/app/api/dependencies.py
index 8423802..f22f200 100644
--- a/app/api/dependencies.py
+++ b/app/api/dependencies.py
@@ -152,7 +152,7 @@ async def get_current_bucket(
) -> Bucket:
"""
Get the Bucket from the database based on the name in the path.
- Reject the request if user has no READ permission for this bucket.
+ Reject the request if user has no permission for this bucket.
FastAPI Dependency Injection Function
diff --git a/app/crud/crud_bucket.py b/app/crud/crud_bucket.py
index 1f4cb29..193dd64 100644
--- a/app/crud/crud_bucket.py
+++ b/app/crud/crud_bucket.py
@@ -5,7 +5,6 @@ from sqlalchemy.orm import joinedload
from app.models.bucket import Bucket
from app.models.bucket_permission import BucketPermission as BucketPermissionDB
-from app.models.bucket_permission import PermissionEnum
from app.schemas.bucket import BucketIn as BucketInSchema
@@ -34,7 +33,7 @@ class CRUDBucket:
@staticmethod
async def get_for_user(db: AsyncSession, uid: str) -> list[Bucket]:
"""
- Get all buckets where the given user has READ permissions for.
+ Get all buckets where the given user is the owner or has any permission.
Parameters
----------
@@ -57,7 +56,6 @@ class CRUDBucket:
WHERE bucket.owner_id = %s OR (EXISTS
(SELECT 1 FROM bucketpermission
WHERE bucket.name = bucketpermission.bucket_name AND bucketpermission.user_id = %s
- AND(bucketpermission.permissions = %s OR bucketpermission.permissions = %s)
AND(datediff(now(), bucketpermission.`from`) <= %s OR bucketpermission.`from` IS NULL)
AND(datediff(now(), bucketpermission.`to`) >= %s OR bucketpermission.`to` IS NULL)))
"""
@@ -68,12 +66,6 @@ class CRUDBucket:
or_(
Bucket.owner_id == uid,
Bucket.permissions.any(BucketPermissionDB.user_id == uid)
- .where(
- or_(
- BucketPermissionDB.permissions == PermissionEnum.READ,
- BucketPermissionDB.permissions == PermissionEnum.READWRITE,
- )
- )
.where(
or_(
func.datediff(func.now(), BucketPermissionDB.from_) >= 0,
diff --git a/app/main.py b/app/main.py
index 0f34442..05bdfca 100644
--- a/app/main.py
+++ b/app/main.py
@@ -23,7 +23,7 @@ def custom_generate_unique_id(route: APIRoute) -> str:
app = FastAPI(
title="S3-Proxy",
- version="1.1.1",
+ version="1.1.2",
description=description,
contact={
"name": "Daniel Goebel",
diff --git a/app/schemas/bucket_permission.py b/app/schemas/bucket_permission.py
index 1af02fa..9e4f7cd 100644
--- a/app/schemas/bucket_permission.py
+++ b/app/schemas/bucket_permission.py
@@ -75,11 +75,12 @@ class BucketPermissionIn(BucketPermissionParameters):
"Action": [],
"Condition": {},
}
+ bucket_policy["Action"] += ["s3:ListBucket"]
if self.permission == PermissionEnum.READ or self.permission == PermissionEnum.READWRITE:
- bucket_policy["Action"] += ["s3:ListBucket"]
obj_policy["Action"] += ["s3:GetObject"]
if self.permission == PermissionEnum.WRITE or self.permission == PermissionEnum.READWRITE:
obj_policy["Action"] += ["s3:DeleteObject", "s3:PutObject"]
+ bucket_policy["Action"] += ["s3:DeleteObject"]
if self.to_timestamp is not None:
obj_policy["Condition"]["DateLessThan"] = {
"aws:CurrentTime": self.to_timestamp.strftime("%Y-%m-%dT%H:%M:%SZ")
@@ -96,7 +97,7 @@ class BucketPermissionIn(BucketPermissionParameters):
del bucket_policy["Condition"]
if len(obj_policy["Condition"]) == 0:
del obj_policy["Condition"]
- return [obj_policy] if self.permission == PermissionEnum.WRITE else [obj_policy, bucket_policy]
+ return [obj_policy, bucket_policy]
class BucketPermissionOut(BucketPermissionIn):
diff --git a/app/tests/crud/test_bucket.py b/app/tests/crud/test_bucket.py
index cd4467e..73eed98 100644
--- a/app/tests/crud/test_bucket.py
+++ b/app/tests/crud/test_bucket.py
@@ -169,7 +169,8 @@ class TestBucketCRUDGet:
buckets = await CRUDBucket.get_for_user(db, random_second_user.uid)
- assert len(buckets) == 0
+ assert len(buckets) == 1
+ assert buckets[0] == random_bucket
@pytest.mark.asyncio
async def test_get_bucket_with_valid_time_permission(
diff --git a/app/tests/unit/test_bucket_permission_scheme.py b/app/tests/unit/test_bucket_permission_scheme.py
index 817b149..22448f5 100644
--- a/app/tests/unit/test_bucket_permission_scheme.py
+++ b/app/tests/unit/test_bucket_permission_scheme.py
@@ -63,7 +63,7 @@ class TestPermissionPolicyPermissionType(_TestPermissionPolicy):
"""
random_base_permission.permission = PermissionEnum.WRITE
stmts = random_base_permission.map_to_bucket_policy_statement(user_id=random_lower_string())
- assert len(stmts) == 1
+ assert len(stmts) == 2
object_stmt = stmts[0]
with pytest.raises(KeyError):
@@ -72,6 +72,13 @@ class TestPermissionPolicyPermissionType(_TestPermissionPolicy):
assert "s3:PutObject" in object_stmt["Action"]
assert "s3:DeleteObject" in object_stmt["Action"]
+ bucket_stmt = stmts[1]
+ with pytest.raises(KeyError):
+ assert bucket_stmt["Condition"]
+ assert len(bucket_stmt["Action"]) == 2
+ assert "s3:ListBucket" in bucket_stmt["Action"]
+ assert "s3:DeleteObject" in bucket_stmt["Action"]
+
def test_READWRITE_permission(self, random_base_permission: BucketPermissionIn) -> None:
"""
Test for converting a READWRITE Permission into a bucket policy statement.
@@ -96,8 +103,9 @@ class TestPermissionPolicyPermissionType(_TestPermissionPolicy):
bucket_stmt = stmts[1]
with pytest.raises(KeyError):
assert bucket_stmt["Condition"]
- assert len(bucket_stmt["Action"]) == 1
- assert bucket_stmt["Action"][0] == "s3:ListBucket"
+ assert len(bucket_stmt["Action"]) == 2
+ assert "s3:ListBucket" in bucket_stmt["Action"]
+ assert "s3:DeleteObject" in bucket_stmt["Action"]
class TestPermissionPolicyCondition(_TestPermissionPolicy):
--
GitLab