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

"Save"ing user properties may overwrite newer data (racy behaviour) #707

Open
andoks opened this issue Jan 22, 2025 · 3 comments
Open

"Save"ing user properties may overwrite newer data (racy behaviour) #707

andoks opened this issue Jan 22, 2025 · 3 comments
Labels
enhancement New feature or request

Comments

@andoks
Copy link
Contributor

andoks commented Jan 22, 2025

As of 0.26.2 it seems like storing a users' properties will overwrite (at least some) properties that have been updated between opening the User UI in the browser and when clicking "Save" (in particular the "E-mail verified" field)

Repro:

  1. Create a user in the Rauthy web UI per
  2. Before opening the "Set password" link, open the user in the user list in the Rauthy web UI, observe that the "E-mail verified" property is "false" (untoggled)
  3. In a different browser session, open the "Set password" link, and set a password
  4. Back in the first browser session, duplicate the Rauthy web UI tab (keep the old one without refreshing), and open the user again in the new tab. Observe that in the new tab, the "E-mail verified" property is set to true, while in the old tab it is false
  5. In the "old tab" (where the "E-mail verified" field still is false), add another role to the user and click "Save"
  6. Observe in the "new tab" of the Rauthy web UI, that the "E-mail verified" property still is true. Then do a refresh of the page, open the user again, and observe that is now false.

Some discussions of strategies of how to handle this on stack overflow

What I would prefer happen would be that either the operator gets an error that the data they are writing is "stale" and has been changed in between their own changes, or only the fields that was actually edited are updated.

Used the ghcr.io/sebadob/rauthy:0.26.2-lite docker image

@sebadob
Copy link
Owner

sebadob commented Jan 22, 2025

Of course this would lead to overwriting values set in another session and I would expect exactly that behavior, if you execute the steps you noted down. Yes, you could implement mechanisms against this, but this is a huge amount of work with very tiny impact, since this usually does not happen.
You would basically need to re-fetch all data before each single modification in the UI and do a full comparison before submitting changes. But then you can still have the same issues if something happens in between. The only way of mitigating this properly would be by adding revision or version numbers or something like that to each table in the database which would be a very, very huge amount of work with, again, tiny impact in real life.

If you would like to submit a PR, please do so, but I don't see me spending many days or even weeks of work to implement this. It would require thousands of lines of code and all "quick" solutions will have the same issues to some degree. This does not only apply to user data, but to any data in all places, if you for instance have multiple admins.

The only thing I can consider is, when I am doing UI updates for /users during #642 at some point, that I only submit changed data. The amount of work for it is somewhat small. This will not fix the issue at all, but might reduce the impact to only actively changed values. The backend is prepared for updating only parts of a user at least.

Btw 0.26.2 is more than 3 months old and there have been quite a few fixes regarding user attributes and stuff like that, especially some cache invalidations have been fixed.

@sebadob sebadob added the enhancement New feature or request label Jan 22, 2025
@andoks
Copy link
Contributor Author

andoks commented Jan 22, 2025

Yeah, I totally get that it is a lot of work, feel free to close this as "wont-do" or something like that if you prefer to keep the issue-list clean. I just wanted to report it as I stumbled upon issues with this exact scenario while testing.

Btw 0.26.2 is more than 3 months old and there have been quite a few fixes regarding user attributes and stuff like that, especially some cache invalidations have been fixed.

Thanks for the heads up 🙂

@sebadob
Copy link
Owner

sebadob commented Jan 22, 2025

I will leave it open for now as a reminder, because I can do something about it with not too much work for user updates at least, which is the situation where it will happen most likely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants