Merge pull request #202 from waterkip/GH-invalid-id_for_metadata_is_p…

…ossible

Rename id to issuer for Net::SAML2::SP

lib/Net/SAML2/Protocol/AuthnRequest.pm

@@ -7,6 +7,7 @@ use MooseX::Types::Common::String qw/ NonEmptySimpleStr /;
 use XML::Generator;
 use List::Util        qw(any);
 use URN::OASIS::SAML2 qw(:urn BINDING_HTTP_POST);
+use Net::SAML2::Util ();
 
 with 'Net::SAML2::Role::ProtocolMessage';
 
@@ -179,9 +180,10 @@ around BUILDARGS => sub {
 
     my %params = @_;
     if ($params{nameid_format} && !defined $params{nameidpolicy_format}) {
-        warn "You are using nameid_format, this field has changed to "
-          . "nameidpolicy_format. This field will be used for other purposes "
-          . "in an upcoming release. Please change your code ASAP.";
+        Net::SAML2::Util::deprecation_warning "You are using nameid_format, "
+          . "this field has changed to nameidpolicy_format. This field will "
+          . "be used for other purposes in an upcoming release. Please change "
+          . "your code ASAP.";
         $params{nameidpolicy_format} = $params{nameid_format};
     }
 

lib/Net/SAML2/SP.pm

@@ -19,17 +19,18 @@ use Net::SAML2::Protocol::LogoutRequest;
 use Net::SAML2::Util ();
 use URN::OASIS::SAML2 qw(:bindings :urn);
 use XML::Generator;
+use Net::SAML2::Types qw(XsdID);
 
 # ABSTRACT: SAML Service Provider object
 
 =head1 SYNOPSIS
 
-  my $sp = Net::SAML2::SP->new(
-    id   => 'http://localhost:3000',
-    url  => 'http://localhost:3000',
-    cert => 'sign-nopw-cert.pem',
-    key => 'sign-nopw-key.pem',
-  );
+my $sp = Net::SAML2::SP->new(
+    issuer => 'http://localhost:3000',
+    url    => 'http://localhost:3000',
+    cert   => 'sign-nopw-cert.pem',
+    key    => 'sign-nopw-key.pem',
+);
 
 =head1 METHODS
 
@@ -44,6 +45,10 @@ Arguments:
 
 =over
 
+=item B<id>
+
+The ID attribute used in the EntityDescription tag
+
 =item B<url>
 
 Base for all SP service URLs
@@ -52,7 +57,7 @@ Base for all SP service URLs
 
 The error URI. Can be relative to the base URI or a regular URI
 
-=item B<id>
+=item B<issuer>
 
 SP's identity URI.
 
@@ -156,7 +161,10 @@ Consumer Services.
 =cut
 
 has 'url'    => (isa => Uri, is => 'ro', required => 1, coerce => 1);
-has 'id'     => (isa => 'Str', is => 'ro', required => 1);
+
+has 'id'        => (isa => XsdID, is => 'ro', builder => '_build_id');
+has 'issuer'    => (isa => 'Str', is => 'ro', required => 1);
+
 has 'cert'   => (isa => 'Str', is => 'ro', required => 1, predicate => 'has_cert');
 has 'key'    => (isa => 'Str', is => 'ro', required => 1);
 has 'cacert' => (isa => 'Str', is => 'rw', required => 0, predicate => 'has_cacert');
@@ -196,6 +204,12 @@ around BUILDARGS => sub {
 
     my %args = @_;
 
+    if (!exists $args{issuer} && exists $args{id}) {
+      Net::SAML2::Util::deprecation_warning
+        "id has been renamed to issuer and should be used instead";
+      $args{issuer} = delete $args{id};
+    }
+
     if (!$args{single_logout_service}) {
         #warn "Deprecation warning, please upgrade your code to use ..";
         my @slo;
@@ -270,6 +284,20 @@ around BUILDARGS => sub {
     return $self->$orig(%args);
 };
 
+sub _build_id {
+    my $self = shift;
+
+    # This allows current clients to override the builder without changing
+    # their code
+    if (my $f = $self->can('generate_sp_desciptor_id')) {
+        Net::SAML2::Util::deprecation_warning
+          "generate_sp_desciptor_id has been deprecated, please override " .
+          "_build_id yourself or supply the ID to the constructor";
+          return $f->();
+    }
+    return Net::SAML2::Util::generate_id();
+}
+
 sub _build_encryption_key_text {
     my ($self) = @_;
 
@@ -323,7 +351,7 @@ sub authn_request {
 
     return Net::SAML2::Protocol::AuthnRequest->new(
         issueinstant        => DateTime->now,
-        issuer              => $self->id,
+        issuer              => $self->issuer,
         destination         => $destination,
         nameidpolicy_format => $nameid_format || '',
         %params,
@@ -356,7 +384,7 @@ sub logout_request {
     my ($self, $destination, $nameid, $nameid_format, $session, $params) = @_;
 
     my $logout_req = Net::SAML2::Protocol::LogoutRequest->new(
-        issuer      => $self->id,
+        issuer      => $self->issuer,
         destination => $destination,
         nameid      => $nameid,
         session     => $session,
@@ -391,7 +419,7 @@ sub logout_response {
 
     my $status_uri = Net::SAML2::Protocol::LogoutResponse->status_uri($status);
     my $logout_req = Net::SAML2::Protocol::LogoutResponse->new(
-        issuer      => $self->id,
+        issuer      => $self->issuer,
         destination => $destination,
         status      => $status_uri,
         response_to => $response_to,
@@ -412,7 +440,7 @@ sub artifact_request {
     my ($self, $destination, $artifact) = @_;
 
     my $artifact_request = Net::SAML2::Protocol::ArtifactResolve->new(
-        issuer       => $self->id,
+        issuer       => $self->issuer,
         destination  => $destination,
         artifact     => $artifact,
         issueinstant => DateTime->now,
@@ -539,17 +567,6 @@ sub post_binding {
     );
 }
 
-=head2 generate_sp_desciptor_id ( )
-
-Returns the Net::SAML2 unique ID from Net::SAML2::Util::generate_id.
-
-=cut
-
-sub generate_sp_desciptor_id {
-    my $self = shift;
-    return Net::SAML2::Util::generate_id();
-}
-
 =head2 generate_metadata( )
 
 Generate the metadata XML document for this SP.
@@ -572,8 +589,8 @@ sub generate_metadata {
     return $x->xml( $x->EntityDescriptor(
         $md,
         {
-            entityID => $self->id,
-            ID       => $self->generate_sp_desciptor_id(),
+            entityID => $self->issuer,
+            ID       => $self->id,
         },
         $x->SPSSODescriptor(
             $md,

lib/Net/SAML2/Util.pm

@@ -11,12 +11,17 @@ use Exporter qw(import);
 
 our @EXPORT_OK = qw(
     generate_id
+    deprecation_warning
 );
 
 sub generate_id {
     return 'NETSAML2_' . unpack 'H*', random_pseudo_bytes(32);
 }
 
+sub deprecation_warning {
+    warn "NET::SAML2 deprecation warning: " . shift . "\n";
+}
+
 
 1;
 

t/lib/Test/Net/SAML2/Util.pm

@@ -41,7 +41,7 @@ our %EXPORT_TAGS = (
 
 sub net_saml2_sp {
     return Net::SAML2::SP->new(
-        id     => 'Some entity ID',
+        issuer => 'Some entity ID',
         cert   => 't/sign-nopw-cert.pem',
         key    => 't/sign-nopw-cert.pem',
         cacert => 't/cacert.pem',