From d757e67854d806dd90d361c71659bfece30d6093 Mon Sep 17 00:00:00 2001 From: ildyria Date: Sat, 5 Nov 2022 15:32:20 +0100 Subject: [PATCH 1/5] fix smart albums rights --- public/Lychee-front | 2 +- public/dist/main.js | 45 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/public/Lychee-front b/public/Lychee-front index ae7ddd03cfa..0213428297d 160000 --- a/public/Lychee-front +++ b/public/Lychee-front @@ -1 +1 @@ -Subproject commit ae7ddd03cfa406a07e548f6c3c66f6f9e0587564 +Subproject commit 0213428297df7fc52c391b1cdb6ed569c7da70b7 diff --git a/public/dist/main.js b/public/dist/main.js index fd5ad959368..fa8ddea5511 100644 --- a/public/dist/main.js +++ b/public/dist/main.js @@ -2732,21 +2732,58 @@ album.apply_nsfw_filter = function () { /** * Determines whether the user can upload to the currently active album. * - * For special cases of no album / smart album / etc. we return true. - * It's only for regular, non-matching albums that we return false. + * It is safe to call this method even if no album is loaded at all. + * In this case, the method simply returns `false` (even for admin users). + * + * If no user is authenticated or the authenticated user has no upload + * capabilities, the method returns `false` (even for albums owned by the + * user). + * For admin users the method returns `true`. + * + * For non-admin, authenticated users with the upload capability + * + * - the method returns `true` for smart albums + * - the method returns `true` for regular albums if and only if the album is + * owned by the currently authenticated user. * * @returns {boolean} */ album.isUploadable = function () { + // If no album is loaded, nobody (not even the admin) can upload photos. + // We must check this first, before we test for the admin short-cut. + // + // Particular photo actions (such as starring/unstarring a photo) assume + // that the corresponding album is loaded, because their code use + // `album.getPhotoId` under the hood. + // (Note, this is a bug on its own.) + // In the special view mode for single photos no album is loaded, even if + // the currently authenticated user had the right to load (and see) the + // album. + // Hence, invoking those actions without a properly loaded album, results + // in exceptions. + // The method `header.setMode` relies on this method to decide whether + // particular action buttons shall be hidden in single photo view. + // If the admin is authenticated and opens the view mode, those "buggy" + // actions must be hidden. + if (album.json === null) { + return false; + } + if (lychee.rights.is_admin) { return true; } - if (lychee.publicMode || !lychee.rights.may_upload) { + if (lychee.user === null || lychee.publicMode || !lychee.rights.may_upload) { return false; } + // Smart albums are considered to be owned by everybody and hence get + // a pass + if (album.isSmartID(album.json.id)) { + return true; + } + // TODO: Comparison of numeric user IDs (instead of names) should be more robust - return album.json === null || lychee.user !== null && album.json.owner_name === lychee.user.username; + return album.json.owner_name === lychee.user.username; }; /** From 3fe48ebbcb0e5ff01daaa31bcf3a85bcf0a8dc4b Mon Sep 17 00:00:00 2001 From: ildyria Date: Sat, 5 Nov 2022 15:34:55 +0100 Subject: [PATCH 2/5] skip back-end CICD if only front-end changed --- .github/workflows/CICD.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index c064493b39e..1cad9e4634b 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -7,11 +7,13 @@ on: - '**/*.md' - 'public/dist/*.js' - 'public/dist/**/*.js' + - 'public/Lychee-front' pull_request: paths-ignore: - '**/*.md' - 'public/dist/*.js' - 'public/dist/**/*.js' + - 'public/Lychee-front' # Allow manually triggering the workflow. workflow_dispatch: From 26e016fff43a109027dfd39fcb44c529a4f81908 Mon Sep 17 00:00:00 2001 From: ildyria Date: Sat, 5 Nov 2022 23:33:16 +0100 Subject: [PATCH 3/5] remove warnings --- .github/workflows/CICD.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 1cad9e4634b..675f125ff99 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -25,7 +25,7 @@ jobs: if: (github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository) steps: - name: Cancel Previous Runs - uses: styfle/cancel-workflow-action@0.7.0 + uses: styfle/cancel-workflow-action@0.11.0 with: access_token: ${{ github.token }} From cdea67d1b8599331418614c5b8f7930f66076cf5 Mon Sep 17 00:00:00 2001 From: Matthias Nagel Date: Sun, 6 Nov 2022 13:36:02 +0100 Subject: [PATCH 4/5] Synced frontend --- public/Lychee-front | 2 +- public/dist/leaflet.markercluster.js.map | 0 public/dist/main.css | 0 public/dist/main.js | 97 +++++++++++++++++++++++- public/dist/view.js | 37 ++++++++- 5 files changed, 130 insertions(+), 6 deletions(-) mode change 100755 => 100644 public/dist/leaflet.markercluster.js.map mode change 100755 => 100644 public/dist/main.css diff --git a/public/Lychee-front b/public/Lychee-front index 0213428297d..0e023592409 160000 --- a/public/Lychee-front +++ b/public/Lychee-front @@ -1 +1 @@ -Subproject commit 0213428297df7fc52c391b1cdb6ed569c7da70b7 +Subproject commit 0e023592409f160974a44fff69b552a61fc41990 diff --git a/public/dist/leaflet.markercluster.js.map b/public/dist/leaflet.markercluster.js.map old mode 100755 new mode 100644 diff --git a/public/dist/main.css b/public/dist/main.css old mode 100755 new mode 100644 diff --git a/public/dist/main.js b/public/dist/main.js index fa8ddea5511..3d49fb49a71 100644 --- a/public/dist/main.js +++ b/public/dist/main.js @@ -2746,9 +2746,24 @@ album.apply_nsfw_filter = function () { * - the method returns `true` for regular albums if and only if the album is * owned by the currently authenticated user. * + * Note, for the time being this method contains a work-around in case + * no album is loaded, but the root view is visible. + * Currently, this is necessary, because this method is (erroneously) called + * for the root view as well. + * In order to determine whether the work-around for the root view needs to + * be applied, this method checks if the root view is visible based on the + * visibility of the corresponding headers. + * Hence, the caller must ensure that the appropriate header is set first + * in order to obtain a correct result from this method. + * * @returns {boolean} */ album.isUploadable = function () { + // Work-around in case this method is called for the root view + if (visible.albums()) { + return lychee.rights.is_admin || lychee.user !== null && !lychee.publicMode && lychee.rights.may_upload; + } + // If no album is loaded, nobody (not even the admin) can upload photos. // We must check this first, before we test for the admin short-cut. // @@ -2869,6 +2884,26 @@ var albums = { */ albums.load = function () { var showRootAlbum = function showRootAlbum() { + // DO NOT change the order of `header.setMode` and `view.albums.init`. + // The latter relies on the header being set correctly. + // + // `view.albums.init` builds the HTML of the albums view (note the + // plural-s). + // Internally, this exploits code for regular albums which in + // turn calls `album.isUploadabe` (note the missing plural-s) to + // check whether the current album supports drag-&-drop. + // In order to return the correct value `album.isUploadabe` resorts + // to a hack: if no (regular) album is loaded `album.isUploadabe` + // normally returns `false` except the root album is visible. + // In that case `album.isUploadabe` returns a "fake" `true`. + // However, in order to do so `album.isUploadabe` needs to check + // whether the root album is visible which is determined by the + // visibility of the corresponding header. + // That is why the header needs to be set first. + // + // However, the actual bug is to call `album.isUploadable` for the + // root view. + // TODO: Fix the bug described above. header.setMode("albums"); view.albums.init(); lychee.animate(lychee.content, "contentZoomIn"); @@ -3190,6 +3225,37 @@ build.album = function (data) { var formattedCreationTs = lychee.locale.printMonthYear(data.created_at); var formattedMinTs = lychee.locale.printMonthYear(data.min_taken_at); var formattedMaxTs = lychee.locale.printMonthYear(data.max_taken_at); + // The condition below is faulty wrt. to two issues: + // + // a) The condition only checks whether the owning/current album is + // uploadable (aka "editable"), but it does not check whether the + // album at hand whose icon is built is editable. + // But this is of similar importance. + // Currently, we only check whether the album at hand is a smart + // album or tag album which are always considered non-editable. + // But this is only half of the story. + // For example, a regular album might still be non-editable, if the + // current user is not the owner of that album. + // b) This method is not only called if the owning/current album is a + // proper album, but also for the root view. + // However, `album.isUploadable` should not be called for the root + // view. + // + // Moreover, we have to distinguish between "drag" and "drop". + // Doing so would also solve the problems above: + // + // - "Drag": If the current child album at hand can be dragged (away) + // is mostly determined by the user's rights on the parent album. + // Instead of (erroneously) using `album.isUploadable()` for that + // (even for the root view), the "right to drag" should be passed to + // this method as a parameter very much like `disabled` such that this + // method can be used for both regular albums and the root view. + // - "Drop": If something (e.g. a photo) can be dropped onto the child + // album at hand is independent of the user's rights on the containing + // album. + // Whether the child album supports the drop event depends on the type + // of the album (i.e. it must not be a smart or tag album), but also + // on the ownership of the album. var disableDragDrop = !album.isUploadable() || disabled || album.isSmartID(data.id) || data.is_tag_album; var subtitle = formattedCreationTs; @@ -4730,7 +4796,11 @@ header.setMode = function (mode) { tabindex.makeFocusable(_e10); } - if (album.json && album.json.hasOwnProperty("is_share_button_visible") && !album.json.is_share_button_visible) { + if (album.json && album.json.is_share_button_visible === false && ( + // The owner of an album (or the admin) shall always see + // the share button and be unaffected by the settings of + // the album + lychee.user === null || lychee.user.username !== album.json.owner_name) && !lychee.rights.is_admin) { var _e11 = $("#button_share_album"); _e11.hide(); tabindex.makeUnfocusable(_e11); @@ -5205,7 +5275,26 @@ $(document).ready(function () { // Drag and Drop upload .on("dragover", function () { return false; - }, false).on("drop", + }, false) + // In the long run, the "drop" event should not be defined on the + // global document element, but on the `DIV` which corresponds to the + // view onto which something is dropped. + // This would also avoid this highly fragile condition below. + // For example, in order to avoid that a photo unintentionally ends + // up in the root album when someone drops a photo while the + // setting screen is opened, we check for `!visible.config()`. + // This would simply not be necessary, if the drop event was directly + // defined on the albums view where it belongs. + // The conditions whether a user is allowed to upload to the root + // album (cp. `visible.albums()` below) or to a regular album + // (cp. `visible.album()` below) are slightly different. + // Nonetheless, we only have a single method `album.isUploadable` + // which tries to cover both cases and is prone to fail in certain + // corner cases. + // If the drop event was defined on the DIV for the root view and on + // the DIV for an album view, the whole problem would not exist. + // TODO: Fix that + .on("drop", /** @param {jQuery.Event} e */function (e) { if (album.isUploadable() && !visible.contextMenu() && !basicModal.isVisible() && !visible.leftMenu() && !visible.config() && (visible.album() || visible.albums())) { // Detect if dropped item is a file or a link @@ -8999,7 +9088,7 @@ _photo3.setProtectionPolicy = function (photoID) { // users? formElements.requires_link.checked = false; formElements.is_downloadable.checked = album.json.is_downloadable; - formElements.is_share_button_visible = album.json.is_share_button_visible; + formElements.is_share_button_visible.checked = album.json.is_share_button_visible; formElements.has_password.checked = album.json.has_password; } basicModal.hideActionButton(); @@ -9013,7 +9102,7 @@ _photo3.setProtectionPolicy = function (photoID) { formElements.grants_full_photo.checked = lychee.full_photo; formElements.requires_link.checked = lychee.public_photos_hidden; formElements.is_downloadable.checked = lychee.downloadable; - formElements.is_share_button_visible = lychee.share_button_visible; + formElements.is_share_button_visible.checked = lychee.share_button_visible; formElements.has_password.checked = false; } }; diff --git a/public/dist/view.js b/public/dist/view.js index 40d41e35ff6..fcaa1290363 100644 --- a/public/dist/view.js +++ b/public/dist/view.js @@ -680,6 +680,37 @@ build.album = function (data) { var formattedCreationTs = lychee.locale.printMonthYear(data.created_at); var formattedMinTs = lychee.locale.printMonthYear(data.min_taken_at); var formattedMaxTs = lychee.locale.printMonthYear(data.max_taken_at); + // The condition below is faulty wrt. to two issues: + // + // a) The condition only checks whether the owning/current album is + // uploadable (aka "editable"), but it does not check whether the + // album at hand whose icon is built is editable. + // But this is of similar importance. + // Currently, we only check whether the album at hand is a smart + // album or tag album which are always considered non-editable. + // But this is only half of the story. + // For example, a regular album might still be non-editable, if the + // current user is not the owner of that album. + // b) This method is not only called if the owning/current album is a + // proper album, but also for the root view. + // However, `album.isUploadable` should not be called for the root + // view. + // + // Moreover, we have to distinguish between "drag" and "drop". + // Doing so would also solve the problems above: + // + // - "Drag": If the current child album at hand can be dragged (away) + // is mostly determined by the user's rights on the parent album. + // Instead of (erroneously) using `album.isUploadable()` for that + // (even for the root view), the "right to drag" should be passed to + // this method as a parameter very much like `disabled` such that this + // method can be used for both regular albums and the root view. + // - "Drop": If something (e.g. a photo) can be dropped onto the child + // album at hand is independent of the user's rights on the containing + // album. + // Whether the child album supports the drop event depends on the type + // of the album (i.e. it must not be a smart or tag album), but also + // on the ownership of the album. var disableDragDrop = !album.isUploadable() || disabled || album.isSmartID(data.id) || data.is_tag_album; var subtitle = formattedCreationTs; @@ -1344,7 +1375,11 @@ header.setMode = function (mode) { tabindex.makeFocusable(_e9); } - if (album.json && album.json.hasOwnProperty("is_share_button_visible") && !album.json.is_share_button_visible) { + if (album.json && album.json.is_share_button_visible === false && ( + // The owner of an album (or the admin) shall always see + // the share button and be unaffected by the settings of + // the album + lychee.user === null || lychee.user.username !== album.json.owner_name) && !lychee.rights.is_admin) { var _e10 = $("#button_share_album"); _e10.hide(); tabindex.makeUnfocusable(_e10); From a8ec4cf1756338dce94884c6b3c04136939f2162 Mon Sep 17 00:00:00 2001 From: ildyria Date: Sun, 6 Nov 2022 13:55:09 +0100 Subject: [PATCH 5/5] sync Front --- public/Lychee-front | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/Lychee-front b/public/Lychee-front index 0e023592409..fbcff79af52 160000 --- a/public/Lychee-front +++ b/public/Lychee-front @@ -1 +1 @@ -Subproject commit 0e023592409f160974a44fff69b552a61fc41990 +Subproject commit fbcff79af52c3a5db014062a0b5cd04138f03ec6