[pve-devel] [PATCH access-control v5 3/5] refactor API using get/register_standard_option

Stoiko Ivanov s.ivanov at proxmox.com
Thu Jun 21 14:31:45 CEST 2018


Pull out duplicated property definitions in the API into
register_standard_option/get_standard_option calls.
(All parameters, which are thus added to the API calls were optional).

Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
---
 PVE/API2/ACL.pm           |  28 +++++-----
 PVE/API2/AccessControl.pm |  12 ++---
 PVE/API2/Group.pm         |  36 +++++++------
 PVE/API2/Role.pm          |  52 +++++++++++-------
 PVE/API2/User.pm          | 134 ++++++++++++++++++++++------------------------
 5 files changed, 139 insertions(+), 123 deletions(-)

diff --git a/PVE/API2/ACL.pm b/PVE/API2/ACL.pm
index d37771b..3e42ac0 100644
--- a/PVE/API2/ACL.pm
+++ b/PVE/API2/ACL.pm
@@ -6,6 +6,7 @@ use PVE::Cluster qw (cfs_read_file cfs_write_file);
 use PVE::Tools qw(split_list);
 use PVE::AccessControl;
 use PVE::Exception qw(raise_param_exc);
+use PVE::JSONSchema qw(get_standard_option register_standard_option);
 
 use PVE::SafeSyslog;
 
@@ -13,6 +14,17 @@ use PVE::RESTHandler;
 
 use base qw(PVE::RESTHandler);
 
+register_standard_option('acl-propagate', {
+    description => "Allow to propagate (inherit) permissions.",
+    type => 'boolean',
+    optional => 1,
+    default => 1,
+});
+register_standard_option('acl-path', {
+    description => "Access control path",
+    type => 'string',
+});
+
 __PACKAGE__->register_method ({
     name => 'read_acl',
     path => '',
@@ -32,11 +44,11 @@ __PACKAGE__->register_method ({
 	    type => "object",
 	    additionalProperties => 0,
 	    properties => {
-		path => { type => 'string' },
+		propagate => get_standard_option('acl-propagate'),
+		path => get_standard_option('acl-path'),
 		type => { type => 'string', enum => ['user', 'group'] },
 		ugid => { type => 'string' },
 		roleid => { type => 'string' },
-		propagate => { type => 'boolean' },
 	    },
 	},
     },
@@ -90,10 +102,8 @@ __PACKAGE__->register_method ({
     parameters => {
 	additionalProperties => 0,
 	properties => {
-	    path => {
-		description => "Access control path",
-		type => 'string',
-	    },
+	    propagate => get_standard_option('acl-propagate'),
+	    path => get_standard_option('acl-path'),
 	    users => {
 		description => "List of users.",
 		type => 'string',  format => 'pve-userid-list',
@@ -108,12 +118,6 @@ __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',
diff --git a/PVE/API2/AccessControl.pm b/PVE/API2/AccessControl.pm
index afcd2fa..414da3a 100644
--- a/PVE/API2/AccessControl.pm
+++ b/PVE/API2/AccessControl.pm
@@ -205,10 +205,10 @@ __PACKAGE__->register_method ({
 	additionalProperties => 0,
 	properties => {
 	    username => {
-		description => "User name",
-		type => 'string',
-		maxLength => 64,
-		completion => \&PVE::AccessControl::complete_username,
+	        description => "User name",
+	        type => 'string',
+	        maxLength => 64,
+	        completion => \&PVE::AccessControl::complete_username,
 	    },
 	    realm =>  get_standard_option('realm', {
 		description => "You can optionally pass the realm using this parameter. Normally the realm is simply added to the username <username>\@<relam>.",
@@ -301,9 +301,7 @@ __PACKAGE__->register_method ({
     parameters => {
 	additionalProperties => 0,
 	properties => {
-	    userid => get_standard_option('userid', {
-		completion => \&PVE::AccessControl::complete_username,
-	    }),
+	    userid => get_standard_option('userid-completed'),
 	    password => { 
 		description => "The new password.",
 		type => 'string',
diff --git a/PVE/API2/Group.pm b/PVE/API2/Group.pm
index fca8a2a..37f8be2 100644
--- a/PVE/API2/Group.pm
+++ b/PVE/API2/Group.pm
@@ -6,9 +6,18 @@ use PVE::Cluster qw (cfs_read_file cfs_write_file);
 use PVE::AccessControl;
 use PVE::SafeSyslog;
 use PVE::RESTHandler;
+use PVE::JSONSchema qw(get_standard_option register_standard_option);
 
 use base qw(PVE::RESTHandler);
 
+register_standard_option('group-id', {
+    type => 'string',
+    format => 'pve-groupid',
+    completion => \&PVE::AccessControl::complete_group,
+});
+
+register_standard_option('group-comment', { type => 'string', optional => 1 });
+
 __PACKAGE__->register_method ({
     name => 'index', 
     path => '', 
@@ -27,7 +36,8 @@ __PACKAGE__->register_method ({
 	items => {
 	    type => "object",
 	    properties => {
-		groupid => { type => 'string' },
+		groupid => get_standard_option('group-id'),
+		comment => get_standard_option('group-comment'),
 	    },
 	},
 	links => [ { rel => 'child', href => "{groupid}" } ],
@@ -66,8 +76,8 @@ __PACKAGE__->register_method ({
     parameters => {
    	additionalProperties => 0,
 	properties => {
-	    groupid => { type => 'string', format => 'pve-groupid' },
-	    comment => { type => 'string', optional => 1 },
+	    groupid => get_standard_option('group-id'),
+	    comment => get_standard_option('group-comment'),
 	},
     },
     returns => { type => 'null' },
@@ -107,11 +117,8 @@ __PACKAGE__->register_method ({
     parameters => {
    	additionalProperties => 0,
 	properties => {
-	    groupid => {
-		type => 'string', format => 'pve-groupid',
-		completion => \&PVE::AccessControl::complete_group,
-	    },
-	    comment => { type => 'string', optional => 1 },
+	    groupid => get_standard_option('group-id'),
+	    comment => get_standard_option('group-comment'),
 	},
     },
     returns => { type => 'null' },
@@ -149,19 +156,17 @@ __PACKAGE__->register_method ({
     parameters => {
    	additionalProperties => 0,
 	properties => {
-	    groupid => { type => 'string', format => 'pve-groupid' },
+	    groupid => get_standard_option('group-id'),
 	},
     },
     returns => {
 	type => "object",
 	additionalProperties => 0,
 	properties => {
-	    comment => { type => 'string', optional => 1 },
+	    comment => get_standard_option('group-comment'),
 	    members => {
 		type => 'array',
-		items => {
-		    type => "string",
-		},
+		items => get_standard_option('userid-completed')
 	    },
 	},
     },
@@ -198,10 +203,7 @@ __PACKAGE__->register_method ({
     parameters => {
    	additionalProperties => 0,
 	properties => {
-	    groupid => {
-		type => 'string' , format => 'pve-groupid',
-		completion => \&PVE::AccessControl::complete_group,
-	    },
+	    groupid => get_standard_option('group-id'),
 	}
     },
     returns => { type => 'null' },
diff --git a/PVE/API2/Role.pm b/PVE/API2/Role.pm
index adbb4db..80959b0 100644
--- a/PVE/API2/Role.pm
+++ b/PVE/API2/Role.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 use PVE::Cluster qw (cfs_read_file cfs_write_file);
 use PVE::AccessControl;
+use PVE::JSONSchema qw(get_standard_option register_standard_option);
 
 use PVE::SafeSyslog;
 
@@ -11,6 +12,16 @@ use PVE::RESTHandler;
 
 use base qw(PVE::RESTHandler);
 
+register_standard_option('role-id', {
+    type => 'string',
+    format => 'pve-roleid',
+});
+register_standard_option('role-privs', {
+    type => 'string' ,
+    format => 'pve-priv-list',
+    optional => 1,
+});
+
 __PACKAGE__->register_method ({
     name => 'index',
     path => '',
@@ -28,7 +39,9 @@ __PACKAGE__->register_method ({
 	items => {
 	    type => "object",
 	    properties => {
-		roleid => { type => 'string' },
+		roleid => get_standard_option('role-id'),
+		privs =>  get_standard_option('role-privs'),
+		special => { type => 'boolean', optional => 1, default => 0 },
 	    },
 	},
 	links => [ { rel => 'child', href => "{roleid}" } ],
@@ -64,8 +77,8 @@ __PACKAGE__->register_method ({
     parameters => {
 	additionalProperties => 0,
 	properties => {
-	    roleid => { type => 'string', format => 'pve-roleid' },
-	    privs => { type => 'string' , format => 'pve-priv-list', optional => 1 },
+	    roleid => get_standard_option('role-id'),
+	    privs =>  get_standard_option('role-privs'),
 	},
     },
     returns => { type => 'null' },
@@ -100,17 +113,13 @@ __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',
-	    },
+	    roleid => get_standard_option('role-id'),
+	    privs =>  get_standard_option('role-privs'),
+	    append => { type => 'boolean', optional => 1, requires => 'privs' },
 	},
     },
     returns => { type => 'null' },
@@ -137,7 +146,6 @@ __PACKAGE__->register_method ({
 	return undef;
 }});
 
-# fixme: return format!
 __PACKAGE__->register_method ({
     name => 'read_role',
     path => '{roleid}',
@@ -149,10 +157,16 @@ __PACKAGE__->register_method ({
     parameters => {
 	additionalProperties => 0,
 	properties => {
-	    roleid => { type => 'string' , format => 'pve-roleid' },
+	    roleid => get_standard_option('role-id'),
+	},
+    },
+    returns => {
+	type => "object",
+	additionalProperties => 0,
+	properties => {
+	    privs =>  get_standard_option('role-privs'),
 	},
     },
-    returns => {},
     code => sub {
 	my ($param) = @_;
 
@@ -165,7 +179,8 @@ __PACKAGE__->register_method ({
 	die "role '$role' does not exist\n" if !$data;
 
 	return $data;
-}});
+    }
+});
 
 __PACKAGE__->register_method ({
     name => 'delete_role',
@@ -179,8 +194,8 @@ __PACKAGE__->register_method ({
     parameters => {
 	additionalProperties => 0,
 	properties => {
-	    roleid => { type => 'string', format => 'pve-roleid' },
-	}
+	    roleid => get_standard_option('role-id'),
+	},
     },
     returns => { type => 'null' },
     code => sub {
@@ -206,6 +221,7 @@ __PACKAGE__->register_method ({
 	    }, "delete role failed");
 
 	return undef;
-}});
+    }
+});
 
 1;
diff --git a/PVE/API2/User.pm b/PVE/API2/User.pm
index 1dc0293..4c859dc 100644
--- a/PVE/API2/User.pm
+++ b/PVE/API2/User.pm
@@ -6,7 +6,7 @@ use PVE::Exception qw(raise raise_perm_exc);
 use PVE::Cluster qw (cfs_read_file cfs_write_file);
 use PVE::Tools qw(split_list);
 use PVE::AccessControl;
-use PVE::JSONSchema qw(get_standard_option);
+use PVE::JSONSchema qw(get_standard_option register_standard_option);
 
 use PVE::SafeSyslog;
 
@@ -14,6 +14,33 @@ use PVE::RESTHandler;
 
 use base qw(PVE::RESTHandler);
 
+register_standard_option('user-enable', {
+    description => "Enable the account (default). You can set this to '0' to disable the account",
+    type => 'boolean',
+    optional => 1,
+    default => 1,
+});
+register_standard_option('user-expire', {
+    description => "Account expiration date (seconds since epoch). '0' means no expiration date.",
+    type => 'integer',
+    minimum => 0,
+    optional => 1,
+});
+register_standard_option('user-firstname', { type => 'string', optional => 1 });
+register_standard_option('user-lastname', { type => 'string', optional => 1 });
+register_standard_option('user-email', { type => 'string', optional => 1, format => 'email-opt' });
+register_standard_option('user-comment', { type => 'string', optional => 1 });
+register_standard_option('user-keys', {
+    description => "Keys for two factor auth (yubico).",
+    type => 'string',
+    optional => 1,
+});
+register_standard_option('group-list', {
+    type => 'string', format => 'pve-groupid-list',
+    optional => 1,
+    completion => \&PVE::AccessControl::complete_group,
+});
+
 my $extract_user_data = sub {
     my ($data, $full) = @_;
 
@@ -54,7 +81,14 @@ __PACKAGE__->register_method ({
 	items => {
 	    type => "object",
 	    properties => {
-		userid => { type => 'string' },
+	        userid => get_standard_option('userid-completed'),
+	        enable => get_standard_option('user-enable'),
+	        expire => get_standard_option('user-expire'),
+	        firstname => get_standard_option('user-firstname'),
+	        lastname => get_standard_option('user-lastname'),
+	        email => get_standard_option('user-email'),
+	        comment => get_standard_option('user-comment'),
+	        keys => get_standard_option('user-keys'),
 	    },
 	},
 	links => [ { rel => 'child', href => "{userid}" } ],
@@ -109,7 +143,14 @@ __PACKAGE__->register_method ({
     parameters => {
 	additionalProperties => 0,
 	properties => {
-	    userid => get_standard_option('userid'),
+	    userid => get_standard_option('userid-completed'),
+	    enable => get_standard_option('user-enable'),
+	    expire => get_standard_option('user-expire'),
+	    firstname => get_standard_option('user-firstname'),
+	    lastname => get_standard_option('user-lastname'),
+	    email => get_standard_option('user-email'),
+	    comment => get_standard_option('user-comment'),
+	    keys => get_standard_option('user-keys'),
 	    password => {
 		description => "Initial password.",
 		type => 'string',
@@ -117,32 +158,7 @@ __PACKAGE__->register_method ({
 		minLength => 5,
 		maxLength => 64
 	    },
-	    groups => {
-		type => 'string', format => 'pve-groupid-list',
-		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,
-	    },
+	    groups => get_standard_option('group-list'),
 	},
     },
     returns => { type => 'null' },
@@ -199,21 +215,22 @@ __PACKAGE__->register_method ({
     parameters => {
 	additionalProperties => 0,
 	properties => {
-	    userid => get_standard_option('userid'),
+	    userid => get_standard_option('userid-completed'),
 	},
     },
     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 },
+	    enable => get_standard_option('user-enable'),
+	    expire => get_standard_option('user-expire'),
+	    firstname => get_standard_option('user-firstname'),
+	    lastname => get_standard_option('user-lastname'),
+	    email => get_standard_option('user-email'),
+	    comment => get_standard_option('user-comment'),
+	    keys => get_standard_option('user-keys'),
 	    groups => { type => 'array' },
-	}
+	},
+	type => "object"
     },
     code => sub {
 	my ($param) = @_;
@@ -240,39 +257,20 @@ __PACKAGE__->register_method ({
     parameters => {
 	additionalProperties => 0,
 	properties => {
-	    userid => get_standard_option('userid', {
-		completion => \&PVE::AccessControl::complete_username,
-	    }),
-	    groups => {
-		type => 'string', format => 'pve-groupid-list',
-		optional => 1,
-		completion => \&PVE::AccessControl::complete_group,
-	    },
+	    userid => get_standard_option('userid-completed'),
+	    enable => get_standard_option('user-enable'),
+	    expire => get_standard_option('user-expire'),
+	    firstname => get_standard_option('user-firstname'),
+	    lastname => get_standard_option('user-lastname'),
+	    email => get_standard_option('user-email'),
+	    comment => get_standard_option('user-comment'),
+	    keys => get_standard_option('user-keys'),
+	    groups => get_standard_option('group-list'),
 	    append => {
 		type => 'boolean',
 		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' },
@@ -333,9 +331,7 @@ __PACKAGE__->register_method ({
     parameters => {
 	additionalProperties => 0,
 	properties => {
-	    userid => get_standard_option('userid', {
-		completion => \&PVE::AccessControl::complete_username,
-	    }),
+	    userid => get_standard_option('userid-completed'),
 	}
     },
     returns => { type => 'null' },
-- 
2.11.0





More information about the pve-devel mailing list