[pve-devel] [PATCH access-control 2/2] fix #1499: check ACL path validity

Wolfgang Bumiller w.bumiller at proxmox.com
Thu Oct 19 12:19:16 CEST 2017


On Tue, Oct 10, 2017 at 03:44:19PM +0200, Philip Abernethy wrote:
> +sub validate_path {
> +    my $path = shift;
> +    return 0 if $path !~ m'^/(vms|nodes|storage|pool|access/(?:groups|realms))(?:/([[:alnum:]\.\-\_]+))?$';
> +
> +    if ($1 eq 'vms') {PVE::JSONSchema::pve_verify_vmid($2) if $2;}
> +    elsif ($1 eq 'nodes') {PVE::JSONSchema::pve_verify_node_name($2) if $2;}
> +    elsif ($1 eq 'storage') {PVE::JSONSchema::parse_storage_id($2) if $2;}
> +    elsif ($1 eq 'pool') {verify_poolname($2) if $2;}
> +    elsif ($1 eq 'access/realms') {PVE::Auth::Plugin::pve_verify_realm($2) if $2;}

I'm really not really happy with this condensed style if + suffix-if
chain after a long regex (which could be made more readable with the /x
modifier btw., but better keep reading below:)

I wonder if it would make sense to write down a hierarchical definition
of the paths somewhere instead which could contain both the verification
methods as well as allow us to add autocompletion at some point,
something looking roughly like this:

$path_schema = {
    # root node
    completion => \&the_default_completion, # TBD, would simply use the subdirs hash keys
    subdirs => {
        vms => {
            completion => \&vm_compltion, # TBD, would complete existing vmids
            verify => \&verify_vmid,
        },
        nodes => {
            ...
        }
        access => {
            # default completion (inherited) should work
            subdirs => {
                groups => { ... }
                realms => { ... }
            }
        }
    }
};

> +
> +    return 1;
> +}
> +
>  sub userconfig_force_defaults {
>      my ($cfg) = @_;
>  
> -- 
> 2.11.0



More information about the pve-devel mailing list