Better fix and improve unit test (#2337)

This commit is contained in:
Christophe Maudoux 2020-10-03 15:05:13 +02:00
parent 222a6472f4
commit b573dbb789
8 changed files with 277 additions and 95 deletions

View File

@ -484,14 +484,20 @@ sub _displayRegister {
);
# Retrieve user all second factors
my $_2fDevices =
$req->userData->{_2fDevices}
? eval {
from_json( $req->userData->{_2fDevices}, { allow_nonref => 1 } ); }
: undef;
unless ($_2fDevices) {
$self->logger->debug("No 2F Device found");
$_2fDevices = [];
my $_2fDevices = [];
if ( $self->allowedUpdateSfa($req) ) {
$_2fDevices = $req->userData->{_2fDevices}
? eval {
from_json( $req->userData->{_2fDevices}, { allow_nonref => 1 } );
}
: undef;
unless ($_2fDevices) {
$self->logger->debug("No 2F Device found");
$_2fDevices = [];
}
}
else {
$self->userLogger->warn("Do not diplay 2F Devices!");
}
# Parse second factors to display delete button if allowed

View File

@ -36,6 +36,10 @@ sub run {
'No ' . $self->conf->{whatToTrace} . ' found in user data', 500 )
unless $user;
# Check if TOTP can be updated
return $self->p->sendError( $req, 'notAuthorized', 400 )
unless $self->allowedUpdateSfa($req);
# Verification that user has a valid TOTP app
if ( $action eq 'verify' ) {
@ -71,7 +75,7 @@ sub run {
}
$TOTPName =
substr( $TOTPName, 0, $self->conf->{max2FDevicesNameLength} );
$self->logger->debug("TOTP name : $TOTPName");
$self->logger->debug("TOTP name: $TOTPName");
unless ($code) {
$self->userLogger->info('TOTP registration: empty validation form');
@ -101,7 +105,7 @@ sub run {
my $secret = '';
# Reading existing 2FDevices
$self->logger->debug("Looking for 2F Devices ...");
$self->logger->debug("Looking for 2F Devices...");
my $_2fDevices;
if ( $req->userData->{_2fDevices} ) {
$_2fDevices = eval {
@ -128,7 +132,7 @@ sub run {
}
# Loading TOTP secret
$self->logger->debug("Reading TOTP secret if exists ...");
$self->logger->debug("Reading TOTP secret if exists...");
$secret = $_->{_secret} foreach (@totp2f);
return $self->p->sendError( $req, 'totpExistingKey', 200 )
if ( $token->{_totp2fSecret} eq $secret );
@ -141,17 +145,13 @@ sub run {
# Check if user can register one more device
my $maxSize = $self->conf->{max2FDevices};
$self->logger->debug("Nbr 2FDevices = $size / $maxSize");
$self->logger->debug("Registered 2F Device(s): $size / $maxSize");
if ( $size >= $maxSize ) {
$self->userLogger->warn("Max number of 2F devices is reached");
return $self->p->sendError( $req, 'maxNumberof2FDevicesReached',
400 );
}
# Check if TOTP can be stored
return $self->p->sendError( $req, 'notAuthorized', 400 )
unless $self->allowedUpdateSfa($req);
# Store TOTP secret
push @keep,
{
@ -161,7 +161,7 @@ sub run {
epoch => $epoch
};
$self->logger->debug(
"Append 2F Device : { type => 'TOTP', name => $TOTPName }");
"Append 2F Device: { type => 'TOTP', name => $TOTPName }");
$self->p->updatePersistentSession( $req,
{ _2fDevices => to_json( \@keep ) } );
$self->userLogger->notice(
@ -179,7 +179,7 @@ sub run {
my $secret = '';
# Read existing 2FDevices
$self->logger->debug("Looking for 2F Devices ...");
$self->logger->debug("Looking for 2F Devices...");
my $_2fDevices;
if ( $req->userData->{_2fDevices} ) {
$_2fDevices = eval {
@ -207,7 +207,7 @@ sub run {
}
# Loading TOTP secret
$self->logger->debug("Reading TOTP secret if exists ...");
$self->logger->debug("Reading TOTP secret if exists...");
$secret = $_->{_secret} foreach (@totp2f);
if ( ( $req->param('newkey') and $self->conf->{totp2fUserCanChangeKey} )
@ -263,15 +263,14 @@ sub run {
# Check if unregistration is allowed
return $self->p->sendError( $req, 'notAuthorized', 400 )
unless ( $self->conf->{totp2fUserCanRemoveKey}
&& $self->allowedUpdateSfa($req) );
unless $self->conf->{totp2fUserCanRemoveKey};
my $epoch = $req->param('epoch')
or return $self->p->sendError( $req, '"epoch" parameter is missing',
400 );
# Read existing 2FDevices
$self->logger->debug("Loading 2F Devices ...");
$self->logger->debug("Loading 2F Devices...");
my ( $_2fDevices, $TOTPName );
if ( $req->userData->{_2fDevices} ) {
$_2fDevices = eval {
@ -294,7 +293,7 @@ sub run {
else { $_ }
} @$_2fDevices;
$self->logger->debug(
"Delete 2F Device : { type => 'TOTP', epoch => $epoch, name => $TOTPName }"
"Delete 2F Device: { type => 'TOTP', epoch => $epoch, name => $TOTPName }"
);
$self->p->updatePersistentSession( $req,
{ _2fDevices => to_json($_2fDevices) } );

View File

@ -33,10 +33,14 @@ sub run {
'No ' . $self->conf->{whatToTrace} . ' found in user data', 500 )
unless $user;
# Check if U2F key can be updated
return $self->p->sendError( $req, 'notAuthorized', 400 )
unless $self->allowedUpdateSfa($req);
if ( $action eq 'register' ) {
# Read existing 2FDevices
$self->logger->debug("Looking for 2F Devices ...");
$self->logger->debug("Looking for 2F Devices...");
my $_2fDevices;
if ( $req->userData->{_2fDevices} ) {
$_2fDevices = eval {
@ -56,7 +60,7 @@ sub run {
# Check if user can register one more 2F device
my $size = @$_2fDevices;
my $maxSize = $self->conf->{max2FDevices};
$self->logger->debug("Registered 2F Device(s) : $size / $maxSize");
$self->logger->debug("Registered 2F Device(s): $size / $maxSize");
if ( $size >= $maxSize ) {
$self->userLogger->warn("Max number of 2F devices is reached");
return $self->p->sendError( $req, 'maxNumberof2FDevicesReached',
@ -96,7 +100,7 @@ sub run {
if ( $keyHandle and $userKey ) {
# Read existing 2FDevices
$self->logger->debug("Looking for 2F Devices ...");
$self->logger->debug("Looking for 2F Devices...");
my $_2fDevices;
if ( $req->userData->{_2fDevices} ) {
$_2fDevices = eval {
@ -117,10 +121,6 @@ sub run {
$_2fDevices = [];
}
# Check if U2F key can be stored
return $self->p->sendError( $req, 'notAuthorized', 400 )
unless $self->allowedUpdateSfa($req);
my $keyName = $req->param('keyName');
my $epoch = time();
@ -132,7 +132,7 @@ sub run {
}
$keyName =
substr( $keyName, 0, $self->conf->{max2FDevicesNameLength} );
$self->logger->debug("Key name : $keyName");
$self->logger->debug("Key name: $keyName");
push @{$_2fDevices},
{
@ -143,7 +143,7 @@ sub run {
epoch => $epoch
};
$self->logger->debug(
"Append 2F Device : { type => 'U2F', name => $keyName }");
"Append 2F Device: { type => 'U2F', name => $keyName }");
$self->p->updatePersistentSession( $req,
{ _2fDevices => to_json($_2fDevices) } );
$self->userLogger->notice(
@ -250,15 +250,14 @@ sub run {
# Check if unregistration is allowed
return $self->p->sendError( $req, 'notAuthorized', 200 )
unless ( $self->conf->{u2fUserCanRemoveKey}
&& $self->allowedUpdateSfa($req) );
unless $self->conf->{u2fUserCanRemoveKey};
my $epoch = $req->param('epoch')
or return $self->p->sendError( $req, '"epoch" parameter is missing',
400 );
# Read existing 2FDevices
$self->logger->debug("Looking for 2F Devices ...");
$self->logger->debug("Looking for 2F Devices...");
my ( $_2fDevices, $keyName );
if ( $req->userData->{_2fDevices} ) {
$_2fDevices = eval {
@ -302,10 +301,10 @@ sub run {
sub loadUser {
my ( $self, $req ) = @_;
$self->logger->debug("Loading user U2F Devices ...");
$self->logger->debug("Loading user U2F Devices...");
# Read existing 2FDevices
$self->logger->debug("Looking for 2F Devices ...");
$self->logger->debug("Looking for 2F Devices...");
my ( $kh, $uk, $_2fDevices );
my @u2fs = ();
@ -325,7 +324,7 @@ sub loadUser {
# Reading existing U2F keys
foreach (@$_2fDevices) {
$self->logger->debug("Looking for registered U2F key(s) ...");
$self->logger->debug("Looking for registered U2F key(s)...");
if ( $_->{type} eq 'U2F' ) {
unless ( $_->{_userKey} and $_->{_keyHandle} ) {
$self->logger->error(

View File

@ -34,6 +34,16 @@ sub run {
'No ' . $self->conf->{whatToTrace} . ' found in user data', 500 )
unless $user;
# Check if UBK key can be updated
return $self->p->sendHtml(
$req, 'error',
params => {
MAIN_LOGO => $self->conf->{portalMainLogo},
RAW_ERROR => 'notAuthorized',
AUTH_ERROR_TYPE => 'warning',
}
) unless $self->allowedUpdateSfa($req);
if ( $action eq 'register' ) {
my $otp = $req->param('otp');
my $UBKName = $req->param('UBKName');
@ -46,7 +56,7 @@ sub run {
return $self->p->sendError( $req, 'badName', 200 );
}
$UBKName = substr( $UBKName, 0, $self->conf->{max2FDevicesNameLength} );
$self->logger->debug("Yubikey name : $UBKName");
$self->logger->debug("Yubikey name: $UBKName");
if ( $otp
and length($otp) > $self->conf->{yubikey2fPublicIDSize} )
@ -54,7 +64,7 @@ sub run {
my $key = substr( $otp, 0, $self->conf->{yubikey2fPublicIDSize} );
# Read existing 2FDevices
$self->logger->debug("Looking for 2F Devices ...");
$self->logger->debug("Looking for 2F Devices...");
my $_2fDevices;
if ( $req->userData->{_2fDevices} ) {
$_2fDevices = eval {
@ -73,20 +83,10 @@ sub run {
$_2fDevices = [];
}
# Check if UBK key can be stored
return $self->p->sendHtml(
$req, 'error',
params => {
MAIN_LOGO => $self->conf->{portalMainLogo},
RAW_ERROR => 'notAuthorized',
AUTH_ERROR_TYPE => 'warning',
}
) unless $self->allowedUpdateSfa($req);
# Search if the Yubikey is already registered
my $SameUBKFound = 0;
foreach (@$_2fDevices) {
$self->logger->debug("Reading Yubikeys ...");
$self->logger->debug("Reading Yubikeys...");
if ( $_->{_yubikey} eq $key ) {
$SameUBKFound = 1;
last;
@ -108,7 +108,7 @@ sub run {
# Check if user can register one more device
my $size = @$_2fDevices;
my $maxSize = $self->conf->{max2FDevices};
$self->logger->debug("Nbr 2FDevices = $size / $maxSize");
$self->logger->debug("Registered 2F Device(s): $size / $maxSize");
if ( $size >= $maxSize ) {
$self->userLogger->warn("Max number of 2F devices is reached");
return $self->p->sendHtml(
@ -129,7 +129,7 @@ sub run {
epoch => $epoch
};
$self->logger->debug(
"Append 2F Device : { type => 'UBK', name => $UBKName }");
"Append 2F Device: { type => 'UBK', name => $UBKName }");
$self->p->updatePersistentSession( $req,
{ _2fDevices => to_json($_2fDevices) } );
$self->userLogger->notice(
@ -161,15 +161,14 @@ sub run {
# Check if unregistration is allowed
return $self->p->sendError( $req, 'notAuthorized', 400 )
unless ( $self->conf->{yubikey2fUserCanRemoveKey}
&& $self->allowedUpdateSfa($req) );
unless $self->conf->{yubikey2fUserCanRemoveKey};
my $epoch = $req->param('epoch')
or return $self->p->sendError( $req, '"epoch" parameter is missing',
400 );
# Read existing 2FDevices
$self->logger->debug("Looking for 2F Devices ...");
$self->logger->debug("Looking for 2F Devices...");
my ( $_2fDevices, $UBKName );
if ( $req->userData->{_2fDevices} ) {
$_2fDevices = eval {
@ -192,7 +191,7 @@ sub run {
else { $_ }
} @$_2fDevices;
$self->logger->debug(
"Delete 2F Device : { type => 'UBK', epoch => $epoch, name => $UBKName }"
"Delete 2F Device: { type => 'UBK', epoch => $epoch, name => $UBKName }"
);
$self->p->updatePersistentSession( $req,
{ _2fDevices => to_json($_2fDevices) } );

View File

@ -106,6 +106,7 @@ sub createNotification {
sub allowedUpdateSfa {
my ( $self, $req ) = @_;
my $user = $req->userData->{ $self->conf->{whatToTrace} };
my $res = 1;
if ( $self->conf->{impersonationRule} ) {
$self->logger->debug('Impersonation plugin is enabled!');
@ -114,12 +115,12 @@ sub allowedUpdateSfa {
$req->userData->{_user} )
{
$self->userLogger->warn(
'Impersonation in progress! User is not allowed to update SFA.'
"Impersonation in progress! $user is not allowed to update SFA."
);
undef $res;
}
else {
$self->userLogger->info('User is allowed to update SFA');
$self->userLogger->info("$user is allowed to update SFA");
}
}
return $res;

View File

@ -5,7 +5,7 @@ use IO::String;
BEGIN {
require 't/test-lib.pm';
}
my $maintests = 27;
my $maintests = 57;
SKIP: {
require Lemonldap::NG::Common::TOTP;
@ -36,45 +36,29 @@ SKIP: {
}
);
## Try to impersonate
ok( $res = $client->_get( '/', accept => 'text/html' ), 'Get Menu', );
my ( $host, $url, $query ) =
expectForm( $res, '#', undef, 'user', 'password', 'spoofId' );
$query =~ s/user=/user=dwho/;
$query =~ s/password=/password=dwho/;
$query =~ s/spoofId=/spoofId=rtyler/;
# Try to authenticate
# -------------------
ok(
$res = $client->_post(
'/',
IO::String->new($query),
length => length($query),
accept => 'text/html',
'/', IO::String->new('user=rtyler&password=rtyler'),
length => 27
),
'Auth query'
);
my $id = expectCookie($res);
expectRedirection( $res, 'http://auth.example.com/' );
# Get Menu
# ------------------------
ok(
$res = $client->_get(
'/',
cookie => "lemonldap=$id",
accept => 'text/html'
),
'Get Menu',
'Get Menu'
);
expectOK($res);
ok(
$res->[2]->[0] =~
m%<span trspan="connectedAs">Connected as</span> rtyler%,
'Connected as dwho'
) or print STDERR Dumper( $res->[2]->[0] );
expectAuthenticatedAs( $res, 'rtyler' );
ok( $res->[2]->[0] =~ m%<span trspan="sfaManager">sfaManager</span>%,
'sfaManager link found' )
or print STDERR Dumper( $res->[2]->[0] );
## Try to register a TOTP
# TOTP form
@ -105,8 +89,10 @@ SKIP: {
ok( not($@), 'Content is JSON' )
or explain( $res->[2]->[0], 'JSON content' );
my ( $key, $token );
ok( $key = $res->{secret}, 'Found secret' );
ok( $token = $res->{token}, 'Found token' );
ok( $key = $res->{secret}, 'Found secret' ) or print STDERR Dumper($res);
ok( $token = $res->{token}, 'Found token' ) or print STDERR Dumper($res);
ok( $res->{user} eq 'rtyler', 'Found user' )
or print STDERR Dumper($res);
$key = Convert::Base32::decode_base32($key);
# Post code
@ -114,7 +100,7 @@ SKIP: {
ok( $code = Lemonldap::NG::Common::TOTP::_code( undef, $key, 0, 30, 6 ),
'Code' );
ok( $code =~ /^\d{6}$/, 'Code contains 6 digits' );
my $s = "code=$code&token=$token";
my $s = "code=$code&token=$token&TOTPName=myTOTP";
ok(
$res = $client->_post(
'/2fregisters/totp/verify',
@ -127,11 +113,9 @@ SKIP: {
eval { $res = JSON::from_json( $res->[2]->[0] ) };
ok( not($@), 'Content is JSON' )
or explain( $res->[2]->[0], 'JSON content' );
ok( $res->{error} eq 'notAuthorized', 'Not authorized to register a TOTP' )
or explain( $res, 'Bad result' );
ok( $res->{result} == 1, 'TOTP is registered' );
## Tru to register an U2F key
# U2F form
## Try to register an U2F key
ok(
$res = $client->_get(
'/2fregisters/u',
@ -200,11 +184,12 @@ JjTJecOOS+88fK8qL1TrYv5rapIdqUI7aQ==
version => "U2F_V2"
}
);
( $host, $url, $query );
my ( $host, $url, $query );
$query = Lemonldap::NG::Common::FormEncode::build_urlencoded(
registration => $registrationData,
challenge => $res->[2]->[0],
);
ok(
$res = $client->_post(
'/2fregisters/u/registration', IO::String->new($query),
@ -214,6 +199,181 @@ JjTJecOOS+88fK8qL1TrYv5rapIdqUI7aQ==
),
'Push registration data'
);
expectOK($res);
eval { $data = JSON::from_json( $res->[2]->[0] ) };
ok( not($@), ' Content is JSON' )
or explain( [ $@, $res->[2] ], 'JSON content' );
ok( $data->{result} == 1, 'U2F key is registered' )
or explain( $data, '"result":1' );
$client->logout($id);
## Try to impersonate
ok( $res = $client->_get( '/', accept => 'text/html' ), 'Get Menu', );
( $host, $url, $query ) =
expectForm( $res, '#', undef, 'user', 'password', 'spoofId' );
$query =~ s/user=/user=rtyler/;
$query =~ s/password=/password=rtyler/;
$query =~ s/spoofId=/spoofId=dwho/;
ok(
$res = $client->_post(
'/',
IO::String->new($query),
length => length($query),
accept => 'text/html',
),
'Auth query'
);
( $host, $url, $query ) = expectForm( $res, undef, '/2fchoice', 'token' );
$query .= '&sf=totp';
ok(
$res = $client->_post(
'/2fchoice',
IO::String->new($query),
length => length($query),
accept => 'text/html',
),
'Post TOTP choice'
);
( $host, $url, $query ) =
expectForm( $res, undef, '/totp2fcheck', 'token' );
ok( $code = Lemonldap::NG::Common::TOTP::_code( undef, $key, 0, 30, 6 ),
'Code' );
$query =~ s/code=/code=$code/;
ok(
$res = $client->_post(
'/totp2fcheck', IO::String->new($query),
length => length($query),
),
'Post code'
);
$id = expectCookie($res);
# Get Menu
# ------------------------
ok(
$res = $client->_get(
'/',
cookie => "lemonldap=$id",
accept => 'text/html'
),
'Get Menu',
);
expectOK($res);
expectAuthenticatedAs( $res, 'dwho' );
# 2fregisters
ok(
$res = $client->_get(
'/2fregisters',
cookie => "lemonldap=$id",
accept => 'text/html',
),
'Form 2fregisters'
);
ok( $res->[2]->[0] =~ /<span id="msg" trspan="choose2f">/,
'Found choose 2F' )
or print STDERR Dumper( $res->[2]->[0] );
ok( $res->[2]->[0] !~ m%<span device=\'(TOTP|U2F)\' epoch=\'\d{10}\'%g,
'No 2F device found' )
or print STDERR Dumper( $res->[2]->[0] );
## Try to register a TOTP
# TOTP form
ok(
$res = $client->_get(
'/2fregisters/totp',
cookie => "lemonldap=$id",
accept => 'text/html',
),
'Form registration'
);
ok( $res->[2]->[0] =~ /totpregistration\.(?:min\.)?js/, 'Found TOTP js' )
or print STDERR Dumper( $res->[2]->[0] );
ok(
$res->[2]->[0] =~ qr%<img src="/static/common/logos/logo_llng_old.png"%,
'Found custom Main Logo'
) or print STDERR Dumper( $res->[2]->[0] );
# JS query
ok(
$res = $client->_post(
'/2fregisters/totp/getkey', IO::String->new(''),
cookie => "lemonldap=$id",
length => 0,
),
'Get new key'
);
eval { $res = JSON::from_json( $res->[2]->[0] ) };
ok( not($@), 'Content is JSON' )
or explain( $res->[2]->[0], 'JSON content' );
ok( $res->{error} eq 'notAuthorized', 'Not authorized to register a TOTP' )
or explain( $res, 'Bad result' );
# Try to unregister TOTP
ok(
$res = $client->_post(
'/2fregisters/totp/delete',
IO::String->new("epoch=1234567890"),
length => 16,
cookie => "lemonldap=$id",
),
'Delete TOTP query'
);
eval { $data = JSON::from_json( $res->[2]->[0] ) };
ok( not($@), ' Content is JSON' )
or explain( [ $@, $res->[2] ], 'JSON content' );
ok(
$data->{error} eq 'notAuthorized',
'Not authorized to unregister a TOTP'
) or explain( $data, 'Bad result' );
# Try to verify TOTP
$s = "code=123456&token=1234567890&TOTPName=myTOTP";
ok(
$res = $client->_post(
'/2fregisters/totp/verify',
IO::String->new($s),
length => length($s),
cookie => "lemonldap=$id",
),
'Post code'
);
eval { $data = JSON::from_json( $res->[2]->[0] ) };
ok( not($@), ' Content is JSON' )
or explain( [ $@, $res->[2] ], 'JSON content' );
ok( $data->{error} eq 'notAuthorized', 'Not authorized to verify a TOTP' )
or explain( $data, 'Bad result' );
## Try to register an U2F key
# U2F form
ok(
$res = $client->_get(
'/2fregisters/u',
cookie => "lemonldap=$id",
accept => 'text/html',
),
'Form registration'
);
ok( $res->[2]->[0] =~ /u2fregistration\.(?:min\.)?js/, 'Found U2F js' );
ok(
$res->[2]->[0] =~ qr%<img src="/static/common/logos/logo_llng_old.png"%,
'Found custom Main Logo'
) or print STDERR Dumper( $res->[2]->[0] );
# Ajax registration request
ok(
$res = $client->_post(
'/2fregisters/u/register', IO::String->new(''),
accept => 'application/json',
cookie => "lemonldap=$id",
length => 0,
),
'Get registration challenge'
);
eval { $data = JSON::from_json( $res->[2]->[0] ) };
ok( not($@), ' Content is JSON' )
or explain( [ $@, $res->[2] ], 'JSON content' );
@ -222,6 +382,24 @@ JjTJecOOS+88fK8qL1TrYv5rapIdqUI7aQ==
'Not authorized to register an U2F key'
) or explain( $data, 'Bad result' );
# Try to unregister U2F key
ok(
$res = $client->_post(
'/2fregisters/u/delete',
IO::String->new("epoch=1234567890"),
length => 16,
cookie => "lemonldap=$id",
),
'Delete U2F key query'
);
eval { $data = JSON::from_json( $res->[2]->[0] ) };
ok( not($@), ' Content is JSON' )
or explain( [ $@, $res->[2] ], 'JSON content' );
ok(
$data->{error} eq 'notAuthorized',
'Not authorized to unregister an U2F key'
) or explain( $data, 'Bad result' );
$client->logout($id);
}

View File

@ -47,7 +47,7 @@ SKIP: {
'Get Menu'
);
ok( $res->[2]->[0] !~ m%<span trspan="sfaManager">sfaManager</span>%,
'sfaManager link found' )
'sfaManager link not found' )
or print STDERR Dumper( $res->[2]->[0] );
# TOTP form
@ -113,7 +113,7 @@ SKIP: {
eval { $res = JSON::from_json( $res->[2]->[0] ) };
ok( not($@), 'Content is JSON' )
or explain( $res->[2]->[0], 'JSON content' );
ok( $res->{result} == 1, 'Key is registered' );
ok( $res->{result} == 1, 'TOTP is registered' );
# Try to sign-in
$client->logout($id);

View File

@ -46,7 +46,7 @@ SKIP: {
'Get Menu'
);
ok( $res->[2]->[0] !~ m%<span trspan="sfaManager">sfaManager</span>%,
'sfaManager link found' )
'sfaManager link not found' )
or print STDERR Dumper( $res->[2]->[0] );
# U2F form