[tests-only][full-ci] added test to enable disable Secure Viewer permissions role for federated shares #10823

PrajwolAmatya · 2025-01-02T08:38:59Z

Description

This PR adds tests to enable and disable the share permissions role Secure VIewer for federated shares.

Related Issue

[QA] test cases for enabling and disabling sharing roles #10711

Test Coverage for #10824 and #10822

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests only (no source changes)

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

sonarqubecloud · 2025-01-08T08:30:21Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

PrajwolAmatya · 2025-02-03T08:37:59Z

For federated user, with API request the resources can be shared with permissions role Secure Viewer, which is not the case when sharing the resources via web UI, as only two permissions role Can View and Can Edit are available for federated share. Issue is reported in #10824, and there has not been confirmation with the behavior. So, currently moved this PR to blocked.

PrajwolAmatya · 2025-02-11T08:23:24Z

Making this PR back to review and added failing scenarios to expected to failure file.

tests/acceptance/features/apiOcm/enableDisablePermissionsRole.feature

amrita-shrestha

what about allowed permission

PrajwolAmatya · 2025-02-13T03:40:15Z

what about allowed permission

These test cases would be good to have:

pre-condition: enable role -> share -> disable role

roles to check: Secure Viewer, Editor Without Version, Denied

This is the part of issue to check for the additional roles. Also, the test coverage for issues #10824 and #10822. So only the added scenarios will fit in this PR.

tests/acceptance/features/apiOcm/enableDisablePermissionsRole.feature

prashant-gurung899 · 2025-02-24T04:25:28Z

LGTM 👍

saw-jan · 2025-02-24T05:21:03Z

Are you going to add tests coverage for allowed permission roles or not?

Not under the mentioned issue. Test for enabling and disabling permissions role are mostly for the additional roles. If we want to add smillar tests for allowed permissions role then we can create a separate issue and work on that.

@saw-jan @nirajacharya2 @Salipa-Gurung what you think? Should we add tests scenario for allowed permission role or not. If yes this please create new issue or add point in existing related issue.

If you want to check disabling of default roles then please only check this scenario:

Given share with Editor role
When disable Editor role
Then check the share permissions (Should be viewer or something lower permission)

PrajwolAmatya · 2025-02-24T10:28:37Z

If you want to check disabling of default roles then please only check this scenario:
Given share with Editor role
When disable Editor role
Then check the share permissions (Should be viewer or something lower permission)

Will do that in next PR. 👍
point added in this issue : #10711

amrita-shrestha · 2025-02-25T09:15:43Z

tests/acceptance/bootstrap/ArchiverContext.php

+	 * @return void
+	 */
+	public function userShouldBeAbleToDownloadArchiveOfFederatedSharedFolder(string $user, string $resource): void {
+		$queryString = $this->getArchiverQueryString($user, $resource, 'remoteItemIds');


i had seen use of $this->archiverContext->getArchiverUrl in tests/acceptance/bootstrap/SpacesContext.php.
So, confused about the perfect place to add this step definition
@saw-jan @nirajacharya2 what do you think about where should we add a related step- definition if that step is part of space context also.

* @When /^public downloads the folder "([^"]*)" from the last created public link using the public files API$/ * @When user :user tries to download the space :spaceName owned by user :owner using the WebDAV API * @When /^user "([^"]*)" (?:downloads|tries to download) the space "([^"]*)" using the WebDAV API$/

this is fine.

amrita-shrestha · 2025-02-25T09:21:55Z

tests/acceptance/bootstrap/SpacesContext.php

