diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 94e00b948fca934313529e240866160e6128e544..ad7d19fbccf5e6f430fcf5174cb3157aa16aa7d5 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 81c327a317f8fcb74da56cbe8385db759ede2de0..db4ef25e13a0cd01ed3d9f37831f44ab29f97494 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 8423802c6b86dde94c8e22d499fbd07eb15f6798..f22f200f165a973294d674184a57c6de205bcf12 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 1f4cb29f53d9f106b19d5c5edfde621e6422b25d..193dd640292e54e155a606f388665458a9ad39e8 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 0f344427e4120555fbc33257cc71bf8cb1aa84b9..05bdfca0240be1ff01d59f887b600268d09ce3b8 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 1af02fae5d2daf4a56961250ef2ae6b7f6db5523..9e4f7cd4f5ec4b942208f5c2ba19a913d995e48a 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 cd4467e1a1bbbaacfac062a3ee9bdddb90fde57f..73eed98a743838f940300766d2d4396ddc3c9215 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 817b149361d82b8b1bdc373c59292400eaa64dae..22448f5467bb8361b3c111d9e50f90eb9c164361 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):