Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(server): Check asset permissions in bulk #5329

Conversation

adamantike
Copy link
Contributor

Modify Access repository, to evaluate asset permissions in bulk. Queries have been validated to match what they currently generate for single ids.

Queries:

  • asset album access:
-- Before
SELECT 1 AS "row_exists" FROM (SELECT 1 AS dummy_column) "dummy_table" WHERE EXISTS (
  SELECT 1
  FROM "albums" "AlbumEntity"
    LEFT JOIN "albums_assets_assets" "AlbumEntity_AlbumEntity__AlbumEntity_assets"
      ON "AlbumEntity_AlbumEntity__AlbumEntity_assets"."albumsId"="AlbumEntity"."id"
    LEFT JOIN "assets" "AlbumEntity__AlbumEntity_assets"
      ON "AlbumEntity__AlbumEntity_assets"."id"="AlbumEntity_AlbumEntity__AlbumEntity_assets"."assetsId"
      AND "AlbumEntity__AlbumEntity_assets"."deletedAt" IS NULL
    LEFT JOIN "albums_shared_users_users" "AlbumEntity_AlbumEntity__AlbumEntity_sharedUsers"
      ON "AlbumEntity_AlbumEntity__AlbumEntity_sharedUsers"."albumsId"="AlbumEntity"."id"
    LEFT JOIN "users" "AlbumEntity__AlbumEntity_sharedUsers"
      ON "AlbumEntity__AlbumEntity_sharedUsers"."id"="AlbumEntity_AlbumEntity__AlbumEntity_sharedUsers"."usersId"
      AND "AlbumEntity__AlbumEntity_sharedUsers"."deletedAt" IS NULL
  WHERE
    (
      ("AlbumEntity"."ownerId" = $1 AND "AlbumEntity__AlbumEntity_assets"."id" = $2)
      OR ("AlbumEntity__AlbumEntity_sharedUsers"."id" = $3 AND "AlbumEntity__AlbumEntity_assets"."id" = $4)
      OR ("AlbumEntity"."ownerId" = $5 AND "AlbumEntity__AlbumEntity_assets"."livePhotoVideoId" = $6)
      OR ("AlbumEntity__AlbumEntity_sharedUsers"."id" = $7 AND "AlbumEntity__AlbumEntity_assets"."livePhotoVideoId" = $8)
    )
    AND "AlbumEntity"."deletedAt" IS NULL
)
LIMIT 1

-- After
SELECT
  "asset"."id" AS "assetId",
  "asset"."livePhotoVideoId" AS "livePhotoVideoId"
FROM "albums" "album"
  INNER JOIN "albums_assets_assets" "album_asset"
    ON "album_asset"."albumsId"="album"."id"
  INNER JOIN "assets" "asset"
    ON "asset"."id"="album_asset"."assetsId"
    AND "asset"."deletedAt" IS NULL
  LEFT JOIN "albums_shared_users_users" "album_sharedUsers"
    ON "album_sharedUsers"."albumsId"="album"."id"
  LEFT JOIN "users" "sharedUsers"
    ON "sharedUsers"."id"="album_sharedUsers"."usersId"
    AND "sharedUsers"."deletedAt" IS NULL
WHERE
  (
    "album"."ownerId" = $1
    OR "sharedUsers"."id" = $2
  )
  AND (
    "asset"."id" IN ($3, $4)
    OR "asset"."livePhotoVideoId" IN ($5, $6)
  )
  AND "album"."deletedAt" IS NULL
  • asset owner access:
-- Before
SELECT 1 AS "row_exists" FROM (SELECT 1 AS dummy_column) "dummy_table" WHERE EXISTS (
  SELECT 1
  FROM "assets" "AssetEntity"
  WHERE
    "AssetEntity"."id" = $1
    AND "AssetEntity"."ownerId" = $2
)
LIMIT 1

-- After
SELECT
  "AssetEntity"."id" AS "AssetEntity_id"
