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

[RBAC] Fix an issue (race) with remote to local group sync during concurrent authentication #4105

Merged
merged 7 commits into from
May 4, 2018

Conversation

Kami
Copy link
Member

@Kami Kami commented May 3, 2018

This pull request fixes issue described in #4103.

If a user tried to authenticate against StackStorm API concurrently in a short time frame, there was a race condition which would cause an exception to be thrown and potentially not all the remote group mappings for that user to be synchronized.

Keep in mind that this issue only affects deployments which use RBAC with remote LDAP groups to local RBAC roles synchronization feature enabled.

In addition to that, the error is not fatal (authentication would succeed, as designed), but it could potentially result in not all the mappings for a user to be synchronized (depending on when the race occurred and number of mappings defined on disk).

Since the RBAC functionality follows "whitelist" approach that behavior is fail safe and has no negative security implications - the worst case (race condition occurring) would mean not all the mappings are synchronized and user has access to less / not all the resources that are defined in the mappings.

Resolves #4103.

Kami added 4 commits May 3, 2018 09:36
authenticate concurrency with StackStorm API and LDAP to RBAC role group
mappings were enabled.

Issue would manifest itself while trying to create a rbac role
assignments in the database.
@Kami Kami added this to the 2.8.0 milestone May 3, 2018
@Kami Kami requested a review from m4dcoder May 3, 2018 12:11
Copy link
Contributor

@bigmstone bigmstone left a comment

Choose a reason for hiding this comment

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

LGTM

@Kami Kami merged commit d067815 into master May 4, 2018
@Kami Kami deleted the concurrent_apply_fix branch May 4, 2018 08:38
@Kami Kami modified the milestones: 2.8.0, 2.7.2 May 8, 2018
@Kami Kami mentioned this pull request May 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Concurrent auth failure due to duplicate DB write conflict
2 participants