Skip to content

Commit

Permalink
Rename options
Browse files Browse the repository at this point in the history
  • Loading branch information
dorssel committed Apr 17, 2024
1 parent 2e9f49d commit eddebaf
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 59 deletions.
2 changes: 1 addition & 1 deletion UnitTests/Parse_policy_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void add_Success()
mock.Setup(m => m.PolicyAdd(It.IsAny<PolicyRule>(),
It.IsNotNull<IConsole>(), It.IsAny<CancellationToken>())).Returns(Task.FromResult(ExitCode.Success));

Test(ExitCode.Success, mock, "policy", "add", "--type", "bind", "--access", "allow",
Test(ExitCode.Success, mock, "policy", "add", "--effect", "Allow", "--operation", "AutoBind",
"--busid", TestBusId.ToString(), "--hardware-id", TestHardwareId.ToString());
}

Expand Down
33 changes: 22 additions & 11 deletions Usbipd/CommandHandlersCli.cs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ Task<ExitCode> ICommandHandlers.List(bool usbids, IConsole console, Cancellation
{
state = "Incompatible hub";
}
else if (Policy.AllowBind(device))
else if (Policy.IsAutoBindAllowed(device))
{
state = "Allowed";

Check warning on line 143 in Usbipd/CommandHandlersCli.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/CommandHandlersCli.cs#L143

Added line #L143 was not covered by tests
}
Expand Down Expand Up @@ -362,9 +362,9 @@ async Task<ExitCode> ICommandHandlers.AttachWsl(BusId busId, bool autoAttach, st
console.ReportError($"There is no device with busid '{busId}'.");
return ExitCode.Failure;
}
if (!device.Guid.HasValue)
if (!device.Guid.HasValue && !Policy.IsAutoBindAllowed(device))
{
console.ReportError($"Device is not shared; run 'usbipd bind -b {busId}' as administrator first.");
console.ReportError($"Device is not shared; run 'usbipd bind --busid {busId}' as administrator first.");

Check warning on line 367 in Usbipd/CommandHandlersCli.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/CommandHandlersCli.cs#L367

Added line #L367 was not covered by tests
return ExitCode.Failure;
}
// We allow auto-attach on devices that are already attached.
Expand Down Expand Up @@ -500,18 +500,18 @@ Task<ExitCode> ICommandHandlers.PolicyList(IConsole console, CancellationToken c
{
var policyRules = RegistryUtils.GetPolicyRules();
console.WriteLine("Policy rules:");
console.WriteLine($"{"GUID",-36} {"TYPE",-4} {"ACCESS",-6} {"BUSID",-5} {"VID:PID",-9}");
console.WriteLine($"{"GUID",-36} {"EFFECT",-6} {"OPERATION",-9} {"BUSID",-5} {"VID:PID",-9}");

Check warning on line 503 in Usbipd/CommandHandlersCli.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/CommandHandlersCli.cs#L501-L503

Added lines #L501 - L503 were not covered by tests
foreach (var rule in policyRules)
{
console.Write($"{rule.Key,-36} ");
console.Write($"{rule.Value.Type,-4} ");
console.Write($"{(rule.Value.Allow ? "Allow" : "Deny"),-6} ");
switch (rule.Value.Type)
console.Write($"{rule.Value.Effect,-6} ");
console.Write($"{rule.Value.Operation,-9} ");

Check warning on line 508 in Usbipd/CommandHandlersCli.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/CommandHandlersCli.cs#L506-L508

Added lines #L506 - L508 were not covered by tests
switch (rule.Value.Operation)
{
case PolicyRuleType.Bind:
var ruleBind = (PolicyRuleBind)rule.Value;
console.Write($"{(ruleBind.BusId.HasValue ? ruleBind.BusId.Value : string.Empty),-5} ");
console.Write($"{(ruleBind.HardwareId.HasValue ? ruleBind.HardwareId.Value : string.Empty),-9}");
case PolicyRuleOperation.AutoBind:
var autoBind = (PolicyRuleAutoBind)rule.Value;

Check warning on line 512 in Usbipd/CommandHandlersCli.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/CommandHandlersCli.cs#L512

Added line #L512 was not covered by tests
console.Write($"{(autoBind.BusId.HasValue ? autoBind.BusId.Value : string.Empty),-5} ");
console.Write($"{(autoBind.HardwareId.HasValue ? autoBind.HardwareId.Value : string.Empty),-9}");
break;
}
console.WriteLine(string.Empty);

Check warning on line 517 in Usbipd/CommandHandlersCli.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/CommandHandlersCli.cs#L517

Added line #L517 was not covered by tests
Expand All @@ -536,4 +536,15 @@ Task<ExitCode> ICommandHandlers.PolicyRemove(Guid guid, IConsole console, Cancel
RegistryUtils.RemovePolicyRule(guid);
return Task.FromResult(ExitCode.Success);

Check warning on line 537 in Usbipd/CommandHandlersCli.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/CommandHandlersCli.cs#L536-L537

Added lines #L536 - L537 were not covered by tests
}

Task<ExitCode> ICommandHandlers.PolicyRemoveAll(IConsole console, CancellationToken cancellationToken)
{
if (!CheckWriteAccess(console))
{
return Task.FromResult(ExitCode.AccessDenied);

Check warning on line 544 in Usbipd/CommandHandlersCli.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/CommandHandlersCli.cs#L544

Added line #L544 was not covered by tests
}

RegistryUtils.RemovePolicyRuleAll();
return Task.FromResult(ExitCode.Success);

Check warning on line 548 in Usbipd/CommandHandlersCli.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/CommandHandlersCli.cs#L547-L548

Added lines #L547 - L548 were not covered by tests
}
}
2 changes: 1 addition & 1 deletion Usbipd/ConnectedClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ static IEnumerable<UsbDevice> GetDevicesAvailableForClient(IPAddress ipAddress)
// Device is already bound, so available for attach.
return true;
}
return Policy.AllowBind(device, ipAddress);
return Policy.IsAutoBindAllowed(device, ipAddress);
});

