Skip to content

Commit 6c63ed1

Browse files
waterkipwaterkip
committed
Make attributes in NameID optional
The Format attribute in NameID will only be set if there is a nameid format configured. The NameQualifier and SPNameQualifier are only set when this is configured by setting `include_name_qualifier`. This is because the SAML spec says: > The NameQualifier and SPNameQualifier attributes SHOULD be omitted unless the > element or format explicitly defines their use and semantics. The SAML spec also says that if the NameID Format is `urn:oasis:names:tc:SAML:2.0:nameidformat:persistent` tese two options must be set. We support this. If there is an affiliation_group_id than we use that instead of the destination ID because of the same spec. > In any case, the <saml:NameID> content in the request and its associated > SPProvidedID attribute MUST contain the most recent name identifier > information established between the providers for the principal. > > In the case of an identifier with a Format of > urn:oasis:names:tc:SAML:2.0:nameidformat:persistent, the NameQualifier > attribute MUST contain the unique identifier of the identity provider that > created the identifier. If the identifier was established between the > identity provider and an affiliation group of which the service provider is a > member, then the SPNameQualifier attribute MUST contain the unique identifier > of the affiliation group. Otherwise, it MUST contain the unique identifier of > the service provider. These attributes MAY be omitted if they would otherwise > match the value of the containing protocol message's <Issuer> element, but > this is NOT RECOMMENDED due to the opportunity for confusion. For more information see: http://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf Signed-off-by: Wesley Schwengle <waterkip@cpan.org>
1 parent 1206d4a commit 6c63ed1

File tree

3 files changed

+149
-46
lines changed

3 files changed

+149
-46
lines changed

‎lib/Net/SAML2/Protocol/LogoutRequest.pm

Lines changed: 91 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use strict;
2-
use warnings;
31
package Net::SAML2::Protocol::LogoutRequest;
42
# VERSION
53

@@ -8,6 +6,7 @@ use MooseX::Types::Common::String qw/ NonEmptySimpleStr /;
86
use MooseX::Types::URI qw/ Uri /;
97
use Net::SAML2::XML::Util qw/ no_comments /;
108
use XML::Generator;
9+
use URN::OASIS::SAML2 qw(:urn);
1110

1211
with 'Net::SAML2::Role::ProtocolMessage';
1312

@@ -63,8 +62,48 @@ sent regardless
6362

6463
has 'session' => (isa => NonEmptySimpleStr, is => 'ro', required => 1);
6564
has 'nameid' => (isa => NonEmptySimpleStr, is => 'ro', required => 1);
66-
has 'nameid_format' => (isa => NonEmptySimpleStr, is => 'ro', required => 0);
67-
has 'destination' => (isa => NonEmptySimpleStr, is => 'ro', required => 0);
65+
has 'nameid_format' => (
66+
isa => NonEmptySimpleStr,
67+
is => 'ro',
68+
required => 0,
69+
predicate => 'has_nameid_format'
70+
);
71+
has 'destination' => (
72+
isa => NonEmptySimpleStr,
73+
is => 'ro',
74+
required => 0,
75+
predicate => 'has_destination'
76+
);
77+
78+
has sp_provided_id => (
79+
isa => NonEmptySimpleStr,
80+
is => 'ro',
81+
required => 0,
82+
predicate => 'has_sp_provided_id'
83+
);
84+
85+
has affiliation_group_id => (
86+
isa => NonEmptySimpleStr,
87+
is => 'ro',
88+
required => 0,
89+
predicate => 'has_affiliation_group_id'
90+
);
91+
92+
has include_name_qualifier =>
93+
(isa => 'Bool', is => 'ro', required => 0, default => 0);
94+
95+
around BUILDARGS => sub {
96+
my $orig = shift;
97+
my $self = shift;
98+
my %args = @_;
99+
100+
if ($args{nameid_format} && $args{nameid_format} eq 'urn:oasis:names:tc:SAML:2.0:nameidformat:persistent') {
101+
$args{include_name_qualifier} = 1;
102+
}
103+
104+
return $self->$orig(%args);
105+
};
106+
68107

