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

Fix user creation as root #2082

Merged
merged 8 commits into from
Dec 19, 2022
Merged

Conversation

ThomasLaPiana
Copy link
Contributor

@ThomasLaPiana ThomasLaPiana commented Dec 19, 2022

Closes #2077

Code Changes

  • add a test to catch the 403 caused by creating a user as root
  • add the fix

Steps to Confirm

  • Run nox -s dev
  • Login as the root user
  • Create a user, assert the user is created
  • Edit a user, assert the user's data is updated
  • Login as the created user, assert that the user is able to act in some way

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation:
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Description Of Changes

This regression was caused by the removal of the fideslib references from our codebase. fideslib was designed to have a common override pattern (with safe backstop) for the verify_oauth_client API dependency — which is not being overridden correctly now that the code is in the main codebase. This PR switches out the version of that method designed to be overloaded with the current method in use by the rest of the codebase.

@ThomasLaPiana ThomasLaPiana self-assigned this Dec 19, 2022
@ThomasLaPiana
Copy link
Contributor Author

@sanders41 I believe I broke this with the fideslib merge. What I think might be happening here is that the root users isn't getting created/seeded, but I'm not sure

@seanpreston seanpreston marked this pull request as ready for review December 19, 2022 18:09
Copy link
Contributor

@adamsachs adamsachs left a comment

Choose a reason for hiding this comment

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

looks good to me, and i verified that things are working now on nox -s test_env just to be sure. thanks for the updated PR description to explain the changes!

@seanpreston seanpreston merged commit 2f3b0c0 into main Dec 19, 2022
@seanpreston seanpreston deleted the ThomasLaPiana-fix-root-permissions branch December 19, 2022 18:43
SteveDMurphy pushed a commit that referenced this pull request Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating a user with the root user returns a 403
4 participants