New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new maven profile for IBM Liberty Profile #263

Open
johnament opened this Issue Oct 6, 2014 · 69 comments

Comments

Projects
None yet
7 participants
@johnament
Contributor

johnament commented Oct 6, 2014

No description provided.

@NottyCode

This comment has been minimized.

Show comment
Hide comment
@NottyCode

NottyCode Oct 6, 2014

The Liberty profile beta supports many Java EE 7 technologies with Servlet 3.1 and Websockets 1.0 being the most complete. At this time neither are 100% CTS compliant although we are making progress with each of our beta updates. The beta is updated roughly every 4 weeks. To get these features both need to be configured into the server. The default server template configures for servlet-3.0 and jsp-2.2 only.

The Java Servlet 3.1, Concurrency Utilities for Java EE 1.0, Java Websocket API 1.0, and JSON Parsing 1.0 will be released on December 9th 2014. To get these capabilities the server.xml needs to be updated to add:

<featureManager>
    <feature>concurrent-1.0</feature>
    <feature>jsonp-1.0</feature>
    <feature>servlet-3.1</feature>
    <feature>websocket-1.0</feature>
</featureManager>

We have run these samples against Liberty (although not using the maven plugin hence no contribution). @arun-gupta tried to run some of the samples on an earlier beta where the Liberty profile did not process the servlet 3.1 deployment descriptor schema. Liberty profile currently supports the servlet 3.1 deployment descriptor schema provided the servlet 3.1 feature is configured.

At the time this was written based on the Liberty profile beta the following samples can run:

Java Batch 1.0
batch.batch-listeners, batch.batchlet-simple, batch.chunk-checkpoint, batch.chuck-csv-database, batch.chunk-exception, batch.chunk-optional-processor, batch.chunk-simple-nobeans, batch.chunk-simple, batch.decision, batch.flow, batch.multiple-steps, batch.split

Concurrency Utilities for Java EE 1.0
concurrency.dynamicproxy, concurrency.managedscheduledexecutor, concurrency.managedthreadfactory

Enterprise JavaBeans 3.2
ejb.async-ejb, ejb.singelton

Java API for XML RESTful Services 2.0
jaxrs.async-client, jaxrs.beanparam, jaxrs.beanvalidation, jaxrs.dynamicfilter, jaxrs.fileupload, jaxrs.filter-itnerceptor, jaxrs.interceptor, jaxrs.invocation, jaxrs.invocation-async, jaxrs.jaxrs-endpoint, jaxrs.mapping-exceptions, jaxrs.moxy, jaxrs.readwriter, jaxrs.readwriter-json, jaxrs.request-binding, jaxrs.singleton

Java Persistence Architecture 2.1
jpa.criteria, jpa.datasourcedefinition, jpa.datasourcedefinition-annotation-pu, jpa.datasourcedefinition-webxml-pu, jpa.dynamic-named-query, jpa.entitygraph, jpa.extended-pc, jpa.jndi-context, jpa.jpa-converter, jpa.listeners, jpa.listeners-injection, jpa.locking-optimistic, jpa.locking-pessimistic, jpa.native-sql, jpa.native-sql-resultset-mapping, jpa.pu-typesafe, jpa.schema-gen-metadata, jpa.schema-gen-scripts, jpa.schema-gen-scripts-external, jpa.schema-gen-scripts-generate

JSON Parsing 1.0
json.object-builder, json.object-reader, json.streaming-generate, json.streaming-parser

Java Servlet 3.1
servlet.async-servlet, servlet.cookies, servlet.error-mapping, servlet.event-listeners, servlet.nonblocking, servlet.web-fragment

Java Websocket 1.0
websocket.endpoint-config, websocket.endpoint-javatypes, websocket.endpoint-partial, websocket.endpoint-programmatic-async, websocket.endpoint-programmatic-config, websocket.endpoint-programmatic-injection, websocket.endpoint-programmatic-partial, websocket.endpoint-singleton, websocket.httpsession, websocket.messagesize, websocket.parameters, websocket.properties, websocket.subprotocol

Anything not listed has either not been run by us, or it ran with errors.

As noted by @aslakknutsen the WebSphere Application Server for Developers license doesn’t include use on a build server, but it does allow people to run these samples and associated tests on a developer desktop so I don’t think that restriction should preclude the creation of a profile for Liberty profile.

NottyCode commented Oct 6, 2014

The Liberty profile beta supports many Java EE 7 technologies with Servlet 3.1 and Websockets 1.0 being the most complete. At this time neither are 100% CTS compliant although we are making progress with each of our beta updates. The beta is updated roughly every 4 weeks. To get these features both need to be configured into the server. The default server template configures for servlet-3.0 and jsp-2.2 only.

The Java Servlet 3.1, Concurrency Utilities for Java EE 1.0, Java Websocket API 1.0, and JSON Parsing 1.0 will be released on December 9th 2014. To get these capabilities the server.xml needs to be updated to add:

<featureManager>
    <feature>concurrent-1.0</feature>
    <feature>jsonp-1.0</feature>
    <feature>servlet-3.1</feature>
    <feature>websocket-1.0</feature>
</featureManager>

We have run these samples against Liberty (although not using the maven plugin hence no contribution). @arun-gupta tried to run some of the samples on an earlier beta where the Liberty profile did not process the servlet 3.1 deployment descriptor schema. Liberty profile currently supports the servlet 3.1 deployment descriptor schema provided the servlet 3.1 feature is configured.

At the time this was written based on the Liberty profile beta the following samples can run:

Java Batch 1.0
batch.batch-listeners, batch.batchlet-simple, batch.chunk-checkpoint, batch.chuck-csv-database, batch.chunk-exception, batch.chunk-optional-processor, batch.chunk-simple-nobeans, batch.chunk-simple, batch.decision, batch.flow, batch.multiple-steps, batch.split

Concurrency Utilities for Java EE 1.0
concurrency.dynamicproxy, concurrency.managedscheduledexecutor, concurrency.managedthreadfactory

Enterprise JavaBeans 3.2
ejb.async-ejb, ejb.singelton

Java API for XML RESTful Services 2.0
jaxrs.async-client, jaxrs.beanparam, jaxrs.beanvalidation, jaxrs.dynamicfilter, jaxrs.fileupload, jaxrs.filter-itnerceptor, jaxrs.interceptor, jaxrs.invocation, jaxrs.invocation-async, jaxrs.jaxrs-endpoint, jaxrs.mapping-exceptions, jaxrs.moxy, jaxrs.readwriter, jaxrs.readwriter-json, jaxrs.request-binding, jaxrs.singleton

Java Persistence Architecture 2.1
jpa.criteria, jpa.datasourcedefinition, jpa.datasourcedefinition-annotation-pu, jpa.datasourcedefinition-webxml-pu, jpa.dynamic-named-query, jpa.entitygraph, jpa.extended-pc, jpa.jndi-context, jpa.jpa-converter, jpa.listeners, jpa.listeners-injection, jpa.locking-optimistic, jpa.locking-pessimistic, jpa.native-sql, jpa.native-sql-resultset-mapping, jpa.pu-typesafe, jpa.schema-gen-metadata, jpa.schema-gen-scripts, jpa.schema-gen-scripts-external, jpa.schema-gen-scripts-generate

JSON Parsing 1.0
json.object-builder, json.object-reader, json.streaming-generate, json.streaming-parser

Java Servlet 3.1
servlet.async-servlet, servlet.cookies, servlet.error-mapping, servlet.event-listeners, servlet.nonblocking, servlet.web-fragment

Java Websocket 1.0
websocket.endpoint-config, websocket.endpoint-javatypes, websocket.endpoint-partial, websocket.endpoint-programmatic-async, websocket.endpoint-programmatic-config, websocket.endpoint-programmatic-injection, websocket.endpoint-programmatic-partial, websocket.endpoint-singleton, websocket.httpsession, websocket.messagesize, websocket.parameters, websocket.properties, websocket.subprotocol

Anything not listed has either not been run by us, or it ran with errors.

As noted by @aslakknutsen the WebSphere Application Server for Developers license doesn’t include use on a build server, but it does allow people to run these samples and associated tests on a developer desktop so I don’t think that restriction should preclude the creation of a profile for Liberty profile.

@arun-gupta

This comment has been minimized.

Show comment
Hide comment
@arun-gupta

arun-gupta Oct 6, 2014

Contributor

@notatibm Thanks a lot for detailed summary!

Will you be adding a profile ?

Contributor

arun-gupta commented Oct 6, 2014

@notatibm Thanks a lot for detailed summary!

Will you be adding a profile ?

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Oct 6, 2014

Contributor

although not using the maven plugin hence no contribution

@notatibm Didn't Arquillian work with Liberty or was there another issue? It looks like there IS an Arquillian container for Liberty available (see http://arquillian.org/modules/arquillian-wlp-managed-8.5-container-adapter)

