Should perform audience checking.

[jaredhanson]

This strategy doesn't check the audience of the assertion, opening up a potential security hole in which a user is authenticated using an assertion that was intended for a different service provider.

[sonnymai]

I ran into this today as well. I think something like this will check for the audience (assuming it's the same as the issuer) sonnymai@578c6dd

[ghost]

Wont this be solved by recipient?

[srd90]

@markstos isn't this issue already fixed?
First pull request seems to be #138 which was later closed in favor of #253

[srd90]

AFAIK there is going to be another major version upgrade due separation of passport-saml and node-saml.

It could be a good place to make audience checking ( which was implemented at #253 ) mandatory configuration value. At the moment someone could replay one login response to multiple service providers which use passport-saml (and which have not enabled audience checking) as long as those SPs use same IdP. Replaying is possible until notOnOrAfter is passed (assuming that developer has not turned off that check by setting acceptedClockSkewMs value to negative value in which case that captured login response can be reused until IdP's signing certificate expires…maybe few years).

Replaying possibility opens up also scenarios where user could get authenticated login session to system where he/she should not have access in the first place (i.e. IdP would not have allowed login to that particular system due lack of some privilege etc.).

Safe default for audience checking would be to require it and developer would have to explicitly turn it off (although I can't imagine any valid reason to not validate audience).

ping @markstos

[cjbarth]

I believe this has been completed in node-saml/node-saml#25