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

Redirect to /user/edit path only needed if we are admin. #135

Merged

Conversation

jpbostic
Copy link

@jpbostic jpbostic commented Sep 2, 2020

to:
cc: @subspacecommunity/subspace-maintainers
related to:
resolves: #134

Background

While evaluating an instance of Subspace running in its docker container (subspacecommunity/subspace:latest), I noticed that when I login as a normal user (via SAML), deletion of device profiles works but I get an error page after I click the "Delete device?" popup.

Running in debug mode showed a "permission denied" on the /user/edit path. When looking at a clone of the repo, I realized that the profileDeleteHandler is trying to redirect back to the /user/edit path even when I'm not logged in as an admin. Normal behavior should be to only redirect to /user/edit if I'm admin and otherwise move on (the redirect to the normal "success" path is immediately below this and it was obviously never being reached).

Changes

  • HTTP redirect after successful device profile deletion only goes to /user/edit if the current user is an admin. Otherwise, continues and is redirected to the normal URL.

Testing

This was tested on a VM running a docker image built from these changes. Profiles were added to a normal account and then deleted using both an admin account and that regular account. Both ways now work without error.

@sonarcloud
Copy link

sonarcloud bot commented Sep 2, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@jack1902 jack1902 self-requested a review October 2, 2020 07:40
@jack1902 jack1902 added the bug Something isn't working label Oct 2, 2020
Copy link

@jack1902 jack1902 left a comment

Choose a reason for hiding this comment

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

I no longer work on this project, so leaving this review to remove it from my list of PRs on Github

@gchamon
Copy link

gchamon commented May 13, 2021

@jpbostic just deployed in production with my fork at https://github.com/waycarbon/subspace with no issues. Thanks!

@gchamon gchamon requested review from agonbar and a team May 17, 2021 01:15
@gchamon
Copy link

gchamon commented May 27, 2021

@subspacecommunity/subspace-maintainers I believe this is ok to merge. Need a couple more eyes on it though.

Copy link
Collaborator

@agonbar agonbar left a comment

Choose a reason for hiding this comment

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

Yup, worked just fine.

Many thanks for the contribution!

@agonbar agonbar merged commit 6c4cbed into subspacecommunity:master Jun 3, 2021
@agonbar
Copy link
Collaborator

agonbar commented Jun 3, 2021

@all-contributors please add @jpbostic for code

@allcontributors
Copy link

@agonbar

I've put up a pull request to add @jpbostic! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Normal users get error on successful device profile deletion
5 participants