Check warning on line 79 in Usbipd/ConnectedClient.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/ConnectedClient.cs#L74-L79

Added lines #L74 - L79 were not covered by tests
}

Expand Down
1 change: 1 addition & 0 deletions Usbipd/ICommandHandlers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,5 @@ interface ICommandHandlers
public Task<ExitCode> PolicyAdd(PolicyRule rule, IConsole console, CancellationToken cancellationToken);
public Task<ExitCode> PolicyList(IConsole console, CancellationToken cancellationToken);
public Task<ExitCode> PolicyRemove(Guid guid, IConsole console, CancellationToken cancellationToken);
public Task<ExitCode> PolicyRemoveAll(IConsole console, CancellationToken cancellationToken);
}
7 changes: 4 additions & 3 deletions Usbipd/Policy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@ static class Policy
{
// If client == null, then it is ignored. This should only be used for listing the devices locally
// to determine if the device is allowed for at least one client.
public static bool AllowBind(UsbDevice device, IPAddress? client = null)
// NOTE: client == null is currently also used by 'attach -wsl', until we can reliably detect WSL connections.
public static bool IsAutoBindAllowed(UsbDevice device, IPAddress? client = null)
{
// Firewalling is not supported yet.
_ = client;

var rules = RegistryUtils.GetPolicyRules();
var allowed = rules.Values.Where(r => r.Allow);
var denied = rules.Values.Where(r => !r.Allow);
var allowed = rules.Values.Where(r => r.Effect == PolicyRuleEffect.Allow);
var denied = rules.Values.Where(r => r.Effect == PolicyRuleEffect.Deny);

Check warning on line 21 in Usbipd/Policy.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/Policy.cs#L19-L21

Added lines #L19 - L21 were not covered by tests

return allowed.Any(r => r.Matches(device)) && !denied.Any(r => r.Matches(device));
}
Expand Down
2 changes: 1 addition & 1 deletion Usbipd/PolicyRule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace Usbipd;

abstract record PolicyRule(bool Allow, PolicyRuleType Type)
abstract record PolicyRule(PolicyRuleEffect Effect, PolicyRuleOperation Operation)
{
public abstract bool IsValid();

Expand Down
8 changes: 4 additions & 4 deletions Usbipd/PolicyRuleBind.cs → Usbipd/PolicyRuleAutoBind.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

namespace Usbipd;

sealed record PolicyRuleBind(bool allow, BusId? BusId, VidPid? HardwareId)
: PolicyRule(allow, PolicyRuleType.Bind)
sealed record PolicyRuleAutoBind(PolicyRuleEffect effect, BusId? BusId, VidPid? HardwareId)

Check warning on line 10 in Usbipd/PolicyRuleAutoBind.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/PolicyRuleAutoBind.cs#L10

Added line #L10 was not covered by tests
: PolicyRule(effect, PolicyRuleOperation.AutoBind)
{
const string BusIdName = "BusId";
const string HardwareIdName = "HardwareId";
Expand Down Expand Up @@ -46,7 +46,7 @@ public override void Save(RegistryKey registryKey)
}
}

Check warning on line 47 in Usbipd/PolicyRuleAutoBind.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/PolicyRuleAutoBind.cs#L47

Added line #L47 was not covered by tests

public static PolicyRuleBind Load(bool allow, RegistryKey registryKey)
public static PolicyRuleAutoBind Load(PolicyRuleEffect access, RegistryKey registryKey)
{
BusId? busId = null;

Check warning on line 51 in Usbipd/PolicyRuleAutoBind.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/PolicyRuleAutoBind.cs#L51

Added line #L51 was not covered by tests
if (Automation.BusId.TryParse(registryKey.GetValue(BusIdName) as string ?? string.Empty, out var parsedBusId))
Expand All @@ -58,6 +58,6 @@ public static PolicyRuleBind Load(bool allow, RegistryKey registryKey)
{
hardwareId = parsedHardwareId;

Check warning on line 59 in Usbipd/PolicyRuleAutoBind.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/PolicyRuleAutoBind.cs#L59

Added line #L59 was not covered by tests
}
return new(allow, busId, hardwareId);
return new(access, busId, hardwareId);

Check warning on line 61 in Usbipd/PolicyRuleAutoBind.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/PolicyRuleAutoBind.cs#L61

Added line #L61 was not covered by tests
}
}
2 changes: 1 addition & 1 deletion Usbipd/PolicyRuleAccess.cs → Usbipd/PolicyRuleEffect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace Usbipd;

