[wildfly-dev] Regression after WFLY-5298; request#authenticate does nothing now
arjan tijms
arjan.tijms at gmail.com
Mon Sep 28 10:58:13 EDT 2015
Hi,
On Mon, Sep 28, 2015 at 7:49 AM, Stuart Douglas
<stuart.w.douglas at gmail.com> wrote:
> Can you verify that this PR: https://github.com/wildfly/wildfly/pull/8204
> fixes your issues?
I did a quick test run, but now really everything breaks.
In UndertowDeploymentInfoService the following line occurs:
deploymentInfo.setJaspiAuthenticationMechanism(new
JASPIAuthenticationMechanism(authMethod, securityDomain));
But the constructor arguments are reversed.
The arguments are defined as:
public JASPIAuthenticationMechanism(final String securityDomain,
final String configuredAuthMethod) {
When I reverse the arguments to their correct order, the same tests as
before fail unfortunately.
This individual case can be rather easily debugged manually by cloning
the project using Eclipse/JBoss tools from
https://github.com/javaee-samples/javaee7-samples Then importing just
the jaspic and test-utils directories as Maven projects, and then
deploying "jaspic-basic-authentication" using Add and Remove on the
server.
If you then request:
http://localhost:8080/jaspic-basic-authentication/public/servlet?doLogin
You should see the same output as:
http://localhost:8080/jaspic-basic-authentication/protected/servlet?doLogin
Hope this helps
Kind regards,
Arjan Tijms
>
> Basically it reverts the recent changes, and instead installs a handler
> after authenticate to handle requests that do not require authentication.
>
> Stuart
>
> On Mon, 28 Sep 2015 at 09:54 Jason T. Greene <jason.greene at redhat.com>
> wrote:
>>
>> The MIT license also grants the ability to relicense.
>>
>> On Sep 27, 2015, at 6:50 PM, Stuart Douglas <stuart.w.douglas at gmail.com>
>> wrote:
>>
>> This was deliberately disabled to match the behavior of EAP6. It can be
>> controlled by the proactive-authentication attribute on the servlet
>> container. These JAPIC changes have been driven by this change, in order to
>> get the TCK to pass without proactive auth. I will look into it today.
>>
>> If you can contribute your tests it would be greatly appreciated, we do
>> not have much coverage (and apparently neither does the TCK, as it does not
>> pick up these issues). As I understand it if you are the sole author you
>> retain copyright and can re-license them under whatever license you like.
>>
>> Stuart
>>
>> On Sat, 26 Sep 2015 at 01:15 arjan tijms <arjan.tijms at gmail.com> wrote:
>>>
>>> Hi,
>>>
>>> I discovered some more issues originating from 5298:
>>>
>>> pre-emptive authentication on a public page doesn't work anymore
>>> either. It still worked fine in WildFly 9.0.1.
>>>
>>> This can be easily seen when running the JASPIC tests from
>>> https://github.com/javaee-samples/javaee7-samples/tree/master/jaspic
>>>
>>> For the basic authentication tests, the following now fail:
>>>
>>> Failed tests:
>>>
>>> testPublicPageNotRememberLogin(org.javaee7.jaspic.basicauthentication.BasicAuthenticationPublicTest)
>>>
>>> testPublicPageLoggedin(org.javaee7.jaspic.basicauthentication.BasicAuthenticationPublicTest)
>>>
>>> These tests don't rely on request#authenticate, but depend on
>>> automatic calling of a SAM at the beginning of a request. After manual
>>> inspection it's clear that the SAM is called, but its outcome is not
>>> being applied.
>>>
>>> Kind regards,
>>> Arjan
>>>
>>>
>>>
>>>
>>>
>>> On Fri, Sep 25, 2015 at 3:18 PM, arjan tijms <arjan.tijms at gmail.com>
>>> wrote:
>>> > Hi,
>>> >
>>> > I checked again on the just released WildFly 10.0 CR2, but
>>> > unfortunately the code is still severely broken now.
>>> >
>>> > There are two main issues, and they're both in this fragment in
>>> > JASPIAuthenticationMechanism:
>>> >
>>> > if(isValid == null) {
>>> > isValid = createJASPIAuthenticationManager().isValid(messageInfo,
>>> > new Subject(), JASPI_HTTP_SERVLET_LAYER,
>>> > attachment.getApplicationIdentifier(), new JBossCallbackHandler());
>>> > }
>>> >
>>> > The first problem is the "isValid == null" check. After the first call
>>> > to request#authenticate in a given request this will always be
>>> > non-null. The result is that a request for programmatic authentication
>>> > will effectively be ignored the first time.
>>> >
>>> > The second problem is that this passes in the JBossCallbackHandler,
>>> > but this doesn't know how to handle JASPIC callbacks and this will
>>> > result in an exception like the following:
>>> >
>>> > javax.security.auth.callback.UnsupportedCallbackException: PBOX00014:
>>> > org.jboss.security.auth.callback.JBossCallbackHandler does not handle
>>> > a callback of type
>>> > javax.security.auth.message.callback.CallerPrincipalCallback
>>> > at
>>> > org.jboss.security.auth.callback.JBossCallbackHandler.handleCallBack(JBossCallbackHandler.java:138)
>>> > at
>>> > org.jboss.security.auth.callback.JBossCallbackHandler.handle(JBossCallbackHandler.java:87)
>>> >
>>> > The code should pass in a JASPICallbackHandler here.
>>> >
>>> > Hope this can be fixed. Perhaps it's just a matter of removing the
>>> > "isValid == null" check and passing in the right callback handler.
>>> >
>>> > Kind regards,
>>> > Arjan Tijms
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > On Wed, Sep 23, 2015 at 5:58 PM, arjan tijms <arjan.tijms at gmail.com>
>>> > wrote:
>>> >> p.s. if I just revert JASPIAuthenticationMechanism to the previous
>>> >> version, but leaving in the new JASPICInitialHandler, then everything
>>> >> seems to work again. This is a bit of hacky workaround perhaps, but in
>>> >> some quick testing it does do the trick.
>>> >>
>>> >> On Wed, Sep 23, 2015 at 3:31 PM, arjan tijms <arjan.tijms at gmail.com>
>>> >> wrote:
>>> >>> Hi,
>>> >>>
>>> >>> It looks like that after WFLY-5298 (this commit specifically
>>> >>>
>>> >>> https://github.com/wildfly/wildfly/commit/121a305c59c3619bb747681c62d099dfddd82709#diff-540388fb45365d1d79353d8b4552bcf6)
>>> >>> HttpServletRequest#authenticate does not longer do anything.
>>> >>>
>>> >>> HttpServletRequest#authenticate calls though to
>>> >>> JASPIAuthenticationMechanism#authenticate.
>>> >>>
>>> >>> There it now obtains the attachment that was set by the new
>>> >>> JASPICInitialHandler, which calls the SAM at the beginning of the
>>> >>> request. And then uses the stored "isValid" outcome directly, without
>>> >>> calling the SAM again.
>>> >>>
>>> >>> See the code below:
>>> >>>
>>> >>> public AuthenticationMechanismOutcome authenticate(final
>>> >>> HttpServerExchange exchange, final SecurityContext sc) {
>>> >>> JASPICAttachment attachment =
>>> >>> exchange.getAttachment(JASPICAttachment.ATTACHMENT_KEY);
>>> >>>
>>> >>> AuthenticationMechanismOutcome outcome;
>>> >>> Account authenticatedAccount = null;
>>> >>>
>>> >>> boolean isValid = attachment.isValid();
>>> >>> final ServletRequestContext requestContext =
>>> >>> attachment.getRequestContext();
>>> >>> final JASPIServerAuthenticationManager sam =
>>> >>> attachment.getSam();
>>> >>> final JASPICallbackHandler cbh = attachment.getCbh();
>>> >>>
>>> >>> GenericMessageInfo messageInfo = attachment.getMessageInfo();
>>> >>> if (isValid) {
>>> >>> // The CBH filled in the JBOSS SecurityContext, we need
>>> >>> to
>>> >>> create an Undertow account based on that
>>> >>> org.jboss.security.SecurityContext jbossSct =
>>> >>> SecurityActions.getSecurityContext();
>>> >>> authenticatedAccount =
>>> >>> createAccount(attachment.getCachedAccount(), jbossSct);
>>> >>> }
>>> >>>
>>> >>> This is not correct I think. The code should call the SAM once again
>>> >>> and use the outcome from that call.
>>> >>>
>>> >>> Am I missing something, or was the new call to the SAM simply
>>> >>> forgotten at this point?
>>> >>>
>>> >>> Kind regards,
>>> >>> Arjan Tijms
>>> _______________________________________________
>>> wildfly-dev mailing list
>>> wildfly-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>
>> _______________________________________________
>> wildfly-dev mailing list
>> wildfly-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
More information about the wildfly-dev
mailing list