FROM "assets" "AssetEntity"
WHERE
  "AssetEntity"."id" IN ($1, $2)
  AND "AssetEntity"."ownerId" = $3
  • asset partner access:
-- Before
SELECT 1 AS "row_exists" FROM (SELECT 1 AS dummy_column) "dummy_table" WHERE EXISTS (
  SELECT 1
  FROM "partners" "PartnerEntity"
    LEFT JOIN "users" "PartnerEntity__PartnerEntity_sharedWith"
      ON "PartnerEntity__PartnerEntity_sharedWith"."id"="PartnerEntity"."sharedWithId"
      AND "PartnerEntity__PartnerEntity_sharedWith"."deletedAt" IS NULL
    LEFT JOIN "users" "PartnerEntity__PartnerEntity_sharedBy"
      ON "PartnerEntity__PartnerEntity_sharedBy"."id"="PartnerEntity"."sharedById"
      AND "PartnerEntity__PartnerEntity_sharedBy"."deletedAt" IS NULL
    LEFT JOIN "assets" "0aabe9f4a62b794e2c24a074297e534f51a4ac6c"
      ON "0aabe9f4a62b794e2c24a074297e534f51a4ac6c"."ownerId"="PartnerEntity__PartnerEntity_sharedBy"."id"
      AND "0aabe9f4a62b794e2c24a074297e534f51a4ac6c"."deletedAt" IS NULL
    LEFT JOIN "users" "PartnerEntity__sharedBy"
      ON "PartnerEntity__sharedBy"."id"="PartnerEntity"."sharedById"
      AND "PartnerEntity__sharedBy"."deletedAt" IS NULL
    LEFT JOIN "users" "PartnerEntity__sharedWith"
      ON "PartnerEntity__sharedWith"."id"="PartnerEntity"."sharedWithId"
      AND "PartnerEntity__sharedWith"."deletedAt" IS NULL
  WHERE
    "PartnerEntity__PartnerEntity_sharedWith"."id" = $1
    AND "0aabe9f4a62b794e2c24a074297e534f51a4ac6c"."id" = $2
)
LIMIT 1

-- After
SELECT
  "asset"."id" AS "assetId"
FROM "partners" "partner"
  INNER JOIN "users" "sharedBy"
    ON "sharedBy"."id"="partner"."sharedById"
    AND "sharedBy"."deletedAt" IS NULL
  INNER JOIN "assets" "asset"
    ON "asset"."ownerId"="sharedBy"."id"
    AND "asset"."deletedAt" IS NULL
WHERE
  "partner"."sharedWithId" = $1
  AND "asset"."id" IN ($2, $3)
  • asset shared link access:
-- Before
SELECT 1 AS "row_exists" FROM (SELECT 1 AS dummy_column) "dummy_table" WHERE EXISTS (
  SELECT 1
  FROM "shared_links" "SharedLinkEntity"
    LEFT JOIN "albums" "SharedLinkEntity__SharedLinkEntity_album"
      ON "SharedLinkEntity__SharedLinkEntity_album"."id"="SharedLinkEntity"."albumId"
      AND "SharedLinkEntity__SharedLinkEntity_album"."deletedAt" IS NULL
    LEFT JOIN "albums_assets_assets" "760f12c00d97bdcec1ce224d1e3bf449859942b6"
      ON "760f12c00d97bdcec1ce224d1e3bf449859942b6"."albumsId"="SharedLinkEntity__SharedLinkEntity_album"."id"
    LEFT JOIN "assets" "4a35f463ae8c5544ede95c4b6d9ce8c686b6bfe6"
      ON "4a35f463ae8c5544ede95c4b6d9ce8c686b6bfe6"."id"="760f12c00d97bdcec1ce224d1e3bf449859942b6"."assetsId"
      AND "4a35f463ae8c5544ede95c4b6d9ce8c686b6bfe6"."deletedAt" IS NULL
    LEFT JOIN "shared_link__asset" "SharedLinkEntity__SharedLinkEntity_assets_SharedLinkEntity"
      ON "SharedLinkEntity__SharedLinkEntity_assets_SharedLinkEntity"."sharedLinksId"="SharedLinkEntity"."id"
    LEFT JOIN "assets" "SharedLinkEntity__SharedLinkEntity_assets"
      ON "SharedLinkEntity__SharedLinkEntity_assets"."id"="SharedLinkEntity__SharedLinkEntity_assets_SharedLinkEntity"."assetsId"
      AND "SharedLinkEntity__SharedLinkEntity_assets"."deletedAt" IS NULL
  WHERE (
    ("SharedLinkEntity"."id" = $1 AND "4a35f463ae8c5544ede95c4b6d9ce8c686b6bfe6"."id" = $2)
    OR ("SharedLinkEntity"."id" = $3 AND "SharedLinkEntity__SharedLinkEntity_assets"."id" = $4)
    OR ("SharedLinkEntity"."id" = $5 AND "4a35f463ae8c5544ede95c4b6d9ce8c686b6bfe6"."livePhotoVideoId" = $6)
    OR ("SharedLinkEntity"."id" = $7 AND "SharedLinkEntity__SharedLinkEntity_assets"."livePhotoVideoId" = $8)
  )
)
LIMIT 1

