From 1e719b2ad1748afc15b69a1d9134540f40924e48 Mon Sep 17 00:00:00 2001 From: "karel.picman@lbcfree.net" Date: Thu, 6 May 2021 10:23:08 +0200 Subject: [PATCH] Files remain locked after content editing --- lib/redmine_dmsf/lockable.rb | 23 +++++++------- lib/redmine_dmsf/webdav/dmsf_resource.rb | 30 +++++++++---------- .../webdav/dmsf_webdav_unlock_test.rb | 4 +-- 3 files changed, 30 insertions(+), 27 deletions(-) diff --git a/lib/redmine_dmsf/lockable.rb b/lib/redmine_dmsf/lockable.rb index c3187fd0..505471e8 100644 --- a/lib/redmine_dmsf/lockable.rb +++ b/lib/redmine_dmsf/lockable.rb @@ -90,23 +90,25 @@ module RedmineDmsf # By using the path upwards, surely this would be quicker? def locked_for_user?(args = nil) return false unless locked? - b_shared = nil + shared = nil self.dmsf_path.each do |entity| locks = entity.locks || entity.lock(false) next if locks.empty? locks.each do |lock| next if lock.expired? # In case we're in between updates + owner = args[:owner] if args + owner ||= User.current&.login if lock.owner if lock.lock_scope == :scope_exclusive - return true if (lock.user&.id != User.current.id) || ((lock.owner != (args ? args[:owner] : nil))) + return true if (lock.user&.id != User.current.id) || (lock.owner != owner) else - b_shared = true if b_shared.nil? - if b_shared && (lock.user&.id == User.current.id) && (lock.owner == (args ? args[:owner] : nil)) || + shared = true if shared.nil? + if shared && (lock.user&.id == User.current.id) && (lock.owner == owner) || (args && (args[:scope] == 'shared')) - b_shared = false + shared = false end end end - return true if b_shared + return true if shared end false end @@ -114,6 +116,7 @@ module RedmineDmsf def unlock!(force_file_unlock_allowed = false, owner = nil) raise DmsfLockError.new(l(:warning_file_not_locked)) unless self.locked? existing = self.lock(true) + destroyed = false # If its empty its a folder that's locked (not root) if existing.empty? || (!self.dmsf_folder.nil? && self.dmsf_folder.locked?) raise DmsfLockError.new(l(:error_unlock_parent_locked)) @@ -126,18 +129,18 @@ module RedmineDmsf if (existing.count == 1) && (existing[0].lock_scope == :exclusive) existing[0].destroy else - b_destroyed = false existing.each do |lock| + owner = User.current&.login if lock.owner && owner.nil? if ((lock.user&.id == User.current.id) && (lock.owner == owner)) || User.current.admin? lock.destroy - b_destroyed = true + destroyed = true break end end # At first it was going to be allowed for someone with force_file_unlock to delete all shared by default # Instead, they by default remove themselves from shared lock, and everyone from shared lock if they're not # on said lock - if !b_destroyed && (User.current.allowed_to?(:force_file_unlock, self.project) || force_file_unlock_allowed) + if !destroyed && (User.current.allowed_to?(:force_file_unlock, self.project) || force_file_unlock_allowed) locks.delete_all end end @@ -145,6 +148,6 @@ module RedmineDmsf reload locks.reload end - true + end end diff --git a/lib/redmine_dmsf/webdav/dmsf_resource.rb b/lib/redmine_dmsf/webdav/dmsf_resource.rb index e872ebdd..fc7fdd15 100644 --- a/lib/redmine_dmsf/webdav/dmsf_resource.rb +++ b/lib/redmine_dmsf/webdav/dmsf_resource.rb @@ -479,6 +479,7 @@ module RedmineDmsf return super(token) end if token.nil? || token.empty? || (token == '<(null)>') || User.current.anonymous? + Rails.logger.info ">>> bad token 2: #{token}" BadRequest else if token =~ /([a-f0-9]{8}-[a-f0-9]{4}-4[a-f0-9]{3}-[89aAbB][a-f0-9]{3}-[a-f0-9]{12})/ @@ -486,21 +487,20 @@ module RedmineDmsf else return BadRequest end - begin - l = DmsfLock.find(token) - # Additional case: if a user tries to unlock the file instead of the folder that's locked - # This should throw forbidden as only the lock at level initiated should be unlocked - entity = file || folder - return NoContent unless entity&.locked? - l_entity = l.file || l.folder - if l_entity != entity - Forbidden - else - entity.unlock! - NoContent - end - rescue - BadRequest + l = DmsfLock.find_by_uuid(token) + unless l + return NoContent + end + # Additional case: if a user tries to unlock the file instead of the folder that's locked + # This should throw forbidden as only the lock at level initiated should be unlocked + entity = file || folder + return NoContent unless entity&.locked? + l_entity = l.file || l.folder + if l_entity != entity + Forbidden + else + entity.unlock! + NoContent end end end diff --git a/test/integration/webdav/dmsf_webdav_unlock_test.rb b/test/integration/webdav/dmsf_webdav_unlock_test.rb index 977c53b3..44c628e9 100644 --- a/test/integration/webdav/dmsf_webdav_unlock_test.rb +++ b/test/integration/webdav/dmsf_webdav_unlock_test.rb @@ -58,7 +58,7 @@ class DmsfWebdavUnlockTest < RedmineDmsf::Test::IntegrationTest assert_response :success process :unlock, "/dmsf/webdav/#{@file2.project.identifier}/#{@file2.name}", params: nil, headers: @admin.merge!({ HTTP_DEPTH: 'infinity', HTTP_TIMEOUT: 'Infinite', HTTP_LOCK_TOKEN: l.uuid }) - assert_response :bad_request + assert_response :no_content end def test_unlock_folder_wrong_path @@ -89,7 +89,7 @@ class DmsfWebdavUnlockTest < RedmineDmsf::Test::IntegrationTest process :unlock, "/dmsf/webdav/#{@folder2.project.identifier}/#{@folder2.dmsf_folder.title}/#{@folder2.title}", params: nil, headers: @jsmith.merge!({ HTTP_DEPTH: 'infinity', HTTP_TIMEOUT: 'Infinite', HTTP_LOCK_TOKEN: l.uuid }) - assert_response :bad_request + assert_response :no_content end def test_unlock_file_in_subproject