Fix return of REST-returned permissions when auth_list is set - #62680
Conversation
|
Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. |
|
It says CI is failing, but it looks like it's failing even before running? It fails in the pre-setup stage, failing to contact some python host. |
dwoz
left a comment
There was a problem hiding this comment.
Please force keep_acl_in_token to true when eauth backend is rest and also document the change.
@Ch3LL Pong! 🙂 Thanks for patience with response, been busy with work. However; Yes, I would love to have this merged in by 3006 as well. I will prioritize this task and look into forcing |
|
Just following up here. We really want to get this in by tomorrow. Any luck with implementing the feedback? Anything I can help with? |
|
I've already implemented, just need to finish writing the test. Will push in a few hours. @Ch3LL |
|
Thank you so much! This is gonna be great to get in :) |
I am still getting familiar with the source code. Feedback/LMK if any changes and I can look at them immediately. |
|
PR description updated |
|
CI fails on |
|
Yeah those are unrelated. We are fixing up some flakyness and test failures right now and I"ll rebase when they're passing. thanks this looks great. I'll just get @dwoz to re-review. |
|
Congratulations on your first PR being merged! 🎉 |
⏭️ Continuation of #62027
What does this PR do?
salt-api: salt/netapi/rest_cherrypy/app.pyWhen the auth method is set to
rest, the permission must be taken viatoken["auth_list"].auth_liststores the data fromaclmethod inrest.py.Note: Per review feedback, this PR also forces
keep_acl_in_tokento True ifrestis present underexternal_auth. This change has been properly documented in relevant places.This PR compared to the now closed #62027 adds tests and changelog entry.
What issues does this PR fix or reference?
Fixes: #62022
Previous Behavior
It always takes * permission in configuration, and therefore fails including the ACL returned by REST during authentication.
New Behavior
Includes the permission returned by REST during authentication, by storing it in the token.
Note: Per review feedback, this PR also forces
keep_acl_in_tokento True ifrestis present underexternal_auth. This change has been properly documented in relevant places.Merge requirements satisfied?
Docs