[pve-devel] [PATCH access-control 05/10] move TFA api path into its own module

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

and remove old modification api

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/PVE/API2/AccessControl.pm | 211 +-----------------------------
 src/PVE/API2/Makefile         |   1 +
 src/PVE/API2/TFA.pm           | 239 ++++++++++++++++++++++++++++++++++
 src/PVE/AccessControl.pm      |  69 ----------
 src/PVE/CLI/pveum.pm          |   3 +-
 5 files changed, 248 insertions(+), 275 deletions(-)
 create mode 100644 src/PVE/API2/TFA.pm

diff --git a/src/PVE/API2/AccessControl.pm b/src/PVE/API2/AccessControl.pm
index 8fa3606..5d78c6f 100644
--- a/src/PVE/API2/AccessControl.pm
+++ b/src/PVE/API2/AccessControl.pm
@@ -20,6 +20,7 @@ use PVE::API2::Group;
 use PVE::API2::Role;
 use PVE::API2::ACL;
 use PVE::API2::OpenId;
+use PVE::API2::TFA;
 use PVE::Auth::Plugin;
 use PVE::OTP;
 
@@ -61,6 +62,11 @@ __PACKAGE__->register_method ({
     path => 'openid',
 });
 
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::TFA",
+    path => 'tfa',
+});
+
 __PACKAGE__->register_method ({
     name => 'index',
     path => '',
@@ -464,211 +470,6 @@ sub verify_user_tfa_config {
     PVE::OTP::oath_verify_otp($value, $secret, $step, $digits);
 }
 
-__PACKAGE__->register_method ({
-    name => 'change_tfa',
-    path => 'tfa',
-    method => 'PUT',
-    permissions => {
-	description => 'A user can change their own u2f or totp token.',
-	check => [ 'or',
-		   ['userid-param', 'self'],
-		   [ 'and',
-		     [ 'userid-param', 'Realm.AllocateUser'],
-		     [ 'userid-group', ['User.Modify']]
-		   ]
-	    ],
-    },
-    protected => 1, # else we can't access shadow files
-    allowtoken => 0, # we don't want tokens to change the regular user's TFA settings
-    description => "Change user u2f authentication.",
-    parameters => {
-	additionalProperties => 0,
-	properties => {
-	    userid => get_standard_option('userid', {
-		completion => \&PVE::AccessControl::complete_username,
-	    }),
-	    password => {
-		optional => 1, # Only required if not root@pam
-		description => "The current password.",
-		type => 'string',
-		minLength => 5,
-		maxLength => 64,
-	    },
-	    action => {
-		description => 'The action to perform',
-		type => 'string',
-		enum => [qw(delete new confirm)],
-	    },
-	    response => {
-		optional => 1,
-		description =>
-		    'Either the the response to the current u2f registration challenge,'
-		    .' or, when adding TOTP, the currently valid TOTP value.',
-		type => 'string',
-	    },
-	    key => {
-		optional => 1,
-		description => 'When adding TOTP, the shared secret value.',
-		type => 'string',
-		format => 'pve-tfa-secret',
-	    },
-	    config => {
-		optional => 1,
-		description => 'A TFA configuration. This must currently be of type TOTP of not set at all.',
-		type => 'string',
-		format => 'pve-tfa-config',
-		maxLength => 128,
-	    },
-	}
-    },
-    returns => { type => 'object' },
-    code => sub {
-	my ($param) = @_;
-
-	die "TODO!\n";
-
-	my $rpcenv = PVE::RPCEnvironment::get();
-	my $authuser = $rpcenv->get_user();
-
-	my $action = delete $param->{action};
-	my $response = delete $param->{response};
-	my $password = delete($param->{password}) // '';
-	my $key = delete($param->{key});
-	my $config = delete($param->{config});
-
-	my ($userid, $ruid, $realm) = PVE::AccessControl::verify_username($param->{userid});
-	$rpcenv->check_user_exist($userid);
-
-	# Only root may modify root
-	raise_perm_exc() if $userid eq 'root@pam' && $authuser ne 'root@pam';
-
-	# Regular users need to confirm their password to change u2f settings.
-	if ($authuser ne 'root@pam') {
-	    raise_param_exc({ 'password' => 'password is required to modify u2f data' })
-		if !defined($password);
-	    my $domain_cfg = cfs_read_file('domains.cfg');
-	    my $cfg = $domain_cfg->{ids}->{$realm};
-	    die "auth domain '$realm' does not exist\n" if !$cfg;
-	    my $plugin = PVE::Auth::Plugin->lookup($cfg->{type});
-	    $plugin->authenticate_user($cfg, $realm, $ruid, $password);
-	}
-
-	if ($action eq 'delete') {
-	    PVE::AccessControl::user_set_tfa($userid, $realm, undef, undef);
-	    PVE::Cluster::log_msg('info', $authuser, "deleted u2f data for user '$userid'");
-	} elsif ($action eq 'new') {
-	    if (defined($config)) {
-		$config = PVE::Auth::Plugin::parse_tfa_config($config);
-		my $type = delete($config->{type});
-		my $tfa_cfg = {
-		    keys => $key,
-		    config => $config,
-		};
-		verify_user_tfa_config($type, $tfa_cfg, $response);
-		PVE::AccessControl::user_set_tfa($userid, $realm, $type, $tfa_cfg);
-	    } else {
-		# The default is U2F:
-		my $u2f = get_u2f_instance($rpcenv);
-		my $challenge = $u2f->registration_challenge()
-		    or raise("failed to get u2f challenge");
-		$challenge = decode_json($challenge);
-		PVE::AccessControl::user_set_tfa($userid, $realm, 'u2f', $challenge);
-		return $challenge;
-	    }
-	} elsif ($action eq 'confirm') {
-	    raise_param_exc({ 'response' => "confirm action requires the 'response' parameter to be set" })
-		if !defined($response);
-
-	    my ($type, $u2fdata) = PVE::AccessControl::user_get_tfa($userid, $realm, 'FIXME');
-	    raise("no u2f data available")
-		if (!defined($type) || $type ne 'u2f');
-
-	    my $challenge = $u2fdata->{challenge}
-		or raise("no active challenge");
-
-	    my $u2f = get_u2f_instance($rpcenv);
-	    $u2f->set_challenge($challenge);
-	    my ($keyHandle, $publicKey) = $u2f->registration_verify($response);
-	    PVE::AccessControl::user_set_tfa($userid, $realm, 'u2f', {
-		keyHandle => $keyHandle,
-		publicKey => $publicKey, # already base64 encoded
-	    });
-	} else {
-	    die "invalid action: $action\n";
-	}
-
-	return {};
-    }});
-
-__PACKAGE__->register_method({
-    name => 'verify_tfa',
-    path => 'tfa',
-    method => 'POST',
-    permissions => { user => 'all' },
-    protected => 1, # else we can't access shadow files
-    allowtoken => 0, # we don't want tokens to access TFA information
-    description => 'Finish a u2f challenge.',
-    parameters => {
-	additionalProperties => 0,
-	properties => {
-	    response => {
-		type => 'string',
-		description => 'The response to the current authentication challenge.',
-	    },
-	}
-    },
-    returns => {
-	type => 'object',
-	properties => {
-	    ticket => { type => 'string' },
-	    # cap
-	}
-    },
-    code => sub {
-	my ($param) = @_;
-
-	my $rpcenv = PVE::RPCEnvironment::get();
-	my $authuser = $rpcenv->get_user();
-	my ($username, undef, $realm) = PVE::AccessControl::verify_username($authuser);
-
-	my ($tfa_type, $tfa_data) = PVE::AccessControl::user_get_tfa($username, $realm, 0);
-	if (!defined($tfa_type)) {
-	    raise('no u2f data available');
-	}
-
-	eval {
-	    if ($tfa_type eq 'u2f') {
-		my $challenge = $rpcenv->get_u2f_challenge()
-		   or raise('no active challenge');
-
-		my $keyHandle = $tfa_data->{keyHandle};
-		my $publicKey = $tfa_data->{publicKey};
-		raise("incomplete u2f setup")
-		    if !defined($keyHandle) || !defined($publicKey);
-
-		my $u2f = get_u2f_instance($rpcenv, $publicKey, $keyHandle);
-		$u2f->set_challenge($challenge);
-
-		my ($counter, $present) = $u2f->auth_verify($param->{response});
-		# Do we want to do anything with these?
-	    } else {
-		# sanity check before handing off to the verification code:
-		my $keys = $tfa_data->{keys} or die "missing tfa keys\n";
-		my $config = $tfa_data->{config} or die "bad tfa entry\n";
-		PVE::AccessControl::verify_one_time_pw($tfa_type, $authuser, $keys, $config, $param->{response});
-	    }
-	};
-	if (my $err = $@) {
-	    my $clientip = $rpcenv->get_client_ip() || '';
-	    syslog('err', "authentication verification failure; rhost=$clientip user=$authuser msg=$err");
-	    die PVE::Exception->new("authentication failure\n", code => 401);
-	}
-
-	return {
-	    ticket => PVE::AccessControl::assemble_ticket($authuser),
-	    cap => $rpcenv->compute_api_permission($authuser),
-	}
-    }});
 
 __PACKAGE__->register_method({
     name => 'permissions',
diff --git a/src/PVE/API2/Makefile b/src/PVE/API2/Makefile
index 4e49037..2817f48 100644
--- a/src/PVE/API2/Makefile
+++ b/src/PVE/API2/Makefile
@@ -6,6 +6,7 @@ API2_SOURCES= 		 	\
 	Role.pm		 	\
 	Group.pm	 	\
 	User.pm			\
+	TFA.pm			\
 	OpenId.pm
 
 .PHONY: install
diff --git a/src/PVE/API2/TFA.pm b/src/PVE/API2/TFA.pm
new file mode 100644
index 0000000..76daef9
--- /dev/null
+++ b/src/PVE/API2/TFA.pm
@@ -0,0 +1,239 @@
+package PVE::API2::TFA;
+
+use strict;
+use warnings;
+
+use PVE::AccessControl;
+use PVE::Cluster qw(cfs_read_file);
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::Exception qw(raise raise_perm_exc raise_param_exc);
+use PVE::RPCEnvironment;
+
+use PVE::API2::AccessControl; # for old login api get_u2f_instance method
+
+use PVE::RESTHandler;
+
+use base qw(PVE::RESTHandler);
+
+### OLD API
+
+__PACKAGE__->register_method({
+    name => 'verify_tfa',
+    path => '',
+    method => 'POST',
+    permissions => { user => 'all' },
+    protected => 1, # else we can't access shadow files
+    allowtoken => 0, # we don't want tokens to access TFA information
+    description => 'Finish a u2f challenge.',
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    response => {
+		type => 'string',
+		description => 'The response to the current authentication challenge.',
+	    },
+	}
+    },
+    returns => {
+	type => 'object',
+	properties => {
+	    ticket => { type => 'string' },
+	    # cap
+	}
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+	my ($username, undef, $realm) = PVE::AccessControl::verify_username($authuser);
+
+	my ($tfa_type, $tfa_data) = PVE::AccessControl::user_get_tfa($username, $realm, 0);
+	if (!defined($tfa_type)) {
+	    raise('no u2f data available');
+	}
+
+	eval {
+	    if ($tfa_type eq 'u2f') {
+		my $challenge = $rpcenv->get_u2f_challenge()
+		   or raise('no active challenge');
+
+		my $keyHandle = $tfa_data->{keyHandle};
+		my $publicKey = $tfa_data->{publicKey};
+		raise("incomplete u2f setup")
+		    if !defined($keyHandle) || !defined($publicKey);
+
+		my $u2f = PVE::API2::AccessControl::get_u2f_instance($rpcenv, $publicKey, $keyHandle);
+		$u2f->set_challenge($challenge);
+
+		my ($counter, $present) = $u2f->auth_verify($param->{response});
+		# Do we want to do anything with these?
+	    } else {
+		# sanity check before handing off to the verification code:
+		my $keys = $tfa_data->{keys} or die "missing tfa keys\n";
+		my $config = $tfa_data->{config} or die "bad tfa entry\n";
+		PVE::AccessControl::verify_one_time_pw($tfa_type, $authuser, $keys, $config, $param->{response});
+	    }
+	};
+	if (my $err = $@) {
+	    my $clientip = $rpcenv->get_client_ip() || '';
+	    syslog('err', "authentication verification failure; rhost=$clientip user=$authuser msg=$err");
+	    die PVE::Exception->new("authentication failure\n", code => 401);
+	}
+
+	return {
+	    ticket => PVE::AccessControl::assemble_ticket($authuser),
+	    cap => $rpcenv->compute_api_permission($authuser),
+	}
+    }});
+
+### END OLD API
+
+my $TFA_TYPE_SCHEMA = {
+    type => 'string',
+    description => 'TFA Entry Type.',
+    enum => [qw(totp u2f webauthn recovery yubico)],
+};
+
+my %TFA_INFO_PROPERTIES = (
+    id => {
+	type => 'string',
+	description => 'The id used to reference this entry.',
+    },
+    description => {
+	type => 'string',
+	description => 'User chosen description for this entry.',
+    },
+    created => {
+	type => 'integer',
+	description => 'Creation time of this entry as unix epoch.',
+    },
+    enable => {
+	type => 'boolean',
+	description => 'Whether this TFA entry is currently enabled.',
+	optional => 1,
+	default => 1,
+    },
+);
+
+my $TYPED_TFA_ENTRY_SCHEMA = {
+    type => 'object',
+    description => 'TFA Entry.',
+    properties => {
+	type => $TFA_TYPE_SCHEMA,
+	%TFA_INFO_PROPERTIES,
+    },
+};
+
+my $TFA_ID_SCHEMA = {
+    type => 'string',
+    description => 'A TFA entry id.',
+};
+
+__PACKAGE__->register_method ({
+    name => 'list_user_tfa',
+    path => '{userid}',
+    method => 'GET',
+    permissions => {
+	check => [ 'or',
+	    ['userid-param', 'self'],
+	    ['userid-group', ['User.Modify', 'Sys.Audit']],
+	],
+    },
+    protected => 1, # else we can't access shadow files
+    allowtoken => 0, # we don't want tokens to change the regular user's TFA settings
+    description => 'List TFA configurations of users.',
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    userid => get_standard_option('userid', {
+		completion => \&PVE::AccessControl::complete_username,
+	    }),
+	}
+    },
+    returns => {
+	description => "A list of the user's TFA entries.",
+	type => 'array',
+	items => $TYPED_TFA_ENTRY_SCHEMA,
+    },
+    code => sub {
+	my ($param) = @_;
+	my $tfa_cfg = cfs_read_file('priv/tfa.cfg');
+	return $tfa_cfg->api_list_user_tfa($param->{userid});
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'get_tfa_entry',
+    path => '{userid}/{id}',
+    method => 'GET',
+    permissions => {
+	check => [ 'or',
+	    ['userid-param', 'self'],
+	    ['userid-group', ['User.Modify', 'Sys.Audit']],
+	],
+    },
+    protected => 1, # else we can't access shadow files
+    allowtoken => 0, # we don't want tokens to change the regular user's TFA settings
+    description => 'A requested TFA entry if present.',
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    userid => get_standard_option('userid', {
+		completion => \&PVE::AccessControl::complete_username,
+	    }),
+	    id => $TFA_ID_SCHEMA,
+	}
+    },
+    returns => $TYPED_TFA_ENTRY_SCHEMA,
+    code => sub {
+	my ($param) = @_;
+	my $tfa_cfg = cfs_read_file('priv/tfa.cfg');
+	return $tfa_cfg->api_get_tfa_entry($param->{userid}, $param->{id});
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'list_tfa',
+    path => '',
+    method => 'GET',
+    permissions => {
+	description => "Returns all or just the logged-in user, depending on privileges.",
+	user => 'all',
+    },
+    protected => 1, # else we can't access shadow files
+    allowtoken => 0, # we don't want tokens to change the regular user's TFA settings
+    description => 'List TFA configurations of users.',
+    parameters => {
+	additionalProperties => 0,
+	properties => {}
+    },
+    returns => {
+	description => "The list tuples of user and TFA entries.",
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => {
+		userid => {
+		    type => 'string',
+		    description => 'User this entry belongs to.',
+		},
+		entries => {
+		    type => 'array',
+		    items => $TYPED_TFA_ENTRY_SCHEMA,
+		},
+	    },
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+
+
+	my $top_level_allowed = ($authuser eq 'root@pam');
+
+	my $tfa_cfg = cfs_read_file('priv/tfa.cfg');
+	return $tfa_cfg->api_list_tfa($authuser, $top_level_allowed);
+    }});
+
+1;
diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 29d22ac..c3d3d16 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1720,75 +1720,6 @@ my $USER_CONTROLLED_TFA_TYPES = {
     oath => 1,
 };
 
