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

Resolve authentication issues in Proxmox VE 5.x and earlier versions #386

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mojtabamoaddab
Copy link

Change auth ticket header to 'Cookie' for compatibility with Proxmox VE 5.x and earlier

@Tinyblargon
Copy link
Collaborator

@mojtabamoaddab I'll test this change later.

Do note that we do not keep in mind older versions of PVE when adding new features. We do however keep the version specific code intact.

@Tinyblargon Tinyblargon self-requested a review November 27, 2024 23:10
@Tinyblargon Tinyblargon added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files test/needen This PR has to be tested labels Nov 27, 2024
@Tinyblargon Tinyblargon added the type/enhancement An improvement of existing functionality label Dec 5, 2024
@Tinyblargon Tinyblargon added test/done This PR has been tested and the result was succesfull and removed test/needen This PR has to be tested labels Dec 17, 2024
@Tinyblargon
Copy link
Collaborator

@mojtabamoaddab Tested it against PVE 8.3 with Cookie and Cookie + Authorization. Could you test against your PVE 5 if it accepts the following:

	} else if s.AuthTicket != "" {
		req.Header["Authorization"] = []string{"PVEAuthCookie=" + s.AuthTicket}
		req.Header["Cookie"] = []string{"PVEAuthCookie=" + s.AuthTicket}
		req.Header["CSRFPreventionToken"] = []string{s.CsrfToken}
	}

Preferably we can set both so it doesn't break in the future when Cookie gets removed.

Could you post a link to the PVE docs explaining the Cookie?

@mojtabamoaddab
Copy link
Author

@Tinyblargon, Thank you for your consideration.

I have tested the Cookie + Authorization method on Proxmox VE versions 5.4 and 5.0, and it worked fine.


According to the Proxmox wiki (https://pve.proxmox.com/wiki/Proxmox_VE_API#Ticket_Cookie), in the section titled "Ticket Cookie," it states:

You need to pass the returned ticket with a cookie to any further request. curl -k -b "PVEAuthCookie=PVE:root@pam:4EEC61E2::rsKoApxDTLYPn6H3NNT6iP2mv..." https://10.0.0.1:8006/api2/json/

Additionally, both official and community Proxmox API clients utilize only the Cookie header for auth tickets.

For example:
Official client (perl):
https://git.proxmox.com/?p=pve-apiclient.git;a=blob;f=src/PVE/APIClient/LWP.pm;h=f753109478867986894ecf2227f67fda7fdb00be;hb=94d38f0aba67ad04bf20159f605d5f7380cf7b58#l93

proxmoxer (python):
https://github.com/proxmoxer/proxmoxer/blob/develop/proxmoxer/backends/https.py#L97

and also github.com/Telmate/proxmox-api-go before 707644b:

req.Header.Add("Cookie", "PVEAuthCookie="+s.AuthTicket)

@Tinyblargon
Copy link
Collaborator

@mojtabamoaddab I'm not sure why 707644b even changed it. For the api token, we did use the authorization header. This feels like some just made the headers the same. Sadly, it's not attached to a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS Denotes a PR that changes 0-9 lines, ignoring generated files test/done This PR has been tested and the result was succesfull type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants