Commit 0b1f6b5c authored by Christophe Maudoux's avatar Christophe Maudoux 🐛
Browse files

Fix warnings with confirmation (#1603)

parent 245913da
......@@ -56,8 +56,11 @@ sub addRoutes {
# New key and conf save
->addRoute(
confs =>
{ newRSAKey => 'newRSAKey', raw => 'newRawConf', '*' => 'newConf' },
confs => {
newRSAKey => 'newRSAKey',
raw => 'newRawConf',
'*' => 'newConf'
},
['POST']
)
......@@ -137,11 +140,12 @@ sub prx {
return $self->sendError( $req,
$response->code . " (" . $response->message . ")", 400 );
}
unless ( $response->header('Content-Type') =~
m#^(?:application/json|(?:application|text)/.*xml).*$# )
unless ( $response->header('Content-Type')
=~ m#^(?:application/json|(?:application|text)/.*xml).*$# )
{
return $self->sendError( $req,
'Content refused for security reason (neither XML or JSON)', 400 );
'Content refused for security reason (neither XML or JSON)',
400 );
}
return $self->sendJSONresponse( $req, { content => $response->content } );
}
......@@ -233,6 +237,8 @@ sub newConf {
$res->{details}->{ '__' . $t . '__' } = $parser->$t
if ( @{ $parser->$t } );
}
$res->{details}->{'__needConfirmation__'} = $parser->{needConfirmation}
if ( @{ $parser->{needConfirmation} } && !$req->params('force') );
if ( $res->{result} ) {
if ( $self->{demoMode} ) {
$res->{message} = '__demoModeOn__';
......@@ -240,7 +246,9 @@ sub newConf {
else {
my %args;
$args{force} = 1 if ( $req->params('force') );
my $s = $self->confAcc->saveConf( $parser->newConf, %args );
my $s = CONFIG_WAS_CHANGED;
$s = $self->confAcc->saveConf( $parser->newConf, %args )
unless ( @{ $parser->{needConfirmation} } && !$args{force} );
if ( $s > 0 ) {
$self->userLogger->notice(
'User ' . $self->userId($req) . " has stored conf $s" );
......@@ -259,7 +267,8 @@ sub newConf {
$res->{result} = 0;
if ( $s == CONFIG_WAS_CHANGED ) {
$res->{needConfirm} = 1;
$res->{message} .= '__needConfirmation__';
$res->{message} .= '__needConfirmation__'
unless @{ $parser->{needConfirmation} };
}
else {
$res->{message} = $Lemonldap::NG::Common::Conf::msg;
......@@ -325,8 +334,8 @@ sub applyConf {
$self->api->checkConf();
# Get apply section values
my %reloadUrls =
%{ $self->confAcc->getLocalConf( APPLYSECTION, undef, 0 ) };
my %reloadUrls
= %{ $self->confAcc->getLocalConf( APPLYSECTION, undef, 0 ) };
if ( !%reloadUrls && $newConf->{reloadUrls} ) {
%reloadUrls = %{ $newConf->{reloadUrls} };
}
......@@ -342,10 +351,10 @@ sub applyConf {
my $targetUrl = $url->scheme . "://" . $host;
$targetUrl .= ":" . $url->port if defined( $url->port );
$targetUrl .= $url->full_path;
$r =
HTTP::Request->new( 'GET', $targetUrl,
$r = HTTP::Request->new( 'GET', $targetUrl,
HTTP::Headers->new( Host => $url->host ) );
if ( defined $url->userinfo && $url->userinfo =~ /^([^:]+):(.*)$/ )
if ( defined $url->userinfo
&& $url->userinfo =~ /^([^:]+):(.*)$/ )
{
$r->authorization_basic( $1, $2 );
}
......@@ -353,8 +362,10 @@ sub applyConf {
my $response = $self->ua->request($r);
if ( $response->code != 200 ) {
$status->{$host} =
"Error " . $response->code . " (" . $response->message . ")";
$status->{$host}
= "Error "
. $response->code . " ("
. $response->message . ")";
$self->logger->error( "Apply configuration for $host: error "
. $response->code . " ("
. $response->message
......@@ -373,13 +384,13 @@ sub diff {
my ( $self, $req, @path ) = @_;
return $self->sendError( $req, 'to many arguments in path info', 400 )
if (@path);
my @cfgNum =
( scalar( $req->param('conf1') ), scalar( $req->param('conf2') ) );
my @cfgNum
= ( scalar( $req->param('conf1') ), scalar( $req->param('conf2') ) );
my @conf;
$self->logger->debug(" Loading confs");
# Load the 2 configurations
for ( my $i = 0 ; $i < 2 ; $i++ ) {
for ( my $i = 0; $i < 2; $i++ ) {
if ( %{ $self->currentConf }
and $cfgNum[$i] == $self->currentConf->{cfgNum} )
{
......@@ -390,7 +401,7 @@ sub diff {
{ cfgNum => $cfgNum[$i], raw => 1, noCache => 1 } );
return $self->sendError(
$req,
"Configuration $cfgNum[$i] not available $Lemonldap::NG::Common::Conf::msg",
"Configuration $cfgNum[$i] not available $Lemonldap::NG::Common::Conf::msg",
400
) unless ( $conf[$i] );
}
......@@ -398,8 +409,7 @@ sub diff {
require Lemonldap::NG::Manager::Conf::Diff;
return $self->sendJSONresponse(
$req,
[
$self->Lemonldap::NG::Manager::Conf::Diff::diff(
[ $self->Lemonldap::NG::Manager::Conf::Diff::diff(
$conf[0], $conf[1]
)
]
......
......@@ -46,7 +46,8 @@ has warnings => (
hdebug( 'warnings contains', $_[0]->{warnings} );
}
);
has changes => ( is => 'rw', isa => 'ArrayRef', default => sub { return [] } );
has changes =>
( is => 'rw', isa => 'ArrayRef', default => sub { return [] } );
has message => (
is => 'rw',
isa => 'Str',
......@@ -55,10 +56,10 @@ has message => (
hdebug( "Message becomes " . $_[0]->{message} );
}
);
has needConfirmation =>
( is => 'rw', isa => 'ArrayRef', default => sub { return [] } );
# Booleans
has needConfirm =>
( is => 'rw', isa => 'ArrayRef', default => sub { return [] } );
has confChanged => (
is => 'rw',
isa => 'Bool',
......@@ -124,14 +125,15 @@ sub scanTree {
# Set cfgNum to ref cfgNum (will be changed when saving), set other
# metadata and set a value to the key if empty
$self->newConf->{cfgNum} = $self->req->params('cfgNum');
$self->newConf->{cfgAuthor} =
$self->req->userData->{ $Lemonldap::NG::Handler::Main::tsv->{whatToTrace}
$self->newConf->{cfgAuthor}
= $self->req->userData
->{ $Lemonldap::NG::Handler::Main::tsv->{whatToTrace}
|| '_whatToTrace' } // "anonymous";
$self->newConf->{cfgAuthorIP} = $self->req->address;
$self->newConf->{cfgDate} = time;
$self->newConf->{cfgVersion} = $VERSION;
$self->newConf->{key} ||=
join( '', map { chr( int( rand(94) ) + 33 ) } ( 1 .. 16 ) );
$self->newConf->{key}
||= join( '', map { chr( int( rand(94) ) + 33 ) } ( 1 .. 16 ) );
return 1;
}
......@@ -216,11 +218,11 @@ sub _scanNodes {
}
# Other sub levels
elsif ( $leaf->{id} =~
/^($specialNodeKeys)\/([^\/]+)\/([^\/]+)(?:\/(.*))?$/io )
elsif ( $leaf->{id}
=~ /^($specialNodeKeys)\/([^\/]+)\/([^\/]+)(?:\/(.*))?$/io )
{
my ( $base, $key, $oldName, $target, $h ) =
( $1, $newNames{$2}, $2, $3, $4 );
my ( $base, $key, $oldName, $target, $h )
= ( $1, $newNames{$2}, $2, $3, $4 );
hdebug(
"Special node chield subnode detected $leaf->{id}",
" base $base, key $key, target $target, h "
......@@ -233,16 +235,16 @@ sub _scanNodes {
if ( $target =~ /^(?:locationRules|exportedHeaders|post)$/ ) {
if ( $leaf->{cnodes} ) {
hdebug(' unopened subnode');
$self->newConf->{$target}->{$key} =
$self->refConf->{$target}->{$oldName} // {};
$self->newConf->{$target}->{$key}
= $self->refConf->{$target}->{$oldName} // {};
}
elsif ($h) {
hdebug(' 4 levels');
if ( $target eq 'locationRules' ) {
hdebug(' locationRules');
my $k =
$leaf->{comment}
my $k
= $leaf->{comment}
? "(?#$leaf->{comment})$leaf->{re}"
: $leaf->{re};
$self->set( $target, $key, $k, $leaf->{data} );
......@@ -306,16 +308,18 @@ sub _scanNodes {
# SAML
elsif ( $base =~ /^saml(?:S|ID)PMetaDataNodes$/ ) {
hdebug('SAML');
if ( defined $leaf->{data} and ref( $leaf->{data} ) eq 'ARRAY' )
if ( defined $leaf->{data}
and ref( $leaf->{data} ) eq 'ARRAY' )
{
hdebug(" SAML data is an array, serializing");
$leaf->{data} = join ';', @{ $leaf->{data} };
}
if ( $target =~ /^saml(?:S|ID)PMetaDataExportedAttributes$/ ) {
if ( $target =~ /^saml(?:S|ID)PMetaDataExportedAttributes$/ )
{
if ( $leaf->{cnodes} ) {
hdebug(" $target: unopened node");
$self->newConf->{$target}->{$key} =
$self->refConf->{$target}->{$oldName} // {};
$self->newConf->{$target}->{$key}
= $self->refConf->{$target}->{$oldName} // {};
}
elsif ($h) {
hdebug(" $target: opened node");
......@@ -330,14 +334,16 @@ sub _scanNodes {
}
elsif ( $target =~ /^saml(?:S|ID)PMetaDataXML$/ ) {
hdebug(" $target");
$self->set( $target, [ $oldName, $key ],
$target, $leaf->{data} );
$self->set(
$target, [ $oldName, $key ],
$target, $leaf->{data}
);
}
elsif ( $target =~ /^saml(?:ID|S)PMetaDataOptions/ ) {
my $optKey = $&;
hdebug(" $base sub key: $target");
if ( $target =~
/^(?:$samlIDPMetaDataNodeKeys|$samlSPMetaDataNodeKeys)/o
if ( $target
=~ /^(?:$samlIDPMetaDataNodeKeys|$samlSPMetaDataNodeKeys)/o
)
{
$self->set(
......@@ -347,7 +353,8 @@ sub _scanNodes {
}
else {
push @{ $self->errors },
{ message => "Unknown SAML metadata option $target" };
{ message =>
"Unknown SAML metadata option $target" };
return 0;
}
}
......@@ -365,7 +372,8 @@ sub _scanNodes {
if ( $target =~ /^oidc(?:O|R)PMetaDataOptions$/ ) {
hdebug(" $target: looking for subnodes");
$self->_scanNodes($subNodes);
$self->set( $target, $key, $leaf->{title}, $leaf->{data} );
$self->set( $target, $key, $leaf->{title},
$leaf->{data} );
}
elsif ( $target =~ /^oidcOPMetaData(?:JSON|JWKS)$/ ) {
hdebug(" $target");
......@@ -375,8 +383,8 @@ sub _scanNodes {
hdebug(" $target");
if ( $leaf->{cnodes} ) {
hdebug(' unopened');
$self->newConf->{$target}->{$key} =
$self->refConf->{$target}->{$oldName} // {};
$self->newConf->{$target}->{$key}
= $self->refConf->{$target}->{$oldName} // {};
}
elsif ($h) {
hdebug(' opened');
......@@ -394,8 +402,8 @@ sub _scanNodes {
if ( $target eq 'oidcRPMetaDataOptionsExtraClaims' ) {
if ( $leaf->{cnodes} ) {
hdebug(' unopened');
$self->newConf->{$target}->{$key} =
$self->refConf->{$target}->{$oldName} // {};
$self->newConf->{$target}->{$key}
= $self->refConf->{$target}->{$oldName} // {};
}
elsif ($h) {
hdebug(' opened');
......@@ -407,8 +415,8 @@ sub _scanNodes {
$self->_scanNodes($subNodes);
}
}
elsif ( $target =~
/^(?:$oidcOPMetaDataNodeKeys|$oidcRPMetaDataNodeKeys)/o
elsif ( $target
=~ /^(?:$oidcOPMetaDataNodeKeys|$oidcRPMetaDataNodeKeys)/o
)
{
$self->set(
......@@ -418,7 +426,8 @@ sub _scanNodes {
}
else {
push @{ $self->errors },
{ message => "Unknown OIDC metadata option $target" };
{ message =>
"Unknown OIDC metadata option $target" };
return 0;
}
}
......@@ -437,14 +446,15 @@ sub _scanNodes {
if ( $target =~ /^cas(?:App|Srv)MetaDataOptions$/ ) {
hdebug(" $target: looking for subnodes");
$self->_scanNodes($subNodes);
$self->set( $target, $key, $leaf->{title}, $leaf->{data} );
$self->set( $target, $key, $leaf->{title},
$leaf->{data} );
}
elsif ( $target =~ /^cas(?:App|Srv)MetaDataExportedVars$/ ) {
hdebug(" $target");
if ( $leaf->{cnodes} ) {
hdebug(' unopened');
$self->newConf->{$target}->{$key} =
$self->refConf->{$target}->{$oldName} // {};
$self->newConf->{$target}->{$key}
= $self->refConf->{$target}->{$oldName} // {};
}
elsif ($h) {
hdebug(' opened');
......@@ -462,8 +472,8 @@ sub _scanNodes {
if ( $target eq 'casSrvMetaDataOptionsProxiedServices' ) {
if ( $leaf->{cnodes} ) {
hdebug(' unopened');
$self->newConf->{$target}->{$key} =
$self->refConf->{$target}->{$oldName} // {};
$self->newConf->{$target}->{$key}
= $self->refConf->{$target}->{$oldName} // {};
}
elsif ($h) {
hdebug(' opened');
......@@ -475,8 +485,8 @@ sub _scanNodes {
$self->_scanNodes($subNodes);
}
}
elsif ( $target =~
/^(?:$casSrvMetaDataNodeKeys|$casAppMetaDataNodeKeys)/o
elsif ( $target
=~ /^(?:$casSrvMetaDataNodeKeys|$casAppMetaDataNodeKeys)/o
)
{
$self->set(
......@@ -486,7 +496,8 @@ sub _scanNodes {
}
else {
push @{ $self->errors },
{ message => "Unknown CAS metadata option $target" };
{ message =>
"Unknown CAS metadata option $target" };
return 0;
}
}
......@@ -513,26 +524,25 @@ sub _scanNodes {
hdebug( $leaf->{title} );
if ( $leaf->{cnodes} ) {
hdebug(' unopened');
$self->newConf->{applicationList} =
$self->refConf->{applicationList} // {};
$self->newConf->{applicationList}
= $self->refConf->{applicationList} // {};
}
else {
$self->_scanNodes($subNodes) or return 0;
# Check for deleted
my @listCatRef =
map { $self->refConf->{applicationList}->{$_}->{catname} }
my @listCatRef
= map { $self->refConf->{applicationList}->{$_}->{catname} }
keys %{ $self->refConf->{applicationList} };
my @listCatNew =
map { $self->newConf->{applicationList}->{$_}->{catname} }
my @listCatNew
= map { $self->newConf->{applicationList}->{$_}->{catname} }
keys(
%{
ref $self->newConf->{applicationList}
%{ ref $self->newConf->{applicationList}
? $self->newConf->{applicationList}
: {}
}
);
for ( my $i = 0 ; $i < @listCatNew ; $i++ ) {
for ( my $i = 0; $i < @listCatNew; $i++ ) {
if ( not( defined $listCatRef[$i] )
or $listCatRef[$i] ne $listCatNew[$i] )
{
......@@ -567,7 +577,8 @@ sub _scanNodes {
unless ( defined $knownCat->{$cat} ) {
push @{ $self->{errors} },
{ message =>
"Fatal: sub cat/app before parent ($leaf->{id})" };
"Fatal: sub cat/app before parent ($leaf->{id})"
};
return 0;
}
$cn = $cn->{ $knownCat->{$cat} };
......@@ -585,17 +596,15 @@ sub _scanNodes {
$knownCat->{__id}++;
my $s = $knownCat->{$app} = sprintf '%04d-cat',
$knownCat->{__id};
$cn->{$s} =
{ catname => $leaf->{title}, type => 'category' };
$cn->{$s} = { catname => $leaf->{title}, type => 'category' };
unless ($cmp->{$app}
and $cmp->{$app}->{catname} eq $cn->{$s}->{catname} )
{
$self->confChanged(1);
push @{ $self->changes },
{
key => join(
', ', 'applicationList', @path, $leaf->{title}
),
key => join( ', ',
'applicationList', @path, $leaf->{title} ),
new => $cn->{$s}->{catname},
old => ( $cn->{$s} ? $cn->{$s}->{catname} : undef )
};
......@@ -622,8 +631,8 @@ sub _scanNodes {
hdebug(' new app');
$knownCat->{__id}++;
my $name = sprintf( '%04d-app', $knownCat->{__id} );
$cn->{$name} =
{ type => 'application', options => $leaf->{data} };
$cn->{$name}
= { type => 'application', options => $leaf->{data} };
$cn->{$name}->{options}->{name} = $leaf->{title};
unless ( $cmp->{$app} ) {
$self->confChanged(1);
......@@ -669,8 +678,8 @@ sub _scanNodes {
$self->newConf->{grantSessionRules} = {};
foreach my $n (@$subNodes) {
hdebug(" looking at $n subnode");
my $k =
$n->{re} . ( $n->{comment} ? "##$n->{comment}" : '' );
my $k = $n->{re}
. ( $n->{comment} ? "##$n->{comment}" : '' );
$self->newConf->{grantSessionRules}->{$k} = $n->{data};
$count++;
unless ( defined $ref->{$k} ) {
......@@ -706,7 +715,8 @@ sub _scanNodes {
if ( $leaf->{data} ) {
unless ( ref $leaf->{data} eq 'ARRAY' ) {
push @{ $self->{errors} },
{ message => 'Malformed openIdIDPList ' . $leaf->{data} };
{ message => 'Malformed openIdIDPList '
. $leaf->{data} };
return 0;
}
$self->set( $name, join( ';', @{ $leaf->{data} } ) );
......@@ -739,7 +749,8 @@ sub _scanNodes {
$self->newConf->{$name} = {};
foreach my $node ( @{ $leaf->{nodes} } ) {
my $tmp;
$tmp->{$_} = $node->{data}->{$_} foreach (qw(type for));
$tmp->{$_} = $node->{data}->{$_}
foreach (qw(type for));
$tmp->{over} = {};
foreach ( @{ $node->{data}->{over} } ) {
$tmp->{over}->{ $_->[0] } = $_->[1];
......@@ -775,14 +786,15 @@ sub _scanNodes {
}
$self->newConf->{$name}->{ $n->{title} } = $n->{data};
$count++;
unless ( defined $self->refConf->{$name}->{ $n->{title} } )
unless (
defined $self->refConf->{$name}->{ $n->{title} } )
{
$self->confChanged(1);
push @{ $self->changes },
{ key => $name, new => $n->{title}, };
}
elsif (
$self->refConf->{$name}->{ $n->{title} } ne $n->{data} )
elsif ( $self->refConf->{$name}->{ $n->{title} } ne
$n->{data} )
{
$self->confChanged(1);
push @{ $self->changes },
......@@ -834,13 +846,13 @@ sub _scanNodes {
@oldKeys = keys %{ $self->refConf->{$name}->{$host} };
}
foreach my $prm ( @{ $getHost->{h} } ) {
$self->newConf->{$name}->{$host}->{ $prm->{k} } =
$prm->{v};
if (
!$change
$self->newConf->{$name}->{$host}->{ $prm->{k} }
= $prm->{v};
if (!$change
and (
not defined(
$self->refConf->{$name}->{$host}->{ $prm->{k} }
$self->refConf->{$name}->{$host}
->{ $prm->{k} }
)
or $self->newConf->{$name}->{$host}->{ $prm->{k} }
ne $self->refConf->{$name}->{$host}->{ $prm->{k} }
......@@ -962,7 +974,8 @@ sub set {
push @{ $self->changes },
{
key => join( ', ', @path, $target ),
old => $confs[0]->{$target} // $self->defaultValue($target),
old => $confs[0]->{$target}
// $self->defaultValue($target),
new => $confs[1]->{$target}
};
}
......@@ -1037,7 +1050,8 @@ sub _unitTest {
# Check if key exists
unless ($attr) {
push @{ $self->errors }, { message => "__unknownKey__: $key" };
push @{ $self->errors },
{ message => "__unknownKey__: $key" };
$res = 0;
next;
}
......@@ -1064,8 +1078,7 @@ sub _unitTest {
$res = 0
unless (
$self->_execTest(
{
keyTest => $attr->{keyTest} // $type->{keyTest},
{ keyTest => $attr->{keyTest} // $type->{keyTest},
keyMsgFail => $attr->{keyMsgFail}
// $type->{keyMsgFail},
test => $attr->{test} // $type->{test},
......@@ -1103,7 +1116,7 @@ sub _execTest {
my ( $self, $test, $value, $key, $attr, $msg, $conf ) = @_;
my $ref;
die
"Malformed test for $key: only regexp ref or sub are accepted (type \"$ref\")"
"Malformed test for $key: only regexp ref or sub are accepted (type \"$ref\")"
unless ( $ref = ref($test) and $ref =~ /^(CODE|Regexp|HASH)$/ );
if ( $ref eq 'CODE' ) {
my ( $r, $m ) = ( $test->( $value, $conf, $attr ) );
......@@ -1165,7 +1178,7 @@ sub _globalTest {
eval {
( $res, $msg ) = $sub->();
if ( $res == -1 ) {
push @{ $self->needConfirm }, { message => $msg };
push @{ $self->needConfirmation }, { message => $msg };
}
elsif ($res) {
if ($msg) {
......
......@@ -609,9 +609,11 @@ sub tests {
# Warn if Mailrest plugin is enabled without Token or Captcha
checkMailResetSecurity => sub {
return 1 unless ( $conf->{portalDisplayResetPassword} );
return ( 1,
return ( -1,
'"passwordMailReset" plugin is enabled without CSRF Token or Captcha required !!!'
) unless ( $conf->{requireToken} or $conf->{captcha_mail_enabled} );
)
unless ( $conf->{requireToken}
or $conf->{captcha_mail_enabled} );
# Return
return 1;
......
......@@ -144,7 +144,7 @@ llapp.controller 'TreeCtrl', [
title: ''
message: ''
items: []
$scope.confirmNeeded = true if data.message == '__needConfirmation__'
$scope.confirmNeeded = true if data.needConfirm
$scope.message.message = data.message if data.message
if data.details
for m of data.details when m != '__changes__'
......
......@@ -170,7 +170,7 @@ This file contains:
message: '',
items: []