PS if/when Liberty is adding support for JASPIC and you have issues with the JASPIC tests, please let me know if I can help (you can contact me privately if needed). I executed a Java EE 6 variant of the tests against WebSphere and initially nothing passed. But when I added the user and groups that the JASPIC tests use to some internal repository of WebSphere many tests did pass (see http://arjan-tijms.omnifaces.org/2012/11/implementing-container-authentication.html for more details).

Contributor

arjantijms commented Oct 6, 2014

although not using the maven plugin hence no contribution

@notatibm Didn't Arquillian work with Liberty or was there another issue? It looks like there IS an Arquillian container for Liberty available (see http://arquillian.org/modules/arquillian-wlp-managed-8.5-container-adapter)

PS if/when Liberty is adding support for JASPIC and you have issues with the JASPIC tests, please let me know if I can help (you can contact me privately if needed). I executed a Java EE 6 variant of the tests against WebSphere and initially nothing passed. But when I added the user and groups that the JASPIC tests use to some internal repository of WebSphere many tests did pass (see http://arjan-tijms.omnifaces.org/2012/11/implementing-container-authentication.html for more details).

@NottyCode

This comment has been minimized.

Show comment
Hide comment
@NottyCode

NottyCode Oct 6, 2014

@arun-gupta I'd love to do it, but I can't do it right now so if someone else chooses to pick it up in the meantime that is fine by me.

@arjantijms There is an Arquillian plugin for Liberty profile I think what @arun-gupta is looking for is the maven updates to enable that for these samples. I'll do my best to remember to contact you about JASPIC.

NottyCode commented Oct 6, 2014

@arun-gupta I'd love to do it, but I can't do it right now so if someone else chooses to pick it up in the meantime that is fine by me.

@arjantijms There is an Arquillian plugin for Liberty profile I think what @arun-gupta is looking for is the maven updates to enable that for these samples. I'll do my best to remember to contact you about JASPIC.

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Apr 19, 2015

Contributor

@notatibm @arun-gupta

I just added a PR that adds the mentioned Arquillian container for Liberty to pom.xml. See #299

As discussed on SO the JASPIC samples had to be adjusted to wrap the war that's normally used for testing in a dummy EAR to satisfy the somewhat unfortunate requirement that Liberty has that a group to role mapping file can only be stored in an EAR.

In order to run the JASPIC tests a cheat had to be added via a user repository that contains exactly the user and group that's used by the JASPIC tests. In reality Liberty fails 100% of the JASPIC tests, but with this cheat in place at least the tests are actually attempted. For real JASPIC usage a special noop user registry would have to be installed (one that simply validates every user and group). I have in fact created this, but I'm not entire sure how to reference such a feature from Maven (it's an .esa archive) and install it to Liberty from the tests.

Unfortunately quite a few JASPIC tests fail, but of course Liberty is still in beta.

The following tests failed:

  • testPublicPageNotRememberLogin (org.javaee7.jaspic.basicauthentication.BasicAuthenticationPublicTest)
  • testPublicPageLoggedin (org.javaee7.jaspic.basicauthentication.BasicAuthenticationPublicTest)
  • testProtectedAccessIsStateless (org.javaee7.jaspic.basicauthentication.BasicAuthenticationStatelessTest)
  • testPublicServletWithLoginCallingEJB (org.javaee7.jaspic.ejbpropagation.ProtectedEJBPropagationTest)
    testProtectedServletWithLoginCallingEJB (org.javaee7.jaspic.ejbpropagation.ProtectedEJBPropagationTest)
  • testProtectedServletWithLoginCallingEJB (org.javaee7.jaspic.ejbpropagation.PublicEJBPropagationLogoutTest)
  • testProtectedServletWithLoginCallingEJB(org.javaee7.jaspic.ejbpropagation.PublicEJBPropagationTest)
  • testLogout(org.javaee7.jaspic.lifecycle.AuthModuleMethodInvocationTest) SAM method cleanSubject not called, but should have been
  • testJoinSessionIsOptional (org.javaee7.jaspic.registersession.RegisterSessionTest)
  • testRemembersSession (org.javaee7.jaspic.registersession.RegisterSessionTest)
  • testResponseWrapping (org.javaee7.jaspic.wrapping.WrappingTest) Response wrapped by SAM did not arrive in Servlet.
  • testRequestWrapping(org.javaee7.jaspic.wrapping.WrappingTest) Request wrapped by SAM did not arrive in Servlet.

Specifically the EJB, register session (new JASPIC 1.1 feature) and request/response wrapper tests completely failed (all tests in those sections failed).

Contributor

arjantijms commented Apr 19, 2015

@notatibm @arun-gupta

I just added a PR that adds the mentioned Arquillian container for Liberty to pom.xml. See #299

As discussed on SO the JASPIC samples had to be adjusted to wrap the war that's normally used for testing in a dummy EAR to satisfy the somewhat unfortunate requirement that Liberty has that a group to role mapping file can only be stored in an EAR.

In order to run the JASPIC tests a cheat had to be added via a user repository that contains exactly the user and group that's used by the JASPIC tests. In reality Liberty fails 100% of the JASPIC tests, but with this cheat in place at least the tests are actually attempted. For real JASPIC usage a special noop user registry would have to be installed (one that simply validates every user and group). I have in fact created this, but I'm not entire sure how to reference such a feature from Maven (it's an .esa archive) and install it to Liberty from the tests.

Unfortunately quite a few JASPIC tests fail, but of course Liberty is still in beta.

The following tests failed:

  • testPublicPageNotRememberLogin (org.javaee7.jaspic.basicauthentication.BasicAuthenticationPublicTest)
  • testPublicPageLoggedin (org.javaee7.jaspic.basicauthentication.BasicAuthenticationPublicTest)
  • testProtectedAccessIsStateless (org.javaee7.jaspic.basicauthentication.BasicAuthenticationStatelessTest)
  • testPublicServletWithLoginCallingEJB (org.javaee7.jaspic.ejbpropagation.ProtectedEJBPropagationTest)
    testProtectedServletWithLoginCallingEJB (org.javaee7.jaspic.ejbpropagation.ProtectedEJBPropagationTest)
  • testProtectedServletWithLoginCallingEJB (org.javaee7.jaspic.ejbpropagation.PublicEJBPropagationLogoutTest)
  • testProtectedServletWithLoginCallingEJB(org.javaee7.jaspic.ejbpropagation.PublicEJBPropagationTest)
  • testLogout(org.javaee7.jaspic.lifecycle.AuthModuleMethodInvocationTest) SAM method cleanSubject not called, but should have been
  • testJoinSessionIsOptional (org.javaee7.jaspic.registersession.RegisterSessionTest)
  • testRemembersSession (org.javaee7.jaspic.registersession.RegisterSessionTest)
  • testResponseWrapping (org.javaee7.jaspic.wrapping.WrappingTest) Response wrapped by SAM did not arrive in Servlet.
  • testRequestWrapping(org.javaee7.jaspic.wrapping.WrappingTest) Request wrapped by SAM did not arrive in Servlet.

Specifically the EJB, register session (new JASPIC 1.1 feature) and request/response wrapper tests completely failed (all tests in those sections failed).

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Apr 21, 2015

Contributor

One thing to consider, as mentioned by @notatibm

the WebSphere Application Server for Developers license doesn’t include use on a build server

Would it be an idea to ask special permission for this?

it does allow people to run these samples and associated tests on a developer desktop

Alternatively, someone could run the tests for Liberty periodically on a desktop and publish the test results.

What do you think?

Contributor

arjantijms commented Apr 21, 2015

One thing to consider, as mentioned by @notatibm

the WebSphere Application Server for Developers license doesn’t include use on a build server

Would it be an idea to ask special permission for this?

it does allow people to run these samples and associated tests on a developer desktop

Alternatively, someone could run the tests for Liberty periodically on a desktop and publish the test results.

What do you think?

@NottyCode

This comment has been minimized.

Show comment
Hide comment
@NottyCode

NottyCode Apr 21, 2015

@arjantijms since I made that comment we updated the license of the version on WASdev.net that would allow use on a build server, provided the heap is under 2Gb. The actual limitation is no more than 2Gb per organization, so if you ran 2 builds at the same time it would need to be 1Gb each build. Also not sure in this case what the organization would be. I don't know if that helps.

IANAL

NottyCode commented Apr 21, 2015

@arjantijms since I made that comment we updated the license of the version on WASdev.net that would allow use on a build server, provided the heap is under 2Gb. The actual limitation is no more than 2Gb per organization, so if you ran 2 builds at the same time it would need to be 1Gb each build. Also not sure in this case what the organization would be. I don't know if that helps.

IANAL

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Apr 21, 2015

Contributor

@notatibm right, I almost forgot about that. 2GB would be more than enough I guess and I don't think @arun-gupta does concurrent builds on the same server.

IANAL either, but it sounds like it should not be an issue indeed ;)

P.s. what do you think about the results from the JASPIC test?

Contributor

arjantijms commented Apr 21, 2015

@notatibm right, I almost forgot about that. 2GB would be more than enough I guess and I don't think @arun-gupta does concurrent builds on the same server.

IANAL either, but it sounds like it should not be an issue indeed ;)

P.s. what do you think about the results from the JASPIC test?

@kwsutter

This comment has been minimized.

Show comment
Hide comment
@kwsutter

kwsutter Apr 22, 2015

@arjantijms, We're looking into the JASPIC test failures you highlighted.

kwsutter commented Apr 22, 2015

@arjantijms, We're looking into the JASPIC test failures you highlighted.

@arkarkala

This comment has been minimized.

Show comment
Hide comment
@arkarkala

arkarkala Apr 22, 2015

@arjantijms have you implemented your own JASPIC provider and configured it as a Liberty product extension feature? We only provide the support to plug your own JASPIC provider and do not ship one.

arkarkala commented Apr 22, 2015

@arjantijms have you implemented your own JASPIC provider and configured it as a Liberty product extension feature? We only provide the support to plug your own JASPIC provider and do not ship one.

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Apr 22, 2015

Contributor

@arkarkala for the tests nothing was configured as a Liberty product extension feature, although I'm not 100% sure what you mean with a JASPIC provider. It sounds like there may be a confusion with a JACC provider, which is indeed something you don't seem to ship (I'm pretty sure this is a spec violation as well, but that's an entirely different story).

For this case it's only about JASPIC. JACC is not involved.

For the test run, Liberty was downloaded from the Liberty beta page, and server.xml configured as indicated here: https://github.com/javaee-samples/javaee7-samples/blob/master/README.md

In order to by pass a spec violation in Liberty where the callback handler tries to validate users and groups with a user registry, a basicRegistry was configured in server.xml that contains exactly the users and groups used by the test. This itself should not be needed, but without it nothing at all runs.

The exact details about the Server Auth Module (SAM) that was used is in the source code of the tests, but as a summary:

JASPIC itself basically works on Liberty using this spec defined method of installing a SAM. The installed SAM is called at the right moment before the Servlet/filter chain is invoked, but an amount of JASPIC features such as the ability to wrap a request and response apparently do not work.

Contributor

arjantijms commented Apr 22, 2015

@arkarkala for the tests nothing was configured as a Liberty product extension feature, although I'm not 100% sure what you mean with a JASPIC provider. It sounds like there may be a confusion with a JACC provider, which is indeed something you don't seem to ship (I'm pretty sure this is a spec violation as well, but that's an entirely different story).

For this case it's only about JASPIC. JACC is not involved.

For the test run, Liberty was downloaded from the Liberty beta page, and server.xml configured as indicated here: https://github.com/javaee-samples/javaee7-samples/blob/master/README.md

In order to by pass a spec violation in Liberty where the callback handler tries to validate users and groups with a user registry, a basicRegistry was configured in server.xml that contains exactly the users and groups used by the test. This itself should not be needed, but without it nothing at all runs.

The exact details about the Server Auth Module (SAM) that was used is in the source code of the tests, but as a summary:

JASPIC itself basically works on Liberty using this spec defined method of installing a SAM. The installed SAM is called at the right moment before the Servlet/filter chain is invoked, but an amount of JASPIC features such as the ability to wrap a request and response apparently do not work.

@arkarkala

This comment has been minimized.

Show comment
Hide comment
@arkarkala

arkarkala Apr 22, 2015

@arjantijms Yes, I did mean JASPIC (not JACC). But since you are registering your config provider in your code there should not be a need for an extension feature. Not all changes went into Beta - so you might be hitting these. We will investigate.

Regarding the userRegistry (UR) issue, did you have to specify the same user/group (test/architect) or configure a UR with any (dummy) user only - the latter indicating that you need a UR but is not used for authentication?

arkarkala commented Apr 22, 2015

@arjantijms Yes, I did mean JASPIC (not JACC). But since you are registering your config provider in your code there should not be a need for an extension feature. Not all changes went into Beta - so you might be hitting these. We will investigate.

Regarding the userRegistry (UR) issue, did you have to specify the same user/group (test/architect) or configure a UR with any (dummy) user only - the latter indicating that you need a UR but is not used for authentication?

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Apr 22, 2015

Contributor

@arkarkala

I did mean JASPIC (not JACC).

Okay, it's just that the term "JASPIC provider" did not ring a bell, while the term "JACC provider" is quite common.

But since you are registering your config provider in your code there should not be a need for an extension feature.

True, there should not be a need for this. But to work around the spec violation where usernames/groups are being validated one is unfortunately needed for practical purposes.

For the test however there was no extension feature used.

Regarding the userRegistry (UR) issue, did you have to specify the same user/group (test/architect) or configure a UR with any (dummy) user only

Well, given the following server.xml that was modified once with the following UR:

<basicRegistry id="basic">
    <user name="test" password="not needed"/>
    <group name="architect"/>
</basicRegistry>

If you would change the user name to say "test-does-not-exist", then every test fails immediately when the callback is being handled in the auth module. The log mentions something about an unknown user (I'd have to repeat the test to see what the exact exception is).

When I do not provide a UR at all, the tests also fails, but instead of an unknown user exception a null pointer exception is logged.

So my conclusion was that a UR needs to be present AND it actually needs to contain the user and group that the tests use.

In private tests on my workstation I have executed a parallel test run where I did not add the basicRegistry, but instead installed my own custom user registry as an extension feature. Here I could see that Liberty actually calls a couple of validate methods (isValidUser, isValidGroup). When I return "false" from these methods handling the callback does not work, when I return "true" the callback works.

I'm just about to finish a blog post with some details about this custom user registry.

At any length, using either the basic registry with the test/architect username/group or the noop custom registry yields the same results; in both situations the tests that fail and succeed are the same.

Contributor

arjantijms commented Apr 22, 2015

@arkarkala

I did mean JASPIC (not JACC).

Okay, it's just that the term "JASPIC provider" did not ring a bell, while the term "JACC provider" is quite common.

But since you are registering your config provider in your code there should not be a need for an extension feature.

True, there should not be a need for this. But to work around the spec violation where usernames/groups are being validated one is unfortunately needed for practical purposes.

For the test however there was no extension feature used.

Regarding the userRegistry (UR) issue, did you have to specify the same user/group (test/architect) or configure a UR with any (dummy) user only

Well, given the following server.xml that was modified once with the following UR:

<basicRegistry id="basic">
    <user name="test" password="not needed"/>
    <group name="architect"/>
</basicRegistry>

If you would change the user name to say "test-does-not-exist", then every test fails immediately when the callback is being handled in the auth module. The log mentions something about an unknown user (I'd have to repeat the test to see what the exact exception is).

When I do not provide a UR at all, the tests also fails, but instead of an unknown user exception a null pointer exception is logged.

So my conclusion was that a UR needs to be present AND it actually needs to contain the user and group that the tests use.

In private tests on my workstation I have executed a parallel test run where I did not add the basicRegistry, but instead installed my own custom user registry as an extension feature. Here I could see that Liberty actually calls a couple of validate methods (isValidUser, isValidGroup). When I return "false" from these methods handling the callback does not work, when I return "true" the callback works.

I'm just about to finish a blog post with some details about this custom user registry.

At any length, using either the basic registry with the test/architect username/group or the noop custom registry yields the same results; in both situations the tests that fail and succeed are the same.

@arkarkala

This comment has been minimized.

Show comment
Hide comment
@arkarkala

arkarkala Apr 22, 2015

@arjantijms , are you seeing the isValid calls made in the latest Beta (April/May) too?

arkarkala commented Apr 22, 2015

@arjantijms , are you seeing the isValid calls made in the latest Beta (April/May) too?

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Apr 22, 2015

Contributor

@arkarkala

are you seeing the isValid calls made in the latest Beta (April/May) too?

Yes. The archive I downloaded was wlp-beta-javaee7-2015.4.0.0.zip and when starting the server reported Launching defaultServer (WebSphere Application Server 2015.4.0.0/wlp-1.0.9.20150407-2322

I just finished the article which contains some more details about the extra test with the custom user registry that I've been doing. See http://arjan-tijms.omnifaces.org/2015/04/testing-jaspic-11-on-ibm-liberty-ee-7.html

Contributor

arjantijms commented Apr 22, 2015

@arkarkala

are you seeing the isValid calls made in the latest Beta (April/May) too?

Yes. The archive I downloaded was wlp-beta-javaee7-2015.4.0.0.zip and when starting the server reported Launching defaultServer (WebSphere Application Server 2015.4.0.0/wlp-1.0.9.20150407-2322

I just finished the article which contains some more details about the extra test with the custom user registry that I've been doing. See http://arjan-tijms.omnifaces.org/2015/04/testing-jaspic-11-on-ibm-liberty-ee-7.html

@arkarkala

This comment has been minimized.

Show comment
Hide comment
@arkarkala

arkarkala Apr 23, 2015

@arjantijms Can you provide the following trace - com.ibm.ws.security.=all:com.ibm.ws.webcontainer.=all so we can see when these isValid calls are being made?

Majority of our customers do use UR so we made it mandatory. Yes, there are few situations where it is not required (for eg, when one does not use the default authentication AND authorization) and having some dummy values in it would be sufficient - they should not be used. We might make the UR configuration optional to handle these cases sometime in future - you can also open Request For Feature (RFE) - https://www.ibm.com/developerworks/rfe/execute?use_case=changeRequestLanding

There are also some scenarios where a custom authentication is performed (for eg using JASPIC), WAS can do the authorization without consulting the registry. For eg, when all valid (authenticated) users are assigned to the role(s) inside the application-bnd as shown below.

  <security-role name="architect">
          <special-subject type="ALL_AUTHENTICATED_USERS" />
  </security-role>

arkarkala commented Apr 23, 2015

@arjantijms Can you provide the following trace - com.ibm.ws.security.=all:com.ibm.ws.webcontainer.=all so we can see when these isValid calls are being made?

Majority of our customers do use UR so we made it mandatory. Yes, there are few situations where it is not required (for eg, when one does not use the default authentication AND authorization) and having some dummy values in it would be sufficient - they should not be used. We might make the UR configuration optional to handle these cases sometime in future - you can also open Request For Feature (RFE) - https://www.ibm.com/developerworks/rfe/execute?use_case=changeRequestLanding

There are also some scenarios where a custom authentication is performed (for eg using JASPIC), WAS can do the authorization without consulting the registry. For eg, when all valid (authenticated) users are assigned to the role(s) inside the application-bnd as shown below.

  <security-role name="architect">
          <special-subject type="ALL_AUTHENTICATED_USERS" />
  </security-role>
@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Apr 23, 2015

Contributor

@arkarkala

Can you provide the following trace - com.ibm.ws.security.=all:com.ibm.ws.webcontainer.=all so we can see when these isValid calls are being made?

I'm on a different computer now, but I'll provide these later today. It should also be easy for yourself to see this. This test repository is set up such that everything is fully reproducible. Nothing in the tests depends on anything that's specific to my environment.

If you follow the instructions from https://github.com/javaee-samples/javaee7-samples/blob/master/README.md you should be able to easily get the exact same results.

Majority of our customers do use UR so we made it mandatory.

The problem here is that the JASPIC specification doesn't allow this. Liberty is not fully Java EE 7 compatible as long as it mandates this.

The spec lead, Ron Monzillo said the following about this:

The behavior attributed to WebSphere 8.5, WRT the processing of the JASPIC CallerPrincipalCallback is NOT compatible with the JASPIC specification.

The CallerPrincipalCallback(s) must be able to support the case where the user registry is integrated within the SAM, including for the purpose of providing user group memberships.

For the special case of password based validation, A SAM can invoke the container provided CallbackHandler to handle a PasswordValidationCallback; in which case the CallbackHandler will return a failure result if the username and/or password combination does not exist in the user registry integrated with the container's CallbackHandler. In that case, the SAM would return a failed (or continuation) authentication result and would NOT invoke the CallbackHandler to handle the CallerPrincipalCallback.

Ultimately the TCK should have flagged this, but unfortunately it didn't.

Contributor

arjantijms commented Apr 23, 2015

@arkarkala

Can you provide the following trace - com.ibm.ws.security.=all:com.ibm.ws.webcontainer.=all so we can see when these isValid calls are being made?

I'm on a different computer now, but I'll provide these later today. It should also be easy for yourself to see this. This test repository is set up such that everything is fully reproducible. Nothing in the tests depends on anything that's specific to my environment.

If you follow the instructions from https://github.com/javaee-samples/javaee7-samples/blob/master/README.md you should be able to easily get the exact same results.

Majority of our customers do use UR so we made it mandatory.

The problem here is that the JASPIC specification doesn't allow this. Liberty is not fully Java EE 7 compatible as long as it mandates this.

The spec lead, Ron Monzillo said the following about this:

The behavior attributed to WebSphere 8.5, WRT the processing of the JASPIC CallerPrincipalCallback is NOT compatible with the JASPIC specification.

The CallerPrincipalCallback(s) must be able to support the case where the user registry is integrated within the SAM, including for the purpose of providing user group memberships.

For the special case of password based validation, A SAM can invoke the container provided CallbackHandler to handle a PasswordValidationCallback; in which case the CallbackHandler will return a failure result if the username and/or password combination does not exist in the user registry integrated with the container's CallbackHandler. In that case, the SAM would return a failed (or continuation) authentication result and would NOT invoke the CallbackHandler to handle the CallerPrincipalCallback.

Ultimately the TCK should have flagged this, but unfortunately it didn't.

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms May 4, 2015

Contributor

@arkarkala @notatibm Just wondering, any luck with the JASPIC issues yet?

Contributor

arjantijms commented May 4, 2015

@arkarkala @notatibm Just wondering, any luck with the JASPIC issues yet?

@kwsutter

This comment has been minimized.

Show comment
Hide comment
@kwsutter

kwsutter May 4, 2015

Hi Arjan,
We've made fantastic progress with the samples... One of the developers
will be replying very soon, but he took a couple days of vacation. But,
it's looking very good.

Thanks for your patience.
Kevin

On Mon, May 4, 2015 at 2:05 AM, Arjan Tijms notifications@github.com
wrote:

@arkarkala https://github.com/arkarkala @notatibm
https://github.com/notatibm Just wondering, any luck with the JASPIC
issues yet?


Reply to this email directly or view it on GitHub
#263 (comment)
.

kwsutter commented May 4, 2015

Hi Arjan,
We've made fantastic progress with the samples... One of the developers
will be replying very soon, but he took a couple days of vacation. But,
it's looking very good.

Thanks for your patience.
Kevin

On Mon, May 4, 2015 at 2:05 AM, Arjan Tijms notifications@github.com
wrote:

@arkarkala https://github.com/arkarkala @notatibm
https://github.com/notatibm Just wondering, any luck with the JASPIC
issues yet?


Reply to this email directly or view it on GitHub
#263 (comment)
.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben May 11, 2015

Hi Arjan,

I've been working on the JASPIC problems you reported on the WebSphere Liberty profile. I have all of the failures that you reported working in the new May beta. I've been using the github/maven test environment as documented here on github.com/javaee-samples/javaee7-samples, which does work as advertised, no problem.

You will see one failure - async-authentication - that I didn't get fixed in the beta. I have fixed it. If you need that fix let me know.

Here are the changes you'll need to make.

  1. You will now only need the following features in server.xml, specifically you no longer need to include jaspic-1.1:

     <featureManager>
        <feature>javaee-7.0</feature>
        <feature>localConnector-1.0</feature>
    </featureManager>
  2. Please add the following to your server.xml:

     <webContainer allowQueryParamWithNoEqual="true"/>

    The reason for this is that I found a number of failures were caused by a difference between Liberty and other Java EE app servers in the default behavior of the method HttpServletRequest.getParameter(). This method was returning null in your SAMs even though the parameter was specified. I originally resolved this problem by changing your tests to add an "=true" value to the params. i.e.

          getFromServerPath("protected/servlet?doLogin=true")

    But the allowQueryParamWithNoEqual property will make the Liberty behavior return non null, allowing you to leave your test servlet invocations unchanged.

  3. In your tests where you check for a null principal you need to change to check for "UNAUTHENTICATED". i.e.:

        assertTrue(response.contains("web username: UNAUTHENTICATED"));

    instead of:

        assertTrue(response.contains("web username: null"));

    or in order to keep your tests container independent:

       assertTrue(response.contains("web username: null") || 
                  response.contains("web username: UNAUTHENTICATED"));

    This is consistent with the JASPIC spec which says the container may "establish the container’s representation of the unauthenticated caller". WebSphere's unauthenticated representation is a Subject with a Principal with name UNAUTHENTICATED. It looks like the other app servers you test against represent an unauthenticated caller with a null Principal name.

  4. For the user registry configuration, all you need to add is the group information in your server.xml as shown below. This is needed because you use our container authorization for the group.

    <basicRegistry id="basic">
       <group name="architect" />
    </basicRegistry>

    As you can see no need for the user "test". You will see isValid checks being made but if the user/group does not exist we will use callback values directly.

    We agree that there are some scenarios (those that do not utilize built-in WAS authentication and authorization) where user registry may not be required. We will look into this.

Thanks for your input. It is our intention to make the Liberty profile as easy for individual developers such as yourself to use as Glassfish, Tomcat, Widlfly, etc., while also scaling to the largest enterprise production workloads. We hear you and we appreciate it.

Paul

paulben commented May 11, 2015

Hi Arjan,

I've been working on the JASPIC problems you reported on the WebSphere Liberty profile. I have all of the failures that you reported working in the new May beta. I've been using the github/maven test environment as documented here on github.com/javaee-samples/javaee7-samples, which does work as advertised, no problem.

You will see one failure - async-authentication - that I didn't get fixed in the beta. I have fixed it. If you need that fix let me know.

Here are the changes you'll need to make.

  1. You will now only need the following features in server.xml, specifically you no longer need to include jaspic-1.1:

     <featureManager>
        <feature>javaee-7.0</feature>
        <feature>localConnector-1.0</feature>
    </featureManager>
  2. Please add the following to your server.xml:

     <webContainer allowQueryParamWithNoEqual="true"/>

    The reason for this is that I found a number of failures were caused by a difference between Liberty and other Java EE app servers in the default behavior of the method HttpServletRequest.getParameter(). This method was returning null in your SAMs even though the parameter was specified. I originally resolved this problem by changing your tests to add an "=true" value to the params. i.e.

          getFromServerPath("protected/servlet?doLogin=true")

    But the allowQueryParamWithNoEqual property will make the Liberty behavior return non null, allowing you to leave your test servlet invocations unchanged.

  3. In your tests where you check for a null principal you need to change to check for "UNAUTHENTICATED". i.e.:

        assertTrue(response.contains("web username: UNAUTHENTICATED"));

    instead of:

        assertTrue(response.contains("web username: null"));

    or in order to keep your tests container independent:

       assertTrue(response.contains("web username: null") || 
                  response.contains("web username: UNAUTHENTICATED"));

    This is consistent with the JASPIC spec which says the container may "establish the container’s representation of the unauthenticated caller". WebSphere's unauthenticated representation is a Subject with a Principal with name UNAUTHENTICATED. It looks like the other app servers you test against represent an unauthenticated caller with a null Principal name.

  4. For the user registry configuration, all you need to add is the group information in your server.xml as shown below. This is needed because you use our container authorization for the group.

    <basicRegistry id="basic">
       <group name="architect" />
    </basicRegistry>

    As you can see no need for the user "test". You will see isValid checks being made but if the user/group does not exist we will use callback values directly.

    We agree that there are some scenarios (those that do not utilize built-in WAS authentication and authorization) where user registry may not be required. We will look into this.

Thanks for your input. It is our intention to make the Liberty profile as easy for individual developers such as yourself to use as Glassfish, Tomcat, Widlfly, etc., while also scaling to the largest enterprise production workloads. We hear you and we appreciate it.

Paul

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms May 11, 2015

Contributor

I have all of the failures that you reported working in the new May beta.

Sounds great, thanks for looking into this!

You will see one failure - async-authentication - that I didn't get fixed in the beta.

That's a peculiar one, I didn't see that one failing in the previous test run I did (the one from #263 (comment))

specifically you no longer need to include jaspic-1.1:

That's a great improvement, I noticed this in the release notes of the latest beta that was released last week.

The reason for this is that I found a number of failures were caused by a difference between Liberty and other Java EE app servers in the default behavior of the method HttpServletRequest.getParameter().

I'm very sorry that I didn't mentioned this before. Indeed, I noticed the same thing right away. I glanced over the Servlet spec and if I'm not mistaken this may a bug in Liberty, but I wanted to double check to make sure first and then forgot about it. In my local test I changed this for Liberty just like you did.

This is consistent with the JASPIC spec which says the container may "establish the container’s representation of the unauthenticated caller"

I think this is not entirely correct. I remember talking with Ron Monzillo about this and it really has to be null. Only in EJB it's allowed (almost required really) to return something container specific (which incidentally is something I hope to be able to address in the security EG, but that's for Java EE 8)

The fragment you mentioned is in the Javadoc of the CallerPrincipalCallback, which for convenience is also printed on page 93 of the JASPIC 1.1 spec:

The CallbackHandler must use the argument Principal to establish the caller principal associated with the invocation being processed by the container. When the argument Principal is null, the handler must establish the container’s representation of the unauthenticated caller principal.

So internally, there can be a server specific representation. Which means a specific Principal implementation (e.g. com.ibm.SomePrincipal) or an actual null. A JACC provider would get to see this, and would need to take this into account.

However, for Servlets the HttpServletRequest#getUserPrincipal() method is very clear that a null must be returned if the user has not been authenticated:

Principal getUserPrincipal()
Returns a java.security.Principal object containing the name of the current authenticated user. If the user has not been authenticated, the method returns null.

See http://docs.oracle.com/javaee/7/api/javax/servlet/http/HttpServletRequest.html#getUserPrincipal()

There's a similar requirement for getRemoteUser(), and the requirement is repeated in the description of methods such as isUserInRole() and login().

I do agree that the wording about "container’s representation of the unauthenticated caller principal" can be confusing.

We will look into this.

Would be really great if something can be done here, thanks in advance!

Thanks for your input.

You're welcome, thanks again for looking into this.

Contributor

arjantijms commented May 11, 2015

I have all of the failures that you reported working in the new May beta.

Sounds great, thanks for looking into this!

You will see one failure - async-authentication - that I didn't get fixed in the beta.

That's a peculiar one, I didn't see that one failing in the previous test run I did (the one from #263 (comment))

specifically you no longer need to include jaspic-1.1:

That's a great improvement, I noticed this in the release notes of the latest beta that was released last week.

The reason for this is that I found a number of failures were caused by a difference between Liberty and other Java EE app servers in the default behavior of the method HttpServletRequest.getParameter().

I'm very sorry that I didn't mentioned this before. Indeed, I noticed the same thing right away. I glanced over the Servlet spec and if I'm not mistaken this may a bug in Liberty, but I wanted to double check to make sure first and then forgot about it. In my local test I changed this for Liberty just like you did.

This is consistent with the JASPIC spec which says the container may "establish the container’s representation of the unauthenticated caller"

I think this is not entirely correct. I remember talking with Ron Monzillo about this and it really has to be null. Only in EJB it's allowed (almost required really) to return something container specific (which incidentally is something I hope to be able to address in the security EG, but that's for Java EE 8)

The fragment you mentioned is in the Javadoc of the CallerPrincipalCallback, which for convenience is also printed on page 93 of the JASPIC 1.1 spec:

The CallbackHandler must use the argument Principal to establish the caller principal associated with the invocation being processed by the container. When the argument Principal is null, the handler must establish the container’s representation of the unauthenticated caller principal.

So internally, there can be a server specific representation. Which means a specific Principal implementation (e.g. com.ibm.SomePrincipal) or an actual null. A JACC provider would get to see this, and would need to take this into account.

However, for Servlets the HttpServletRequest#getUserPrincipal() method is very clear that a null must be returned if the user has not been authenticated:

Principal getUserPrincipal()
Returns a java.security.Principal object containing the name of the current authenticated user. If the user has not been authenticated, the method returns null.

See http://docs.oracle.com/javaee/7/api/javax/servlet/http/HttpServletRequest.html#getUserPrincipal()

There's a similar requirement for getRemoteUser(), and the requirement is repeated in the description of methods such as isUserInRole() and login().

I do agree that the wording about "container’s representation of the unauthenticated caller principal" can be confusing.

We will look into this.

Would be really great if something can be done here, thanks in advance!

Thanks for your input.

You're welcome, thanks again for looking into this.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben May 11, 2015

Correct, the async-authentication failure was not there before. It was introduced by my changes to fix the failures you reported. I didn't catch it in time for the beta, but it is fixed now.

We're investigating the getUserPrincipal issue.

paulben commented May 11, 2015

Correct, the async-authentication failure was not there before. It was introduced by my changes to fix the failures you reported. I didn't catch it in time for the beta, but it is fixed now.

We're investigating the getUserPrincipal issue.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben May 12, 2015

Yep, that's a bug in Liberty getUserPrincipal. I coded up a fix and the tests run clean without the UNAUTHENTICATED change.

paulben commented May 12, 2015

Yep, that's a bug in Liberty getUserPrincipal. I coded up a fix and the tests run clean without the UNAUTHENTICATED change.

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms May 12, 2015

Contributor

I coded up a fix and the tests run clean without the UNAUTHENTICATED change.

That's really great news! :) Thanks for your efforts.

Btw, I'm working on a few more tests but those aren't ready yet.

They concern the ability to include/forward to a resource from the SAM (new requirement in JASPIC 1.1) , setting a specific HTTP status code from a SAM and explicitly testing whether the response can still be written to when a SAM's secureResponse() method is called.

Contributor

arjantijms commented May 12, 2015

I coded up a fix and the tests run clean without the UNAUTHENTICATED change.

That's really great news! :) Thanks for your efforts.

Btw, I'm working on a few more tests but those aren't ready yet.

They concern the ability to include/forward to a resource from the SAM (new requirement in JASPIC 1.1) , setting a specific HTTP status code from a SAM and explicitly testing whether the response can still be written to when a SAM's secureResponse() method is called.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben May 14, 2015

Hi Arjan,
Have you been able to get the forward to work on any container? I'm able to get include working on Liberty. But I found that on return from the forward the response object is committed and closed. The spec requires the MessageInfo object to contain the request and response objects that the runtime uses for the web request being processed, and it also requires that those req/res objects be used for the forward, so we're not seeing an easy way around this problem.

The Servlet spec and the Javadoc for RequestDispatcher are very clear about the response being committed and closed on return from forward. It's looking to us like there is a conflict between the requirements of the JASPIC spec and the Servlet spec on this particular item.

Also it seems to us that this usage - doing a forward while processing a request and then, on return from the forward, continuing processing the original request and ultimately invoking the original request target - is not the intended use for a forward. A forward is intended "replace" the original target, with the results of the forward returned to the user agent, which is consistent with the response being committed and closed.

So...we would be interested to know what your experience is with the forward on other containers.

Thanks, Paul

paulben commented May 14, 2015

Hi Arjan,
Have you been able to get the forward to work on any container? I'm able to get include working on Liberty. But I found that on return from the forward the response object is committed and closed. The spec requires the MessageInfo object to contain the request and response objects that the runtime uses for the web request being processed, and it also requires that those req/res objects be used for the forward, so we're not seeing an easy way around this problem.

The Servlet spec and the Javadoc for RequestDispatcher are very clear about the response being committed and closed on return from forward. It's looking to us like there is a conflict between the requirements of the JASPIC spec and the Servlet spec on this particular item.

Also it seems to us that this usage - doing a forward while processing a request and then, on return from the forward, continuing processing the original request and ultimately invoking the original request target - is not the intended use for a forward. A forward is intended "replace" the original target, with the results of the forward returned to the user agent, which is consistent with the response being committed and closed.

So...we would be interested to know what your experience is with the forward on other containers.

Thanks, Paul

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms May 14, 2015

Contributor

@paulben

Hi Paul,

Have you been able to get the forward to work on any container?

It's been a while since I tested this, but originally (for JASPIC 1.0) it already worked in GlassFish and Geronimo, but failed on JBoss, WebLogic and WebsSphere.

Also it seems to us that this usage - doing a forward while processing a request and then, on return from the forward, continuing processing the original request and ultimately invoking the original request target - is not the intended use for a forward.

Indeed, that's of course not the intended use for a forward.

The main use case is to be able to implement Servlet's native FORM authentication using a portable JASPIC authentication module. Servlet's FORM asks for a forward to a login page and of course doesn't invoke the original target then.

I personally did the proposal for this feature to be included in JASPIC 1.1 and discussed it with Ron Monzillo. See here: https://java.net/jira/browse/JASPIC_SPEC-7

The exact text that ended up in the spec indeed doesn't quite capture the full intend.

You could argue though that the text "Under the constraints defined by RequestDispatcher" indirectly defines that JASPIC indeed does not ask for the possibility to also invoke the original request target after a forward. The constraints for dispatching via a forward (Servlet 3.1, section 9.4) simply do not allow this.

You could perhaps interpret this in the same way as that a Servlet can not forward to two other Servlets either, even though I think the spec does not explicitly say this. It's implied by the specification that says a dispatcher MUST close the response before returning from a forward.

There's a small (indirect) clarification in the JASPIC 1.1 spec about this. In C.7.3 it says:

In Section 3.8.3.1, “validateRequest Before Service Invocation”, added clarification to description of processing for SEND_CONTINUE, especially to allow for forwards to a login page within an authentication module.

My interpretation would be that after having called RequestDispatcher#forward an authentication module SHOULD NOT return SUCCESS, since this would cause the container to invoke the resource, which would then get to see a closed response object.

It's debatable what should be returned in that case though. The original discussion mentioned "SEND_SUCCESS", but the JASPIC 1.1 spec has been adjusted such that a "SEND_CONTINUE" can be used after having used a forward.

If Liberty's JASPIC implementation behaves the same as WebSphere's 8.5 one, then possibly all that's needed here might be the removal of the constraint where WebSphere enforced that only the "302, 303, 307 and 401" status codes are allowed when SEND_CONTINUE is returned.

Hope this helps.

Contributor

arjantijms commented May 14, 2015

@paulben

Hi Paul,

Have you been able to get the forward to work on any container?

It's been a while since I tested this, but originally (for JASPIC 1.0) it already worked in GlassFish and Geronimo, but failed on JBoss, WebLogic and WebsSphere.

Also it seems to us that this usage - doing a forward while processing a request and then, on return from the forward, continuing processing the original request and ultimately invoking the original request target - is not the intended use for a forward.

Indeed, that's of course not the intended use for a forward.

The main use case is to be able to implement Servlet's native FORM authentication using a portable JASPIC authentication module. Servlet's FORM asks for a forward to a login page and of course doesn't invoke the original target then.

I personally did the proposal for this feature to be included in JASPIC 1.1 and discussed it with Ron Monzillo. See here: https://java.net/jira/browse/JASPIC_SPEC-7

The exact text that ended up in the spec indeed doesn't quite capture the full intend.

You could argue though that the text "Under the constraints defined by RequestDispatcher" indirectly defines that JASPIC indeed does not ask for the possibility to also invoke the original request target after a forward. The constraints for dispatching via a forward (Servlet 3.1, section 9.4) simply do not allow this.

You could perhaps interpret this in the same way as that a Servlet can not forward to two other Servlets either, even though I think the spec does not explicitly say this. It's implied by the specification that says a dispatcher MUST close the response before returning from a forward.

There's a small (indirect) clarification in the JASPIC 1.1 spec about this. In C.7.3 it says:

In Section 3.8.3.1, “validateRequest Before Service Invocation”, added clarification to description of processing for SEND_CONTINUE, especially to allow for forwards to a login page within an authentication module.

My interpretation would be that after having called RequestDispatcher#forward an authentication module SHOULD NOT return SUCCESS, since this would cause the container to invoke the resource, which would then get to see a closed response object.

It's debatable what should be returned in that case though. The original discussion mentioned "SEND_SUCCESS", but the JASPIC 1.1 spec has been adjusted such that a "SEND_CONTINUE" can be used after having used a forward.

If Liberty's JASPIC implementation behaves the same as WebSphere's 8.5 one, then possibly all that's needed here might be the removal of the constraint where WebSphere enforced that only the "302, 303, 307 and 401" status codes are allowed when SEND_CONTINUE is returned.

Hope this helps.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben May 14, 2015

Yes that helps thanks. Definitely agree that the intent (and the implied flows) are not at all clear from the spec.

So just to summarize: the behavior on SEND_CONTINUE with status code other than 302, 303, 307 or 401 is to simply return the response object as is. Essentially the provider is not authenticating the request, it's just displaying a login form, and the user agent will then need to invoke the original target again somehow with the results of the login form submission.

Can you let me know if that's correct?

paulben commented May 14, 2015

Yes that helps thanks. Definitely agree that the intent (and the implied flows) are not at all clear from the spec.

So just to summarize: the behavior on SEND_CONTINUE with status code other than 302, 303, 307 or 401 is to simply return the response object as is. Essentially the provider is not authenticating the request, it's just displaying a login form, and the user agent will then need to invoke the original target again somehow with the results of the login form submission.

Can you let me know if that's correct?

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms May 15, 2015

Contributor

Essentially the provider is not authenticating the request, it's just displaying a login form

That's correct.

and the user agent will then need to invoke the original target again somehow with the results of the login form submission.

It doesn't necessarily need to be the original target. In Servlet FORM for instance the postback is a virtual URL named "j_security_check". There's no resource behind this URL, just a name the SAM is listening too.

There the flow is thus:

  • User accesses /protected/foo
  • SAM sees /protected/foo is protected and user not authenticated
  • SAM remembers original request, forwards to /login
  • /login renders a form with "/test_bar" as the action (postback url)
  • User fills out form, submits to /test_bar
  • SAM checks every incoming URL, if requestUri is /test_bar redirects to original URL
  • SAM checks again, if request details saved, restores original request by wrapping current request and lets original target be invoked

Of course this is just a flow, but the above is roughly how the FORM authentication method is specified.

Contributor

arjantijms commented May 15, 2015

Essentially the provider is not authenticating the request, it's just displaying a login form

That's correct.

and the user agent will then need to invoke the original target again somehow with the results of the login form submission.

It doesn't necessarily need to be the original target. In Servlet FORM for instance the postback is a virtual URL named "j_security_check". There's no resource behind this URL, just a name the SAM is listening too.

There the flow is thus:

  • User accesses /protected/foo
  • SAM sees /protected/foo is protected and user not authenticated
  • SAM remembers original request, forwards to /login
  • /login renders a form with "/test_bar" as the action (postback url)
  • User fills out form, submits to /test_bar
  • SAM checks every incoming URL, if requestUri is /test_bar redirects to original URL
  • SAM checks again, if request details saved, restores original request by wrapping current request and lets original target be invoked

Of course this is just a flow, but the above is roughly how the FORM authentication method is specified.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben May 15, 2015

Gotcha, totally clear now. Thanks.

We can make that change easily enough. One concern is that it appears to be an incompatible change of behavior from the 1.0 spec - a SEND_CONTINUE that causes us to fail the request today would now return the response as is. So we want to carefully consider how to proceed as we want provider portability between our 1.0 and 1.1 impl's as much as possible.

From your new feature request for forward/include (https://java.net/jira/browse/JASPIC_SPEC-7) Ron said this re our 1.0 impl:

"I think the websphere implementation is probably most correct (in terms of what the spec currently requires, asthey are apparently enforcing the following for SEND_CONTINUE..."

Also thinking what if the forward returns 302, 303, 307, or 401? That could be ambiguous to the container.

I'm wondering if maybe a new AuthStatus value might be useful here? Something like SEND_RETURN which would tell the container to return the response as is, specifically do not try to set anything in the response as it may very well be committed. Then the container's SEND_CONTINUE processing could remain compatible with 1.0.

Paul

paulben commented May 15, 2015

Gotcha, totally clear now. Thanks.

We can make that change easily enough. One concern is that it appears to be an incompatible change of behavior from the 1.0 spec - a SEND_CONTINUE that causes us to fail the request today would now return the response as is. So we want to carefully consider how to proceed as we want provider portability between our 1.0 and 1.1 impl's as much as possible.

From your new feature request for forward/include (https://java.net/jira/browse/JASPIC_SPEC-7) Ron said this re our 1.0 impl:

"I think the websphere implementation is probably most correct (in terms of what the spec currently requires, asthey are apparently enforcing the following for SEND_CONTINUE..."

Also thinking what if the forward returns 302, 303, 307, or 401? That could be ambiguous to the container.

I'm wondering if maybe a new AuthStatus value might be useful here? Something like SEND_RETURN which would tell the container to return the response as is, specifically do not try to set anything in the response as it may very well be committed. Then the container's SEND_CONTINUE processing could remain compatible with 1.0.

Paul

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms May 15, 2015

Contributor

@paulben

a SEND_CONTINUE that causes us to fail the request today would now return the response as is.

True

So we want to carefully consider how to proceed as we want provider portability between our 1.0 and 1.1 impl's as much as possible.

I hear you, however it might be debatable how much this is really an issue for this particular point.

JASPIC 1.0 said the following:

return AuthStatus.SEND_CONTINUE – If it has established a response (available to the runtime by calling messageInfo.getResponseMessage) that must be sent by the runtime for the request validation to be effectively continued by the client. The module must have set the HTTP status code of the response to a value (for example, HTTP 401 unauthorized, HTTP 303 see other, or HTTP 307 temporary redirect) that will indicate to the client that it should retry the request.

There are a few points to take into account here:

First is that the spec text mentioned 401, 303, 307 as examples, but doesn't say anything definitive about which status codes should be set, not even which ranges. It also says nothing about which status codes should not be set.

Second it does not say whether the runtime should do anything to validate this behavior. In the spirit of what the spec asked before validating the response code might have been correct from a certain point of view, but this was never explicitly mandated. JASPIC 1.1 then clarified that it's the auth module's responsibility to set the right response code for a particular flow that's specific to a given auth module.

Basically following the second point is that no JASPIC implementation I'm aware of actually does anything with return values. All the implementations out there (including the RI), exclusively check for SUCCESS or not. Whether this is a good thing is another debate, but it does mean that no portable SAM could have depended on the runtime throwing an exception when the module did not set the right response code.

Finally, just in my personal opinion, it's perhaps not so bad if a module that previously would not have worked on WebSphere's JASPIC 1.0 would now work on Liberty's JASPIC 1.1. It's another server and a newer version of the spec after all. There are a few more examples of such things in Java EE where a constraint was lifted. E.g. the EJB 3.0 and earlier spec said that an EJB was not allowed to access the file system. If I'm not mistaken 3.1 lifted this. If a server actively enforced this constraint then an EJB accessing the file system would not have worked on a EJB 3.0 impl of that server, but would have to work on an EJB 3.1 one.

Still, maybe if this is really an issue for IBM than you could set a flag in server.xml? There's already some configuration for JASPIC there. E.g. something like jaspic_validate_send_continue_status_codes and then have the user set it to "302, 303, 307, 401" if they would like to have the old WebSphere specific behavior?

Also thinking what if the forward returns 302, 303, 307, or 401? That could be ambiguous to the container.

I think it probably should not matter.

The SAM can either forward to another resource, or write to the response itself. Just as it can retrieve the username/roles via its own code or delegate that to say a JAAS login module.

The only thing that matters to the container is the outcome, not the way in which this outcome was established. So it probably shouldn't matter if the SAM would use the response object to set a 302 directly, or it passed the response object to another class which sets the 302, or it forwards to a Servlet which sets the 302.

Something like SEND_RETURN which would tell the container to return the response as is, specifically do not try to set anything in the response as it may very well be committed.

Something like this might be discussed for a newer version of JASPIC, but that's likely not going to help much for JASPIC 1.1/Java EE 7. If it was really a show stopper, then maybe a new MR of the spec could be asked for, but that's a big step.

There's also the issue that it probably wouldn't help much, as all existing authentication modules that I checked are already returning SEND_CONTINUE after having forwarded.

Contributor

arjantijms commented May 15, 2015

@paulben

a SEND_CONTINUE that causes us to fail the request today would now return the response as is.

True

So we want to carefully consider how to proceed as we want provider portability between our 1.0 and 1.1 impl's as much as possible.

I hear you, however it might be debatable how much this is really an issue for this particular point.

JASPIC 1.0 said the following:

return AuthStatus.SEND_CONTINUE – If it has established a response (available to the runtime by calling messageInfo.getResponseMessage) that must be sent by the runtime for the request validation to be effectively continued by the client. The module must have set the HTTP status code of the response to a value (for example, HTTP 401 unauthorized, HTTP 303 see other, or HTTP 307 temporary redirect) that will indicate to the client that it should retry the request.

There are a few points to take into account here:

First is that the spec text mentioned 401, 303, 307 as examples, but doesn't say anything definitive about which status codes should be set, not even which ranges. It also says nothing about which status codes should not be set.

Second it does not say whether the runtime should do anything to validate this behavior. In the spirit of what the spec asked before validating the response code might have been correct from a certain point of view, but this was never explicitly mandated. JASPIC 1.1 then clarified that it's the auth module's responsibility to set the right response code for a particular flow that's specific to a given auth module.

Basically following the second point is that no JASPIC implementation I'm aware of actually does anything with return values. All the implementations out there (including the RI), exclusively check for SUCCESS or not. Whether this is a good thing is another debate, but it does mean that no portable SAM could have depended on the runtime throwing an exception when the module did not set the right response code.

Finally, just in my personal opinion, it's perhaps not so bad if a module that previously would not have worked on WebSphere's JASPIC 1.0 would now work on Liberty's JASPIC 1.1. It's another server and a newer version of the spec after all. There are a few more examples of such things in Java EE where a constraint was lifted. E.g. the EJB 3.0 and earlier spec said that an EJB was not allowed to access the file system. If I'm not mistaken 3.1 lifted this. If a server actively enforced this constraint then an EJB accessing the file system would not have worked on a EJB 3.0 impl of that server, but would have to work on an EJB 3.1 one.

Still, maybe if this is really an issue for IBM than you could set a flag in server.xml? There's already some configuration for JASPIC there. E.g. something like jaspic_validate_send_continue_status_codes and then have the user set it to "302, 303, 307, 401" if they would like to have the old WebSphere specific behavior?

Also thinking what if the forward returns 302, 303, 307, or 401? That could be ambiguous to the container.

I think it probably should not matter.

The SAM can either forward to another resource, or write to the response itself. Just as it can retrieve the username/roles via its own code or delegate that to say a JAAS login module.

The only thing that matters to the container is the outcome, not the way in which this outcome was established. So it probably shouldn't matter if the SAM would use the response object to set a 302 directly, or it passed the response object to another class which sets the 302, or it forwards to a Servlet which sets the 302.

Something like SEND_RETURN which would tell the container to return the response as is, specifically do not try to set anything in the response as it may very well be committed.

Something like this might be discussed for a newer version of JASPIC, but that's likely not going to help much for JASPIC 1.1/Java EE 7. If it was really a show stopper, then maybe a new MR of the spec could be asked for, but that's a big step.

There's also the issue that it probably wouldn't help much, as all existing authentication modules that I checked are already returning SEND_CONTINUE after having forwarded.

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms May 22, 2015

Contributor

@paulben

I've added tests for the forwarding as mentioned before, and one new test that tests whether the second form of the CallerPrincipalCallback (where you pass a custom Principal in instead of a string) works correctly.

Liberty fails this test too (I tested with the previous beta, wlp-1.0.9.20150407-2322). Instead of the custom principal that was set, HttpServletRequest#getUserPrincipal returns an instance of
com.ibm.ws.security.authentication.principals.WSPrincipal.

Contributor

arjantijms commented May 22, 2015

@paulben

I've added tests for the forwarding as mentioned before, and one new test that tests whether the second form of the CallerPrincipalCallback (where you pass a custom Principal in instead of a string) works correctly.

Liberty fails this test too (I tested with the previous beta, wlp-1.0.9.20150407-2322). Instead of the custom principal that was set, HttpServletRequest#getUserPrincipal returns an instance of
com.ibm.ws.security.authentication.principals.WSPrincipal.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben May 28, 2015

Hi Arjan, that's now fixed.

Paul

paulben commented May 28, 2015

Hi Arjan, that's now fixed.

Paul

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben Jul 1, 2015

Arjan,
FYI the include tests are failing and we are investigating. That last bunch of tests came in right at the end of our release cycle and we didn't have time to get them all working in the 8556 release. For now a workaround I've found is making the included servlet have asyncSupported=true:

@WebServlet(urlPatterns = "/includedServlet", asyncSupported=true)

paulben commented Jul 1, 2015

Arjan,
FYI the include tests are failing and we are investigating. That last bunch of tests came in right at the end of our release cycle and we didn't have time to get them all working in the 8556 release. For now a workaround I've found is making the included servlet have asyncSupported=true:

@WebServlet(urlPatterns = "/includedServlet", asyncSupported=true)
@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Nov 13, 2015

Contributor

@paulben @notatibm

I've just tested the latest versions of Liberty, namely 8.5.5.7 and beta-2015-10. See http://arjan-tijms.omnifaces.org/2015/11/the-state-of-portable-authentication.html

I've adjusted the tests so that <webContainer allowQueryParamWithNoEqual="true"/> is not necessarily needed. This makes it a tiny bit easier to use Liberty with the tests I think.

Forwards and custom principals now work, and async works again too, so that's a great result.

Unfortunately there were some failures with calling request#logout, and includes from a SAM. Also the new register session feature introduced in JASPIC 1.1 (see http://arjan-tijms.omnifaces.org/2013/04/whats-new-in-java-ee-7s-authentication.html#RegisterSession) doesn't seem to work.

Also obtaining and calling CDI beans via CDI.current.select(...) doesn't work (it does work in the RI and JBoss).

Hope this feedback helps, let me know if you need more details

Contributor

arjantijms commented Nov 13, 2015

@paulben @notatibm

I've just tested the latest versions of Liberty, namely 8.5.5.7 and beta-2015-10. See http://arjan-tijms.omnifaces.org/2015/11/the-state-of-portable-authentication.html

I've adjusted the tests so that <webContainer allowQueryParamWithNoEqual="true"/> is not necessarily needed. This makes it a tiny bit easier to use Liberty with the tests I think.

Forwards and custom principals now work, and async works again too, so that's a great result.

Unfortunately there were some failures with calling request#logout, and includes from a SAM. Also the new register session feature introduced in JASPIC 1.1 (see http://arjan-tijms.omnifaces.org/2013/04/whats-new-in-java-ee-7s-authentication.html#RegisterSession) doesn't seem to work.

Also obtaining and calling CDI beans via CDI.current.select(...) doesn't work (it does work in the RI and JBoss).

Hope this feedback helps, let me know if you need more details

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben Nov 13, 2015

Thanks Arjan. The last time I pulled and ran the tests, several months ago, the only failure was a jsf cdi test, so something has changed :-). I'll pull the latest tests and run them and investigate.

Paul

paulben commented Nov 13, 2015

Thanks Arjan. The last time I pulled and ran the tests, several months ago, the only failure was a jsf cdi test, so something has changed :-). I'll pull the latest tests and run them and investigate.

Paul

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Nov 13, 2015

Contributor

@paulben

Ok thanks! I'll do a double check, at least the logout failure is likely incorrect since I forgot to adjust that for the <webContainer allowQueryParamWithNoEqual="true"/> change.

The "invoke CDI from SAM" tests are new. There's one include failing that uses JSF, but that's very likely a JSF problem (although it's remarkable that both Mojarra and Myfaces fail here).

Contributor

arjantijms commented Nov 13, 2015

@paulben

Ok thanks! I'll do a double check, at least the logout failure is likely incorrect since I forgot to adjust that for the <webContainer allowQueryParamWithNoEqual="true"/> change.

The "invoke CDI from SAM" tests are new. There's one include failing that uses JSF, but that's very likely a JSF problem (although it's remarkable that both Mojarra and Myfaces fail here).

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben Nov 13, 2015

@arjantijms @notatibm @arkarkala @kwsutter

Yes I just verified that logout and the register-session are due to

<webContainer allowQueryParamWithNoEqual="true"/>

So that leaves 9, and 8 of those are CDI...hmmm. We are investigating the remaining failures.

Paul

paulben commented Nov 13, 2015

@arjantijms @notatibm @arkarkala @kwsutter

Yes I just verified that logout and the register-session are due to

<webContainer allowQueryParamWithNoEqual="true"/>

So that leaves 9, and 8 of those are CDI...hmmm. We are investigating the remaining failures.

Paul

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Nov 13, 2015

Contributor

@paulben

Indeed, logout and register-session are due to that query parameter. I'll update the test right away. Thanks!

The ones that are still failing are these:

Include:

  1. dispatching testBasicIncludeViaPublicResource Failure: Response did not contain output from target Servlet after included one.
  2. dispatching-jsf-cdi testCDIIncludeViaPublicResource Failure: Response did not contain output from target Servlet after included one.
  3. dispatching-jsf-cdi testJSFwithCDIIncludeViaPublicResource Failure: Response did not contain output from JSF view that SAM included.
  4. dispatching-jsf-cdi testJSFIncludeViaPublicResource Failure: Response did not contain output from JSF view that SAM included.

Obtain/invoke CDI:

  1. invoke-ejb-cdi protectedInvokeCDIFromSecureResponse Failure: Response did not contain output from CDI bean for secureResponse for protected resource. (note: this is not required by the spec)
  2. invoke-ejb-cdi protectedInvokeCDIFromValidateRequest Failure: Response did not contain output from CDI bean for validateRequest for protected resource. (note: this is not required by the spec)
  3. invoke-ejb-cdi publicInvokeCDIFromSecureResponse Failure: Response did not contain output from CDI bean for secureResponse for public resource. (note: this is not required by the spec)
  4. invoke-ejb-cdi publicInvokeCDIFromValidateRequest Failure: Response did not contain output from CDI bean for validateRequest for public resource. (note: this is not required by the spec)
  5. invoke-ejb-cdi publicInvokeCDIFromCleanSubject Failure: Response did not contain output from CDI bean for cleanSubject for public resource. (note: this is not required by the spec)

Indeed, 8 of those use CDI. It could be that for the include that uses CDI this is a coincidence, since the basis include is failing too, but that's just a guess.

Thanks again for investigating these.

Contributor

arjantijms commented Nov 13, 2015

@paulben

Indeed, logout and register-session are due to that query parameter. I'll update the test right away. Thanks!

The ones that are still failing are these:

Include:

  1. dispatching testBasicIncludeViaPublicResource Failure: Response did not contain output from target Servlet after included one.
  2. dispatching-jsf-cdi testCDIIncludeViaPublicResource Failure: Response did not contain output from target Servlet after included one.
  3. dispatching-jsf-cdi testJSFwithCDIIncludeViaPublicResource Failure: Response did not contain output from JSF view that SAM included.
  4. dispatching-jsf-cdi testJSFIncludeViaPublicResource Failure: Response did not contain output from JSF view that SAM included.

Obtain/invoke CDI:

  1. invoke-ejb-cdi protectedInvokeCDIFromSecureResponse Failure: Response did not contain output from CDI bean for secureResponse for protected resource. (note: this is not required by the spec)
  2. invoke-ejb-cdi protectedInvokeCDIFromValidateRequest Failure: Response did not contain output from CDI bean for validateRequest for protected resource. (note: this is not required by the spec)
  3. invoke-ejb-cdi publicInvokeCDIFromSecureResponse Failure: Response did not contain output from CDI bean for secureResponse for public resource. (note: this is not required by the spec)
  4. invoke-ejb-cdi publicInvokeCDIFromValidateRequest Failure: Response did not contain output from CDI bean for validateRequest for public resource. (note: this is not required by the spec)
  5. invoke-ejb-cdi publicInvokeCDIFromCleanSubject Failure: Response did not contain output from CDI bean for cleanSubject for public resource. (note: this is not required by the spec)

Indeed, 8 of those use CDI. It could be that for the include that uses CDI this is a coincidence, since the basis include is failing too, but that's just a guess.

Thanks again for investigating these.

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Nov 25, 2015

Contributor

@paulben @notatibm

Just curious, any updates for the JASPIC failures?

I also found a JPA/JDBC issue with Liberty (tried the 2015-12 beta). Don't know if this is the best place to report this but the problem is that it does not load a JDBC driver for an embedded data source from WEB-INF/lib.

Liberty documents that for this case a shared library should be set specifically for a certain application. See http://www14.software.ibm.com/webapp/wsbroker/redirect?version=cord&product=was-nd-mp&topic=rwlp_ds_appdefined

The spec is not entirely clear on this, however the Java EE spec lead (Bill Shanon) has clarified that loading the JDBC driver from WEB-INF/lib in this case should work.

A few of the tests that depend on this are:

Would be great if the Liberty team could take a look at these failures as well.

Contributor

arjantijms commented Nov 25, 2015

@paulben @notatibm

Just curious, any updates for the JASPIC failures?

I also found a JPA/JDBC issue with Liberty (tried the 2015-12 beta). Don't know if this is the best place to report this but the problem is that it does not load a JDBC driver for an embedded data source from WEB-INF/lib.

Liberty documents that for this case a shared library should be set specifically for a certain application. See http://www14.software.ibm.com/webapp/wsbroker/redirect?version=cord&product=was-nd-mp&topic=rwlp_ds_appdefined

The spec is not entirely clear on this, however the Java EE spec lead (Bill Shanon) has clarified that loading the JDBC driver from WEB-INF/lib in this case should work.

A few of the tests that depend on this are:

Would be great if the Liberty team could take a look at these failures as well.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben Nov 25, 2015

@arjantijms @notatibm @arkarkala @kwsutter

Hi Arjan,
I've fixed an include bug that makes the following tests pass:

dispatching-jsf-cd testCDIIncludeViaPublicResource
dispatching testBasicIncludeViaPublicResource

That fix should be in the next beta. That leaves the 5 CDI failures in invoke-ejb-cdi, which we are still investigating, plus the 2 jsf that all containers fail.

Yes we will investigate the JPA/JDBC issue, thanks for the heads up.

Paul

paulben commented Nov 25, 2015

@arjantijms @notatibm @arkarkala @kwsutter

Hi Arjan,
I've fixed an include bug that makes the following tests pass:

dispatching-jsf-cd testCDIIncludeViaPublicResource
dispatching testBasicIncludeViaPublicResource

That fix should be in the next beta. That leaves the 5 CDI failures in invoke-ejb-cdi, which we are still investigating, plus the 2 jsf that all containers fail.

Yes we will investigate the JPA/JDBC issue, thanks for the heads up.

Paul

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Nov 25, 2015

Contributor

That fix should be in the next beta

Sounds great, looking forward to test it and update the result table.

Thanks for investigating the other issues.

Contributor

arjantijms commented Nov 25, 2015

That fix should be in the next beta

Sounds great, looking forward to test it and update the result table.

Thanks for investigating the other issues.

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Nov 26, 2015

Contributor

@paulben @notatibm @arkarkala @kwsutter

Another test that doesn't pass on Liberty, closely related to JASPIC, is the JACC test at https://github.com/javaee-samples/javaee7-samples/tree/master/jacc

This fails pretty early when just trying to obtain the Subject:

Subject subject = (Subject) PolicyContext.getContext("javax.security.auth.Subject.container");

The following exception is thrown:

unknown handler key
    at javax.security.jacc.PolicyContext.getContext(PolicyContext.java:309)
    at org.javaee7.jacc.contexts.servlet.SubjectServlet.doGet(SubjectServlet.java:43)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:687)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
    at com.ibm.ws.webcontainer.servlet.ServletWrapper.service(ServletWrapper.java:1287)

I'm about to add a test for JASPIC to see if the identity it sets is correctly propagated to JACC, but if JACC doesn't work at all on Liberty that new test will surely fail there.

The problem seems to be that Liberty does not ship with a default JACC provider. This is mentioned in the description of the "jacc-1.5" feature:

"you need to add the third party JACC provider which is not a part of the WebSphere Application Server Liberty profile."

Almost as usual the spec and the TCK are not entirely clear on this, but the JACC spec lead (Ron Monzillo) clarified that a default JACC provider should be available (without needing any configuration).

Contributor

arjantijms commented Nov 26, 2015

@paulben @notatibm @arkarkala @kwsutter

Another test that doesn't pass on Liberty, closely related to JASPIC, is the JACC test at https://github.com/javaee-samples/javaee7-samples/tree/master/jacc

This fails pretty early when just trying to obtain the Subject:

Subject subject = (Subject) PolicyContext.getContext("javax.security.auth.Subject.container");

The following exception is thrown:

unknown handler key
    at javax.security.jacc.PolicyContext.getContext(PolicyContext.java:309)
    at org.javaee7.jacc.contexts.servlet.SubjectServlet.doGet(SubjectServlet.java:43)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:687)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
    at com.ibm.ws.webcontainer.servlet.ServletWrapper.service(ServletWrapper.java:1287)

I'm about to add a test for JASPIC to see if the identity it sets is correctly propagated to JACC, but if JACC doesn't work at all on Liberty that new test will surely fail there.

The problem seems to be that Liberty does not ship with a default JACC provider. This is mentioned in the description of the "jacc-1.5" feature:

"you need to add the third party JACC provider which is not a part of the WebSphere Application Server Liberty profile."

Almost as usual the spec and the TCK are not entirely clear on this, but the JACC spec lead (Ron Monzillo) clarified that a default JACC provider should be available (without needing any configuration).

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Dec 1, 2015

Contributor

@paulben

I finally got around to adding the test I mentioned last week. Sorry for the delay. It can be found here: https://github.com/javaee-samples/javaee7-samples/tree/master/jaspic/jacc-propagation

As predicted, it indeed doesn't seem to run correctly on Liberty. The results are:

callingJACCWhenAuthenticated(org.javaee7.jaspic.jaccpropagation.JACCPropagationProtectedTest): JACC doesn't seem to be available.
callingJACCWhenAuthenticated(org.javaee7.jaspic.jaccpropagation.JACCPropagationPublicTest): JACC doesn't seem to be available.
callingJACCWhenNotAuthenticated(org.javaee7.jaspic.jaccpropagation.JACCPropagationPublicTest): JACC doesn't seem to be available.
Contributor

arjantijms commented Dec 1, 2015

@paulben

I finally got around to adding the test I mentioned last week. Sorry for the delay. It can be found here: https://github.com/javaee-samples/javaee7-samples/tree/master/jaspic/jacc-propagation

As predicted, it indeed doesn't seem to run correctly on Liberty. The results are:

callingJACCWhenAuthenticated(org.javaee7.jaspic.jaccpropagation.JACCPropagationProtectedTest): JACC doesn't seem to be available.
callingJACCWhenAuthenticated(org.javaee7.jaspic.jaccpropagation.JACCPropagationPublicTest): JACC doesn't seem to be available.
callingJACCWhenNotAuthenticated(org.javaee7.jaspic.jaccpropagation.JACCPropagationPublicTest): JACC doesn't seem to be available.
@NottyCode

This comment has been minimized.

Show comment
Hide comment
@NottyCode

NottyCode Dec 1, 2015

We look forward to participating in that discussion with Ron during the next JACC spec revision.

NottyCode commented Dec 1, 2015

We look forward to participating in that discussion with Ron during the next JACC spec revision.

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Dec 2, 2015

Contributor

@notatibm

Thanks a lot for being open to this.

In theory though it's already required for Java EE 7, but I understand this may be a difficult to support at this point.

For the Java EE security EG, which now possibly relies on this functionality being available, I started the discussion here: https://java.net/projects/javaee-security-spec/lists/jsr375-experts/archive/2015-12/message/0

Feel free to join that conversation. Thanks in advance ;)

Contributor

arjantijms commented Dec 2, 2015

@notatibm

Thanks a lot for being open to this.

In theory though it's already required for Java EE 7, but I understand this may be a difficult to support at this point.

For the Java EE security EG, which now possibly relies on this functionality being available, I started the discussion here: https://java.net/projects/javaee-security-spec/lists/jsr375-experts/archive/2015-12/message/0

Feel free to join that conversation. Thanks in advance ;)

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Jan 19, 2016

Contributor

@paulben @notatibm

I retested the latest Liberty 2016.1 beta, and includes now generally work, which is great news!

Additionally one extra CDI case now works as well, but there are still 4 CDI related failures left. See http://arjan-tijms.omnifaces.org/2016/01/java-ee-7-server-liberty-9-beta-20161.html

Contributor

arjantijms commented Jan 19, 2016

@paulben @notatibm

I retested the latest Liberty 2016.1 beta, and includes now generally work, which is great news!

Additionally one extra CDI case now works as well, but there are still 4 CDI related failures left. See http://arjan-tijms.omnifaces.org/2016/01/java-ee-7-server-liberty-9-beta-20161.html

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Jul 8, 2016

Contributor

@paulben @notatibm

I just discovered a new failure with Liberty and JASPIC. As it appears, when a SAM is installed and HttpServletRequest#authenticate is called, the SAM is not invoked at all. Section 3.9.3 of the spec contains the details about this.

I've tested against the latest Liberty 16.0.0.2 as well as a somewhat older beta (2016.5) to check if it's a recent regression, but both failed.

A new test for this has been added at https://github.com/javaee-samples/javaee7-samples/tree/master/jaspic/programmatic-authentication

I also made it easier to test against Liberty by adding an "embedded-managed" profile that automatically downloads Liberty from maven. You can test for the above mentioned failure by doing the following from the top level of the EE 7 samples project:

mvn clean install -pl "test-utils,util" -am
cd jaspic
mvn clean test -P liberty-embedded-arquillian

This will print near the end of the test run:

Failed tests: 
  ProgrammaticAuthenticationTest.testAuthenticateFailFirstTwice:46->assertAuthenticated:65 Calling request.authenticate should have returned true, but did not.
  ProgrammaticAuthenticationTest.testAuthenticate:32->assertAuthenticated:65 Calling request.authenticate should have returned true, but did not.
  ProgrammaticAuthenticationTest.testAuthenticateFailFirstOnce:39->assertAuthenticated:65 Calling request.authenticate should have returned true, but did not.

Running the test war manually in a browser reveals that Liberty responds to the request#authenticate call with an HTTP Basic authentication header.

Contributor

arjantijms commented Jul 8, 2016

@paulben @notatibm

I just discovered a new failure with Liberty and JASPIC. As it appears, when a SAM is installed and HttpServletRequest#authenticate is called, the SAM is not invoked at all. Section 3.9.3 of the spec contains the details about this.

I've tested against the latest Liberty 16.0.0.2 as well as a somewhat older beta (2016.5) to check if it's a recent regression, but both failed.

A new test for this has been added at https://github.com/javaee-samples/javaee7-samples/tree/master/jaspic/programmatic-authentication

I also made it easier to test against Liberty by adding an "embedded-managed" profile that automatically downloads Liberty from maven. You can test for the above mentioned failure by doing the following from the top level of the EE 7 samples project:

mvn clean install -pl "test-utils,util" -am
cd jaspic
mvn clean test -P liberty-embedded-arquillian

This will print near the end of the test run:

Failed tests: 
  ProgrammaticAuthenticationTest.testAuthenticateFailFirstTwice:46->assertAuthenticated:65 Calling request.authenticate should have returned true, but did not.
  ProgrammaticAuthenticationTest.testAuthenticate:32->assertAuthenticated:65 Calling request.authenticate should have returned true, but did not.
  ProgrammaticAuthenticationTest.testAuthenticateFailFirstOnce:39->assertAuthenticated:65 Calling request.authenticate should have returned true, but did not.

Running the test war manually in a browser reveals that Liberty responds to the request#authenticate call with an HTTP Basic authentication header.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben Jul 19, 2016

@arjantijms @notatibm

Thanks for the heads up Arjan. I'll take a look at it.

Paul

paulben commented Jul 19, 2016

@arjantijms @notatibm

Thanks for the heads up Arjan. I'll take a look at it.

Paul

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Jul 19, 2016

Contributor

@paulben

You're welcome, thank you too. If you need more information just let me know.

Contributor

arjantijms commented Jul 19, 2016

@paulben

You're welcome, thank you too. If you need more information just let me know.

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Aug 23, 2016

Contributor

@paulben

Already made any progress with looking at the issue? Also, the return value of the request#authenticate is not super clearly defined in the spec. JBoss and GlassFish have interpreted it somewhat differently for instance. Let me know if I can be of some help there.

Contributor

arjantijms commented Aug 23, 2016

@paulben

Already made any progress with looking at the issue? Also, the return value of the request#authenticate is not super clearly defined in the spec. JBoss and GlassFish have interpreted it somewhat differently for instance. Let me know if I can be of some help there.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben Aug 24, 2016

@arjantijms

Yep, that's a bug, which surprised me as we have a test for programmatic authenticate, but obviously the test has a hole in it. We'll try to have a fix in the next beta.

I saw your comment in the servlet re GlassFish and JBoss authenticate. I'll keep that in mind for this fix.

Paul

paulben commented Aug 24, 2016

@arjantijms

Yep, that's a bug, which surprised me as we have a test for programmatic authenticate, but obviously the test has a hole in it. We'll try to have a fix in the next beta.

I saw your comment in the servlet re GlassFish and JBoss authenticate. I'll keep that in mind for this fix.

Paul

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Aug 24, 2016

Contributor

@paulben

Thanks, looking forward to the next beta ;)

Contributor

arjantijms commented Aug 24, 2016

@paulben

Thanks, looking forward to the next beta ;)

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Aug 27, 2016

Contributor

@notatibm @paulben

I managed to make CDI work inside a SAM by activating the request scope early on Liberty beta 2016.8.

In fact, currently CDI actually already does work from within a SAM, but only for the application scope and other scopes that do not depend on the current request.

The code I used is this one:

From within the validateRequest method of a SAM:

Object weldInitialListener = request.getServletContext().getAttribute("org.jboss.weld.servlet.WeldInitialListener");
ServletRequestEvent event = new ServletRequestEvent(request.getServletContext(), request);

ELProcessor elProcessor = new ELProcessor();
elProcessor.defineBean("weldInitialListener", weldInitialListener);
elProcessor.defineBean("event", event);
elProcessor.eval("weldInitialListener.requestInitialized(event)");

Weld already has support for nested invocations to the weldInitialListener, so when Liberty calls the same method later on it's most likely silently ignored. Now clearly this is a hacky solution that I know you can't possibly support officially.

Maybe the best solution until the (EE) spec clearly defines this situation is to provide a switch that Liberty users can use in ibm-application-bnd.xml,

e.g. <servlet throwRequestInitialized="early" /> or something like that?

That would mean the request initialised event would be thrown as early as a request is available, which I guess is approximately just before you call a JACC provider for the first time in a request (in com.ibm.ws.security.authorization.jacc.web.impl.WebSecurityValidatorImpl#checkDataConstraints)

What do you think?

Contributor

arjantijms commented Aug 27, 2016

@notatibm @paulben

I managed to make CDI work inside a SAM by activating the request scope early on Liberty beta 2016.8.

In fact, currently CDI actually already does work from within a SAM, but only for the application scope and other scopes that do not depend on the current request.

The code I used is this one:

From within the validateRequest method of a SAM:

Object weldInitialListener = request.getServletContext().getAttribute("org.jboss.weld.servlet.WeldInitialListener");
ServletRequestEvent event = new ServletRequestEvent(request.getServletContext(), request);

ELProcessor elProcessor = new ELProcessor();
elProcessor.defineBean("weldInitialListener", weldInitialListener);
elProcessor.defineBean("event", event);
elProcessor.eval("weldInitialListener.requestInitialized(event)");

Weld already has support for nested invocations to the weldInitialListener, so when Liberty calls the same method later on it's most likely silently ignored. Now clearly this is a hacky solution that I know you can't possibly support officially.

Maybe the best solution until the (EE) spec clearly defines this situation is to provide a switch that Liberty users can use in ibm-application-bnd.xml,

e.g. <servlet throwRequestInitialized="early" /> or something like that?

That would mean the request initialised event would be thrown as early as a request is available, which I guess is approximately just before you call a JACC provider for the first time in a request (in com.ibm.ws.security.authorization.jacc.web.impl.WebSecurityValidatorImpl#checkDataConstraints)

What do you think?

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben Sep 5, 2016

@arjantijms @notatibm

Arjan, Sure we can look into doing something along those lines. ATM thinking we'd like to keep it JASPIC specific rather than a general servlet / ibm-application-bnd thing.

Also I have the authenticate tests working, should be in the next beta.

Paul

paulben commented Sep 5, 2016

@arjantijms @notatibm

Arjan, Sure we can look into doing something along those lines. ATM thinking we'd like to keep it JASPIC specific rather than a general servlet / ibm-application-bnd thing.

Also I have the authenticate tests working, should be in the next beta.

Paul

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Sep 6, 2016

Contributor

@paulben @notatibm

JASPIC specific should work I guess. If people need CDI in their security modules they can use JASPIC then, and if they don't need it they can use either JASPIC or the IBM specific artefacts at their choice.

Only extra decision point would be whether to activate or not before the first JACC check (which is typically for the WebUserDataPermission permission). Depending on how Liberty's request processing pipeline is setup internally this may already happen automatically (like what happens in JBoss or GlassFish/Payara).

Also I have the authenticate tests working, should be in the next beta.

Great, I'll be curious to see if you went with the "Jboss behavior" or the "GlassFish behavior" wrt the return value. Looking forward anyway to it. Thanks!

Contributor

arjantijms commented Sep 6, 2016

@paulben @notatibm

JASPIC specific should work I guess. If people need CDI in their security modules they can use JASPIC then, and if they don't need it they can use either JASPIC or the IBM specific artefacts at their choice.

Only extra decision point would be whether to activate or not before the first JACC check (which is typically for the WebUserDataPermission permission). Depending on how Liberty's request processing pipeline is setup internally this may already happen automatically (like what happens in JBoss or GlassFish/Payara).

Also I have the authenticate tests working, should be in the next beta.

Great, I'll be curious to see if you went with the "Jboss behavior" or the "GlassFish behavior" wrt the return value. Looking forward anyway to it. Thanks!

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Sep 7, 2016

Contributor

@paulben @notatibm

We've also included Liberty as a CI test target in the JSR 375 RI (Soteria). It does work against 16.0.2 and not the latest beta, since those latest betas do not seem to be available in Maven central (I asked on Twitter as well).

The tests can be started as follows:

git clone https://github.com/javaee-security-spec/soteria.git
cd soteria
mvn clean install -P liberty -P bundled

A lot of them pass already, but for the BASIC authentication mechanism there's an exception that I need to look into still:

java.lang.NullPointerException: 
   at javax.xml.bind.DatatypeConverter.parseBase64Binary(DatatypeConverter.java:97)
   at org.glassfish.soteria.mechanisms.BasicAuthenticationMechanism.getCredentials(BasicAuthenticationMechanism.java:106)
   at org.glassfish.soteria.mechanisms.BasicAuthenticationMechanism.validateRequest(BasicAuthenticationMechanism.java:80)
   at org.glassfish.soteria.mechanisms.BasicAuthenticationMechanism$Proxy$_$$_WeldClientProxy.validateRequest(Unknown Source)
   at org.glassfish.soteria.mechanisms.jaspic.HttpBridgeServerAuthModule.validateRequest(HttpBridgeServerAuthModule.java:106)
   at org.glassfish.soteria.mechanisms.jaspic.DefaultServerAuthContext.validateRequest(DefaultServerAuthContext.java:75)
   at com.ibm.ws.security.jaspi.JaspiServiceImpl.authenticate(JaspiServiceImpl.java:399)
   at [internal classes]

Looks like the JDK javax.xml.bind.DatatypeConverter.parseBase64Binary(String) for some reason doesn't work in Liberty, but works on at least JBoss and GlassFish.

Contributor

arjantijms commented Sep 7, 2016

@paulben @notatibm

We've also included Liberty as a CI test target in the JSR 375 RI (Soteria). It does work against 16.0.2 and not the latest beta, since those latest betas do not seem to be available in Maven central (I asked on Twitter as well).

The tests can be started as follows:

git clone https://github.com/javaee-security-spec/soteria.git
cd soteria
mvn clean install -P liberty -P bundled

A lot of them pass already, but for the BASIC authentication mechanism there's an exception that I need to look into still:

java.lang.NullPointerException: 
   at javax.xml.bind.DatatypeConverter.parseBase64Binary(DatatypeConverter.java:97)
   at org.glassfish.soteria.mechanisms.BasicAuthenticationMechanism.getCredentials(BasicAuthenticationMechanism.java:106)
   at org.glassfish.soteria.mechanisms.BasicAuthenticationMechanism.validateRequest(BasicAuthenticationMechanism.java:80)
   at org.glassfish.soteria.mechanisms.BasicAuthenticationMechanism$Proxy$_$$_WeldClientProxy.validateRequest(Unknown Source)
   at org.glassfish.soteria.mechanisms.jaspic.HttpBridgeServerAuthModule.validateRequest(HttpBridgeServerAuthModule.java:106)
   at org.glassfish.soteria.mechanisms.jaspic.DefaultServerAuthContext.validateRequest(DefaultServerAuthContext.java:75)
   at com.ibm.ws.security.jaspi.JaspiServiceImpl.authenticate(JaspiServiceImpl.java:399)
   at [internal classes]

Looks like the JDK javax.xml.bind.DatatypeConverter.parseBase64Binary(String) for some reason doesn't work in Liberty, but works on at least JBoss and GlassFish.

@NottyCode

This comment has been minimized.

Show comment
Hide comment
@NottyCode

NottyCode Sep 7, 2016

@arjantijms we don't put the betas in maven central, we would want to be able to remove them and maven doesn't really allow for removal.

NottyCode commented Sep 7, 2016

@arjantijms we don't put the betas in maven central, we would want to be able to remove them and maven doesn't really allow for removal.

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Sep 7, 2016

Contributor

@notatibm That makes sense. Wouldn't there be another maven repo (e.g. IBM specific one) where it could be downloaded from? Or the sonatype snapshot repo?

Contributor

arjantijms commented Sep 7, 2016

@notatibm That makes sense. Wouldn't there be another maven repo (e.g. IBM specific one) where it could be downloaded from? Or the sonatype snapshot repo?

@NottyCode

This comment has been minimized.

Show comment
Hide comment
@NottyCode

NottyCode Sep 7, 2016

@arjantijms not that contains them. We used to release our maven artifacts into an IBM registry, but it was basically just an http website, rather than an actual repository, so didn't really work very well for people. I wouldn't want to reuse it since peoples expectations are that things from there do not go away. We could replicate, but we haven't had a lot of demand. Although you are the second person to ask recently so perhaps we should consider it.

NottyCode commented Sep 7, 2016

@arjantijms not that contains them. We used to release our maven artifacts into an IBM registry, but it was basically just an http website, rather than an actual repository, so didn't really work very well for people. I wouldn't want to reuse it since peoples expectations are that things from there do not go away. We could replicate, but we haven't had a lot of demand. Although you are the second person to ask recently so perhaps we should consider it.

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Sep 7, 2016

Contributor

Although you are the second person to ask recently so perhaps we should consider it.

Okay, thanks for at least considering it then ;)

Contributor

arjantijms commented Sep 7, 2016

Although you are the second person to ask recently so perhaps we should consider it.

Okay, thanks for at least considering it then ;)

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Sep 29, 2016

Contributor

@paulben

Also I have the authenticate tests working, should be in the next beta.

Just to be sure, that next beta is not 2016.9, or is it?

I tried that one as well as the stable 16.0.0.3, but request.authenticate in both cases brings up the BASIC dialog again instead of going to the SAM.

Just asking in case it should be in there and I'm doing something wrong somehow.

Contributor

arjantijms commented Sep 29, 2016

@paulben

Also I have the authenticate tests working, should be in the next beta.

Just to be sure, that next beta is not 2016.9, or is it?

I tried that one as well as the stable 16.0.0.3, but request.authenticate in both cases brings up the BASIC dialog again instead of going to the SAM.

Just asking in case it should be in there and I'm doing something wrong somehow.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben Sep 30, 2016

@arjantijms

No, you're not doing anything wrong. Sorry for the confusion, I was thinking next in terms of our internal cutoff schedule. I should have said the beta after next. The fix was too late to make 2016.9, it will be in 2016.10.

Paul

paulben commented Sep 30, 2016

@arjantijms

No, you're not doing anything wrong. Sorry for the confusion, I was thinking next in terms of our internal cutoff schedule. I should have said the beta after next. The fix was too late to make 2016.9, it will be in 2016.10.

Paul

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Sep 30, 2016

Contributor

@paulben

Ah, ok. No worries, thanks for the clarification ;)

Contributor

arjantijms commented Sep 30, 2016

@paulben

Ah, ok. No worries, thanks for the clarification ;)

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Sep 30, 2016

