From 3a5ae7a0e6a0f3483a43752994bcb03336ff498e Mon Sep 17 00:00:00 2001 From: Stoiko Ivanov Date: Thu, 21 Jun 2018 14:31:45 +0200 Subject: [PATCH] refactor API using get/register_standard_option 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 --- 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 \@.", @@ -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.39.2