Fix regression in #2085 (#2224)

Clearing all hidden form values was a mistake as it breaks SAML when the
redirection URL contains a query string. We should keep existing hidden
fields. In the context of OIDC request, we clear them before redirection
to avoid #2085
This commit is contained in:
Maxime Besson 2020-05-28 19:15:04 +02:00
parent 827d06cded
commit 33a5496e55
2 changed files with 27 additions and 25 deletions

View File

@ -707,10 +707,7 @@ sub run {
$session_state
);
$self->logger->debug("Redirect user to $response_url");
$req->urldc($response_url);
return PE_REDIRECT;
return $self->_redirectToUrl($req, $response_url);
}
# Implicit Flow
@ -780,10 +777,7 @@ sub run {
$session_state
);
$self->logger->debug("Redirect user to $response_url");
$req->urldc($response_url);
return PE_REDIRECT;
return $self->_redirectToUrl($req, $response_url);
}
# Hybrid Flow
@ -885,9 +879,7 @@ sub run {
$session_state
);
$self->logger->debug("Redirect user to $response_url");
$req->urldc($response_url);
return PE_REDIRECT;
return $self->_redirectToUrl($req, $response_url);
}
$self->logger->debug("None flow has been selected");
@ -969,9 +961,7 @@ sub run {
$self->buildLogoutResponse( $post_logout_redirect_uri,
$state );
$self->logger->debug("Redirect user to $response_url");
$req->urldc($response_url);
return PE_REDIRECT;
return $self->_redirectToUrl($req, $response_url);
}
return $req->param('confirm') == 1
? ( $err ? $err : PE_LOGOUT_OK )
@ -2323,4 +2313,15 @@ sub _generateIDToken {
return $self->createIDToken( $id_token_payload_hash, $rp );
}
sub _redirectToUrl {
my ($self, $req, $response_url) = @_;
# We must clear hidden form fields saved from the request (#2085)
$self->p->clearHiddenFormValue($req);
$self->logger->debug("Redirect user to $response_url");
$req->urldc($response_url);
return PE_REDIRECT;
}
1;

View File

@ -524,19 +524,20 @@ sub buildOutgoingHiddenForm {
my ( $self, $req, $method ) = @_;
my @keys = keys %{ $req->{portalHiddenFormValues} };
# Redirection URL contains query string. Before displaying a form,
# we must set the query string parameters as form fields so they can
# be preserved #2085
if ( lc $method eq 'get' ) {
my $uri = URI->new( $req->{urldc} );
my %query_params = $uri->query_form;
my $uri = URI->new( $req->{urldc} );
my %query_params = $uri->query_form;
if (%query_params) {
$self->logger->debug(
# Redirection URL contains query string. Before displaying a form,
# we must set the query string parameters as form fields so they can
# be preserved #2085
if (%query_params) {
$self->logger->debug(
"urldc contains query parameters, setting them as hidden form values"
);
$self->clearHiddenFormValue($req);
foreach ( keys %query_params ) {
$self->setHiddenFormValue( $req, $_, $query_params{$_}, "", 0 );
);
foreach ( keys %query_params ) {
$self->setHiddenFormValue( $req, $_, $query_params{$_}, "", 0 );
}
}
}