-- After
SELECT
  "assets"."id" AS "assetId",
  "assets"."livePhotoVideoId" AS "assetLivePhotoVideoId",
  "albumAssets"."id" AS "albumAssetId",
  "albumAssets"."livePhotoVideoId" AS "albumAssetLivePhotoVideoId"
FROM "shared_links" "sharedLink"
  LEFT JOIN "albums" "album"
    ON "album"."id"="sharedLink"."albumId"
    AND "album"."deletedAt" IS NULL
  LEFT JOIN "shared_link__asset" "assets_sharedLink"
    ON "assets_sharedLink"."sharedLinksId"="sharedLink"."id"
  LEFT JOIN "assets" "assets"
    ON "assets"."id"="assets_sharedLink"."assetsId"
    AND "assets"."deletedAt" IS NULL
  LEFT JOIN "albums_assets_assets" "album_albumAssets"
    ON "album_albumAssets"."albumsId"="album"."id"
  LEFT JOIN "assets" "albumAssets"
    ON "albumAssets"."id"="album_albumAssets"."assetsId"
    AND "albumAssets"."deletedAt" IS NULL
WHERE
  "sharedLink"."id" = $1
  AND (
    "assets"."id" IN ($2, $3)
    OR "albumAssets"."id" IN ($4, $5)
    OR "assets"."livePhotoVideoId" IN ($6, $7)
    OR "albumAssets"."livePhotoVideoId" IN ($8, $9)
  )

