Re: [jetty-dev] Better default identity handling

[Greg Wilkins <gregw@xxxxxxxxxxx>]

David,

I have no problem with this in general... but there is a practical problem.

Because of the DefferedAuthentication type and the ability of application
code to call the new authenticate methods, this now means that an associate
might happen not from the SecurityHandler call.    This causes two problem:

1) How does the security handler get hold of the returned Object so that
disassociate can be called?

2) What happens if an async handler calls authenticate outside of the
scope of the security Handler (hmmm I think this is a problem anyway).


So I'm not sure if you can have a null auth meaning no established
identity.  I think you always need to have a DeferredAuthentication.

Ping me in your morning and we can discuss on IRC.

cheers



David Jencks wrote:
> In geronimo we're having a problem installing default identity on
> threads that don't actually authenticate.  After studying the situation
> for a while I think the following solution is about the best possible. 
> It lets the IdentityService be notified whenever a request is going to
> be handled: if the identity passed in is null it is free to do whatever
> it wants such as establish a default identity.   Since "null" now means
> "no established identity" we also need a disassociate method, and this
> give the opportunity to restore whatever context info might have been
> previously present.  Thoughts or objections?  The Geronimo issue
> tracking our problems is 
> https://issues.apache.org/jira/browse/GERONIMO-4756
> 
> many thanks
> david jencks
> 
> Index:
> jetty-security/src/main/java/org/eclipse/jetty/security/SecurityHandler.java
> 
> ===================================================================
> ---
> jetty-security/src/main/java/org/eclipse/jetty/security/SecurityHandler.java       
> (revision 578)
> +++
> jetty-security/src/main/java/org/eclipse/jetty/security/SecurityHandler.java       
> (working copy)
> @@ -408,6 +408,7 @@
>              boolean isAuthMandatory = isAuthMandatory(baseRequest,
> base_response, constraintInfo);
> 
>              // check authentication
> +            Object previousIdentity = null;
>              try
>              {
>                  final Authenticator authenticator = _authenticator;
> @@ -429,7 +430,7 @@
>                  {
>                      Authentication.User userAuth =
> (Authentication.User)authentication;
>                      baseRequest.setAuthentication(authentication);
> -                   
> _identityService.associate(userAuth.getUserIdentity());
> +                    previousIdentity =
> _identityService.associate(userAuth.getUserIdentity());
> 
>                      if (isAuthMandatory)
>                      {
> @@ -467,10 +468,12 @@
>                      }
>                      else
>                          authenticator.secureResponse(request, response,
> isAuthMandatory, null);
> +                    //TODO fish previousIdentity out of something.
>                  }
>                  else
>                  {
>                      baseRequest.setAuthentication(authentication);
> +                    previousIdentity = _identityService.associate(null);
>                      handler.handle(pathInContext, baseRequest, request,
> response);
>                      authenticator.secureResponse(request, response,
> isAuthMandatory, null);
>                  }
> @@ -483,7 +486,7 @@
>              }
>              finally
>              {
> -                _identityService.associate(null);
> +                _identityService.disassociate(previousIdentity);
>              }
>          }
>          else
> Index:
> jetty-security/src/main/java/org/eclipse/jetty/security/IdentityService.java
> 
> ===================================================================
> ---
> jetty-security/src/main/java/org/eclipse/jetty/security/IdentityService.java       
> (revision 578)
> +++
> jetty-security/src/main/java/org/eclipse/jetty/security/IdentityService.java       
> (working copy)
> @@ -37,7 +37,9 @@
>       * method and then again with a null argument as that call exits.
>       * @param user The current user or null for no user to associated.
>       */
> -    void associate(UserIdentity user);
> +    Object associate(UserIdentity user);
> +
> +    void disassociate(Object previous);
> 
>      /* ------------------------------------------------------------ */
>      /**
> Index:
> jetty-security/src/main/java/org/eclipse/jetty/security/DefaultIdentityService.java
> 
> ===================================================================
> ---
> jetty-security/src/main/java/org/eclipse/jetty/security/DefaultIdentityService.java
> (revision 578)
> +++
> jetty-security/src/main/java/org/eclipse/jetty/security/DefaultIdentityService.java
> (working copy)
> @@ -42,10 +42,14 @@
>       * If there are roles refs present in the scope, then wrap the
> UserIdentity
>       * with one that uses the role references in the {@link
> UserIdentity#isUserInRole(String)}
>       */
> -    public void associate(UserIdentity user)
> +    public Object associate(UserIdentity user)
>      {
> +        return null;
>      }
> 
> +    public void disassociate(Object previous) {
> +    }
> +
>      public Object setRunAs(UserIdentity user, RunAsToken token)
>      {
>          return token;
> _______________________________________________
> jetty-dev mailing list
> jetty-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/jetty-dev