cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Bernhardt <jbernha...@talend.com>
Subject AW: cxf-fediz git commit: [FEDIZ-168] Support SAML Token without Audience Restriction in Fediz Plugin
Date Thu, 02 Jun 2016 10:18:03 GMT
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:coheigea@apache.org]
> Gesendet: Donnerstag, 2. Juni 2016 12:10
> An: dev@cxf.apache.org
> 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 <jbernhardt@talend.com>
> 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:jbernhardt@talend.com]
> > Gesendet: Donnerstag, 2. Juni 2016 08:55
> > An: coheigea@apache.org; dev@cxf.apache.org
> > Cc: jbernhardt@apache.org
> > 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:coheigea@apache.org]
> > Gesendet: Mittwoch, 1. Juni 2016 17:52
> > An: dev@cxf.apache.org<mailto:dev@cxf.apache.org>
> > Cc: jbernhardt@apache.org<mailto:jbernhardt@apache.org>
> > 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, <jbernhardt@apache.org<mailto:
> > jbernhardt@apache.org>> 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
Mime
View raw message