Skip to content

if user has enough permission, allow to increase permission#35614

Merged
DeepDiver1975 merged 1 commit into
masterfrom
fix-35528
Jun 24, 2019
Merged

if user has enough permission, allow to increase permission#35614
DeepDiver1975 merged 1 commit into
masterfrom
fix-35528

Conversation

@karakayasemi

@karakayasemi karakayasemi commented Jun 20, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

After #35447 merged, it has become that only node owners can increase permissions on a share. It has lead to this regression: #35528

This PR checks Session User's permission on the share node. If user has enough permission, it let session user to increase permission of the share.

Related Issue

Motivation and Context

Solving bug.

How Has This Been Tested?

Followed the scenario described in the issue. Also, we have acceptance tests that are covering bug scenario.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • 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:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

@karakayasemi karakayasemi added this to the development milestone Jun 20, 2019
@karakayasemi karakayasemi self-assigned this Jun 20, 2019
sharidas
sharidas approved these changes Jun 20, 2019
View reviewed changes

@sharidas sharidas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

DeepDiver1975
DeepDiver1975 reviewed Jun 20, 2019
View reviewed changes
Comment thread apps/files_sharing/lib/Controller/Share20OcsController.php Outdated
DeepDiver1975
DeepDiver1975 requested changes Jun 20, 2019
View reviewed changes

@DeepDiver1975 DeepDiver1975 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please have a look at my comment above - I'm not happy ....

@patrickjahns
patrickjahns requested a review from sharidas June 20, 2019 21:34
@patrickjahns

patrickjahns commented Jun 20, 2019

Copy link
Copy Markdown
Contributor

@karakayasemi
please add unit tests to cover your change and outline intended behavior

jvillafanez
jvillafanez reviewed Jun 21, 2019
View reviewed changes
Comment thread apps/files_sharing/lib/Controller/Share20OcsController.php Outdated
DeepDiver1975
DeepDiver1975 requested changes Jun 21, 2019
View reviewed changes
Comment thread apps/files_sharing/lib/Controller/Share20OcsController.php Outdated
jvillafanez
jvillafanez reviewed Jun 21, 2019
View reviewed changes
Comment thread apps/files_sharing/lib/Controller/Share20OcsController.php Outdated
DeepDiver1975
DeepDiver1975 approved these changes Jun 21, 2019
View reviewed changes
Comment thread apps/files_sharing/tests/Controller/Share20OcsControllerTest.php

public function strictSubsetOfDataProvider() {
return [
[\bindec('11111'), \bindec('0111'), true],

@DeepDiver1975 DeepDiver1975 Jun 21, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@karakayasemi
karakayasemi requested a review from jvillafanez June 21, 2019 13:54
@micbar

micbar commented Jun 24, 2019

Copy link
Copy Markdown
Contributor

@karakayasemi new tests merged into release branch. Could you rebase?

@phil-davis

phil-davis commented Jun 24, 2019

Copy link
Copy Markdown
Contributor

Note: acceptance tests in master stable10 and release-10.2.1 are all the same. So for getting the first version of this PR reviewed, master base should be fine.
Then we also make the backport PRs to stable10 and release-10.2.1 and confirm CI is good there also.

@micbar

micbar commented Jun 24, 2019

Copy link
Copy Markdown
Contributor

@phil-davis @karakayasemi Sorry for my misleading comment.
Anyway, I would give CI a last chance to finish correctly.

@patrickjahns

patrickjahns commented Jun 24, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

@ownclouders rebase rebase bot is ☠️

This was referenced Jun 24, 2019
Merged
Merged
@owncloud owncloud deleted a comment from codecov Bot Jun 24, 2019
@owncloud owncloud deleted a comment from codecov Bot Jun 24, 2019
@karakayasemi

karakayasemi commented Jun 24, 2019

Copy link
Copy Markdown
Contributor Author

Codecov came for backport PRs. I deleted old codecov report comments. I will try my chance with this way, started a new build.

@DeepDiver1975
DeepDiver1975 merged commit 9b31528 into master Jun 24, 2019
@delete-merged-branch
delete-merged-branch Bot deleted the fix-35528 branch June 24, 2019 20:05
@phil-davis

phil-davis commented Jun 25, 2019

Copy link
Copy Markdown
Contributor

Backports
stable10 #35634
release-10.2.1 #35633

@lock lock Bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DeepDiver1975 DeepDiver1975 DeepDiver1975 approved these changes
@phil-davis phil-davis phil-davis approved these changes
@micbar micbar Awaiting requested review from micbar
@sharidas sharidas Awaiting requested review from sharidas
@jvillafanez jvillafanez Awaiting requested review from jvillafanez
+1 more reviewer
@IljaN IljaN IljaN approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@karakayasemi karakayasemi

Labels

3 - To Review

Projects

None yet

Milestone

development

Development

Successfully merging this pull request may close these issues.

Permission of reshare cannot be increased

8 participants

@karakayasemi @patrickjahns @micbar @phil-davis @IljaN @DeepDiver1975 @jvillafanez @sharidas