public enum PolicyRuleAccess
public enum PolicyRuleEffect
{
Allow,
Deny,
Expand Down
4 changes: 2 additions & 2 deletions Usbipd/PolicyRuleType.cs → Usbipd/PolicyRuleOperation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace Usbipd;

public enum PolicyRuleType
public enum PolicyRuleOperation
{
Bind,
AutoBind,
}
72 changes: 48 additions & 24 deletions Usbipd/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -416,28 +416,28 @@ await commandHandlers.List(invocationContext.ParseResult.HasOption(usbidsOption)
// policy
//
var policyCommand = new Command("policy", "Manage policy rules\0"
+ "Policy rules allow or deny specific functionality, such as bind and attach.\n");
+ "Policy rules allow or deny specific operations.");
{
//
// policy add --access <ACCESS>
// policy add --effect <EFFECT>
//
var accessOption = new Option<PolicyRuleAccess>(
aliases: ["--access", "-a"]
var effectOption = new Option<PolicyRuleEffect>(
aliases: ["--effect", "-e"]
)
{
ArgumentHelpName = "ACCESS",
ArgumentHelpName = "EFFECT",
Description = "Allow or Deny",
IsRequired = true,
};
//
// policy add --type <TYPE>
// policy add --operation <OPERATION>
//
var typeOption = new Option<PolicyRuleType>(
aliases: ["--type", "-t"]
var operationOption = new Option<PolicyRuleOperation>(
aliases: ["--operation", "-o"]
)
{
ArgumentHelpName = "TYPE",
Description = "Add a policy rule of type <TYPE>",
ArgumentHelpName = "OPERATION",
Description = "Currently only supports 'AutoBind'",
IsRequired = true,
};
//
Expand Down Expand Up @@ -472,8 +472,8 @@ await commandHandlers.List(invocationContext.ParseResult.HasOption(usbidsOption)
+ "\n"
+ AtLeastOneOfRequiredText(busIdOption, hardwareIdOption))
{
accessOption,
typeOption,
effectOption,
operationOption,
busIdOption,
hardwareIdOption,
};
Expand All @@ -483,15 +483,15 @@ await commandHandlers.List(invocationContext.ParseResult.HasOption(usbidsOption)
});
addCommand.SetHandler(async invocationContext =>
{
var policyRuleType = invocationContext.ParseResult.GetValueForOption(typeOption);
invocationContext.ExitCode = (int)await (policyRuleType switch
var operation = invocationContext.ParseResult.GetValueForOption(operationOption);
invocationContext.ExitCode = (int)await (operation switch
{
PolicyRuleType.Bind =>
commandHandlers.PolicyAdd(new PolicyRuleBind(invocationContext.ParseResult.GetValueForOption(accessOption) == PolicyRuleAccess.Allow,
PolicyRuleOperation.AutoBind =>
commandHandlers.PolicyAdd(new PolicyRuleAutoBind(invocationContext.ParseResult.GetValueForOption(effectOption),
invocationContext.ParseResult.GetValueForOptionOrNull(busIdOption),
invocationContext.ParseResult.GetValueForOptionOrNull(hardwareIdOption)),
invocationContext.Console, invocationContext.GetCancellationToken()),
_ => throw new UnexpectedResultException($"Unexpected policy rule type '{policyRuleType}'."),
_ => throw new UnexpectedResultException($"Unexpected policy rule operation '{operation}'."),

Check warning on line 494 in Usbipd/Program.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/Program.cs#L494

Added line #L494 was not covered by tests
});
});
policyCommand.AddCommand(addCommand);
Expand All @@ -501,7 +501,7 @@ await commandHandlers.List(invocationContext.ParseResult.HasOption(usbidsOption)
// policy list
//
var listCommand = new Command("list", "List policy rules\0"
+ "List all policy rules.\n");
+ "List all policy rules.");
listCommand.SetHandler(async invocationContext =>
{
invocationContext.ExitCode = (int)
Expand All @@ -511,7 +511,17 @@ await commandHandlers.List(invocationContext.ParseResult.HasOption(usbidsOption)
}
{
//
// policy remove --guid <GUID>
// policy remove [--all]
//
var allOption = new Option<bool>(
aliases: ["--all", "-a"]
)
{
Description = "Remove all policy rules",
Arity = ArgumentArity.Zero,
};
//
// policy remove [--guid <GUID>]
//
var guidOption = new Option<Guid>(
aliases: ["--guid", "-g"],
Expand All @@ -520,22 +530,36 @@ await commandHandlers.List(invocationContext.ParseResult.HasOption(usbidsOption)
{
ArgumentHelpName = "GUID",
Description = "Remove the policy rule having <GUID>",
IsRequired = true,
}.AddCompletions(completionContext => CompletionGuard(completionContext, () =>
RegistryUtils.GetPolicyRules().Select(r => r.Key.ToString("D"))));
//
// policy remove
//
var removeCommand = new Command("remove", "Remove a policy rule\0"
+ "Remove an existing policy rule. The resulting policy will be effective immediately.\n")
+ "Remove existing policy rules. The resulting policy will be effective immediately.\n"
+ "\n"
+ OneOfRequiredText(allOption, guidOption))
{
allOption,
guidOption,
};
removeCommand.AddValidator(commandResult =>
{
ValidateOneOf(commandResult, allOption, guidOption);
});
removeCommand.SetHandler(async invocationContext =>
{
invocationContext.ExitCode = (int)
await commandHandlers.PolicyRemove(invocationContext.ParseResult.GetValueForOption(guidOption),
invocationContext.Console, invocationContext.GetCancellationToken());
if (invocationContext.ParseResult.HasOption(allOption))
{
invocationContext.ExitCode = (int)
await commandHandlers.PolicyRemoveAll(invocationContext.Console, invocationContext.GetCancellationToken());

Check warning on line 555 in Usbipd/Program.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/Program.cs#L554-L555

Added lines #L554 - L555 were not covered by tests
}
else
{
invocationContext.ExitCode = (int)
await commandHandlers.PolicyRemove(invocationContext.ParseResult.GetValueForOption(guidOption),
invocationContext.Console, invocationContext.GetCancellationToken());
}
});
policyCommand.AddCommand(removeCommand);
}
Expand Down
30 changes: 19 additions & 11 deletions Usbipd/RegistryUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ static RegistryKey OpenBaseKey(bool writable)
const string BusIdName = "BusId";
const string IPAddressName = "IPAddress";
const string PolicyName = "Policy";
const string TypeName = "Type";
const string AllowName = "Allow";
const string EffectName = "Effect";
const string OperationName = "Operation";

/// <summary>
/// <see langword="null"/> if not installed
Expand Down Expand Up @@ -252,8 +252,8 @@ public static Guid AddPolicyRule(PolicyRule rule)
}
var guid = Guid.NewGuid();
using var ruleKey = GetPolicyKey(true).CreateSubKey($"{guid:B}");
ruleKey.SetValue(AllowName, rule.Allow ? 1 : 0);
ruleKey.SetValue(TypeName, rule.Type.ToString());
ruleKey.SetValue(EffectName, rule.Effect.ToString());
ruleKey.SetValue(OperationName, rule.Operation.ToString());
rule.Save(ruleKey);
return guid;
}

Check warning on line 259 in Usbipd/RegistryUtils.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/RegistryUtils.cs#L253-L259

Added lines #L253 - L259 were not covered by tests
Expand All @@ -264,6 +264,15 @@ public static void RemovePolicyRule(Guid guid)
policyKey.DeleteSubKeyTree(guid.ToString("B"), false);
}

Check warning on line 265 in Usbipd/RegistryUtils.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/RegistryUtils.cs#L263-L265

Added lines #L263 - L265 were not covered by tests

public static void RemovePolicyRuleAll()
{
using var policyKey = GetPolicyKey(true);

Check warning on line 269 in Usbipd/RegistryUtils.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/RegistryUtils.cs#L269

Added line #L269 was not covered by tests
foreach (var subKeyName in policyKey.GetSubKeyNames())
{
policyKey.DeleteSubKeyTree(subKeyName, false);

Check warning on line 272 in Usbipd/RegistryUtils.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/RegistryUtils.cs#L272

Added line #L272 was not covered by tests
}
}

Check warning on line 274 in Usbipd/RegistryUtils.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/RegistryUtils.cs#L274

Added line #L274 was not covered by tests

/// <summary>
/// Enumerates all rules.
/// <para>
Expand Down Expand Up @@ -291,22 +300,21 @@ public static SortedDictionary<Guid, PolicyRule> GetPolicyRules()
{
continue;

Check warning on line 301 in Usbipd/RegistryUtils.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/RegistryUtils.cs#L301

Added line #L301 was not covered by tests
}
if (ruleKey.GetValue(AllowName) is not int allowValue)
if (!Enum.TryParse<PolicyRuleEffect>(ruleKey.GetValue(EffectName) as string, true, out var effect))
{
// Must exist and be a DWORD.
// Must exist and be a valid enum string.
continue;

Check warning on line 306 in Usbipd/RegistryUtils.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/RegistryUtils.cs#L306

Added line #L306 was not covered by tests
}
var allow = (allowValue != 0);
if (!Enum.TryParse<PolicyRuleType>(ruleKey.GetValue(TypeName) as string, true, out var ruleType))
if (!Enum.TryParse<PolicyRuleOperation>(ruleKey.GetValue(OperationName) as string, true, out var operation))
{
// Must exist and be a valid enum string.
continue;
}
PolicyRule rule;
switch (ruleType)
switch (operation)
{
case PolicyRuleType.Bind:
rule = PolicyRuleBind.Load(allow, ruleKey);
case PolicyRuleOperation.AutoBind:
rule = PolicyRuleAutoBind.Load(effect, ruleKey);

Check warning on line 317 in Usbipd/RegistryUtils.cs

View check run for this annotation

Codecov / codecov/patch

Usbipd/RegistryUtils.cs#L317

Added line #L317 was not covered by tests
break;
default:
// Invalid, ignore.
Expand Down

0 comments on commit eddebaf

Please sign in to comment.