[pve-devel] [PATCH access-control v3 1/2] refactor API by unifying duplicate properties
Stoiko Ivanov
s.ivanov at proxmox.com
Tue Jun 12 12:41:48 CEST 2018
* The JSONSchema for most API calls share some properties, which this
refactoring pulls out in a common_$type_properties hashref in order to
minimize code duplication (All parameters, which are thus added to the API
calls were optional)
* Additionally for some fields the title property is added (its not used
elswhere) - as preparation for unified CLI handling
* PVE/AccessControl::role_is_special now return 0 instead of '' for false
(Schemavalidation did complain about '')
* For 2 fields a new property (print_width) was added
Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
---
PVE/API2/ACL.pm | 40 ++++++++++++----------
PVE/API2/Group.pm | 38 +++++++++-----------
PVE/API2/Role.pm | 58 ++++++++++++++++---------------
PVE/API2/User.pm | 97 +++++++++++++++++++---------------------------------
PVE/AccessControl.pm | 2 +-
PVE/Auth/Plugin.pm | 2 +-
6 files changed, 107 insertions(+), 130 deletions(-)
diff --git a/PVE/API2/ACL.pm b/PVE/API2/ACL.pm
index d37771b..64d84e3 100644
--- a/PVE/API2/ACL.pm
+++ b/PVE/API2/ACL.pm
@@ -13,6 +13,20 @@ use PVE::RESTHandler;
use base qw(PVE::RESTHandler);
+my $common_acl_properties = {
+ propagate => {
+ description => "Allow to propagate (inherit) permissions.",
+ type => 'boolean',
+ title => 'Propagate',
+ optional => 1,
+ default => 1,
+ },
+ path => {
+ description => "Access control path",
+ title => 'Path',
+ type => 'string',
+ },
+};
__PACKAGE__->register_method ({
name => 'read_acl',
path => '',
@@ -31,13 +45,11 @@ __PACKAGE__->register_method ({
items => {
type => "object",
additionalProperties => 0,
- properties => {
- path => { type => 'string' },
- type => { type => 'string', enum => ['user', 'group'] },
- ugid => { type => 'string' },
- roleid => { type => 'string' },
- propagate => { type => 'boolean' },
- },
+ properties => { (%$common_acl_properties,
+ type => { type => 'string', title => 'Type', enum => ['user', 'group'] },
+ ugid => { type => 'string', title => 'ID'},
+ roleid => { type => 'string', title => 'Role' },
+ ) },
},
},
code => sub {
@@ -89,11 +101,7 @@ __PACKAGE__->register_method ({
description => "Update Access Control List (add or remove permissions).",
parameters => {
additionalProperties => 0,
- properties => {
- path => {
- description => "Access control path",
- type => 'string',
- },
+ properties => { (%$common_acl_properties,
users => {
description => "List of users.",
type => 'string', format => 'pve-userid-list',
@@ -108,18 +116,12 @@ __PACKAGE__->register_method ({
description => "List of roles.",
type => 'string', format => 'pve-roleid-list',
},
- propagate => {
- description => "Allow to propagate (inherit) permissions.",
- type => 'boolean',
- optional => 1,
- default => 1,
- },
delete => {
description => "Remove permissions (instead of adding it).",
type => 'boolean',
optional => 1,
},
- },
+ ) },
},
returns => { type => 'null' },
code => sub {
diff --git a/PVE/API2/Group.pm b/PVE/API2/Group.pm
index fca8a2a..d3c91c8 100644
--- a/PVE/API2/Group.pm
+++ b/PVE/API2/Group.pm
@@ -9,6 +9,16 @@ use PVE::RESTHandler;
use base qw(PVE::RESTHandler);
+my $common_group_properties = {
+ groupid => {
+ type => 'string',
+ format => 'pve-groupid',
+ title => 'Group ID' ,
+ completion => \&PVE::AccessControl::complete_group,
+ },
+ comment => { type => 'string', optional => 1 },
+};
+
__PACKAGE__->register_method ({
name => 'index',
path => '',
@@ -26,9 +36,7 @@ __PACKAGE__->register_method ({
type => 'array',
items => {
type => "object",
- properties => {
- groupid => { type => 'string' },
- },
+ properties => $common_group_properties,
},
links => [ { rel => 'child', href => "{groupid}" } ],
},
@@ -65,10 +73,7 @@ __PACKAGE__->register_method ({
description => "Create new group.",
parameters => {
additionalProperties => 0,
- properties => {
- groupid => { type => 'string', format => 'pve-groupid' },
- comment => { type => 'string', optional => 1 },
- },
+ properties => $common_group_properties,
},
returns => { type => 'null' },
code => sub {
@@ -106,13 +111,7 @@ __PACKAGE__->register_method ({
description => "Update group data.",
parameters => {
additionalProperties => 0,
- properties => {
- groupid => {
- type => 'string', format => 'pve-groupid',
- completion => \&PVE::AccessControl::complete_group,
- },
- comment => { type => 'string', optional => 1 },
- },
+ properties => $common_group_properties,
},
returns => { type => 'null' },
code => sub {
@@ -149,18 +148,18 @@ __PACKAGE__->register_method ({
parameters => {
additionalProperties => 0,
properties => {
- groupid => { type => 'string', format => 'pve-groupid' },
+ %$common_group_properties{'groupid'},
},
},
returns => {
type => "object",
additionalProperties => 0,
properties => {
- comment => { type => 'string', optional => 1 },
+ comment => $common_group_properties->{'comment'},
members => {
type => 'array',
items => {
- type => "string",
+ type => 'string', format => 'pve-userid',
},
},
},
@@ -198,10 +197,7 @@ __PACKAGE__->register_method ({
parameters => {
additionalProperties => 0,
properties => {
- groupid => {
- type => 'string' , format => 'pve-groupid',
- completion => \&PVE::AccessControl::complete_group,
- },
+ %$common_group_properties{'groupid'},
}
},
returns => { type => 'null' },
diff --git a/PVE/API2/Role.pm b/PVE/API2/Role.pm
index adbb4db..860c57c 100644
--- a/PVE/API2/Role.pm
+++ b/PVE/API2/Role.pm
@@ -11,6 +11,18 @@ use PVE::RESTHandler;
use base qw(PVE::RESTHandler);
+my $common_role_properties = {
+ roleid => { type => 'string',
+ format => 'pve-roleid',
+ title => 'Role ID',
+ print_width => 30
+ },
+ privs => { type => 'string' ,
+ format => 'pve-priv-list',
+ optional => 1, title => 'Privileges',
+ },
+};
+
__PACKAGE__->register_method ({
name => 'index',
path => '',
@@ -27,9 +39,9 @@ __PACKAGE__->register_method ({
type => 'array',
items => {
type => "object",
- properties => {
- roleid => { type => 'string' },
- },
+ properties => { (%$common_role_properties,
+ special => { type => 'boolean', optional => 1, default => 0, title => 'Built-In' },
+ ) },
},
links => [ { rel => 'child', href => "{roleid}" } ],
},
@@ -63,10 +75,7 @@ __PACKAGE__->register_method ({
description => "Create new role.",
parameters => {
additionalProperties => 0,
- properties => {
- roleid => { type => 'string', format => 'pve-roleid' },
- privs => { type => 'string' , format => 'pve-priv-list', optional => 1 },
- },
+ properties => $common_role_properties,
},
returns => { type => 'null' },
code => sub {
@@ -100,18 +109,12 @@ __PACKAGE__->register_method ({
permissions => {
check => ['perm', '/access', ['Sys.Modify']],
},
- description => "Create new role.",
+ description => "Update an existing role.",
parameters => {
additionalProperties => 0,
- properties => {
- roleid => { type => 'string', format => 'pve-roleid' },
- privs => { type => 'string' , format => 'pve-priv-list' },
- append => {
- type => 'boolean',
- optional => 1,
- requires => 'privs',
- },
- },
+ properties => { (%$common_role_properties,
+ append => { type => 'boolean', optional => 1, requires => 'privs' },
+ ) },
},
returns => { type => 'null' },
code => sub {
@@ -137,7 +140,6 @@ __PACKAGE__->register_method ({
return undef;
}});
-# fixme: return format!
__PACKAGE__->register_method ({
name => 'read_role',
path => '{roleid}',
@@ -148,11 +150,13 @@ __PACKAGE__->register_method ({
description => "Get role configuration.",
parameters => {
additionalProperties => 0,
- properties => {
- roleid => { type => 'string' , format => 'pve-roleid' },
- },
+ properties => { %$common_role_properties{'roleid'}, },
+ },
+ returns => {
+ type => "object",
+ additionalProperties => 0,
+ properties => { %$common_role_properties{qw(privs)} },
},
- returns => {},
code => sub {
my ($param) = @_;
@@ -165,7 +169,8 @@ __PACKAGE__->register_method ({
die "role '$role' does not exist\n" if !$data;
return $data;
-}});
+ }
+});
__PACKAGE__->register_method ({
name => 'delete_role',
@@ -178,9 +183,7 @@ __PACKAGE__->register_method ({
description => "Delete role.",
parameters => {
additionalProperties => 0,
- properties => {
- roleid => { type => 'string', format => 'pve-roleid' },
- }
+ properties => { %$common_role_properties{'roleid'}, },
},
returns => { type => 'null' },
code => sub {
@@ -206,6 +209,7 @@ __PACKAGE__->register_method ({
}, "delete role failed");
return undef;
-}});
+ }
+});
1;
diff --git a/PVE/API2/User.pm b/PVE/API2/User.pm
index 1dc0293..6449ed0 100644
--- a/PVE/API2/User.pm
+++ b/PVE/API2/User.pm
@@ -14,6 +14,34 @@ use PVE::RESTHandler;
use base qw(PVE::RESTHandler);
+my $common_user_properties = {
+ userid => get_standard_option('userid', {
+ completion => \&PVE::AccessControl::complete_username,
+ }),
+ enable => {
+ title => "Enable",
+ description => "Enable the account (default). You can set this to '0' to disable the account",
+ type => 'boolean',
+ optional => 1,
+ default => 1,
+ },
+ expire => {
+ description => "Account expiration date (seconds since epoch). '0' means no expiration date.",
+ type => 'integer',
+ minimum => 0,
+ optional => 1,
+ },
+ firstname => { type => 'string', optional => 1 },
+ lastname => { type => 'string', optional => 1 },
+ email => { type => 'string', optional => 1, format => 'email-opt' },
+ comment => { type => 'string', optional => 1 },
+ keys => {
+ description => "Keys for two factor auth (yubico).",
+ type => 'string',
+ optional => 1,
+ },
+};
+
my $extract_user_data = sub {
my ($data, $full) = @_;
@@ -53,10 +81,8 @@ __PACKAGE__->register_method ({
type => 'array',
items => {
type => "object",
- properties => {
- userid => { type => 'string' },
+ properties => $common_user_properties,
},
- },
links => [ { rel => 'child', href => "{userid}" } ],
},
code => sub {
@@ -108,8 +134,7 @@ __PACKAGE__->register_method ({
description => "Create new user.",
parameters => {
additionalProperties => 0,
- properties => {
- userid => get_standard_option('userid'),
+ properties => { ( %$common_user_properties,
password => {
description => "Initial password.",
type => 'string',
@@ -122,28 +147,7 @@ __PACKAGE__->register_method ({
optional => 1,
completion => \&PVE::AccessControl::complete_group,
},
- firstname => { type => 'string', optional => 1 },
- lastname => { type => 'string', optional => 1 },
- email => { type => 'string', optional => 1, format => 'email-opt' },
- comment => { type => 'string', optional => 1 },
- keys => {
- description => "Keys for two factor auth (yubico).",
- type => 'string',
- optional => 1,
- },
- expire => {
- description => "Account expiration date (seconds since epoch). '0' means no expiration date.",
- type => 'integer',
- minimum => 0,
- optional => 1,
- },
- enable => {
- description => "Enable the account (default). You can set this to '0' to disable the accout",
- type => 'boolean',
- optional => 1,
- default => 1,
- },
- },
+ )}
},
returns => { type => 'null' },
code => sub {
@@ -204,16 +208,10 @@ __PACKAGE__->register_method ({
},
returns => {
additionalProperties => 0,
- properties => {
- enable => { type => 'boolean' },
- expire => { type => 'integer', optional => 1 },
- firstname => { type => 'string', optional => 1 },
- lastname => { type => 'string', optional => 1 },
- email => { type => 'string', optional => 1 },
- comment => { type => 'string', optional => 1 },
- keys => { type => 'string', optional => 1 },
+ properties => { ( %$common_user_properties{qw(enable expire firstname lastname email comment keys)},
groups => { type => 'array' },
- }
+ ) },
+ type => "object"
},
code => sub {
my ($param) = @_;
@@ -239,10 +237,7 @@ __PACKAGE__->register_method ({
description => "Update user configuration.",
parameters => {
additionalProperties => 0,
- properties => {
- userid => get_standard_option('userid', {
- completion => \&PVE::AccessControl::complete_username,
- }),
+ properties => { (%{$common_user_properties},
groups => {
type => 'string', format => 'pve-groupid-list',
optional => 1,
@@ -253,27 +248,7 @@ __PACKAGE__->register_method ({
optional => 1,
requires => 'groups',
},
- enable => {
- description => "Enable/disable the account.",
- type => 'boolean',
- optional => 1,
- },
- firstname => { type => 'string', optional => 1 },
- lastname => { type => 'string', optional => 1 },
- email => { type => 'string', optional => 1, format => 'email-opt' },
- comment => { type => 'string', optional => 1 },
- keys => {
- description => "Keys for two factor auth (yubico).",
- type => 'string',
- optional => 1,
- },
- expire => {
- description => "Account expiration date (seconds since epoch). '0' means no expiration date.",
- type => 'integer',
- minimum => 0,
- optional => 1
- },
- },
+ ) },
},
returns => { type => 'null' },
code => sub {
diff --git a/PVE/AccessControl.pm b/PVE/AccessControl.pm
index f0fb7dc..f847a8b 100644
--- a/PVE/AccessControl.pm
+++ b/PVE/AccessControl.pm
@@ -501,7 +501,7 @@ create_roles();
sub role_is_special {
my ($role) = @_;
- return exists $special_roles->{$role};
+ return (exists $special_roles->{$role} ? 1 : 0);
}
sub add_role_privs {
diff --git a/PVE/Auth/Plugin.pm b/PVE/Auth/Plugin.pm
index d5d2c06..0bffa2b 100755
--- a/PVE/Auth/Plugin.pm
+++ b/PVE/Auth/Plugin.pm
@@ -76,7 +76,7 @@ sub verify_username {
}
PVE::JSONSchema::register_standard_option('userid', {
- description => "User ID",
+ description => "User ID", title => "User ID",
type => 'string', format => 'pve-userid',
maxLength => 64,
});
--
2.11.0
More information about the pve-devel
mailing list