-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Clean up Proxmox API token handling by stripping whitespace and forma… #9228
Conversation
…tting the token string
622ffaa
to
367bcb7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution!
# Clean and format token components | ||
user = self.proxmox_user.strip() | ||
token_id = self.proxmox_token_id.strip() | ||
token_secret = self.proxmox_token_secret.strip() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering whether this is a good idea. Is token_secret
a user-selected string that can potentially have valid whitespace in it, that would get incorrectly removed by this change? (Most likely spaces, and not newlines.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally, pve tokens should never have spaces. If this is the case the token will be invalid in all cases.
https://pve.proxmox.com/wiki/Proxmox_VE_API#Example:_Use_API_Token
Exemple with token format
curl -H "Authorization: PVEAPIToken=root@pam!monitoring=aaaaaaaaa-bbb-cccc-dddd-ef0123456789" https://10.0.0.1:8006/api2/json/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, what about token IDs and usernames? Can they have spaces?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i try to create local user with space. It's not possible.
userid: invalid format - value 'user name@pam' does not look like a valid user name
(i cant try with other type (LDAP/AD/OpenID))
we can remove strip to self.proxmox_user if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you also test token ID? If that doesn't work either, everything should be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I try for token ID space is not possible :
Method 'POST /access/users/ansible@pam/token/user space' not implemented (501)
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
If nobody objects, I'll merge this at the end of next week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Backport to stable-10: 💚 backport PR created✅ Backport PR branch: Backported as #9288 🤖 @patchback |
#9228) * Clean up Proxmox API token handling by stripping whitespace and formatting the token string * Update plugins/inventory/proxmox.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/inventory/proxmox.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update changelogs/fragments/9228-fix-issue-header.yml Co-authored-by: Felix Fontein <felix@fontein.de> * Update changelogs/fragments/9228-fix-issue-header.yml Co-authored-by: Felix Fontein <felix@fontein.de> --------- Co-authored-by: Felix Fontein <felix@fontein.de> (cherry picked from commit c5855d1)
…ndling by stripping whitespace and forma… (#9288) Clean up Proxmox API token handling by stripping whitespace and forma… (#9228) * Clean up Proxmox API token handling by stripping whitespace and formatting the token string * Update plugins/inventory/proxmox.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/inventory/proxmox.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update changelogs/fragments/9228-fix-issue-header.yml Co-authored-by: Felix Fontein <felix@fontein.de> * Update changelogs/fragments/9228-fix-issue-header.yml Co-authored-by: Felix Fontein <felix@fontein.de> --------- Co-authored-by: Felix Fontein <felix@fontein.de> (cherry picked from commit c5855d1) Co-authored-by: xilmen <romain.scha@gmail.com>
ansible-collections#9228) * Clean up Proxmox API token handling by stripping whitespace and formatting the token string * Update plugins/inventory/proxmox.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/inventory/proxmox.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update changelogs/fragments/9228-fix-issue-header.yml Co-authored-by: Felix Fontein <felix@fontein.de> * Update changelogs/fragments/9228-fix-issue-header.yml Co-authored-by: Felix Fontein <felix@fontein.de> --------- Co-authored-by: Felix Fontein <felix@fontein.de>
ansible-collections#9228) * Clean up Proxmox API token handling by stripping whitespace and formatting the token string * Update plugins/inventory/proxmox.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update plugins/inventory/proxmox.py Co-authored-by: Felix Fontein <felix@fontein.de> * Update changelogs/fragments/9228-fix-issue-header.yml Co-authored-by: Felix Fontein <felix@fontein.de> * Update changelogs/fragments/9228-fix-issue-header.yml Co-authored-by: Felix Fontein <felix@fontein.de> --------- Co-authored-by: Felix Fontein <felix@fontein.de>
SUMMARY
Fix header issue for proxmox dynamic inventory with vault
#9227
ISSUE TYPE
COMPONENT NAME
proxmox inventory
ADDITIONAL INFORMATION
#9227