Repository navigation
Honor deactivation behind a proxy and keep built-in roles - #43
Merged
Merged
Conversation
Behind a trusted proxy, ProxyUser returned the account without checking Active. Deactivating a proxy user through the API succeeded and listed them as deactivated, but they kept full access to the UI and the gateway. They are now refused, with a message naming the reason, and the Users page offers Deactivate for proxy users too. ProxyUser also wrote the proxy's role onto any account with that name, including built-in ones from before YardMaster moved behind the proxy. A proxy user named admin without the admin role demoted the built-in admin, and one with it promoted a built-in user. Going back to built-in sign-in kept those changes, even reopening first-run setup with no admin left. The proxy's role now applies to the request only for built-in accounts, and is stored only on accounts the proxy created. Two first visits at once raced on the insert, and the loser got a UNIQUE error; the insert now ignores the conflict and reads the row. Fixes #28
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #28. Affects only trusted-proxy mode (
YARDMASTER_AUTH=proxy).Deactivation was ignored.
ProxyUserreturned the account without checkingActive.POST /api/admin/users/{id}/activesucceeded and the user list showed the person as deactivated, but they kept full access to the UI and the gateway.ProxyUsernow returnsErrInactive. The UI answers 403 and the gateway 401, both saying the account is deactivated.Built-in accounts were rewritten.
ProxyUserstored the proxy's role on any account with that name, including built-in accounts from before YardMaster moved behind the proxy.adminwithout the admin role demoted the built-in admin. One with it promoted a built-in user.First-visit race. Two concurrent first requests for a new name raced on the insert, and the loser got a UNIQUE error (401). The insert now uses
ON CONFLICT DO NOTHINGand reads the row back.The Identity section of
docs/architecture.mddescribes the behaviour.Tests:
TestProxyUsers,TestProxyUserFirstSightRaceandTestDeactivatedProxyUserIsShutOut. Each problem above reproduces on the old code.