cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Colm O hEigeartaigh <cohei...@apache.org>
Subject Re: cxf-fediz git commit: [FEDIZ-168] Support SAML Token without Audience Restriction in Fediz Plugin
Date Thu, 02 Jun 2016 10:10:12 GMT
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.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
>



-- 
Colm O hEigeartaigh

Talend Community Coder
http://coders.talend.com

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message