gsadmin/PSProxmoxVE
1
0
Fork 0
mirror of https://github.com/GoodOlClint/PSProxmoxVE.git synced 2026-09-03 18:55:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: route the Users cmdlets through UserService (#126) (#198)

Browse Source 
Get-PveRole, Get-PveUser, New-PveRole and Set-PvePermission each built
their own PveHttpClient and hand-rolled requests, while UserService's
GetRoles/GetUsers/CreateRole/SetPermission carried the same requests
with zero callers. Wire the cmdlets to the service and change
SetPermission's propagate/delete parameters from bool with defaults
to nullable bool so the omitted-when-unset shipped behaviour of
Set-PvePermission survives the move.

Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com>
This commit is contained in:
goodolclint-claude[bot]
2026-09-03 00:07:45 +00:00
committed by GitHub
parent 9af67f78e6
commit 48bab3e4cb
6 changed files with 102 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/PSProxmoxVE.Core/Services/UserService.cs
+7 -7
View File
@@ -531,8 +531,8 @@ namespace PSProxmoxVE.Core.Services
/// <param name="users">Comma-separated user IDs.</param>
/// <param name="groups">Comma-separated group names.</param>
/// <param name="tokens">Comma-separated API token IDs (user@realm!tokenid).</param>
/// <param name="propagate">Whether to propagate the permission to sub-paths.</param>
/// <param name="delete">If true, removes the specified ACL entries.</param>
/// <param name="propagate">Whether to propagate the permission to sub-paths; omitted when unset.</param>
/// <param name="delete">If true, removes the specified ACL entries; omitted when unset.</param>
public void SetPermission(
PveSession session,
string path,
@@ -540,8 +540,8 @@ namespace PSProxmoxVE.Core.Services
string? users = null,
string? groups = null,
string? tokens = null,
bool propagate = true,
bool delete = false)
bool? propagate = null,
bool? delete = null)
{
if (session == null) throw new ArgumentNullException(nameof(session));
if (string.IsNullOrWhiteSpace(path)) throw new ArgumentNullException(nameof(path));
@@ -550,13 +550,13 @@ namespace PSProxmoxVE.Core.Services
var formData = new Dictionary<string, string>
{
["path"] = path,
["roles"] = roles,
["propagate"] = propagate ? "1" : "0",
["delete"] = delete ? "1" : "0"
["roles"] = roles
};
if (!string.IsNullOrEmpty(users)) formData["users"] = users!;
if (!string.IsNullOrEmpty(groups)) formData["groups"] = groups!;
if (!string.IsNullOrEmpty(tokens)) formData["tokens"] = tokens!;
if (propagate.HasValue) formData["propagate"] = propagate.Value ? "1" : "0";
if (delete.HasValue) formData["delete"] = delete.Value ? "1" : "0";
Invoke(session, client =>
{
src/PSProxmoxVE/Cmdlets/Users/GetPveRoleCmdlet.cs
+4 -8
View File
@@ -1,7 +1,6 @@
using System.Management.Automation;
using Newtonsoft.Json.Linq;
using PSProxmoxVE.Core.Client;
using PSProxmoxVE.Core.Models.Users;
using PSProxmoxVE.Core.Services;
namespace PSProxmoxVE.Cmdlets.Users
{
@@ -23,16 +22,13 @@ namespace PSProxmoxVE.Cmdlets.Users
protected override void ProcessRecord()
{
var session = GetSession();
using var client = new PveHttpClient(session);
WriteVerbose("Getting roles...");
var json = client.GetAsync("access/roles").GetAwaiter().GetResult();
var root = JObject.Parse(json);
var data = root["data"] as JArray ?? new JArray();
var service = new UserService();
var roles = service.GetRoles(session);
foreach (var item in data)
foreach (var role in roles)
{
var role = item.ToObject<PveRole>()!;
if (!string.IsNullOrEmpty(RoleId) &&
!string.Equals(role.RoleId, RoleId, System.StringComparison.OrdinalIgnoreCase))
continue;
src/PSProxmoxVE/Cmdlets/Users/GetPveUserCmdlet.cs
+4 -8
View File
@@ -1,7 +1,6 @@
using System.Management.Automation;
using Newtonsoft.Json.Linq;
using PSProxmoxVE.Core.Client;
using PSProxmoxVE.Core.Models.Users;
using PSProxmoxVE.Core.Services;
namespace PSProxmoxVE.Cmdlets.Users
{
@@ -31,16 +30,13 @@ namespace PSProxmoxVE.Cmdlets.Users
protected override void ProcessRecord()
{
var session = GetSession();
using var client = new PveHttpClient(session);
WriteVerbose("Getting users...");
var json = client.GetAsync("access/users").GetAwaiter().GetResult();
var root = JObject.Parse(json);
var data = root["data"] as JArray ?? new JArray();
var service = new UserService();
var users = service.GetUsers(session);
foreach (var item in data)
foreach (var user in users)
{
var user = item.ToObject<PveUser>()!;
if (MatchesFilters(user))
WriteObject(user);
}
src/PSProxmoxVE/Cmdlets/Users/NewPveRoleCmdlet.cs
+3 -10
View File
@@ -1,7 +1,6 @@
using System.Collections.Generic;
using System.Management.Automation;
using PSProxmoxVE.Core.Client;
using PSProxmoxVE.Core.Models.Users;
using PSProxmoxVE.Core.Services;
namespace PSProxmoxVE.Cmdlets.Users
{
@@ -32,16 +31,10 @@ namespace PSProxmoxVE.Cmdlets.Users
return;
var session = GetSession();
using var client = new PveHttpClient(session);
WriteVerbose($"Creating role '{RoleId}'...");
var data = new Dictionary<string, string>
{
["roleid"] = RoleId
};
if (!string.IsNullOrEmpty(Privileges)) data["privs"] = Privileges!;
client.PostAsync("access/roles", data).GetAwaiter().GetResult();
var service = new UserService();
service.CreateRole(session, RoleId, Privileges);
WriteObject(new PveRole { RoleId = RoleId, Privileges = Privileges });
}
src/PSProxmoxVE/Cmdlets/Users/SetPvePermissionCmdlet.cs
+15 -16
View File
@@ -1,6 +1,5 @@
using System.Collections.Generic;
using System.Management.Automation;
using PSProxmoxVE.Core.Client;
using PSProxmoxVE.Core.Services;
namespace PSProxmoxVE.Cmdlets.Users
{
@@ -48,26 +47,26 @@ namespace PSProxmoxVE.Cmdlets.Users
return;
var session = GetSession();
using var client = new PveHttpClient(session);
WriteVerbose($"Setting permission for '{UgId}' at '{Path}'...");
var data = new Dictionary<string, string>
{
["path"] = Path,
["roles"] = Role
};
string? users = null, groups = null, tokens = null;
if (string.Equals(Type, "group", System.StringComparison.OrdinalIgnoreCase))
data["groups"] = UgId;
groups = UgId;
else if (string.Equals(Type, "token", System.StringComparison.OrdinalIgnoreCase) || UgId.Contains("!"))
data["tokens"] = UgId;
tokens = UgId;
else
data["users"] = UgId;
users = UgId;
if (Propagate.IsPresent) data["propagate"] = "1";
if (Delete.IsPresent) data["delete"] = "1";
client.PutAsync("access/acl", data).GetAwaiter().GetResult();
var service = new UserService();
service.SetPermission(
session,
Path,
Role,
users: users,
groups: groups,
tokens: tokens,
propagate: Propagate.IsPresent ? true : (bool?)null,
delete: Delete.IsPresent ? true : (bool?)null);
}
}
}
tests/PSProxmoxVE.Core.Tests/Services/UserServiceTests.cs
+69 -2
View File
@@ -358,6 +358,7 @@ namespace PSProxmoxVE.Core.Tests.Services
Assert.Equal(1, result[0].Special);
Assert.Equal("CustomOps", result[2].RoleId);
Assert.Equal(0, result[2].Special);
Assert.Equal("VM.PowerMgmt,VM.Console", result[2].Privileges);
}
[Fact]
@@ -873,7 +874,10 @@ namespace PSProxmoxVE.Core.Tests.Services
d["roles"] == "PVEVMAdmin" &&
d["users"] == "deploy@pve" &&
d["propagate"] == "1" &&
d["delete"] == "0")),
!d.ContainsKey("delete") &&
!d.ContainsKey("groups") &&
!d.ContainsKey("tokens") &&
d.Count == 4)),
Times.Once);
}
@@ -894,7 +898,70 @@ namespace PSProxmoxVE.Core.Tests.Services
d["roles"] == "Administrator" &&
d["groups"] == "admins" &&
d["propagate"] == "0" &&
d["delete"] == "1")),
d["delete"] == "1" &&
!d.ContainsKey("users") &&
!d.ContainsKey("tokens") &&
d.Count == 5)),
Times.Once);
}
[Fact]
public void SetPermission_WithToken_SendsTokensKey()
{
// Arrange
_mockClient.Setup(c => c.PutAsync("access/acl", It.IsAny<Dictionary<string, string>>()))
.ReturnsAsync(@"{""data"":null}");
// Act
_service.SetPermission(_session, "/vms/100", "PVEVMAdmin", tokens: "deploy@pve!ci");
// Assert
_mockClient.Verify(c => c.PutAsync("access/acl",
It.Is<Dictionary<string, string>>(d =>
d["tokens"] == "deploy@pve!ci" &&
!d.ContainsKey("users") &&
!d.ContainsKey("groups") &&
!d.ContainsKey("propagate") &&
!d.ContainsKey("delete") &&
d.Count == 3)),
Times.Once);
}
[Fact]
public void SetPermission_DeleteWithoutPropagate_SendsDeleteOmitsPropagate()
{
// Arrange
_mockClient.Setup(c => c.PutAsync("access/acl", It.IsAny<Dictionary<string, string>>()))
.ReturnsAsync(@"{""data"":null}");
// Act
_service.SetPermission(_session, "/vms/100", "PVEVMAdmin", users: "deploy@pve", delete: true);
// Assert
_mockClient.Verify(c => c.PutAsync("access/acl",
It.Is<Dictionary<string, string>>(d =>
d["delete"] == "1" &&
!d.ContainsKey("propagate") &&
d.Count == 4)),
Times.Once);
}
[Fact]
public void SetPermission_NoPropagateOrDelete_OmitsBothFlags()
{
// Arrange
_mockClient.Setup(c => c.PutAsync("access/acl", It.IsAny<Dictionary<string, string>>()))
.ReturnsAsync(@"{""data"":null}");
// Act
_service.SetPermission(_session, "/vms/100", "PVEVMAdmin", users: "deploy@pve");
// Assert
_mockClient.Verify(c => c.PutAsync("access/acl",
It.Is<Dictionary<string, string>>(d =>
!d.ContainsKey("propagate") &&
!d.ContainsKey("delete") &&
d.Count == 3)),
Times.Once);
}