Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only expose storage location to admins #36094

Merged
merged 1 commit into from
Jan 16, 2023

Conversation

PVince81
Copy link
Member

@PVince81 PVince81 commented Jan 11, 2023

Checklist

@PVince81 PVince81 added the 3. to review Waiting for reviews label Jan 11, 2023
@PVince81 PVince81 added this to the Nextcloud 26 milestone Jan 11, 2023
@PVince81 PVince81 self-assigned this Jan 11, 2023
@PVince81
Copy link
Member Author

/backport to stable25

@nickvergessen
Copy link
Member

Now it's also for group admins, is that good enough, or should we wrap with an additional $this->groupManager->isAdmin($currentLoggedInUser->getUID() ?

@PVince81
Copy link
Member Author

@nickvergessen it's for admins and group admins

I've retested with admins and can confirm

@PVince81
Copy link
Member Author

ah, got it. should we exclude group admins. not sure...

@nickvergessen
Copy link
Member

I would say they also don't need it. Without system access there is no use of the value 🤔

@come-nc
Copy link
Contributor

come-nc commented Jan 12, 2023

I agree groupadmin are not sysadmin, they should not see the value.

@PVince81 PVince81 force-pushed the bugfix/noid/user-info-api-exclude-storage branch from 0d3945f to 141dfc9 Compare January 12, 2023 09:48
@PVince81
Copy link
Member Author

here we go, fixed, rebased, squashed

now only admins see the location

Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@PVince81 PVince81 force-pushed the bugfix/noid/user-info-api-exclude-storage branch from 7412381 to c24884d Compare January 13, 2023 09:31
@PVince81
Copy link
Member Author

ok, I've added an assert, squashed, rebased

@PVince81
Copy link
Member Author

@come-nc @nickvergessen can you approve or are there further concerns ? I've made it admin-only to see the storage location, not subadmins any more

@nickvergessen nickvergessen merged commit 614e3e2 into master Jan 16, 2023
@nickvergessen nickvergessen deleted the bugfix/noid/user-info-api-exclude-storage branch January 16, 2023 21:27
@PVince81
Copy link
Member Author

/backport to stable24

@PVince81
Copy link
Member Author

/backport to stable23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants