Hi Colm, ok let's start with your suggested more restrictive behavior. If the need increases to weaken this behavior we can still provide a change for that.
I'll push my changes in a minut which should cover your suggestions. I would be happy if you could review my changes and provide feedback to me. Best regards Jan > -----Ursprüngliche Nachricht----- > Von: Colm O hEigeartaigh [mailto:[email protected]] > Gesendet: Donnerstag, 2. Juni 2016 12:10 > An: [email protected] > Betreff: Re: cxf-fediz git commit: [FEDIZ-168] Support SAML Token without > Audience Restriction in Fediz Plugin > > Hi Jan, > > Sounds good. I'd go a bit further though and say that if an audience is > present in the SAML token, and no audienceURIs are set in the configuration, > then an error should be thrown. WDYT? > > Colm. > > On Thu, Jun 2, 2016 at 7:58 AM, Jan Bernhardt <[email protected]> > wrote: > > > 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.ja > > va > > ---------------------------------------------------------------------- > > diff --git > > a/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninH > > andler.java > > b/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninH > > andler.java > > index 5119196..00f3559 100644 > > --- > > a/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninH > > andler.java > > +++ > > b/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninH > > andler.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 > > > > > > -- > Colm O hEigeartaigh > > Talend Community Coder > http://coders.talend.com
