Skip to content

Commit

Permalink
Merge pull request #16411 from opf/implementation/55975-use-authentic…
Browse files Browse the repository at this point in the history 
…ation-in-set_permission_command

[#55975] introduced auth strategy for set permissions command
  • Loading branch information
@Kharonus
Kharonus authored Aug 14, 2024
2 parents a96e19f + 031bae9 commit 777f0db
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 202 deletions.
59 changes: 36 additions & 23 deletions .../app/common/storages/peripherals/storage_interaction/nextcloud/set_permissions_command.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,41 +37,53 @@ class SetPermissionsCommand
using ServiceResultRefinements

SUCCESS_XPATH = "/d:multistatus/d:response/d:propstat[d:status[text() = 'HTTP/1.1 200 OK']]/d:prop/nc:acl-list"
def self.call(storage:, path:, permissions:)
new(storage).call(path:, permissions:)

def self.call(storage:, auth_strategy:, path:, permissions:)
new(storage).call(auth_strategy:, path:, permissions:)
end

def initialize(storage)
@storage = storage
@username = storage.username
@password = storage.password
end

def call(path:, permissions:)
if path.blank?
return ServiceResult.failure(errors: StorageError.new(code: :invalid_path))
end
# rubocop:disable Metrics/AbcSize
def call(auth_strategy:, path:, permissions:)
validate_input_data(path).on_failure { return _1 }

with_tagged_logger do
info "Setting permissions #{permissions.inspect} on #{path}"

body = request_xml_body(permissions[:groups], permissions[:users])
# This can raise KeyErrors, we probably should just default to enpty Arrays.
response = OpenProject
.httpx
.basic_auth(@username, @password)
.request(
"PROPPATCH",
UrlBuilder.url(@storage.uri, "remote.php/dav/files", @username, path),
xml: body
)

handle_response(response)
username = Util.origin_user_id(caller: self.class, storage: @storage, auth_strategy:)
.on_failure { return _1 }
.result

Authentication[auth_strategy].call(storage: @storage) do |http|
with_tagged_logger do
info "Setting permissions #{permissions.inspect} on #{path}"

body = request_xml_body(permissions[:groups], permissions[:users])
# This can raise KeyErrors, we probably should just default to empty Arrays.
response = http
.request(
"PROPPATCH",
UrlBuilder.url(@storage.uri, "remote.php/dav/files", username, path),
xml: body
)

handle_response(response)
end
end
end

# rubocop:enable Metrics/AbcSize

private

def validate_input_data(path)
if path.blank?
ServiceResult.failure(errors: StorageError.new(code: :invalid_path))
else
ServiceResult.success
end
end

# rubocop:disable Metrics/AbcSize
def handle_response(response)
error_data = StorageErrorData.new(source: self.class, payload: response)
Expand Down Expand Up @@ -125,6 +137,7 @@ def request_xml_body(groups_permissions, users_permissions)
end
end.to_xml
end

# rubocop:enable Metrics/AbcSize
end
end
Expand Down
4 changes: 2 additions & 2 deletions .../app/common/storages/peripherals/storage_interaction/one_drive/set_permissions_command.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ def delete? = permission_ids.any? && user_ids.empty?
def update? = permission_ids.any? && user_ids.any?
end

def self.call(storage:, path:, permissions:, auth_strategy:)
new(storage).call(path:, permissions:, auth_strategy:)
def self.call(storage:, auth_strategy:, path:, permissions:)
new(storage).call(auth_strategy:, path:, permissions:)
end

def initialize(storage)
Expand Down
6 changes: 3 additions & 3 deletions modules/storages/app/services/storages/nextcloud_group_folder_properties_sync_service.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def set_folders_permissions(remote_admins, project_storage)
}
}

set_permissions.call(storage: @storage, **command_params).on_failure do |service_result|
set_permissions.call(storage: @storage, auth_strategy:, **command_params).on_failure do |service_result|
log_storage_error(service_result.errors, folder:)
add_error(:set_folder_permission, service_result.errors, options: { folder: })
end
Expand Down Expand Up @@ -178,7 +178,7 @@ def hide_inactive_folders(remote_folders)
groups: { "#{@storage.group}": NO_PERMISSIONS }
} }