69108
=head2 new_from_xml( ... )
70109
@@ -88,25 +127,27 @@ sub new_from_xml {
88127
my $dom = no_comments($args{xml});
89128

90129
my $xpath = XML::LibXML::XPathContext->new($dom);
91-
$xpath->registerNs('saml', 'urn:oasis:names:tc:SAML:2.0:assertion');
92-
$xpath->registerNs('samlp', 'urn:oasis:names:tc:SAML:2.0:protocol');
130+
$xpath->registerNs('saml', URN_ASSERTION);
131+
$xpath->registerNs('samlp', URN_PROTOCOL);
93132

94133
my %params = (
95-
id => $xpath->findvalue('/samlp:LogoutRequest/@ID'),
96-
session => $xpath->findvalue('/samlp:LogoutRequest/samlp:SessionIndex'),
97-
issuer => $xpath->findvalue('/samlp:LogoutRequest/saml:Issuer'),
98-
nameid => $xpath->findvalue('/samlp:LogoutRequest/saml:NameID'),
99-
destination => $xpath->findvalue('/samlp:LogoutRequest/@Destination'),
134+
id => $xpath->findvalue('/samlp:LogoutRequest/@ID'),
135+
session => $xpath->findvalue('/samlp:LogoutRequest/samlp:SessionIndex'),
136+
issuer => $xpath->findvalue('/samlp:LogoutRequest/saml:Issuer'),
137+
nameid => $xpath->findvalue('/samlp:LogoutRequest/saml:NameID'),
138+
destination => $xpath->findvalue('/samlp:LogoutRequest/@Destination'),
100139
);
101140

102-
my $nameid_format = $xpath->findvalue('/samlp:LogoutRequest/saml:NameID/@Format');
103-
if ( $nameid_format ne '' ) { $params{nameid_format} = $nameid_format; }
141+
my $nameid_format
142+
= $xpath->findvalue('/samlp:LogoutRequest/saml:NameID/@Format');
104143

105-
my $self = $class->new(
106-
%params
107-
);
144+
$params{nameid_format} = $nameid_format
145+
if NonEmptySimpleStr->check($nameid_format);
108146

109-
return $self;
147+
$params{include_name_qualifier} = $args{include_name_qualifier}
148+
if $args{include_name_qualifier};
149+
150+
return $class->new(%params);
110151
}
111152

112153
=head2 as_xml( )
@@ -116,34 +157,47 @@ Returns the LogoutRequest as XML.
116157
=cut
117158

118159
sub as_xml {
119-
my ($self) = @_;
160+
my $self = shift;
161+
162+
my $x = XML::Generator->new(':pretty=0');
163+
my $saml = ['saml' => URN_ASSERTION];
164+
my $samlp = ['samlp' => URN_PROTOCOL];
120165

121-
my $x = XML::Generator->new(':pretty');
122-
my $saml = ['saml' => 'urn:oasis:names:tc:SAML:2.0:assertion'];
123-
my $samlp = ['samlp' => 'urn:oasis:names:tc:SAML:2.0:protocol'];
124166

125167
$x->xml(
126168
$x->LogoutRequest(
127169
$samlp,
128-
{ ID => $self->id,
129-
IssueInstant => $self->issue_instant,
130-
Destination => $self->destination,
131-
Version => '2.0' },
132-
$x->Issuer(
133-
$saml,
134-
$self->issuer,
135-
),
170+
{
171+
ID => $self->id,
172+
IssueInstant => $self->issue_instant,
173+
$self->has_destination
174+
? (Destination => $self->destination)
175+
: (),
176+
Version => '2.0'
177+
},
178+
$x->Issuer($saml, $self->issuer),
136179
$x->NameID(
137180
$saml,
138-
{ Format => $self->nameid_format,
139-
NameQualifier => $self->destination,
140-
SPNameQualifier => $self->issuer },
141-
$self->nameid,
142-
),
143-
$x->SessionIndex(
144-
$samlp,
145-
$self->session,
181+
{
182+
$self->has_nameid_format
183+
? (Format => $self->nameid_format)
184+
: (),
185+
$self->has_sp_provided_id ? (
186+
SPProvidedID => $self->sp_provided_id
187+
) : (),
188+
$self->include_name_qualifier
189+
? (
190+
$self->has_destination
191+
? (NameQualifier => $self->destination)
192+
: (),
193+
SPNameQualifier =>
194+
$self->has_affiliation_group_id ? $self->affiliation_group_id : $self->issuer
195+
)
196+
: (),
197+
},
198+
$self->nameid
146199
),
200+
$x->SessionIndex($samlp, $self->session),
147201
)
148202
);
149203
}

