From cc5435015dfa3f169b9b6312516b0e879fb87a88 Mon Sep 17 00:00:00 2001 From: Maxime Besson Date: Sat, 5 Mar 2022 00:17:28 +0100 Subject: [PATCH 1/5] Extract 2F common functions into lib --- lemonldap-ng-portal/MANIFEST | 1 + .../NG/Portal/2F/Register/WebAuthn.pm | 2 +- .../lib/Lemonldap/NG/Portal/Lib/2fDevices.pm | 144 ++++++++++++++++++ .../lib/Lemonldap/NG/Portal/Lib/WebAuthn.pm | 114 +------------- 4 files changed, 151 insertions(+), 110 deletions(-) create mode 100644 lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/2fDevices.pm diff --git a/lemonldap-ng-portal/MANIFEST b/lemonldap-ng-portal/MANIFEST index 914a4892e..80c80ee92 100644 --- a/lemonldap-ng-portal/MANIFEST +++ b/lemonldap-ng-portal/MANIFEST @@ -58,6 +58,7 @@ lib/Lemonldap/NG/Portal/Issuer/OpenID.pm lib/Lemonldap/NG/Portal/Issuer/OpenIDConnect.pm lib/Lemonldap/NG/Portal/Issuer/SAML.pm lib/Lemonldap/NG/Portal/Lib/_tokenRule.pm +lib/Lemonldap/NG/Portal/Lib/2fDevices.pm lib/Lemonldap/NG/Portal/Lib/Captcha.pm lib/Lemonldap/NG/Portal/Lib/CAS.pm lib/Lemonldap/NG/Portal/Lib/Choice.pm diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Register/WebAuthn.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Register/WebAuthn.pm index 3ad69a0ed..1d5c7d2a2 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Register/WebAuthn.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Register/WebAuthn.pm @@ -179,7 +179,7 @@ sub _registration { . "\n" ); if ( - $self->find2fByKey( + $self->find2fDevicesByKey( $req, $req->userData, $self->type, "_credentialId", $credential_id ) diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/2fDevices.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/2fDevices.pm new file mode 100644 index 000000000..a9844d8d0 --- /dev/null +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/2fDevices.pm @@ -0,0 +1,144 @@ +package Lemonldap::NG::Portal::Lib::2fDevices; + +use strict; +use Mouse::Role; +use JSON; + +requires qw(p conf logger); + +our $VERSION = '2.0.15'; + +sub update2fDevice { + my ( $self, $req, $info, $type, $key, $value, $update_key, $update_value ) + = @_; + + my $user = $info->{ $self->conf->{whatToTrace} }; + + my $_2fDevices = $self->get2fDevices( $req, $info ); + return 0 unless $_2fDevices; + + my @found = + grep { $_->{type} eq $type and $_->{$key} eq $value } @{$_2fDevices}; + + for my $device (@found) { + $device->{$update_key} = $update_value; + } + + if (@found) { + $self->p->updatePersistentSession( $req, + { _2fDevices => to_json($_2fDevices) }, $user ); + return 1; + } + return 0; +} + +sub add2fDevice { + my ( $self, $req, $info, $device ) = @_; + + my $_2fDevices = $self->get2fDevices( $req, $info ); + + push @{$_2fDevices}, $device; + $self->logger->debug( + "Append 2F Device: { type => $device->{type}, name => $device->{name} }" + ); + $self->p->updatePersistentSession( $req, + { _2fDevices => to_json($_2fDevices) } ); + return 1; +} + +# $devices must be a arrayref of type+epoch hashrefs: +# [ { type => xxx, epoch => xxx }, { type => xxx, epoch => xxx } ] +sub del2fDevices { + my ( $self, $req, $info, $devices ) = @_; + + return 0 unless ( ref($devices) eq "ARRAY" ); + + my $_2fDevices = $self->get2fDevices( $req, $info ); + return 0 unless $_2fDevices; + + my @updated_2fDevices = @{$_2fDevices}; + my $need_update = 0; + + for my $device_spec (@$devices) { + next unless ( ref($device_spec) eq "HASH" ); + my $type = $device_spec->{type}; + my $epoch = $device_spec->{epoch}; + next unless ( $type and $epoch ); + + my $size_before = @updated_2fDevices; + @updated_2fDevices = + grep { not( $_->{type} eq $type and $_->{epoch} eq $epoch ) } + @updated_2fDevices; + if ( @updated_2fDevices < $size_before ) { + $need_update = 1; + $self->logger->debug( + "Deleted 2F Device: { type => $type, epoch => $epoch }"); + } + } + + if ($need_update) { + $self->p->updatePersistentSession( $req, + { _2fDevices => to_json( [@updated_2fDevices] ) } ); + } + return 1; +} + +sub del2fDevice { + my ( $self, $req, $info, $type, $epoch ) = @_; + + return $self->del2fDevices( $req, $info, + [ { type => $type, epoch => $epoch } ] ); +} + +sub find2fDevicesByKey { + my ( $self, $req, $info, $type, $key, $value ) = @_; + + my $_2fDevices = $self->get2fDevices( $req, $info ); + return unless $_2fDevices; + + my @found = + grep { $_->{type} eq $type and $_->{$key} eq $value } @{$_2fDevices}; + return @found; +} + +## @method get2fDevices($req, $info) +# Validate logout request +# @param req Request object +# @param info HashRef of session data +# @return undef or ArrayRef of second factors + +sub get2fDevices { + my ( $self, $req, $info ) = @_; + + my $_2fDevices; + if ( $info->{_2fDevices} ) { + $_2fDevices = + eval { from_json( $info->{_2fDevices}, { allow_nonref => 1 } ); }; + if ($@) { + $self->logger->error("Corrupted session (_2fDevices): $@"); + return; + } + } + else { + # Return new ArrayRef + return []; + } + if ( ref($_2fDevices) eq "ARRAY" ) { + return $_2fDevices; + } + else { + return; + } +} + +sub find2fDevicesByType { + my ( $self, $req, $info, $type ) = @_; + + my $_2fDevices = $self->get2fDevices( $req, $info ); + return unless $_2fDevices; + + return @{$_2fDevices} unless $type; + my @found = grep { $_->{type} eq $type } @{$_2fDevices}; + return @found; +} +1; diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/WebAuthn.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/WebAuthn.pm index f1741ef45..6d0b4446a 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/WebAuthn.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/WebAuthn.pm @@ -7,11 +7,12 @@ use JSON qw(decode_json from_json to_json); use Digest::SHA qw(sha256); use URI; use Carp; +with 'Lemonldap::NG::Portal::Lib::2fDevices'; our $VERSION = '2.0.12'; -has rp_id => ( is => 'rw', lazy => 1, builder => "_build_rp_id" ); -has origin => ( is => 'rw', lazy => 1, builder => "_build_origin" ); +has rp_id => ( is => 'rw', lazy => 1, builder => "_build_rp_id" ); +has origin => ( is => 'rw', lazy => 1, builder => "_build_origin" ); has type => ( is => 'ro', default => 'WebAuthn' ); has verifier => ( is => 'rw', lazy => 1, builder => "_build_verifier" ); @@ -67,7 +68,7 @@ sub generateChallenge { my ( $self, $req, $data ) = @_; # Find webauthn devices for user - my @webauthn_devices = $self->find2fByType( $req, $data, $self->type ); + my @webauthn_devices = $self->find2fDevicesByType( $req, $data, $self->type ); unless (@webauthn_devices) { return; } @@ -138,7 +139,7 @@ sub validateAssertion { # credential.id If the user was identified before the authentication # ceremony was initiated, e.g., via a username or cookie, verify that the # identified user is the owner of credentialSource. - my @webauthn_devices = $self->find2fByType( $req, $data, $self->type ); + my @webauthn_devices = $self->find2fDevicesByType( $req, $data, $self->type ); my @matching_credentials = grep { $_->{_credentialId} eq $credential_id } @webauthn_devices; if ( @matching_credentials < 1 ) { @@ -234,109 +235,4 @@ sub decode_credential { return $credential; } -sub update2fDevice { - my ( $self, $req, $info, $type, $key, $value, $update_key, $update_value ) - = @_; - - my $user = $info->{ $self->conf->{whatToTrace} }; - - my $_2fDevices = $self->get2fDevices( $req, $info ); - return 0 unless $_2fDevices; - - my @found = - grep { $_->{type} eq $type and $_->{$key} eq $value } @{$_2fDevices}; - - for my $device (@found) { - $device->{$update_key} = $update_value; - } - - if (@found) { - $self->p->updatePersistentSession( $req, - { _2fDevices => to_json($_2fDevices) }, $user ); - return 1; - } - return 0; -} - -sub add2fDevice { - my ( $self, $req, $info, $device ) = @_; - - my $_2fDevices = $self->get2fDevices( $req, $info ); - - push @{$_2fDevices}, $device; - $self->logger->debug( - "Append 2F Device: { type => 'Webauthn', name => $device->{name} }"); - $self->p->updatePersistentSession( $req, - { _2fDevices => to_json($_2fDevices) } ); - return 1; -} - -sub del2fDevice { - my ( $self, $req, $info, $type, $epoch ) = @_; - - my $_2fDevices = $self->get2fDevices( $req, $info ); - return 0 unless $_2fDevices; - - my @updated_2fDevices = - grep { not( $_->{type} eq $type and $_->{epoch} eq $epoch ) } - @{$_2fDevices}; - $self->logger->debug( - "Deleted 2F Device: { type => $type, epoch => $epoch }"); - $self->p->updatePersistentSession( $req, - { _2fDevices => to_json( [@updated_2fDevices] ) } ); - return 1; -} - -sub find2fByKey { - my ( $self, $req, $info, $type, $key, $value ) = @_; - - my $_2fDevices = $self->get2fDevices( $req, $info ); - return unless $_2fDevices; - - my @found = - grep { $_->{type} eq $type and $_->{$key} eq $value } @{$_2fDevices}; - return @found; -} - -## @method get2fDevices($req, $info) -# Validate logout request -# @param req Request object -# @param info HashRef of session data -# @return undef or ArrayRef of second factors - -sub get2fDevices { - my ( $self, $req, $info ) = @_; - - my $_2fDevices; - if ( $info->{_2fDevices} ) { - $_2fDevices = - eval { from_json( $info->{_2fDevices}, { allow_nonref => 1 } ); }; - if ($@) { - $self->logger->error("Corrupted session (_2fDevices): $@"); - return; - } - } - else { - # Return new ArrayRef - return []; - } - if ( ref($_2fDevices) eq "ARRAY" ) { - return $_2fDevices; - } - else { - return; - } -} - -sub find2fByType { - my ( $self, $req, $info, $type ) = @_; - - my $_2fDevices = $self->get2fDevices( $req, $info ); - return unless $_2fDevices; - - return @{$_2fDevices} unless $type; - my @found = grep { $_->{type} eq $type } @{$_2fDevices}; - return @found; -} - 1; From 21745359a2f30294c089f94508f862163df49dd0 Mon Sep 17 00:00:00 2001 From: Maxime Besson Date: Tue, 29 Mar 2022 23:08:02 +0200 Subject: [PATCH 2/5] Fix #2716 --- .../lib/Lemonldap/NG/Portal/2F/Engines/Default.pm | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Engines/Default.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Engines/Default.pm index 034f2aa7c..24be564c6 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Engines/Default.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Engines/Default.pm @@ -509,14 +509,6 @@ sub _displayRegister { } } - return [ 302, [ Location => $self->conf->{portal} . $am[0]->{URL} ], [] ] - if ( - @am == 1 - and not( $req->userData->{_2fDevices} - && $req->userData->{_2fDevices} =~ /\w+/ - or $req->data->{sfRegRequired} ) - ); - # Retrieve user all second factors my $_2fDevices = []; unless ( $self->canUpdateSfa($req) ) { @@ -534,6 +526,13 @@ sub _displayRegister { $self->userLogger->warn("Do not display 2F devices!"); } + return [ 302, [ Location => $self->conf->{portal} . $am[0]->{URL} ], [] ] + if ( + @am == 1 + and not( @$_2fDevices + or $req->data->{sfRegRequired} ) + ); + # Parse second factors to display delete button if allowed and upgrade button my $displayUpgBtn = 0; my $action = ''; From 0f6753d188167efd21cfc4994ef8b9466df4603a Mon Sep 17 00:00:00 2001 From: Maxime Besson Date: Wed, 30 Mar 2022 00:06:43 +0200 Subject: [PATCH 3/5] Refactor Webauthn --- .../lib/Lemonldap/NG/Portal/2F/Register/WebAuthn.pm | 10 ++++------ .../lib/Lemonldap/NG/Portal/2F/WebAuthn.pm | 3 +-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Register/WebAuthn.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Register/WebAuthn.pm index 1d5c7d2a2..cc3178557 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Register/WebAuthn.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/Register/WebAuthn.pm @@ -82,11 +82,9 @@ sub _generate_user_handle { sub _registrationchallenge { my ( $self, $req, $user ) = @_; - my @_2fDevices = $self->find2fByType( $req, $req->userData ); - - # Check if user can register one more 2F device - my $size = @_2fDevices; - my $maxSize = $self->conf->{max2FDevices}; + my @alldevices = $self->find2fDevicesByType( $req, $req->userData ); + my $size = @alldevices; + my $maxSize = $self->conf->{max2FDevices}; $self->logger->debug("Registered 2F Device(s): $size / $maxSize"); if ( $size >= $maxSize ) { $self->userLogger->warn("Max number of 2F devices is reached"); @@ -180,7 +178,7 @@ sub _registration { if ( $self->find2fDevicesByKey( - $req, $req->userData, $self->type, + $req, $req->userData, $self->type, "_credentialId", $credential_id ) ) diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/WebAuthn.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/WebAuthn.pm index 8ad27eaeb..c86f3e2f5 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/WebAuthn.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/2F/WebAuthn.pm @@ -36,8 +36,7 @@ sub init { if ( $self->conf->{webauthn2fSelfRegistration} and $self->conf->{webauthn2fActivation} eq '1' ) { - $self->conf->{webauthn2fActivation} = - '$_2fDevices && $_2fDevices =~ /"type"\s*:\s*"WebAuthn"/s'; + $self->conf->{webauthn2fActivation} = 'has2f("WebAuthn")'; } return 0 unless ( $self->Lemonldap::NG::Portal::Main::SecondFactor::init() ); From c85ade2e275fdb97c54f60afb96371b344982d58 Mon Sep 17 00:00:00 2001 From: Maxime Besson Date: Wed, 30 Mar 2022 00:02:22 +0200 Subject: [PATCH 4/5] perldoc for 2fDevices lib --- .../lib/Lemonldap/NG/Portal/Lib/2fDevices.pm | 179 +++++++++++++++++- 1 file changed, 172 insertions(+), 7 deletions(-) diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/2fDevices.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/2fDevices.pm index a9844d8d0..b0525029d 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/2fDevices.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/2fDevices.pm @@ -1,5 +1,25 @@ package Lemonldap::NG::Portal::Lib::2fDevices; +=pod + +=head1 NAME + +Lemonldap::NG::Portal::Lib::2fDevices - Role for registrable second factors + +=head1 DESCRIPTION + +This role provides LemonLDAP::NG modules with a high-level interface to storing +information on registrable second factors into the persistent session. + +It is recommended that _2fDevices is never accessed directly from code outside +of this module + +=head1 METHODS + +=over + +=cut + use strict; use Mouse::Role; use JSON; @@ -8,6 +28,30 @@ requires qw(p conf logger); our $VERSION = '2.0.15'; +=item update2fDevice + +Updates one field of a registered device + + $self->update2fDevice($req, $info, $type, $key, $value, $update_key, $update_value); + +=over 4 + +=item req: Current LemonLDAP::NG request + +=item info: hashref of current session information + +=item type: 'type' field of the device to update + +=item key, value: update the device whose 'key' field equals value + +=item update_key, update_value: set the matched devices' 'update_key' field to update_value + +=back + +Returns true iff the update was sucessful + +=cut + sub update2fDevice { my ( $self, $req, $info, $type, $key, $value, $update_key, $update_value ) = @_; @@ -32,6 +76,27 @@ sub update2fDevice { return 0; } +=item add2fDevice + +Register a new device + + $self->add2fDevice($req, $info, $device); + +=over 4 + +=item req: Current LemonLDAP::NG request + +=item info: hashref of current session information + +=item device: hashref of device details. It must contain at least a 'type', +'name' and 'epoch' key + +=back + +Returns true iff the update was sucessful + +=cut + sub add2fDevice { my ( $self, $req, $info, $device ) = @_; @@ -46,8 +111,28 @@ sub add2fDevice { return 1; } -# $devices must be a arrayref of type+epoch hashrefs: -# [ { type => xxx, epoch => xxx }, { type => xxx, epoch => xxx } ] +=item del2fDevices + +Delete the devices specified in the @$devices array + + $self->del2fDevices($req, $info, $devices); + +=over 4 + +=item req: Current LemonLDAP::NG request + +=item info: hashref of current session information + +=item device: arrayref of type+epoch hashrefs + + [ { type => xxx, epoch => xxx }, { type => xxx, epoch => xxx } ] + +=back + +Returns true iff the update was sucessful + +=cut + sub del2fDevices { my ( $self, $req, $info, $devices ) = @_; @@ -83,6 +168,28 @@ sub del2fDevices { return 1; } +=item del2fDevice + +Delete a single device + + $self->del2fDevice($req, $info, $type, $epoch); + +=over 4 + +=item req: Current LemonLDAP::NG request + +=item info: hashref of current session information + +=item type: type of the device to remove + +=item epoch: timestamp of the device to remove + +=back + +Returns true iff the update was sucessful + +=cut + sub del2fDevice { my ( $self, $req, $info, $type, $epoch ) = @_; @@ -90,6 +197,28 @@ sub del2fDevice { [ { type => $type, epoch => $epoch } ] ); } +=item find2fDevicesByKey + +Find devices from one of its attributes + + $self->find2fDevicesByKey($req, $info, $type, $key, $value); + +=over 4 + +=item req: Current LemonLDAP::NG request + +=item info: hashref of current session information + +=item type: device type + +=item key, value: attribute to search in the device hash and the value to filter on + +=back + +Returns an array of devices for which type, key and value match the supplied ones + +=cut + sub find2fDevicesByKey { my ( $self, $req, $info, $type, $key, $value ) = @_; @@ -101,11 +230,23 @@ sub find2fDevicesByKey { return @found; } -## @method get2fDevices($req, $info) -# Validate logout request -# @param req Request object -# @param info HashRef of session data -# @return undef or ArrayRef of second factors +=item get2fDevices + +Return all registrable devices. + + $self->get2fDevices($req, $info); + +=over 4 + +=item req: Current LemonLDAP::NG request + +=item info: hashref of current session information + +=back + +Returns an arrayref of all registrable devices, or undef if an error occured + +=cut sub get2fDevices { my ( $self, $req, $info ) = @_; @@ -131,6 +272,27 @@ sub get2fDevices { } } +=item find2fDevicesByType + +Return all registrable devices of a certain type. If type is not given, return +all registrable devices + + $self->find2fDevicesByType($req, $info, $type); + +=over 4 + +=item req: Current LemonLDAP::NG request + +=item info: hashref of current session information + +=item type: type of registrable device to return + +=back + +Returns an array of all matching devices + +=cut + sub find2fDevicesByType { my ( $self, $req, $info, $type ) = @_; @@ -142,3 +304,6 @@ sub find2fDevicesByType { return @found; } 1; + +=back + From 3b0a928ba692df2419a829f135e775855950e307 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20OUDOT?= Date: Mon, 16 May 2022 13:38:52 +0000 Subject: [PATCH 5/5] Fix some typos in embedded doc --- .../lib/Lemonldap/NG/Portal/Lib/2fDevices.pm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/2fDevices.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/2fDevices.pm index b0525029d..6656540b8 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/2fDevices.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Lib/2fDevices.pm @@ -48,7 +48,7 @@ Updates one field of a registered device =back -Returns true iff the update was sucessful +Returns true if the update was sucessful =cut @@ -93,7 +93,7 @@ Register a new device =back -Returns true iff the update was sucessful +Returns true if the update was sucessful =cut @@ -129,7 +129,7 @@ Delete the devices specified in the @$devices array =back -Returns true iff the update was sucessful +Returns true if the update was sucessful =cut @@ -186,7 +186,7 @@ Delete a single device =back -Returns true iff the update was sucessful +Returns true if the update was sucessful =cut