.select('asset.id', 'assetId')
.addSelect('asset.livePhotoVideoId', 'livePhotoVideoId')
.where(
new Brackets((qb) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason you have to use the query builder here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two main reasons I found to start using query builders:

  • Previous queries used an array of wheres, which worked fine because the entire query was encapsulated in an EXISTS check. For some of these complex queries, avoiding false positivies is a lot harder without query builders, if we don't want to go with UNION ALL in the SQL (e.g. for each retrieved row, is livePhotoVideoId or assetId the one that matched the WHERE condition.
  • As this bulk check can now potentially validate thousands of IDs in a single call, avoiding the ORM to create the full entities (and instead just get raw rows with a small set of selected columns) should reduce memory usage.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can manually control what field are selected even when not using a query builder btw.

@jrasm91
Copy link
Contributor

jrasm91 commented Nov 27, 2023

Thoughts on adding something like this?

type CheckFn = (ids: Set<string>) => Promise<Set<string>>;

const check = async (ids: Set<string>, ...checkers: CheckFn[]): Promise<Set<string>> => {
  let remaining = new Set(ids);
  let granted = new Set<string>();

  for (const checker of checkers) {
    const results = await checker(remaining);
    granted = setUnion(granted, results);
    remaining = setDifference(remaining, results);
  }

  return granted;
};

// usage
switch(permission) {

  case Permission.ASSET_READ: {
    return check(
      ids,
      (ids) => this.repository.asset.checkOwnerAccess(authUser.id, ids),
      (ids) => this.repository.asset.checkAlbumAccess(authUser.id, ids),
      (ids) => this.repository.asset.checkPartnerAccess(authUser.id, ids),
    );
  }
}

Modify Access repository, to evaluate `asset` permissions in bulk.
Queries have been validated to match what they currently generate for single ids.

Queries:

* `asset` album access:

```sql
-- Before
SELECT 1 AS "row_exists" FROM (SELECT 1 AS dummy_column) "dummy_table" WHERE EXISTS (
  SELECT 1
  FROM "albums" "AlbumEntity"
    LEFT JOIN "albums_assets_assets" "AlbumEntity_AlbumEntity__AlbumEntity_assets"
      ON "AlbumEntity_AlbumEntity__AlbumEntity_assets"."albumsId"="AlbumEntity"."id"
    LEFT JOIN "assets" "AlbumEntity__AlbumEntity_assets"
      ON "AlbumEntity__AlbumEntity_assets"."id"="AlbumEntity_AlbumEntity__AlbumEntity_assets"."assetsId"
      AND "AlbumEntity__AlbumEntity_assets"."deletedAt" IS NULL
    LEFT JOIN "albums_shared_users_users" "AlbumEntity_AlbumEntity__AlbumEntity_sharedUsers"
      ON "AlbumEntity_AlbumEntity__AlbumEntity_sharedUsers"."albumsId"="AlbumEntity"."id"
    LEFT JOIN "users" "AlbumEntity__AlbumEntity_sharedUsers"
      ON "AlbumEntity__AlbumEntity_sharedUsers"."id"="AlbumEntity_AlbumEntity__AlbumEntity_sharedUsers"."usersId"
      AND "AlbumEntity__AlbumEntity_sharedUsers"."deletedAt" IS NULL
  WHERE
    (
      ("AlbumEntity"."ownerId" = $1 AND "AlbumEntity__AlbumEntity_assets"."id" = $2)
      OR ("AlbumEntity__AlbumEntity_sharedUsers"."id" = $3 AND "AlbumEntity__AlbumEntity_assets"."id" = $4)
      OR ("AlbumEntity"."ownerId" = $5 AND "AlbumEntity__AlbumEntity_assets"."livePhotoVideoId" = $6)
      OR ("AlbumEntity__AlbumEntity_sharedUsers"."id" = $7 AND "AlbumEntity__AlbumEntity_assets"."livePhotoVideoId" = $8)
    )
    AND "AlbumEntity"."deletedAt" IS NULL
)
LIMIT 1

-- After
SELECT
  "asset"."id" AS "assetId",
  "asset"."livePhotoVideoId" AS "livePhotoVideoId"
FROM "albums" "album"
  INNER JOIN "albums_assets_assets" "album_asset"
    ON "album_asset"."albumsId"="album"."id"
  INNER JOIN "assets" "asset"
    ON "asset"."id"="album_asset"."assetsId"
    AND "asset"."deletedAt" IS NULL
  LEFT JOIN "albums_shared_users_users" "album_sharedUsers"
    ON "album_sharedUsers"."albumsId"="album"."id"
  LEFT JOIN "users" "sharedUsers"
    ON "sharedUsers"."id"="album_sharedUsers"."usersId"
    AND "sharedUsers"."deletedAt" IS NULL
WHERE
  (
    "album"."ownerId" = $1
    OR "sharedUsers"."id" = $2
  )
  AND (
    "asset"."id" IN ($3, $4)
    OR "asset"."livePhotoVideoId" IN ($5, $6)
  )
  AND "album"."deletedAt" IS NULL
```

* `asset` owner access:

```sql
-- Before
SELECT 1 AS "row_exists" FROM (SELECT 1 AS dummy_column) "dummy_table" WHERE EXISTS (
  SELECT 1
  FROM "assets" "AssetEntity"
  WHERE
    "AssetEntity"."id" = $1
    AND "AssetEntity"."ownerId" = $2
)
LIMIT 1

-- After
SELECT
  "AssetEntity"."id" AS "AssetEntity_id"
FROM "assets" "AssetEntity"
WHERE
  "AssetEntity"."id" IN ($1, $2)
  AND "AssetEntity"."ownerId" = $3
```

* `asset` partner access:

```sql
-- Before
SELECT 1 AS "row_exists" FROM (SELECT 1 AS dummy_column) "dummy_table" WHERE EXISTS (
  SELECT 1
  FROM "partners" "PartnerEntity"
    LEFT JOIN "users" "PartnerEntity__PartnerEntity_sharedWith"
      ON "PartnerEntity__PartnerEntity_sharedWith"."id"="PartnerEntity"."sharedWithId"
      AND "PartnerEntity__PartnerEntity_sharedWith"."deletedAt" IS NULL
    LEFT JOIN "users" "PartnerEntity__PartnerEntity_sharedBy"
      ON "PartnerEntity__PartnerEntity_sharedBy"."id"="PartnerEntity"."sharedById"
      AND "PartnerEntity__PartnerEntity_sharedBy"."deletedAt" IS NULL
    LEFT JOIN "assets" "0aabe9f4a62b794e2c24a074297e534f51a4ac6c"
      ON "0aabe9f4a62b794e2c24a074297e534f51a4ac6c"."ownerId"="PartnerEntity__PartnerEntity_sharedBy"."id"
      AND "0aabe9f4a62b794e2c24a074297e534f51a4ac6c"."deletedAt" IS NULL
    LEFT JOIN "users" "PartnerEntity__sharedBy"
      ON "PartnerEntity__sharedBy"."id"="PartnerEntity"."sharedById"
      AND "PartnerEntity__sharedBy"."deletedAt" IS NULL
    LEFT JOIN "users" "PartnerEntity__sharedWith"
      ON "PartnerEntity__sharedWith"."id"="PartnerEntity"."sharedWithId"
      AND "PartnerEntity__sharedWith"."deletedAt" IS NULL
  WHERE
    "PartnerEntity__PartnerEntity_sharedWith"."id" = $1
    AND "0aabe9f4a62b794e2c24a074297e534f51a4ac6c"."id" = $2
)
LIMIT 1

-- After
SELECT
  "asset"."id" AS "assetId"
FROM "partners" "partner"
  INNER JOIN "users" "sharedBy"
    ON "sharedBy"."id"="partner"."sharedById"
    AND "sharedBy"."deletedAt" IS NULL
  INNER JOIN "assets" "asset"
    ON "asset"."ownerId"="sharedBy"."id"
    AND "asset"."deletedAt" IS NULL
WHERE
  "partner"."sharedWithId" = $1
  AND "asset"."id" IN ($2, $3)
```

* `asset` shared link access:

```sql
-- Before
SELECT 1 AS "row_exists" FROM (SELECT 1 AS dummy_column) "dummy_table" WHERE EXISTS (
  SELECT 1
  FROM "shared_links" "SharedLinkEntity"
    LEFT JOIN "albums" "SharedLinkEntity__SharedLinkEntity_album"
      ON "SharedLinkEntity__SharedLinkEntity_album"."id"="SharedLinkEntity"."albumId"
      AND "SharedLinkEntity__SharedLinkEntity_album"."deletedAt" IS NULL
    LEFT JOIN "albums_assets_assets" "760f12c00d97bdcec1ce224d1e3bf449859942b6"
      ON "760f12c00d97bdcec1ce224d1e3bf449859942b6"."albumsId"="SharedLinkEntity__SharedLinkEntity_album"."id"
    LEFT JOIN "assets" "4a35f463ae8c5544ede95c4b6d9ce8c686b6bfe6"
      ON "4a35f463ae8c5544ede95c4b6d9ce8c686b6bfe6"."id"="760f12c00d97bdcec1ce224d1e3bf449859942b6"."assetsId"
      AND "4a35f463ae8c5544ede95c4b6d9ce8c686b6bfe6"."deletedAt" IS NULL
    LEFT JOIN "shared_link__asset" "SharedLinkEntity__SharedLinkEntity_assets_SharedLinkEntity"
      ON "SharedLinkEntity__SharedLinkEntity_assets_SharedLinkEntity"."sharedLinksId"="SharedLinkEntity"."id"
    LEFT JOIN "assets" "SharedLinkEntity__SharedLinkEntity_assets"
      ON "SharedLinkEntity__SharedLinkEntity_assets"."id"="SharedLinkEntity__SharedLinkEntity_assets_SharedLinkEntity"."assetsId"
      AND "SharedLinkEntity__SharedLinkEntity_assets"."deletedAt" IS NULL
  WHERE (
    ("SharedLinkEntity"."id" = $1 AND "4a35f463ae8c5544ede95c4b6d9ce8c686b6bfe6"."id" = $2)
    OR ("SharedLinkEntity"."id" = $3 AND "SharedLinkEntity__SharedLinkEntity_assets"."id" = $4)
    OR ("SharedLinkEntity"."id" = $5 AND "4a35f463ae8c5544ede95c4b6d9ce8c686b6bfe6"."livePhotoVideoId" = $6)
    OR ("SharedLinkEntity"."id" = $7 AND "SharedLinkEntity__SharedLinkEntity_assets"."livePhotoVideoId" = $8)
  )
)
LIMIT 1

-- After
SELECT
  "assets"."id" AS "assetId",
  "assets"."livePhotoVideoId" AS "assetLivePhotoVideoId",
  "albumAssets"."id" AS "albumAssetId",
  "albumAssets"."livePhotoVideoId" AS "albumAssetLivePhotoVideoId"
FROM "shared_links" "sharedLink"
  LEFT JOIN "albums" "album"
    ON "album"."id"="sharedLink"."albumId"
    AND "album"."deletedAt" IS NULL
  LEFT JOIN "shared_link__asset" "assets_sharedLink"
    ON "assets_sharedLink"."sharedLinksId"="sharedLink"."id"
  LEFT JOIN "assets" "assets"
    ON "assets"."id"="assets_sharedLink"."assetsId"
    AND "assets"."deletedAt" IS NULL
  LEFT JOIN "albums_assets_assets" "album_albumAssets"
    ON "album_albumAssets"."albumsId"="album"."id"
  LEFT JOIN "assets" "albumAssets"
    ON "albumAssets"."id"="album_albumAssets"."assetsId"
    AND "albumAssets"."deletedAt" IS NULL
WHERE
  "sharedLink"."id" = $1
  AND (
    "assets"."id" IN ($2, $3)
    OR "albumAssets"."id" IN ($4, $5)
    OR "assets"."livePhotoVideoId" IN ($6, $7)
    OR "albumAssets"."livePhotoVideoId" IN ($8, $9)
  )
```
@jrasm91 jrasm91 enabled auto-merge (squash) December 2, 2023 02:51
@jrasm91 jrasm91 force-pushed the chore/server/check-asset-permissions-in-bulk branch from 7109883 to a5f058d Compare December 2, 2023 02:52
@jrasm91 jrasm91 merged commit 5aa658d into immich-app:main Dec 2, 2023
18 checks passed
@adamantike adamantike deleted the chore/server/check-asset-permissions-in-bulk branch December 10, 2023 14:20
@adamantike
Copy link
Contributor Author

@jrasm91 sorry for not being able to reply earlier!

The idea of the check function is very interesting. I will give it a try after migrating the pending permissions to be checked in bulk. That will greatly reduce the complexity of the current approach when a single permission makes multiple validations.

@bo0tzz bo0tzz mentioned this pull request Jan 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants