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

We've had generate_sp_desciptor_id to override the ID, but this could
very well become a parameter for the constructor. The current ID
attribute was actually used as the issuer. Rename ID to issuer and and
reintroduce ID. We add some deprecation warnings to users so they are
aware of the change.

Signed-off-by: Wesley Schwengle <waterkip@cpan.org>

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;