From b8977ffb2f183a0e95cd9b50fe2a6338c000bef2 Mon Sep 17 00:00:00 2001 From: maudoux Date: Fri, 12 Apr 2019 22:46:25 +0200 Subject: [PATCH] Manage Viewer routes with function (#1710) & Improve unit tests --- .../lib/Lemonldap/NG/Manager/Viewer.pm | 40 +++++++++++----- lemonldap-ng-manager/t/70-viewer.t | 8 +++- .../t/71-viewer-with-no-diff.t | 46 +++++++++++++++---- 3 files changed, 73 insertions(+), 21 deletions(-) diff --git a/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Viewer.pm b/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Viewer.pm index ea770f262..4ad5774c7 100644 --- a/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Viewer.pm +++ b/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Viewer.pm @@ -11,7 +11,7 @@ use feature 'state'; extends 'Lemonldap::NG::Manager::Conf'; -our $VERSION = '2.0.3'; +our $VERSION = '2.0.4'; ############################# # I. INITIALIZATION METHODS # @@ -26,7 +26,7 @@ sub addRoutes { $self->ua( Lemonldap::NG::Common::UserAgent->new($conf) ); my $hiddenPK = ''; - $hiddenPK = $self->{viewerHiddenKeys} || $conf->{viewerHiddenKeys}; + $hiddenPK = $self->{viewerHiddenKeys}; my @enabledPK = (); my @keys = qw(virtualHosts samlIDPMetaDataNodes samlSPMetaDataNodes applicationList oidcOPMetaDataNodes oidcRPMetaDataNodes @@ -66,17 +66,9 @@ sub addRoutes { view => { diff => { ':conf1' => { ':conf2' => 'viewDiff' } } } ) ->addRoute( 'viewDiff.html', undef, ['GET'] ); } - unless ( $self->{viewerAllowBrowser} ) { - $self->addRoute( - view => { ':cfgNum' => 'rejectKey' }, - ['GET'] - ); - } # Other keys - else { - $self->addRoute( view => { ':cfgNum' => { '*' => 'getKey' } }, ['GET'] ); - } + $self->addRoute( view => { ':cfgNum' => { '*' => 'viewKey' } }, ['GET'] ); } sub getConfByNum { @@ -124,8 +116,32 @@ sub viewDiff { } sub rejectKey { - my ( $self, $req ) = @_; + my ( $self, $req, @args ) = @_; return $self->sendJSONresponse( $req, { 'value' => '_Hidden_' } ); } +sub viewKey { + my ( $self, $req, @args ) = @_; + my $lastConf; + $self->logger->debug("Viewer requested URI -> $req->{env}->{REQUEST_URI}"); + if ( $self->{viewerAllowBrowser} ) { + $self->logger->debug(" No restriction"); + $self->SUPER::getKey( $req, @args ); + } + else { + if ( $req->{env}->{REQUEST_URI} =~ m%/view/(?:latest|\d+/\w+)$% ) { + $self->logger->debug(" $req->{env}->{REQUEST_URI} -> URI allowed"); + $self->SUPER::getKey( $req, @args ); + } + else { + $self->logger->debug( + " $req->{env}->{REQUEST_URI} -> URI FORBIDDEN"); + my $user = $req->{userData}->{_whatToTrace} || 'anonymous'; + $self->userLogger->warn("$user tried to browse configurations!!!"); + $self->rejectKey( $req, @args ); + } + + } +} + 1; diff --git a/lemonldap-ng-manager/t/70-viewer.t b/lemonldap-ng-manager/t/70-viewer.t index 553db18b5..780081bd6 100644 --- a/lemonldap-ng-manager/t/70-viewer.t +++ b/lemonldap-ng-manager/t/70-viewer.t @@ -29,7 +29,7 @@ count(2); # Try to display latest conf $res = &client->jsonResponse('/view/latest'); -ok( $res->{cfgNum} eq '1', 'Browser is allowed' ); +ok( $res->{cfgNum} eq '1', 'Latest conf loaded' ); count(1); ok( @@ -61,6 +61,12 @@ ok( 6 == keys %{ $res->[1] }, 'Right number of keys found' ) or print STDERR Dumper($res); count(2); +# Try to display previous conf +$res = &client->jsonResponse('/view/1'); +ok( $res->{cfgNum} eq '1', 'Browser is allowed' ) +or print STDERR Dumper($res); +count(1); + # Remove new conf `rm -rf t/conf/lmConf-2.json`; diff --git a/lemonldap-ng-manager/t/71-viewer-with-no-diff.t b/lemonldap-ng-manager/t/71-viewer-with-no-diff.t index 8fee87c90..bcd62014b 100644 --- a/lemonldap-ng-manager/t/71-viewer-with-no-diff.t +++ b/lemonldap-ng-manager/t/71-viewer-with-no-diff.t @@ -8,6 +8,7 @@ use JSON qw(from_json); require 't/test-lib.pm'; my $struct = 't/jsonfiles/70-diff.json'; + sub body { return IO::File->new( $struct, 'r' ); } @@ -21,11 +22,13 @@ ok( 'Client object' ); - - # Try to compare confs 1 & 2 -ok( my $res = $client2->_post( '/confs/', 'cfgNum=1&force=1', &body, 'application/json' ), - "Request succeed" ); +ok( + my $res = $client2->_post( + '/confs/', 'cfgNum=1&force=1', &body, 'application/json' + ), + "Request succeed" +); ok( $res->[0] == 200, "Result code is 200" ); my $resBody; ok( $resBody = from_json( $res->[2]->[0] ), "Result body contains JSON text" ); @@ -38,13 +41,40 @@ foreach my $i ( 0 .. 1 ) { ) or print STDERR Dumper($resBody); } count(2); -$res = $client2->jsonResponse('/view/diff/1/2'); -ok( $res->{value} eq '_Hidden_', 'Diff is NOT allowed' ); + +# Test that Conf key value is sent +my $res = $client2->jsonResponse('/view/2/portalDisplayOidcConsents'); +ok( $res->{value} eq '$_oidcConnectedRP', 'Key found' ) + or print STDERR Dumper($res); +count(1); + +# Test that hidden key values are NOT sent +$res = &client->jsonResponse('/view/2/portalDisplayLogout'); +ok( $res->{value} eq '_Hidden_', 'Key is hidden' ) + or explain( $res, 'value => "_Hidden_"' ); +count(1); + +# Browse confs is forbidden +my $res = $client2->jsonResponse('/view/2'); +ok( $res->{value} eq '_Hidden_', 'Key is hidden' ) + or print STDERR Dumper($res); count(1); # Try to display latest conf -$res = $client2->jsonResponse('/view/2'); -ok( $res->{value} eq '_Hidden_', 'Browser is NOT allowed' ); +$res = &client->jsonResponse('/view/latest'); +ok( $res->{cfgNum} eq '2', 'Latest conf loaded' ); +count(1); + +# Try to compare confs +$res = $client2->jsonResponse('/view/diff/1/2'); +ok( $res->{value} eq '_Hidden_', 'Diff is NOT allowed' ) + or print STDERR Dumper($res); +count(1); + +# Try to display latest conf +$res = $client2->jsonResponse('/view/1'); +ok( $res->{value} eq '_Hidden_', 'Browser is NOT allowed' ) + or print STDERR Dumper($res); count(2); # Remove new conf