Contributor

@paulben

I think I discovered another bug in Liberty and JASPIC. It concerns the request wrapping. I was investigating erratic behaviour in Liberty with seemingly random null pointer exceptions.

As it turned out, Liberty is storing the request (and response) wrappers set by a SAM in the global application scope as servlet context attributes (using com.ibm.ws.security.jaspi.servlet.request.wrapper and com.ibm.ws.security.jaspi.servlet.response.wrapper as keys).

E.g. in a Servlet you can do the following:

request.getServletContext().getAttribute("com.ibm.ws.security.jaspi.servlet.request.wrapper");

It seems that the wrapper stored here is automatically applied to each request, but this seems incorrect. The wrapper should only be applied to the request in which it was set. The result is that if a delegating wrapper is used (that delegates to the original Liberty request), things seem to be totally corrupted in those other requests. Even worse, since it's stored in the Servlet context it's already potentially seen by other requests while the request in which authentication took place is still running.

Contributor

arjantijms commented Sep 30, 2016

@paulben

I think I discovered another bug in Liberty and JASPIC. It concerns the request wrapping. I was investigating erratic behaviour in Liberty with seemingly random null pointer exceptions.

As it turned out, Liberty is storing the request (and response) wrappers set by a SAM in the global application scope as servlet context attributes (using com.ibm.ws.security.jaspi.servlet.request.wrapper and com.ibm.ws.security.jaspi.servlet.response.wrapper as keys).

