Shibboleth Developers

[Tom Scavo <>]

comments: draft-mace-shibboleth-arch-protocols-10

(I apologize in advance for the lateness of these comments.)

Bugs:

- [footer, lines 1--2] Join these lines.

- [line 123] s|step 5|steps 5 and 6|

- [line 167] s|[SAMLCore]|the SAML Assertions and Protocols
specification [SAMLCore]|

- [line 167] s|SAML Browser/POST or Browser/Artifact|browser|

- [line 168] s|[SAMLBind]|the Bindings and Profiles specification [SAMLBind]|

- [line 174] s|3.4|3.4.2|

- [lines 201--202] Insert non-breaking space between "section" and "3.2."

- [line 239] s|3.4|3.4.2|

- [line 254] s|rely|relying|

- [line 285] s|profiles|profile|

- [lines 352--353] This is only possible with Browser/POST, right?

- [line 355] s|The|The time|

- [line 360] s|shibboleth%2F|shibboleth|

- [line 467] s|values|value|

- [line 478] s|receive,|receive|

- [line 486] s|lookup,|lookup|

- [lines 502] s|element,|element|

- [line 531] s|http|https|

- [line 571] s|http|https|

- [line 605] s|([Schema2])|[Schema2]|

- [line 707] s|and|and the TARGET parameter|

- [line 763] The label [Schema2] is the wrong font.

- [line 769] The label [LibertyProt] is the wrong font.

Suggestions:

- Add prefixes xsi: and saml2: to section 1.1.

- [line 124] s|SAML request|back-channel request|

- [lines 134--135] Replace these lines with the phrase "wishes to
delegate the task of identity provider discovery (see section 2.4)."

- [line 137] s|then it|it|

- Discuss "Attribute Push" briefly in a separate subsection of section
2.1 or at the end of the step-by-step description on p.5.

- Move subsection 2.1 to the end of section 2.

- [line 222] Append a comma and the phrase "in which case the redirect
mentioned in section 2.2.3 is unnecessary."

- [line 225] s|requests from|requests directly from|

- [lines 232--233] s|the SAML Browser/POST or Browser/Artifact
profiles|a SAML browser profile [SAMLBind]|

- If you're going to include the note in section 2.3, you should also
include a note in section 2.2.3.

- [line 242] s|SAML Browser/POST|Browser/POST|

- [line 243] s|SAML Browser/Artifact|Browser/Artifact|

- [line 244] s|it|the assertion consumer service|

- [lines 245--247] Replace the note with "[SAMLBind] refers to an
assertion consumer service that supports the Browser/Artifact profile
as an artifact receiver service.  In this specification, no such
distinction is made."

- [line 252] s|SAML <samlp:Request> messages|a SAML <samlp:Request> message|

- [line 253] s|the <samlp:AttributeQuery>|a <samlp:AttributeQuery>|

- [lines 309--311] Replace these lines with "The URL of a resource
accessed at the service provider (although an opaque reference MAY be
used) returned by the identity provider as the TARGET parameter in the
response".

- [line 322] Delete the parenthetical remark.

- [line 366] s|request; in such a|request, in which|

- [line 370] s|message|message (so-called attribute push)|

- [line 465] s|and transmitted in|by way of|

- [line 486] s|this|attribute push|

- [line 487] s|this profile within Shibboleth|this profile|

- [lines 504--506] Delete the first sentence (which is redundant) and
join the remaining sentence with line 503.

- [lines 515] s|Additionally, the|The|

- [lines 514--515] Join these two paragraphs.

- [line 541] s|is issuing|issued|

- [line 617] s|SAML|SAML 1.1|

- [line 636] s|includes a metadata specification in|introduces a
metadata specification|

- [line 637] s|this|the SAML 2.0|

- [line 638] s|(see [SAML1Meta])|[SAML1Meta]|

- [line 644] Append the sentence "The value of the Name attribute is a
globally unique reference to the collection of entities enclosed by
the element."

- [line 651] Append the sentence "Other elements of type
md:RoleDescriptorType may be defined and supported, but are beyond the
scope of this specification.

- [line 664] s|value:|value|

- [line 666] s|The|and moreover,|

- [line 666] s|profile|Profile|

- [lines 679--680] Replace these lines with the sentence "Any
<saml2:Attribute> elements SHOULD follow the guidelines on attribute
naming and syntax in section 3.2.4."

- [lines 685--686] Replace these lines with the sentence "Any
<md:RequestedAttribute> elements SHOULD follow the guidelines on
attribute naming and syntax in section 3.2.4."

- [line 710] s|and possibly log files||

Questions/Comments:

- [lines 119--120] This sentence may be true of IdP discovery in
general, but is it true of Shib deployments in particular?

- Browser/Artifact is not referred to in the diagram on p.4, which is
fine.  The remark in the opening paragraph of section 2.1 refers the
reader to [SAMLBind] for more detail regarding Browser/Artifact, which
is also fine.  Browser/Artifact is described in some detail in step 5
on p.5, which unfortunately is confusing.  For clarity, I suggest the
references to Browser/Artifact in step 5 be removed.

- After removing the references to Browser/Artifact from step 5 on
p.5, you have three options:

1) do nothing further re Browser/Artifact in section 2.1.
2) incorporate Browser/Artifact into both the diagram on p.4 and the
step-by-step description on p.5.
3) break section 2.1 into two subsections covering Browser/POST and
Browser/Artifact separately.

- If you choose the latter option, you could end up with something like

2.1.1  Browser/POST Message Flow
2.1.2  Attribute Push
2.1.3  Browser/Artifact Message Flow

- [lines 206--207] Does Shib (the implementation) actually do this?  I
thought the ITS was gone (or subsumed by the SSO service, however you
want to look at it)?

- [line 624] So NameQualifier is optional?  If it is omitted, it
obviously can't be equal to the Issuer attribute.  Moreover, shouldn't
this requirement be true of ALL NameIdentifier formats?

- [line 659] Why MUST a Shibboleth IdP "include the
<md:IDPSSODescriptor> element in its metadata?  A GridShib IdP, for
example, formulates metadata that does not contain this element.

- [lines 675--676] Although I don't have a use case, the keyword
"MUST" in the opening sentence of section 3.4.5 may be overspecified.

- [line 682] Why MUST a Shibboleth SP "include the
<md:SPSSODescriptor> element in its metadata?  An SP (such as a grid
SP) that wishes to advertise only its attribute requesting ability
does not need to specify this element.

- [lines 724--725] Should the WAYF be included in this sentence?

- In section 5.1, the labels [SAMLMeta-xsd] and [SAML1Meta] do not
agree.  Should the latter be called [SAMLMeta]?