‎lib/Net/SAML2/SP.pm

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use Crypt::OpenSSL::X509;
1010
use Digest::MD5 ();
1111
use List::Util qw(first none);
1212
use MooseX::Types::URI qw/ Uri /;
13+
use MooseX::Types::Common::String qw/ NonEmptySimpleStr /;
1314
use Net::SAML2::Binding::POST;
1415
use Net::SAML2::Binding::Redirect;
1516
use Net::SAML2::Binding::SOAP;
@@ -300,11 +301,13 @@ sub logout_request {
300301
my ($self, $destination, $nameid, $nameid_format, $session) = @_;
301302

302303
my $logout_req = Net::SAML2::Protocol::LogoutRequest->new(
303-
issuer => $self->id,
304-
destination => $destination,
305-
nameid => $nameid,
306-
$nameid_format ? (nameid_format => $nameid_format) : (),
307-
session => $session,
304+
issuer => $self->id,
305+
destination => $destination,
306+
nameid => $nameid,
307+
session => $session,
308+
NonEmptySimpleStr->check($nameid_format)
309+
? (nameid_format => $nameid_format)
310+
: (),
308311
);
309312

310313
return $logout_req;

‎t/07-logout-request.t

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use strict;
22
use warnings;
33
use Test::Lib;
44
use Test::Net::SAML2;
5+
use URN::OASIS::SAML2 qw(:urn);
56

67
use Net::SAML2::Protocol::LogoutRequest;
78

@@ -26,15 +27,60 @@ my $xml = $lor->as_xml;
2627

2728
my $xpath = get_xpath(
2829
$xml,
29-
samlp => 'urn:oasis:names:tc:SAML:2.0:protocol',
30-
saml => 'urn:oasis:names:tc:SAML:2.0:assertion',
30+
samlp => URN_PROTOCOL,
31+
saml => URN_ASSERTION,
3132
);
3233

3334
isa_ok($xpath, 'XML::LibXML::XPathContext');
3435

3536
test_xml_attribute_ok($xpath, '/samlp:LogoutRequest/@ID', qr/^NETSAML2_/);
3637
test_xml_attribute_ok($xpath, '/samlp:LogoutRequest/@IssueInstant', 'foo');
37-
test_xml_attribute_ok($xpath, '/samlp:LogoutRequest/saml:NameID/@Format',
38-
$args{nameid_format});
38+
39+
my $name_id = get_single_node_ok($xpath, '/samlp:LogoutRequest/saml:NameID');
40+
is($name_id->getAttribute('Format'), $args{nameid_format});
41+
42+
foreach (qw(NameQualifier SPNameQualifier SPProvidedID)) {
43+
is(
44+
$name_id->getAttribute($_),
45+
undef,
46+
"We don't have $_ as an attribute in the nameid"
47+
);
48+
}
49+
50+
{
51+
my $lor = Net::SAML2::Protocol::LogoutRequest->new(%args,
52+
sp_provided_id => "Some provided ID");
53+
my $xml = $lor->as_xml;
54+
55+
my $xpath = get_xpath(
56+
$xml,
57+
samlp => URN_PROTOCOL,
58+
saml => URN_ASSERTION,
59+
);
60+
my $name_id = get_single_node_ok($xpath, '/samlp:LogoutRequest/saml:NameID');
61+
is(
62+
$name_id->getAttribute('SPProvidedID'),
63+
"Some provided ID",
64+
"We have the SP provided ID"
65+
);
66+
}
67+
68+
{
69+
my $lor = Net::SAML2::Protocol::LogoutRequest->new(%args,
70+
include_name_qualifier => 1);
71+
my $xml = $lor->as_xml;
72+
73+
my $xpath = get_xpath(
74+
$xml,
75+
samlp => URN_PROTOCOL,
76+
saml => URN_ASSERTION,
77+
);
78+
my $name_id = get_single_node_ok($xpath, '/samlp:LogoutRequest/saml:NameID');
79+
is($name_id->getAttribute('SPNameQualifier'),
80+
$args{issuer}, "We the SPNameQualifier");
81+
is($name_id->getAttribute('NameQualifier'),
82+
$args{destination}, ".. and the NameQualifier");
83+
}
84+
3985

4086
done_testing;

0 commit comments

Comments
 (0)