E.g. in a Servlet you can do the following:

request.getServletContext().getAttribute("com.ibm.ws.security.jaspi.servlet.request.wrapper");

It seems that the wrapper stored here is automatically applied to each request, but this seems incorrect. The wrapper should only be applied to the request in which it was set. The result is that if a delegating wrapper is used (that delegates to the original Liberty request), things seem to be totally corrupted in those other requests. Even worse, since it's stored in the Servlet context it's already potentially seen by other requests while the request in which authentication took place is still running.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben Sep 30, 2016

@arjantijms

Indeed, I wasn't thinking of different behavior per request. I'll look into request scoping it.

Paul

paulben commented Sep 30, 2016

@arjantijms

Indeed, I wasn't thinking of different behavior per request. I'll look into request scoping it.

Paul

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Sep 30, 2016

Contributor

@paulben

Okay, thanks for looking into making it request scoped.

Btw, from the stack in a debugger it can be seen that a filter called JaspiServletFilter is setting that wrapped request and response. However, this Filter seems to be executed as the last Filter, not as the absolute first.

What happens now is that if a user supplied Filter also wraps the request, JaspiServletFilter totally discards that and sets the SAM supplied request.

What should happen I think is that the very first Filter in the chain sees the request that the SAM wrapped, so it gets a chance to wrap that, and other Filters down the chain can continue wrapping it until it arrives at the target Servlet.

