From 527f42e4debf1987f001f63f3108b0eefb056256 Mon Sep 17 00:00:00 2001
From: Guillaume Rousse <guillaume.rousse@renater.fr>
Date: Tue, 7 Nov 2017 14:21:47 +0100
Subject: [PATCH] extract metadata only if needed

---
 lib/IdPAccountManager/WebRequest.pm | 106 ++++++++++------------------
 templates/web/select_sp.tt2.html    |  13 ++--
 2 files changed, 41 insertions(+), 78 deletions(-)

diff --git a/lib/IdPAccountManager/WebRequest.pm b/lib/IdPAccountManager/WebRequest.pm
index cdafccd..d7fec1e 100755
--- a/lib/IdPAccountManager/WebRequest.pm
+++ b/lib/IdPAccountManager/WebRequest.pm
@@ -242,86 +242,54 @@ sub req_select_sp {
         return undef;
     }
 
-    my $federation_metadata;
-
-    eval {
-        $federation_metadata = IdPAccountManager::SAMLMetadata->new(
-            file => $self->{configuration}->{federation_metadata_file_path}
-        );
-    };
-    if ($EVAL_ERROR) {
-        push @{ $self->{out}->{errors} }, "internal";
-        $self->{logger}->error("Failed to load federation metadata: $EVAL_ERROR");
-        return undef;
-    }
-
-    $federation_metadata->parse(id => $self->{in}->{sp_entityid});
-
-    ## Create a serviceprovider object to store major parameters for this SP in DB
-    my $service_provider = IdPAccountManager::Data::ServiceProvider->new(
-        db             => $self->{db},
-        entityid       => $self->{in}->{sp_entityid},
-        dev_sp_contact => $self->{configuration}->{dev_sp_contact}
+    # Create a persistent service provider object
+    my $provider = IdPAccountManager::Data::ServiceProvider->new(
+        db       => $self->{db},
+        entityid => $self->{in}->{sp_entityid}
     );
 
-    ## Prepare data
-    my $sp_metadata_as_hashref =
-      $federation_metadata->{federation_metadata_as_hashref}->[0];
-    my @contacts;
-    if (defined $sp_metadata_as_hashref->{contacts}) {
-        foreach my $contact (@{ $sp_metadata_as_hashref->{contacts} }) {
-            my $email = $contact->{EmailAddress};
-            $email =~ s/^(mailto:)//;    ## Remove 'mailto:' prefixes if any
-            push @contacts, $email;
-        }
-    }
-    my $display_name;
-    if (defined $sp_metadata_as_hashref->{display_name}) {
-        ## Use English version of displayName if available
-        if ($sp_metadata_as_hashref->{display_name}->{en}) {
-            $display_name = $sp_metadata_as_hashref->{display_name}->{en};
-            ## Else any language
-        } else {
-            foreach
-              my $lang (keys %{ $sp_metadata_as_hashref->{display_name} })
-            {
-                $display_name =
-                  $sp_metadata_as_hashref->{display_name}->{$lang};
-                last;
-            }
+    if ($provider->load(speculative => 1)) {
+        # already present in DB, nothing todo
+    } else {
+        # extract information from metadata
+        my $metadata;
+
+        eval {
+            $metadata = IdPAccountManager::SAMLMetadata->new(
+                file => $self->{configuration}->{federation_metadata_file_path}
+            );
+        };
+        if ($EVAL_ERROR) {
+            push @{ $self->{out}->{errors} }, "internal";
+            $self->{logger}->error("Failed to load federation metadata: $EVAL_ERROR");
+            return undef;
         }
-    }
 
-    ## Try loading DB object first
-    if ($service_provider->load(speculative => 1)) {
-        $service_provider->contacts(join(',', @contacts));
-        $service_provider->displayname($display_name);
+        my $sps = $metadata->parse(id => $self->{in}->{sp_entityid});
+        if (!@$sps) {
+            push @{ $self->{out}->{errors} }, "no_such_entity";
+            $self->{logger}->errorf(
+                "No such entity %s in metadata", $self->{in}->{sp_entityid}
+            );
+            return undef;
+        }
+        my $sp = $sps->[0];
 
-    } else {
+        # complete persistent object
+        $provider->displayname($sp->{display_name});
+        $provider->contacts(
+            join(',', map { $_->{EmailAddress} } @{$sp->{contacts}})
+        ) if $sp->{contacts};
 
-        $service_provider = IdPAccountManager::Data::ServiceProvider->new(
-            db             => $self->{db},
-            entityid       => $self->{in}->{sp_entityid},
-            contacts       => join(',', @contacts),
-            displayname    => $display_name,
-            dev_sp_contact => $self->{configuration}->{dev_sp_contact}
-        );
-        unless (defined $service_provider) {
+        # save in DB
+        unless ($provider->save()) {
             push @{ $self->{out}->{errors} }, "internal";
-            $self->{logger}->error("Failed to create serviceprovider object");
+            $self->{logger}->error("Failed to save service provider object");
             return undef;
         }
     }
 
-    unless ($service_provider->save()) {
-        push @{ $self->{out}->{errors} }, "internal";
-        $self->{logger}->error("Failed to save serviceprovider object");
-        return undef;
-    }
-
-    $self->{out}->{sp_metadata_as_hashref} =
-      $federation_metadata->{federation_metadata_as_hashref}->[0];
-    $self->{out}->{serviceprovider} = $service_provider;
+    $self->{out}->{provider} = $provider;
     $self->{out}->{subtitle} = 'Select your Service Provider';
 
     return 1;
diff --git a/templates/web/select_sp.tt2.html b/templates/web/select_sp.tt2.html
index 03a7e36..ce4e56a 100644
--- a/templates/web/select_sp.tt2.html
+++ b/templates/web/select_sp.tt2.html
@@ -1,25 +1,20 @@
 <h3>Send email challenge</h3>
-[% IF serviceprovider.displayname %]
-[% SET sp_display_name = serviceprovider.displayname %]
-[% ELSE %]
-[% SET sp_display_name = metadata.entityid %]
-[% END %]
 <div>
-Before you can create test accounts at this Identity Provider, we need to ensure you are a legitimate administrator of "[% sp_display_name %]".
+Before you can create test accounts at this Identity Provider, we need to ensure you are a legitimate administrator of "[% provider.displayname %]".
 </div>
 
 <fieldset class="scrollable">
-[% IF  metadata.contacts.defined %]
+[% IF provider.contacts.defined %]
     <legend>Select your email address</legend>
    <label for="sp_entityid">The email addresses below have been extracted from your SP SAML metadata.<br/>Please select the email address where an email challenge
    can be sent to validate your identity</label>
 
 
 <div class="radio_inline">
-[% FOREACH email IN serviceprovider.list_contacts_as_array.sort %]
+[% FOREACH email IN provider.list_contacts_as_array.sort %]
 <input name="email_address" value="[% email %]" type="radio" class="required"/><label for="email_address">[% email %]</label><br/>
 
-<input type="hidden" name="sp_entityid" value="[% metadata.entityid %]" id="sp_entityid"/>
+<input type="hidden" name="sp_entityid" value="[% provider.entityid %]" id="sp_entityid"/>
 
 [% END %]
 </div>
-- 
GitLab