+	/**
+	 * @Then user :user should be able to download federated shared file :resource
+	 *
+	 * @param string $user
+	 * @param string $resource
+	 *
+	 * @return void
+	 */
+	public function userShouldBeAbleToDownloadFederatedSharedFile(string $user, string $resource): void {


Can't we just modify the existing step for internal sharing? Better modifying step rather than repetition of code

this is not that much of a duplication. If we have a single method that handles everything, it would be messy and difficult to manage. So, we need to always maintain between DRY and readability/maintainability

i don't think readability gets compromised if we just merge two step because only different is "$baseUrl/$davPath/$remoteItemId", this value and as you said this function is small so this will not be complicated logic . Another point is file size, as you can see number of line is crossing 4k.

The current download related step uses makeDavRequest function, and the dav path is set inside that function. To update this path $baseUrl/$davPath/$remoteItemId we need to specify in makeDavRequest function that this is a federated shared file. This makes the function more complex.
That's also the reason I have used HttpRequestHtlper::get() directly in this step. Its just to make things less complex.

How about splitting context file. Dedicated context file for federation?

I can move these steps to OcmContext.php.

ocmcontext is a great place. 👍

sonarqubecloud · 2025-02-28T04:29:57Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[tests-only][full-ci] added test to enable disable Secure Viewer permissions role for federated shares

PrajwolAmatya self-assigned this Jan 2, 2025

PrajwolAmatya added the QA:team label Jan 2, 2025

PrajwolAmatya mentioned this pull request Jan 2, 2025

[QA] test cases for enabling and disabling sharing roles #10711

Open

9 tasks

PrajwolAmatya force-pushed the federation-share branch 2 times, most recently from 85b7a21 to 222314c Compare January 8, 2025 08:27

PrajwolAmatya force-pushed the federation-share branch from 222314c to 9355c3f Compare February 11, 2025 06:44

PrajwolAmatya mentioned this pull request Feb 10, 2025

[QA] Test coverage for OCM bugs #10971

Open

16 tasks

PrajwolAmatya force-pushed the federation-share branch 3 times, most recently from 62be978 to 1bce918 Compare February 11, 2025 07:14

PrajwolAmatya marked this pull request as ready for review February 11, 2025 08:23

PrajwolAmatya requested review from anon-pradip, ishabaral, saw-jan, prashant-gurung899, Salipa-Gurung, amrita-shrestha, nabim777, S-Panta and nirajacharya2 February 11, 2025 08:23

PrajwolAmatya force-pushed the federation-share branch from 1bce918 to 5699738 Compare February 12, 2025 03:56

amrita-shrestha requested changes Feb 12, 2025

View reviewed changes

tests/acceptance/features/apiOcm/enableDisablePermissionsRole.feature Outdated Show resolved Hide resolved

tests/acceptance/features/apiOcm/enableDisablePermissionsRole.feature Show resolved Hide resolved

amrita-shrestha requested changes Feb 12, 2025

View reviewed changes

PrajwolAmatya force-pushed the federation-share branch from 5699738 to 2e0132f Compare February 13, 2025 03:35

PrajwolAmatya requested a review from amrita-shrestha February 13, 2025 03:40

amrita-shrestha changed the title ~~[tests-only][full-ci] added test to enable disable share permissions role for federated shares~~ [tests-only][full-ci] added test to enable disable Secure Viewer permissions role for federated shares Feb 13, 2025

PrajwolAmatya requested a review from prashant-gurung899 February 19, 2025 07:03

PrajwolAmatya force-pushed the federation-share branch 3 times, most recently from 34cab98 to 138eb5c Compare February 21, 2025 03:22

prashant-gurung899 reviewed Feb 21, 2025

View reviewed changes

tests/acceptance/features/apiOcm/enableDisablePermissionsRole.feature Outdated Show resolved Hide resolved

prashant-gurung899 reviewed Feb 21, 2025

View reviewed changes

tests/acceptance/features/apiOcm/enableDisablePermissionsRole.feature Outdated Show resolved Hide resolved

PrajwolAmatya force-pushed the federation-share branch from 138eb5c to 265b7b0 Compare February 24, 2025 03:23

PrajwolAmatya requested a review from prashant-gurung899 February 24, 2025 03:25

PrajwolAmatya force-pushed the federation-share branch from 265b7b0 to 59f2e3b Compare February 25, 2025 04:15

saw-jan approved these changes Feb 25, 2025

View reviewed changes

PrajwolAmatya force-pushed the federation-share branch 3 times, most recently from ed4fdf9 to 3beb419 Compare February 25, 2025 09:00

amrita-shrestha requested changes Feb 25, 2025

View reviewed changes

amrita-shrestha requested a review from saw-jan February 25, 2025 09:22

PrajwolAmatya requested a review from amrita-shrestha February 26, 2025 03:42

PrajwolAmatya force-pushed the federation-share branch 3 times, most recently from 4810439 to 13cb44c Compare February 28, 2025 04:14

added test to enable disable share permissions role for federated shares

316bd72

PrajwolAmatya force-pushed the federation-share branch from 13cb44c to 316bd72 Compare February 28, 2025 04:15

amrita-shrestha approved these changes Feb 28, 2025

View reviewed changes

saw-jan approved these changes Feb 28, 2025

View reviewed changes

saw-jan merged commit c960321 into master Feb 28, 2025
4 checks passed

saw-jan deleted the federation-share branch February 28, 2025 05:33

ownclouders pushed a commit that referenced this pull request Feb 28, 2025

Merge pull request #10823 from owncloud/federation-share

a171eac

[tests-only][full-ci] added test to enable disable Secure Viewer permissions role for federated shares

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tests-only][full-ci] added test to enable disable Secure Viewer permissions role for federated shares #10823