A complication here is that using the standard Servlet API it's not really possible AFAIK to absolutely guarantee a given Filter is the very first to be executed. All other servers therefor do something special (container specific) to set the wrapped request and response.

Contributor

arjantijms commented Sep 30, 2016

@paulben

Okay, thanks for looking into making it request scoped.

Btw, from the stack in a debugger it can be seen that a filter called JaspiServletFilter is setting that wrapped request and response. However, this Filter seems to be executed as the last Filter, not as the absolute first.

What happens now is that if a user supplied Filter also wraps the request, JaspiServletFilter totally discards that and sets the SAM supplied request.

What should happen I think is that the very first Filter in the chain sees the request that the SAM wrapped, so it gets a chance to wrap that, and other Filters down the chain can continue wrapping it until it arrives at the target Servlet.

A complication here is that using the standard Servlet API it's not really possible AFAIK to absolutely guarantee a given Filter is the very first to be executed. All other servers therefor do something special (container specific) to set the wrapped request and response.

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben Sep 30, 2016

@arjantijms

Got the wrappers request scoped, will have to investigate a bit on the wrapping/filter order.

Paul

paulben commented Sep 30, 2016

@arjantijms

Got the wrappers request scoped, will have to investigate a bit on the wrapping/filter order.

Paul

@paulben