-# Delete an entry by setting $data=undef in which case $type is ignored.
-# Otherwise both must be valid.
-sub user_set_tfa {
-    my ($userid, $realm, $type, $data, $cached_usercfg, $cached_domaincfg) = @_;
-
-    if (defined($data) && !defined($type)) {
-	# This is an internal usage error and should not happen
-	die "cannot set tfa data without a type\n";
-    }
-
-    my $user_cfg = $cached_usercfg || cfs_read_file('user.cfg');
-    my $user = $user_cfg->{users}->{$userid};
-
-    my $domain_cfg = $cached_domaincfg || cfs_read_file('domains.cfg');
-    my $realm_cfg = $domain_cfg->{ids}->{$realm};
-    die "auth domain '$realm' does not exist\n" if !$realm_cfg;
-
-    my $realm_tfa = $realm_cfg->{tfa};
-    if (defined($realm_tfa)) {
-	$realm_tfa = PVE::Auth::Plugin::parse_tfa_config($realm_tfa);
-	# If the realm has a TFA setting, we're only allowed to use that.
-	if (defined($data)) {
-	    die "user '$userid' not found\n" if !defined($user);
-	    my $required_type = $realm_tfa->{type};
-	    if ($required_type ne $type) {
-		die "realm '$realm' only allows TFA of type '$required_type\n";
-	    }
-
-	    if (defined($data->{config})) {
-		# XXX: Is it enough if the type matches? Or should the configuration also match?
-	    }
-
-	    # realm-configured tfa always uses a simple key list, so use the user.cfg
-	    $user->{keys} = $data->{keys};
-	} else {
-	    # TFA is enforce by realm, only allow deletion if the whole user gets delete
-	    die "realm '$realm' does not allow removing the 2nd factor\n" if defined($user);
-	}
-    } else {
-	die "user '$userid' not found\n" if !defined($user) && defined($data);
-	# Without a realm-enforced TFA setting the user can add a u2f or totp entry by themselves.
-	# The 'yubico' type requires yubico server settings, which have to be configured on the
-	# realm, so this is not supported here:
-	die "domain '$realm' does not support TFA type '$type'\n"
-	    if defined($data) && !$USER_CONTROLLED_TFA_TYPES->{$type};
-    }
-
-    # Custom TFA entries are stored in priv/tfa.cfg as they can be more complet: u2f uses a
-    # public key and a key handle, TOTP requires the usual totp settings...
-
-    my $tfa_cfg = cfs_read_file('priv/tfa.cfg');
-    my $tfa = ($tfa_cfg->{users}->{$userid} //= {});
-
-    if (defined($data)) {
-	$tfa->{type} = $type;
-	$tfa->{data} = $data;
-	cfs_write_file('priv/tfa.cfg', $tfa_cfg);
-
-	$user->{keys} = "x!$type";
-    } else {
-	delete $tfa_cfg->{users}->{$userid};
-	cfs_write_file('priv/tfa.cfg', $tfa_cfg);
-
-	delete $user->{keys} if defined($user);
-    }
-
-    cfs_write_file('user.cfg', $user_cfg) if defined($user);
-}
-
 sub user_get_tfa : prototype($$$) {
     my ($username, $realm, $new_format) = @_;
 
diff --git a/src/PVE/CLI/pveum.pm b/src/PVE/CLI/pveum.pm
index 5929707..95b5705 100755
--- a/src/PVE/CLI/pveum.pm
+++ b/src/PVE/CLI/pveum.pm
@@ -11,6 +11,7 @@ use PVE::API2::Role;
 use PVE::API2::ACL;
 use PVE::API2::AccessControl;
 use PVE::API2::Domains;
+use PVE::API2::TFA;
 use PVE::CLIFormatter;
 use PVE::CLIHandler;
 use PVE::JSONSchema qw(get_standard_option);
@@ -118,7 +119,7 @@ our $cmddef = {
 	list   => [ 'PVE::API2::User', 'index', [], {}, $print_api_result, $PVE::RESTHandler::standard_output_options],
 	permissions => [ 'PVE::API2::AccessControl', 'permissions', ['userid'], {}, $print_perm_result, $PVE::RESTHandler::standard_output_options],
 	tfa => {
-	    delete => [ 'PVE::API2::AccessControl', 'change_tfa', ['userid'], { action => 'delete', key => undef, config => undef, response => undef, }, ],
+	    delete => [ 'PVE::API2::TFA', 'change_tfa', ['userid'], { action => 'delete', key => undef, config => undef, response => undef, }, ],
 	},
 	token => {
 	    add    => [ 'PVE::API2::User', 'generate_token', ['userid', 'tokenid'], {}, $print_api_result, $PVE::RESTHandler::standard_output_options ],
-- 
2.30.2