[tests-only][full-ci] added test to enable disable Secure Viewer permissions role for federated shares #10823

PrajwolAmatya commented Jan 2, 2025 •

edited by amrita-shrestha

Loading

sonarqubecloud bot commented Jan 8, 2025

PrajwolAmatya commented Feb 3, 2025

PrajwolAmatya commented Feb 11, 2025

amrita-shrestha left a comment •

edited

Loading

PrajwolAmatya commented Feb 13, 2025

prashant-gurung899 commented Feb 24, 2025

saw-jan commented Feb 24, 2025

PrajwolAmatya commented Feb 24, 2025 •

edited by amrita-shrestha

Loading

amrita-shrestha Feb 25, 2025

saw-jan Feb 25, 2025

amrita-shrestha Feb 25, 2025

saw-jan Feb 25, 2025

amrita-shrestha Feb 26, 2025

PrajwolAmatya Feb 26, 2025

amrita-shrestha Feb 27, 2025

PrajwolAmatya Feb 27, 2025 •

edited

Loading

saw-jan Feb 27, 2025

sonarqubecloud bot commented Feb 28, 2025

[tests-only][full-ci] added test to enable disable Secure Viewer permissions role for federated shares #10823

[tests-only][full-ci] added test to enable disable Secure Viewer permissions role for federated shares #10823

Conversation

PrajwolAmatya commented Jan 2, 2025 • edited by amrita-shrestha Loading

Description

Related Issue

Types of changes

Checklist:

sonarqubecloud bot commented Jan 8, 2025

Quality Gate passed

PrajwolAmatya commented Feb 3, 2025

PrajwolAmatya commented Feb 11, 2025

amrita-shrestha left a comment • edited Loading

Choose a reason for hiding this comment

PrajwolAmatya commented Feb 13, 2025

prashant-gurung899 commented Feb 24, 2025

saw-jan commented Feb 24, 2025

PrajwolAmatya commented Feb 24, 2025 • edited by amrita-shrestha Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PrajwolAmatya Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Feb 28, 2025

Quality Gate passed

PrajwolAmatya commented Jan 2, 2025 •

edited by amrita-shrestha

Loading

amrita-shrestha left a comment •

edited

Loading

PrajwolAmatya commented Feb 24, 2025 •

edited by amrita-shrestha

Loading

PrajwolAmatya Feb 27, 2025 •

edited

Loading