set_permissions.call(storage: @storage, **command_params).on_failure do |service_result|
set_permissions.call(storage: @storage, auth_strategy:, **command_params).on_failure do |service_result|
log_storage_error(service_result.errors, folder: path, context: "hide_folder")
add_error(:hide_inactive_folders, service_result.errors, options: { path: })
end
Expand Down Expand Up @@ -263,7 +263,7 @@ def ensure_root_folder_permissions(group_folder, username, group)
}
}

set_permissions.call(storage: @storage, **command_params).on_failure do |service_result|
set_permissions.call(storage: @storage, auth_strategy:, **command_params).on_failure do |service_result|
log_storage_error(service_result.errors, { folder: group_folder })
add_error(:ensure_root_folder_permissions, service_result.errors, options: { group:, username: }).fail!
end
Expand Down
5 changes: 3 additions & 2 deletions modules/storages/app/services/storages/one_drive_managed_folder_sync_service.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def apply_permission_to_folders
end

def set_folder_permissions(folder_id, permissions)
set_permissions.call(storage: @storage, path: folder_id, permissions:, auth_strategy:)
set_permissions.call(storage: @storage, auth_strategy:, path: folder_id, permissions:)
end

def ensure_folders_exist(folder_map)
Expand All @@ -104,7 +104,7 @@ def hide_inactive_folders(folder_map)
info "Hiding folder with ID #{item_id} as it does not belong to any active project"

# FIXME: Set permissions wont ever fail.
set_permissions.call(storage: @storage, path: item_id, permissions:, auth_strategy:)
set_permissions.call(storage: @storage, auth_strategy:, path: item_id, permissions:)
.on_failure do |service_result|
log_storage_error(service_result.errors, item_id:, context: "hide_folder")
add_error(:hide_inactive_folders, service_result.errors, options: { path: folder_map[item_id] })
Expand Down Expand Up @@ -212,6 +212,7 @@ def admin_remote_identities_scope
end

def root_folder = Peripherals::ParentFolder.new("/")

def auth_strategy = userless.call

# @param attribute [Symbol] attribute to which the error will be tied to
Expand Down
172 changes: 0 additions & 172 deletions modules/storages/spec/common/storages/peripherals/registry_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,178 +213,6 @@
end
end

describe "#set_permissions_command" do
let(:path) { "OpenProject/JediProject" }
let(:permissions) do
{
users: {
OpenProject: 31,
"Obi-Wan": 31,
"Qui-Gon": 31
},
groups: {
OpenProject: 0
}
}
end

let(:expected_request_body) do
<<~XML
<?xml version="1.0"?>
<d:propertyupdate xmlns:d="DAV:" xmlns:nc="http://nextcloud.org/ns">
<d:set>
<d:prop>
<nc:acl-list>
<nc:acl>
<nc:acl-mapping-type>group</nc:acl-mapping-type>
<nc:acl-mapping-id>OpenProject</nc:acl-mapping-id>
<nc:acl-mask>31</nc:acl-mask>
<nc:acl-permissions>0</nc:acl-permissions>
</nc:acl>
<nc:acl>
<nc:acl-mapping-type>user</nc:acl-mapping-type>
<nc:acl-mapping-id>OpenProject</nc:acl-mapping-id>
<nc:acl-mask>31</nc:acl-mask>
<nc:acl-permissions>31</nc:acl-permissions>
</nc:acl>
<nc:acl>
<nc:acl-mapping-type>user</nc:acl-mapping-type>
<nc:acl-mapping-id>Obi-Wan</nc:acl-mapping-id>
<nc:acl-mask>31</nc:acl-mask>
<nc:acl-permissions>31</nc:acl-permissions>
</nc:acl>
<nc:acl>
<nc:acl-mapping-type>user</nc:acl-mapping-type>
<nc:acl-mapping-id>Qui-Gon</nc:acl-mapping-id>
<nc:acl-mask>31</nc:acl-mask>
<nc:acl-permissions>31</nc:acl-permissions>
</nc:acl>
</nc:acl-list>
</d:prop>
</d:set>
</d:propertyupdate>
XML
end

