What about making the audienceUris not mandatory. If no audienceUris are provided a SAML token without audience restriction will be accepted. If audienceUris are set, an audience restriction in the saml token must be present?
Best regards Jan Von: Jan Bernhardt [mailto:[email protected]] Gesendet: Donnerstag, 2. Juni 2016 08:55 An: [email protected]; [email protected] Cc: [email protected] Betreff: AW: cxf-fediz git commit: [FEDIZ-168] Support SAML Token without Audience Restriction in Fediz Plugin Hi Colm, thanks for pointing out my error. I’ll fix it ASAP. I’m also fine with enforcing an audience restriction by default, and provide an option to disable this behavior. I will first fix the validAudience change and then later add the configuration support. Best regards. Jan Von: Colm O hEigeartaigh [mailto:[email protected]] Gesendet: Mittwoch, 1. Juni 2016 17:52 An: [email protected]<mailto:[email protected]> Cc: [email protected]<mailto:[email protected]> Betreff: Re: cxf-fediz git commit: [FEDIZ-168] Support SAML Token without Audience Restriction in Fediz Plugin Hi Jan, - boolean validAudience = false; - if (audience != null) { + boolean validAudience = audience == null; + if (validAudience) { This commit means that if the audience is non-null, it won't actually be validated. The boolean return from this method is not actually checked, which explains why the tests didn't fail I suppose. I'm a bit uncomfortable with the change in logic here in general, as it is weakening the default security validation of the RP. How about doing something like adding a "required=true/false" attribute to the "audienceUris" configuration item? If it is true (default), then the token must match one of the given audienceURIs. Otherwise, it must only match if the token contains an audience restriction value and there are audienceURIs specified. Colm. On Wed, Jun 1, 2016 at 10:44 AM, <[email protected]<mailto:[email protected]>> wrote: ---------------------------------------------------------------------- .../java/org/apache/cxf/fediz/core/handler/SigninHandler.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/2d8d8e64/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninHandler.java ---------------------------------------------------------------------- diff --git a/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninHandler.java b/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninHandler.java index 5119196..00f3559 100644 --- a/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninHandler.java +++ b/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninHandler.java @@ -111,9 +111,9 @@ public class SigninHandler<T> implements RequestHandler<T> { protected boolean validateAudienceRestrictions(String audience, String requestURL) { // Validate the AudienceRestriction in Security Token (e.g. SAML) - // against the configured list of audienceURIs - boolean validAudience = false; - if (audience != null) { + boolean validAudience = audience == null; + if (validAudience) { + // validate against the configured list of audienceURIs List<String> audienceURIs = fedizContext.getAudienceUris(); for (String a : audienceURIs) { if (audience.startsWith(a)) { -- Colm O hEigeartaigh Talend Community Coder http://coders.talend.com
