Skip to content

Implemented PVE Token Access#43

Closed
Anuril wants to merge 0 commit into
CpuID:masterfrom
Anuril:master
Closed

Implemented PVE Token Access#43
Anuril wants to merge 0 commit into
CpuID:masterfrom
Anuril:master

Conversation

@Anuril

@Anuril Anuril commented Jun 16, 2023

Copy link
Copy Markdown

I've implemented token login for the API. It's transparent and either username & password an api id and token work.

I'd really like to see this implemented, so if you see any issues, please give feedback so I can fix.

@CpuID CpuID left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks reasonably OK, it's a great start... I've suggested a few things I'd like looked at before I'd feel comfortable with a merge.

Also I don't have a Proxmox instance around to test this works (and we don't have any test coverage in this repo, I'd kind of love some tbh but don't have the cycles to add any + a Proxmox API mock of some sort)... @danhunsaker do you have a Proxmox instance around to test this works as intended at all...?

Comment thread pve2_api.class.php Outdated
Comment thread pve2_api.class.php Outdated
@Anuril

Anuril commented Jun 17, 2023

Copy link
Copy Markdown
Author

do you have a Proxmox instance around

I sent you an E-Mail RE: PVE Instance

@danhunsaker

Copy link
Copy Markdown
Collaborator

Unfortunately, my servers were all stolen, and I haven't been able to replace them yet. I'll see about spinning a couple up in a cloud somewhere, though!

@Anuril

Anuril commented Jun 24, 2023

Copy link
Copy Markdown
Author

Any Update? I can provide Instances to test against if you want.

@Anuril

Anuril commented Jul 5, 2023

Copy link
Copy Markdown
Author

@danhunsaker - I can provide servers for you to test against - just lmk.

@spamhome

spamhome commented Jul 20, 2023

Copy link
Copy Markdown
Contributor

ive create a pull request for a updated and backward compatible version of this class
#44

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants