diff --git a/bin/account-manager-client.pl b/bin/account-manager-client.pl index da4bfd078042e5dd5c48746a5d0060c918d959ae..d10d5073f90a51e0a24e604240da0e225a9285c2 100755 --- a/bin/account-manager-client.pl +++ b/bin/account-manager-client.pl @@ -149,12 +149,11 @@ sub list_test_accounts { } sub parse_federation_metadata { - my $federation_metadata = IdPAccountManager::SAMLMetadata->new(); + my $federation_metadata; eval { - $federation_metadata->load( - federation_metadata_file_path => - $configuration->{federation_metadata_file_path} + $federation_metadata = IdPAccountManager::SAMLMetadata->new( + file => $configuration->{federation_metadata_file_path} ); }; die "unable to load federation metadata: $EVAL_ERROR" if $EVAL_ERROR; diff --git a/lib/IdPAccountManager/SAMLMetadata.pm b/lib/IdPAccountManager/SAMLMetadata.pm index de3a26f7abab224af636f4620badf164f8ea3408..ccda103ba345779571ec59772b7b0ad4a0d2eaee 100644 --- a/lib/IdPAccountManager/SAMLMetadata.pm +++ b/lib/IdPAccountManager/SAMLMetadata.pm @@ -11,36 +11,38 @@ use IdPAccountManager::Tools; sub new { my ($pkg, %args) = @_; - my $self = {}; + die "missing argument 'file'" unless defined $args{file}; + die "non-existing file $args{file}" unless -f $args{file}; + die "non-readable file $args{file}" unless -r $args{file}; - bless $self, $pkg; - - return $self; -} + open(my $handle, '<', $args{file}) + or die "failed to open file $args{file}: $ERRNO"; -## Load metadata -sub load { - my ($self, %args) = @_; + my $parser = XML::LibXML->new(); + die "Failed to initialize XML parser" unless $parser; - die "Missing parameter 'federation_metadata_file_path'" - unless $args{federation_metadata_file_path}; + my $doc; + eval { $doc = $parser->parse_fh($handle) }; + die "Failed to parse file $args{file}: $EVAL_ERROR" + if $EVAL_ERROR; - $self->{federation_metadata_file_path} = - $args{federation_metadata_file_path}; + die "Failed to parse file $args{file}: $EVAL_ERROR" + unless $doc; - die "Failed to read $args{federation_metadata_file_path}: $ERRNO" - unless -r $self->{federation_metadata_file_path}; + my $root = $doc->documentElement(); + my $type = $root->nodeName(); - die "Failed to parse file $args{metadata_file}: $ERRNO" - unless $self->{federation_metadata_as_xml} = - $self->_get_xml_object($args{federation_metadata_file_path}); + die "incorrect root element type '$type' for file $args{file}, should be + 'EntitiesDescriptor'" unless $type =~ /EntitiesDescriptor$/; - my $root = $self->{federation_metadata_as_xml}->documentElement(); + my $self = { + file => $args{file}, + doc => $doc + }; - die "Root element of file $args{federation_metadata_file_path} is of type '%s'; should be 'EntitiesDescriptor'", $root->nodeName() - unless $root->nodeName() =~ /EntitiesDescriptor$/; + bless $self, $pkg; - return 1; + return $self; } ## Parse XML structure of metadata to fill a hashref @@ -48,7 +50,7 @@ sub parse { my ($self, %args) = @_; my %parser_args = ( - metadata_as_xml => $self->{federation_metadata_as_xml}, + metadata_as_xml => $self->{doc}, filter_entity_type => 'sp' ); @@ -70,37 +72,10 @@ sub print { my ($self, $fd) = @_; $fd = \*STDOUT unless $fd; - my $root = $self->{federation_metadata_as_xml}->documentElement(); + my $root = $self->{doc}->documentElement(); print $fd $root->toString(); } -## returns a Lib::XML représenting an XML file -sub _get_xml_object { - my ($self, $metadata_file) = @_; - - die "File $metadata_file not found: $ERRNO" - unless -f $metadata_file; - - die "Failed to open file $metadata_file: $ERRNO" - unless open FH, $metadata_file; - - my $parser = XML::LibXML->new(); - die "Failed to initialize XML parser" unless $parser; - - $parser->line_numbers(1); - my $doc; - - ## Eval() prevents the parsing from killing the main process - eval { $doc = $parser->parse_fh(\*FH) }; - die "Failed to parse file $metadata_file : $EVAL_ERROR" - if $EVAL_ERROR; - - die "Failed to parse file $metadata_file : $EVAL_ERROR" - unless $doc; - - return $doc; -} - ## Parse a SAML federation metadata file sub _parse_saml_metadata { my ($self, %args) = @_; diff --git a/lib/IdPAccountManager/WebRequest.pm b/lib/IdPAccountManager/WebRequest.pm index 0ec86860fcdb3ead151af9a47c481826c5f6c4e5..9dff134d30fb1f49f71e43ef622efa1877151a50 100755 --- a/lib/IdPAccountManager/WebRequest.pm +++ b/lib/IdPAccountManager/WebRequest.pm @@ -249,7 +249,7 @@ sub req_account_wizard { } eval { - $federation_metadata->parse()); + $federation_metadata->parse(); }; if ($EVAL_ERROR) { push @{ $self->{param_out}->{errors} }, "internal"; @@ -279,35 +279,32 @@ sub req_select_sp { return undef; } - my $federation_metadata = IdPAccountManager::SAMLMetadata->new( - logger => $self->{logger} - ); + my $federation_metadata; - unless ( - $federation_metadata->load( - federation_metadata_file_path => - $self->{configuration}->{federation_metadata_file_path} - ) - ) - { + eval { + $federation_metadata = IdPAccountManager::SAMLMetadata->new( + file => $self->{configuration}->{federation_metadata_file_path} + ); + }; + if ($EVAL_ERROR) { push @{ $self->{param_out}->{errors} }, "internal"; $self->{logger}->log( level => LOG_ERROR, - message => "Failed to load federation metadata : $ERRNO" + message => "Failed to load federation metadata: $EVAL_ERROR" ); return undef; } - unless ( + eval { $federation_metadata->parse( filter_entity_id => $self->{param_in}->{sp_entityid} - ) - ) - { + ); + }; + if ($EVAL_ERROR) { push @{ $self->{param_out}->{errors} }, "internal"; $self->{logger}->log( level => LOG_ERROR, - message => "Failed to parse federation metadata : $ERRNO" + message => "Failed to parse federation metadata: $EVAL_ERROR" ); return undef; }