RE: [security-services] Comments on saml-session-token-v1.0-wd04

[Hal Lockhart <hal.lockhart@oracle.com>]

I have uploaded wd05 which I believe resolves all of these points. 

Responses inline.

> -----Original Message-----
> From: Cantor, Scott E. [mailto:cantor.2@osu.edu]
> Sent: Wednesday, February 09, 2011 1:20 PM
> To: Hal Lockhart; security-services@lists.oasis-open.org
> Subject: [security-services] Comments on saml-session-token-v1.0-wd04
> 
> 
> Throughout, s/http/HTTP when the protocol itself is meant.

done

> 
> Front matter: you're defining a new XML namespace, so OASIS 
> will demand that you add that to the front boilerplate, and 
> of course provide a schema artifact (maybe that was in your 
> ZIP, I didn't look).

Well I thought I had posted the .xsd, but I actually posted a different one. However the xsd is not in the repository and referenced from the wiki. 

> 
> Sec 3.1:
> 
> Line 157: <Conditions> should be <saml:Conditions> and in the 
> right font.

done
> 
> In that fourth bullet, I still think we need to either say 
> "only the NotBefore/NotOnOrAfter can be present", or 
> alternatively we need to say something to the effect that the 
> assertion MUST be valid with respect to SAML core (there's a 
> specific section there on validity). It can't be optional to 
> honor other critical content of the assertion, so if you want 
> to obviate the need, the only way is by precluding it being there. 

changed wording to require validation as per section 2.5 of core.
> 
> As a normative suggestion, since you're requiring 
> SubjectConfirmation anyway, why not just preclude Conditions 
> outright, and use the NotBefore/NotOnOrAfter fields of the 
> SubjectConfirmationData? That might be simplest.

I want to permit other Conditions sub-elements, for example, AudienceRestriction so I stuck with Conditions.
> 
> In 6 and 7, I think the references to the TimeLastActive and 
> AuthnInstant attributes probably ought to be more formalized, 
> maybe referring to the full URI names.

done

> 
> In 10, I think by IssueInstant you mean the XML attribute in 
> the assertion, so I'd s/Attribute/XML attribute.

done there and elsewhere

> 
> Sec 4.2:
> 
> The XML attributes there need to be unqualified:
> 
> s/saml:Version/Version
> s/saml:IssueInstant/IssueInstant
> s/saml:Method/Method
> s/saml:Address/Address
> s/saml:NotBefore/NotBefore
> s/saml:NotOnOrAfter/NotOnOrAfter

done

> 
> The NameID text still seems off to me because it reads as 
> "MUST create a <saml:Subject> containing the following..." 
> and then mentions NameID. I think you have to expicitly say 
> for each part of the subject whether the child is required or not.

modified initial statement

> 
> Sec 4.3:
> 
> s/saml:AuthnInstant/AuthnInstant
> 

done 

> Sec 5.1 and 6:
> 
> Looks like some references got embedded as footnote-style 
> numbers, rather than reference labels. Might want to fix that.

fixed these and others

> 
> Sec 7.2:
> 
> Your CookieNameType shouldn't be an abstract type, and you 
> need to define a CookieName element of that type.
> 
> <element name="CookieName type="mdsess:CookieNameType"/>
> 
> Sec 7.3:
> 
> The role extension type is also mistakenly abstract.

Scott, thanks for your off-list help on the schema. I believe it is correct now.

Hal