You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shiro.apache.org by Les Hazlewood <lh...@apache.org> on 2011/08/13 22:00:26 UTC
Re: svn commit: r1157377 - in /shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc:
AuthenticatingFilter.java BasicHttpAuthenticationFilter.java
Hi Jared,
Any reason why this logic can't/shouldn't be in the 'afterCompletion'
method - the safeguards in place for cleanup should also take care of
afterCompletion calls.
In other words, afterCompletion was meant to be overridden by
subclasses whereas cleanup really wasn't.
Thoughts?
Les
On Sat, Aug 13, 2011 at 6:34 AM, <jb...@apache.org> wrote:
> Author: jbunting
> Date: Sat Aug 13 13:34:44 2011
> New Revision: 1157377
>
> URL: http://svn.apache.org/viewvc?rev=1157377&view=rev
> Log:
> SHIRO-283: adding ability to specify "permissive" for authc and authcBasic filters. This will cause unauthenticated users to not be blocked, but will perform appropriate login request (redirect or challenge response) when an UnauthenticatedException is thrown.
>
> Modified:
> shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/AuthenticatingFilter.java
> shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java
>
> Modified: shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/AuthenticatingFilter.java
> URL: http://svn.apache.org/viewvc/shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/AuthenticatingFilter.java?rev=1157377&r1=1157376&r2=1157377&view=diff
> ==============================================================================
> --- shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/AuthenticatingFilter.java (original)
> +++ shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/AuthenticatingFilter.java Sat Aug 13 13:34:44 2011
> @@ -21,10 +21,14 @@ package org.apache.shiro.web.filter.auth
> import org.apache.shiro.authc.AuthenticationException;
> import org.apache.shiro.authc.AuthenticationToken;
> import org.apache.shiro.authc.UsernamePasswordToken;
> +import org.apache.shiro.authz.UnauthenticatedException;
> import org.apache.shiro.subject.Subject;
>
> +import javax.servlet.ServletException;
> import javax.servlet.ServletRequest;
> import javax.servlet.ServletResponse;
> +import java.io.IOException;
> +import java.util.Arrays;
>
> /**
> * An <code>AuthenticationFilter</code> that is capable of automatically performing an authentication attempt
> @@ -33,6 +37,7 @@ import javax.servlet.ServletResponse;
> * @since 0.9
> */
> public abstract class AuthenticatingFilter extends AuthenticationFilter {
> + public static final String PERMISSIVE = "permissive";
>
> //TODO - complete JavaDoc
>
> @@ -104,4 +109,50 @@ public abstract class AuthenticatingFilt
> protected boolean isRememberMe(ServletRequest request) {
> return false;
> }
> +
> + /**
> + * Determines whether the current subject should be allowed to make the current request.
> + * <p/>
> + * The default implementation returns <code>true</code> if the user is authenticated. Will also return
> + * <code>true</code> if the {@link #isLoginRequest} returns false and the "permissive" flag is set.
> + *
> + * @return <code>true</code> if request should be allowed access
> + */
> + @Override
> + protected boolean isAccessAllowed(ServletRequest request, ServletResponse response, Object mappedValue) {
> + return super.isAccessAllowed(request, response, mappedValue) ||
> + (!isLoginRequest(request, response) && isPermissive(mappedValue));
> + }
> +
> + /**
> + * Returns <code>true</code> if the mappedValue contains the {@link #PERMISSIVE} qualifier.
> + *
> + * @return <code>true</code> if this filter should be permissive
> + */
> + protected boolean isPermissive(Object mappedValue) {
> + if(mappedValue != null) {
> + String[] values = (String[]) mappedValue;
> + return Arrays.binarySearch(values, PERMISSIVE) >= 0;
> + }
> + return false;
> + }
> +
> + /**
> + * Overrides the default behavior to call {@link #onAccessDenied} and swallow the exception if the exception is
> + * {@link UnauthenticatedException}.
> + */
> + @Override
> + protected void cleanup(ServletRequest request, ServletResponse response, Exception existing) throws ServletException, IOException {
> + if (existing instanceof UnauthenticatedException || (existing instanceof ServletException && existing.getCause() instanceof UnauthenticatedException))
> + {
> + try {
> + onAccessDenied(request, response);
> + existing = null;
> + } catch (Exception e) {
> + existing = e;
> + }
> + }
> + super.cleanup(request, response, existing);
> +
> + }
> }
>
> Modified: shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java
> URL: http://svn.apache.org/viewvc/shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java?rev=1157377&r1=1157376&r2=1157377&view=diff
> ==============================================================================
> --- shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java (original)
> +++ shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java Sat Aug 13 13:34:44 2011
> @@ -212,6 +212,14 @@ public class BasicHttpAuthenticationFilt
> }
>
> /**
> + * Delegates to {@link #isLoginAttempt(javax.servlet.ServletRequest, javax.servlet.ServletResponse) isLoginAttempt}.
> + */
> + @Override
> + protected final boolean isLoginRequest(ServletRequest request, ServletResponse response) {
> + return this.isLoginAttempt(request, response);
> + }
> +
> + /**
> * Returns the {@link #AUTHORIZATION_HEADER AUTHORIZATION_HEADER} from the specified ServletRequest.
> * <p/>
> * This implementation merely casts the request to an <code>HttpServletRequest</code> and returns the header:
Re: svn commit: r1157377 - in /shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc:
AuthenticatingFilter.java BasicHttpAuthenticationFilter.java
Posted by Jared Bunting <ja...@peachjean.com>.
On 08/15/2011 10:41 AM, Les Hazlewood wrote:
> I'm confused as to why this wouldn't work in afterCompletion.
> afterCompletion is guaranteed to be called at the end of the request
> (in all cases) and is meant to perform subclass-specific final/cleanup
> type of work that you describe.
>
> The cleanup method is primarily infrastructural - its only job is to
> ensure that 1) afterCompletion is always called and 2) if any
> exception is thrown, either originally or as a result of
> afterCompletion, that it is wrapped in a ServletException if it is not
> already an IOException or ServletException.
>
> Based on this, it makes sense to me to have AuthenticatingFilter
> override afterCompletion (it doesn't currently) to do the following:
>
> if (existing instanceof UnauthenticatedException || (existing
> instanceof ServletException && existing.getCause() instanceof
> UnauthenticatedException)) {
> onAccessDenied(request,response);
> }
>
> Thoughts?
>
> Cheers,
>
> Les
But, because of that 2nd guarantee of cleanup, there's no way for me to
actually catch the exception - without rethrowing it. The code you
propose would set return values appropriately, but the exception would
still be propagated up the filter chain - even when it's already been
acknowledged and handled.
Thanks,
Jared
Re: svn commit: r1157377 - in /shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc:
AuthenticatingFilter.java BasicHttpAuthenticationFilter.java
Posted by Les Hazlewood <lh...@apache.org>.
I'm confused as to why this wouldn't work in afterCompletion.
afterCompletion is guaranteed to be called at the end of the request
(in all cases) and is meant to perform subclass-specific final/cleanup
type of work that you describe.
The cleanup method is primarily infrastructural - its only job is to
ensure that 1) afterCompletion is always called and 2) if any
exception is thrown, either originally or as a result of
afterCompletion, that it is wrapped in a ServletException if it is not
already an IOException or ServletException.
Based on this, it makes sense to me to have AuthenticatingFilter
override afterCompletion (it doesn't currently) to do the following:
if (existing instanceof UnauthenticatedException || (existing
instanceof ServletException && existing.getCause() instanceof
UnauthenticatedException)) {
onAccessDenied(request,response);
}
Thoughts?
Cheers,
Les
Re: svn commit: r1157377 - in /shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc:
AuthenticatingFilter.java BasicHttpAuthenticationFilter.java
Posted by Jared Bunting <ja...@peachjean.com>.
I can't see any way for it to work in the 'afterCompletion' method. No
matter what I do in there, the exception will still get thrown, and this
functionality needs to fully catch the exception.
That being said, I can see how cleanup doesn't really appear to be meant
for overriding. Perhaps a more appropriate approach would be to add a
"handleException" sort of method?
I'm thinking of something like this in AdviceFilter:
public void doFilterInternal(ServletRequest request, ServletResponse
response, FilterChain chain)
throws ServletException, IOException {
Exception exception = null;
try {
..........
} catch (Exception e) {
exception = handleException(request, response, e);
} finally {
cleanup(request, response, exception);
}
}
protected Exception handleException(ServletRequest request,
ServletResponse response, Exception e) {
return e;
}
Alternatively, it could go in the cleanup method:
protected void cleanup(ServletRequest request, ServletResponse
response, Exception existing)
throws ServletException, IOException {
Exception exception = existing;
try {
afterCompletion(request, response, exception);
if (log.isTraceEnabled()) {
log.trace("Successfully invoked afterCompletion method.");
}
} catch (Exception e) {
if (exception == null) {
exception = e;
} else {
log.debug("afterCompletion implementation threw an
exception. This will be ignored to " +
"allow the original source exception to be
propagated.", e);
}
}
handleException(e);
}
protected void handleException(ServletRequest request,
ServletResponse response, Exception exception) throws ServletException,
IOException {
if (exception != null) {
if (exception instanceof ServletException) {
throw (ServletException) exception;
} else if (exception instanceof IOException) {
throw (IOException) exception;
} else {
if (log.isDebugEnabled()) {
String msg = "Filter execution resulted in an
unexpected Exception " +
"(not IOException or ServletException as the
Filter API recommends). " +
"Wrapping in ServletException and propagating.";
log.debug(msg);
}
throw new ServletException(exception);
}
}
}
Thoughts? Basically, the goal is to catch an exception occurring
further down the chain, and handle it, without propagating it to the user.
Thanks,
Jared
On 08/13/2011 03:00 PM, Les Hazlewood wrote:
> Hi Jared,
>
> Any reason why this logic can't/shouldn't be in the 'afterCompletion'
> method - the safeguards in place for cleanup should also take care of
> afterCompletion calls.
>
> In other words, afterCompletion was meant to be overridden by
> subclasses whereas cleanup really wasn't.
>
> Thoughts?
>
> Les
>
> On Sat, Aug 13, 2011 at 6:34 AM, <jb...@apache.org> wrote:
>> Author: jbunting
>> Date: Sat Aug 13 13:34:44 2011
>> New Revision: 1157377
>>
>> URL: http://svn.apache.org/viewvc?rev=1157377&view=rev
>> Log:
>> SHIRO-283: adding ability to specify "permissive" for authc and authcBasic filters. This will cause unauthenticated users to not be blocked, but will perform appropriate login request (redirect or challenge response) when an UnauthenticatedException is thrown.
>>
>> Modified:
>> shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/AuthenticatingFilter.java
>> shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java
>>
>> Modified: shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/AuthenticatingFilter.java
>> URL: http://svn.apache.org/viewvc/shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/AuthenticatingFilter.java?rev=1157377&r1=1157376&r2=1157377&view=diff
>> ==============================================================================
>> --- shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/AuthenticatingFilter.java (original)
>> +++ shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/AuthenticatingFilter.java Sat Aug 13 13:34:44 2011
>> @@ -21,10 +21,14 @@ package org.apache.shiro.web.filter.auth
>> import org.apache.shiro.authc.AuthenticationException;
>> import org.apache.shiro.authc.AuthenticationToken;
>> import org.apache.shiro.authc.UsernamePasswordToken;
>> +import org.apache.shiro.authz.UnauthenticatedException;
>> import org.apache.shiro.subject.Subject;
>>
>> +import javax.servlet.ServletException;
>> import javax.servlet.ServletRequest;
>> import javax.servlet.ServletResponse;
>> +import java.io.IOException;
>> +import java.util.Arrays;
>>
>> /**
>> * An <code>AuthenticationFilter</code> that is capable of automatically performing an authentication attempt
>> @@ -33,6 +37,7 @@ import javax.servlet.ServletResponse;
>> * @since 0.9
>> */
>> public abstract class AuthenticatingFilter extends AuthenticationFilter {
>> + public static final String PERMISSIVE = "permissive";
>>
>> //TODO - complete JavaDoc
>>
>> @@ -104,4 +109,50 @@ public abstract class AuthenticatingFilt
>> protected boolean isRememberMe(ServletRequest request) {
>> return false;
>> }
>> +
>> + /**
>> + * Determines whether the current subject should be allowed to make the current request.
>> + * <p/>
>> + * The default implementation returns <code>true</code> if the user is authenticated. Will also return
>> + * <code>true</code> if the {@link #isLoginRequest} returns false and the "permissive" flag is set.
>> + *
>> + * @return <code>true</code> if request should be allowed access
>> + */
>> + @Override
>> + protected boolean isAccessAllowed(ServletRequest request, ServletResponse response, Object mappedValue) {
>> + return super.isAccessAllowed(request, response, mappedValue) ||
>> + (!isLoginRequest(request, response) && isPermissive(mappedValue));
>> + }
>> +
>> + /**
>> + * Returns <code>true</code> if the mappedValue contains the {@link #PERMISSIVE} qualifier.
>> + *
>> + * @return <code>true</code> if this filter should be permissive
>> + */
>> + protected boolean isPermissive(Object mappedValue) {
>> + if(mappedValue != null) {
>> + String[] values = (String[]) mappedValue;
>> + return Arrays.binarySearch(values, PERMISSIVE) >= 0;
>> + }
>> + return false;
>> + }
>> +
>> + /**
>> + * Overrides the default behavior to call {@link #onAccessDenied} and swallow the exception if the exception is
>> + * {@link UnauthenticatedException}.
>> + */
>> + @Override
>> + protected void cleanup(ServletRequest request, ServletResponse response, Exception existing) throws ServletException, IOException {
>> + if (existing instanceof UnauthenticatedException || (existing instanceof ServletException && existing.getCause() instanceof UnauthenticatedException))
>> + {
>> + try {
>> + onAccessDenied(request, response);
>> + existing = null;
>> + } catch (Exception e) {
>> + existing = e;
>> + }
>> + }
>> + super.cleanup(request, response, existing);
>> +
>> + }
>> }
>>
>> Modified: shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java
>> URL: http://svn.apache.org/viewvc/shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java?rev=1157377&r1=1157376&r2=1157377&view=diff
>> ==============================================================================
>> --- shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java (original)
>> +++ shiro/trunk/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java Sat Aug 13 13:34:44 2011
>> @@ -212,6 +212,14 @@ public class BasicHttpAuthenticationFilt
>> }
>>
>> /**
>> + * Delegates to {@link #isLoginAttempt(javax.servlet.ServletRequest, javax.servlet.ServletResponse) isLoginAttempt}.
>> + */
>> + @Override
>> + protected final boolean isLoginRequest(ServletRequest request, ServletResponse response) {
>> + return this.isLoginAttempt(request, response);
>> + }
>> +
>> + /**
>> * Returns the {@link #AUTHORIZATION_HEADER AUTHORIZATION_HEADER} from the specified ServletRequest.
>> * <p/>
>> * This implementation merely casts the request to an <code>HttpServletRequest</code> and returns the header: