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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Dec 20, 2024
Merged

Clean up Proxmox API token handling by stripping whitespace and forma… #9228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
367bcb7
Clean up Proxmox API token handling by stripping whitespace and forma…
xilmen Dec 8, 2024
ae6c035
Update plugins/inventory/proxmox.py
xilmen Dec 8, 2024
c2bb00d
Update plugins/inventory/proxmox.py
xilmen Dec 8, 2024
0b65154
Update changelogs/fragments/9228-fix-issue-header.yml
xilmen Dec 8, 2024
83e3312
Update changelogs/fragments/9228-fix-issue-header.yml
xilmen Dec 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/9228-fix-issue-header.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- proxmox inventory plugin - strip whitespace from ``user``, ``token_id``, and ``token_secret`` (https://github.com/ansible-collections/community.general/issues/9227, https://github.com/ansible-collections/community.general/pull/9228/).
17 changes: 9 additions & 8 deletions plugins/inventory/proxmox.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,32 +275,33 @@ def _get_session(self):
return self.session

def _get_auth(self):

validate_certs = self.get_option('validate_certs')

if validate_certs is False:
from requests.packages.urllib3 import disable_warnings
disable_warnings()

if self.proxmox_password:

credentials = urlencode({'username': self.proxmox_user, 'password': self.proxmox_password})

a = self._get_session()

ret = a.post('%s/api2/json/access/ticket' % self.proxmox_url, data=credentials)

json = ret.json()

self.headers = {
# only required for POST/PUT/DELETE methods, which we are not using currently
# 'CSRFPreventionToken': json['data']['CSRFPreventionToken'],
'Cookie': 'PVEAuthCookie={0}'.format(json['data']['ticket'])
}

else:
# Clean and format token components
user = self.proxmox_user.strip()
token_id = self.proxmox_token_id.strip()
token_secret = self.proxmox_token_secret.strip()
Copy link
Collaborator

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.)

Copy link
Contributor Author

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/

Copy link
Collaborator

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?

Copy link
Contributor Author

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.

Copy link
Collaborator

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.

Copy link
Contributor Author

@xilmen xilmen Dec 9, 2024 •
edited
Loading

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)


# Build token string without newlines
token = f'{user}!{token_id}={token_secret}'

self.headers = {'Authorization': 'PVEAPIToken={0}!{1}={2}'.format(self.proxmox_user, self.proxmox_token_id, self.proxmox_token_secret)}
# Set headers with clean token
self.headers = {'Authorization': f'PVEAPIToken={token}'}

def _get_json(self, url, ignore_errors=None):

Expand Down
Loading