Visitar URL original
fix: Do not allow to set higher permissions on a federated share for … by DeepDiver1975 · Pull Request #40803 · owncloud/core · GitHub
Skip to content

fix: Do not allow to set higher permissions on a federated share for … - #40803

Merged
phil-davis merged 1 commit into
masterfrom
fix/fedsharing-permission-creep
Jun 7, 2023
Merged

fix: Do not allow to set higher permissions on a federated share for …#40803
phil-davis merged 1 commit into
masterfrom
fix/fedsharing-permission-creep

Conversation

@DeepDiver1975

@DeepDiver1975 DeepDiver1975 commented May 22, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

…a resource which was shared with limited permissions

Description

Related Issue

  • Fixes <issue_link>

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

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:
  • Changelog item, see TEMPLATE

@DeepDiver1975
DeepDiver1975 force-pushed the fix/fedsharing-permission-creep branch 2 times, most recently from d209fda to c3bb7e1 Compare May 23, 2023 09:51
@DeepDiver1975
fix: Do not allow to set higher permissions on a federated share for …
24c4292 
…a resource which was shared with limited permissions
@DeepDiver1975
DeepDiver1975 force-pushed the fix/fedsharing-permission-creep branch from c3bb7e1 to 24c4292 Compare May 25, 2023 08:04
@sonarqubecloud

sonarqubecloud Bot commented May 25, 2023

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

72.7% 72.7% Coverage
0.0% 0.0% Duplication

@pako81 pako81 mentioned this pull request Jun 1, 2023
Merged
@jnweiger
jnweiger requested review from pako81 and phil-davis June 1, 2023 09:50
phil-davis
phil-davis reviewed Jun 1, 2023
View reviewed changes

@phil-davis phil-davis 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.

The code looks like it works fine.
I have suggested a change to the name of a function - I think that the current name is misleading (from my reading of the code)

Comment thread apps/federatedfilesharing/lib/FedShareManager.php Resolved
Comment thread apps/federatedfilesharing/lib/Ocm/Permissions.php Resolved
Comment thread changelog/unreleased/40803 Resolved
@owncloud owncloud deleted a comment from update-docs Bot Jun 1, 2023
pako81 pushed a commit to owncloud/docs that referenced this pull request Jun 2, 2023
Pasquale Tripodi
document owncloud/core#40803
8faa7e3
@phil-davis

phil-davis commented Jun 7, 2023

Copy link
Copy Markdown
Contributor

This has been released in 10.12.2 with exactly the code here, which works fine.
I will merge this PR to master so that we get the same code in master
Then I will make a separate PR with my refactoring suggestions - those do not effect the functionality.

@phil-davis
phil-davis merged commit 86ca60e into master Jun 7, 2023
@delete-merged-branch
delete-merged-branch Bot deleted the fix/fedsharing-permission-creep branch June 7, 2023 05:00
@phil-davis phil-davis mentioned this pull request Jun 7, 2023
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@phil-davis phil-davis phil-davis approved these changes
@pako81 pako81 Awaiting requested review from pako81

Assignees

No one assigned

Labels

3 - To Review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@DeepDiver1975 @phil-davis