context "with Nextcloud storage type selected" do
context "with outbound request" do
before do
stub_request(:proppatch, "#{url}/remote.php/dav/files/OpenProject/OpenProject/JediProject")
.with(
body: expected_request_body,
headers: {
"Authorization" => "Basic T3BlblByb2plY3Q6T3BlblByb2plY3RTZWN1cmVQYXNzd29yZA=="
}
)
.to_return(expected_response)
end

context "when permissions can be set" do
let(:expected_response_body) do
<<~XML
<?xml version="1.0"?>
<d:multistatus
xmlns:d="DAV:"
xmlns:s="http://sabredav.org/ns"
xmlns:oc="http://owncloud.org/ns"
xmlns:nc="http://nextcloud.org/ns">
<d:response>
<d:href>/remote.php/dav/files/OpenProject/OpenProject/Project%231</d:href>
<d:propstat>
<d:prop>
<nc:acl-list/>
</d:prop>
<d:status>HTTP/1.1 200 OK</d:status>
</d:propstat>
</d:response>
</d:multistatus>
XML
end
let(:expected_response) do
{
status: 207,
body: expected_response_body,
headers: {}
}
end

it "returns success when permissions can be set" do
result = registry.resolve("nextcloud.commands.set_permissions").call(storage:, path:, permissions:)
expect(result).to be_success
end
end

context "when the password is wrong" do
let(:expected_response_body) do
<<~XML
<?xml version="1.0" encoding="utf-8"?>
<d:error
xmlns:d="DAV:"
xmlns:s="http://sabredav.org/ns">
<s:exception>Sabre\DAV\Exception\NotAuthenticated</s:exception>
<s:message>No public access to this resource., No 'Authorization: Basic' header found. Either the client didn't send one, or the server is misconfigured, No 'Authorization: Bearer' header found. Either the client didn't send one, or the server is mis-configured, No 'Authorization: Basic' header found. Either the client didn't send one, or the server is misconfigured</s:message>
</d:error>
XML
end
let(:expected_response) do
{
status: 401,
body: expected_response_body,
headers: {}
}
end

it "returns failure" do
result = registry.resolve("nextcloud.commands.set_permissions").call(storage:, path:, permissions:)
expect(result).to be_failure
end
end

context "when the NC control user cannot read(see) the project folder" do
let(:expected_response_body) do
<<~XML
<?xml version="1.0" encoding="utf-8"?>
<d:error
xmlns:d="DAV:"
xmlns:s="http://sabredav.org/ns">
<s:exception>Sabre\DAV\Exception\NotFound</s:exception>
<s:message>File with name /OpenProject/JediProject could not be located</s:message>
</d:error>
XML
end
let(:expected_response) do
{
status: 404,
body: expected_response_body,
headers: {}
}
end

it "returns failure" do
result = registry.resolve("nextcloud.commands.set_permissions").call(storage:, path:, permissions:)
expect(result).to be_failure
end
end
end

context "when forbidden values are given as folder" do
it "raises an ArgumentError on nil" do
result = registry.resolve("nextcloud.commands.set_permissions").call(storage:, path: nil, permissions:)

expect(result).to be_failure
expect(result.errors.code).to eq(:invalid_path)
end

it "returns a :invalid_path Failure on empty string" do
result = registry.resolve("nextcloud.commands.set_permissions").call(storage:, path: "", permissions:)

expect(result).to be_failure
expect(result.errors.code).to eq(:invalid_path)
end
end
end
end

describe "#file_ids_query" do
let(:nextcloud_subpath) { "" }
let(:url) { "https://example.com#{nextcloud_subpath}" }
Expand Down

0 comments on commit 777f0db

Please sign in to comment.