This comment has been minimized.

Show comment
Hide comment
@paulben

paulben Oct 14, 2016

@arjantijms

Hi Arjan,
Hopefully will have the request scoped wrapper fix in 2016.11 beta.
Pre EE8 CDI and filter ordering issues still being worked.

Paul

paulben commented Oct 14, 2016

@arjantijms

Hi Arjan,
Hopefully will have the request scoped wrapper fix in 2016.11 beta.
Pre EE8 CDI and filter ordering issues still being worked.

Paul

@arjantijms

This comment has been minimized.

Show comment
Hide comment
@arjantijms

arjantijms Oct 18, 2016

Contributor

@paulben

Hi Paul, sounds good, thanks for the update!

To help with the filtering issues, I extended the wrapping test here: https://github.com/javaee-samples/javaee7-samples/tree/master/jaspic/wrapping

On all servers I tested this on (Tomcat, JBoss, Payara, ...) the result is:

programmatic filter request isWrapped: true
programmatic filter response isWrapped: true
declared filter request isWrapped: true
declared filter response isWrapped: true
servlet request isWrapped: true
servlet response isWrapped: true

For Liberty however it's:

programmatic filter request isWrapped: null
programmatic filter response isWrapped: null
declared filter request isWrapped: null
declared filter response isWrapped: null
servlet request isWrapped: true
servlet response isWrapped: true

Hope this test helps and thanks again for all your work on this.

Contributor

arjantijms commented Oct 18, 2016

@paulben

Hi Paul, sounds good, thanks for the update!

To help with the filtering issues, I extended the wrapping test here: https://github.com/javaee-samples/javaee7-samples/tree/master/jaspic/wrapping

On all servers I tested this on (Tomcat, JBoss, Payara, ...) the result is:

programmatic filter request isWrapped: true
programmatic filter response isWrapped: true
declared filter request isWrapped: true
declared filter response isWrapped: true
servlet request isWrapped: true
servlet response isWrapped: true

For Liberty however it's:

programmatic filter request isWrapped: null
programmatic filter response isWrapped: null
declared filter request isWrapped: null
declared filter response isWrapped: null
servlet request isWrapped: true
servlet response isWrapped: true

Hope this test helps and thanks again for all your work on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment