search

casinthecloud / cas-pac4j-oauth-demo

CAS server demo to test the authentication delegation
https://www.casinthecloud.com
76 stars 62 forks source link

Testing against 1.7.1 SNAPSHOT #17

Closed mmoayyed closed 9 years ago

leleuj commented 9 years ago

@mmoayyed : I just started the demo and the SAML support did not work. I fixed it: https://github.com/leleuj/cas-pac4j-oauth-demo/commit/700585f38995e51ebc2d214d348b86558d4f6574#diff-00f53953a0465356bbaf44c7cd68d167R183

Though, the test SAML IdP is still on error, it seems that no metadata have been uploaded: how did you test the change?

leleuj commented 9 years ago

BTW, I updated the j2e-pac4j-demo to test the new SAML support: https://github.com/pac4j/j2e-pac4j-demo/pull/12

I have the following error:

Caused by: java.lang.NoClassDefFoundError: org/springframework/core/io/Resource
    at org.pac4j.saml.client.SAML2Client.initIdentityProviderMetadataResolver(SAML2Client.java:180)
    at org.pac4j.saml.client.SAML2Client.internalInit(SAML2Client.java:125)
    at org.pac4j.core.util.InitializableObject.init(InitializableObject.java:37)

Very strange: I need Spring now?!

mmoayyed commented 9 years ago

spring core dependency was marked as an optional dependency in pac4j. opensaml v3 cannot function without Spring, as it delegates it metadata management to spring resources. a spring core jar is all you need. this is something that we cannot escape unfortunately.

mmoayyed commented 9 years ago

I am surprised the original pull does not declare spring-core as a dependency. I'll test this once more.

leleuj commented 9 years ago

OK. Thx

mmoayyed commented 9 years ago

I ran the demo. Had to make a few adjustments changing com.github.inspektr to org.jasig.inspektr, but other than that, it loaded just fine.

Testing the Saml2Client link also fails correctly because the metadata that is generated is not recognized by testshib and needs to be uploaded.

leleuj commented 9 years ago

@mmoayyed : Note that the spring-core dependency is not "optional" any more on the 1.7.x branch.

I haven't been unable to make a test with this demo because of the missing metadata.

So I use another demo: https://github.com/pac4j/j2e-pac4j-demo and I get the following error:

Warning:  org.apache.xerces.parsers.SAXParser: Feature 'http://javax.xml.XMLConstants/feature/secure-processing' is not recognized.
Warning:  org.apache.xerces.parsers.SAXParser: Property 'http://javax.xml.XMLConstants/property/accessExternalDTD' is not recognized.
Warning:  org.apache.xerces.parsers.SAXParser: Property 'http://www.oracle.com/xml/jaxp/properties/entityExpansionLimit' is not recognized.
[WARNING] Error for /callback
java.lang.NoClassDefFoundError: org/apache/xml/utils/URI$MalformedURIException
        at org.opensaml.xmlsec.encryption.support.Decrypter.decryptKey(Decrypter.java:693)
        at org.opensaml.xmlsec.encryption.support.Decrypter.decryptKey(Decrypter.java:643)
        at org.opensaml.xmlsec.encryption.support.Decrypter.decryptUsingResolvedEncryptedKey(Decrypter.java:781)
        at org.opensaml.xmlsec.encryption.support.Decrypter.decryptDataToDOM(Decrypter.java:539)
        at org.opensaml.xmlsec.encryption.support.Decrypter.decryptDataToList(Decrypter.java:452)
        at org.opensaml.xmlsec.encryption.support.Decrypter.decryptData(Decrypter.java:412)

A dependency is missing?

mmoayyed commented 9 years ago

You shouldnt run into any missing metadata issues. Testshib should already have the pac4j metadata uploaded, and unless it's been removed/replaced, the tests should run fine. I'll do a quick review and see if I can make sense out of it.

mmoayyed commented 9 years ago

Also, I'll make same change to 1.7.x to make spring non-optional. That was an oversight.

mmoayyed commented 9 years ago

Reviewing: looks like you already removed the optional bit for Spring on 1.7. Thanks.

leleuj commented 9 years ago

Yes, I did remove the optional option according to what we did for the master. My last worry is this missing class...

mmoayyed commented 9 years ago

Sure. I'll take a look at https://github.com/pac4j/j2e-pac4j-demo next

leleuj commented 9 years ago

Do you reproduce the problem with the demo?

mmoayyed commented 9 years ago

First "warning" I see is this:

java.lang.Exception: Timeout scanning annotations
        at org.eclipse.jetty.annotations.AnnotationConfiguration.scanForAnnotations(AnnotationConfiguration.java:570)
        at org.eclipse.jetty.annotations.AnnotationConfiguration.configure(AnnotationConfiguration.java:440)
        at org.eclipse.jetty.webapp.WebAppContext.configure(WebAppContext.java:471)
        at org.eclipse.jetty.webapp.WebAppContext.startContext(WebAppContext.java:1329)
        at org.eclipse.jetty.server.handler.ContextHandler.doStart(ContextHandler.java:741)
        at org.eclipse.jetty.webapp.WebAppContext.doStart(WebAppContext.java:497)
        at org.eclipse.jetty.maven.plugin.JettyWebAppContext.doStart(JettyWebAppContext.java:365)
        at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:68)

Other than that, jetty starts up at 8080. Is there a specific place I should be looking for more logs?

mmoayyed commented 9 years ago

Also, what is the depencency for:

        <dependency>
            <groupId>org.pac4j</groupId>
            <artifactId>j2e-pac4j</artifactId>
            <version>1.1.0</version>
        </dependency>

?

It's pulling in pac4j 1.6.

mmoayyed commented 9 years ago

Other conflicts:

capture

leleuj commented 9 years ago

I already have the "scanning timeout" issue when my hard disk was very slow. I guess it should be ignored most of the time.

leleuj commented 9 years ago

The j2e-pac4j dependency version 1.1.0 is tied to pac4j version 1.7 (https://github.com/pac4j/j2e-pac4j/blob/j2e-pac4j-1.1.0/pom.xml), not 1.6. So it shouldn't be a problem.

leleuj commented 9 years ago

findjar tells me it's a xalan class: http://www.findjar.com/class/org/apache/xml/utils/URI$MalformedURIException.html, but I don't see any dependency on xalan. Any clue?

leleuj commented 9 years ago

It works using the following dependency:

<dependency>
  <groupId>xalan</groupId>
  <artifactId>xalan</artifactId>
  <version>2.7.2</version>
</dependency>

I think it should be added in the pac4j-saml dependencies, shouldn't it?

mmoayyed commented 9 years ago

I do think we need to do something about the annotations timeout issue, because I keep running into it often. I am running on a Quad Core 3Ghz machine with 8GB of ram, a SSD drive that has about 100 GB left on it, and I see this error every time.

In fact, I have feeling it also breaks pac4j IT tests, unless I run things with: mvn clean verify -DskipTests=false -Dorg.eclipse.jetty.annotations.maxWait=120

I think that need to build that maxWait thingy into the plugin config.

mmoayyed commented 9 years ago

More on the dependency conflict. j2e-pac4j is pulling 1.7 but the demo is pulling in 1.7.1 SNAPSHOT by default, which might be causing problems. See:

[INFO] org.leleuj:j2e-pac4j-demo:war:1.0.0-SNAPSHOT
[INFO] +- org.pac4j:j2e-pac4j:jar:1.1.0:compile
**** [INFO] |  \- org.pac4j:pac4j-core:jar:1.7.0:compile*****
[INFO] +- org.pac4j:pac4j-oauth:jar:1.7.1-SNAPSHOT:compile

and if I looked at my demo war:

capture

As for xalan issue, let me dig deeper into this. Where do you see the error? and what actions do you think take to reproduce it? Because I have not been able to do so yet so I must be missing something here.

leleuj commented 9 years ago

About the timeout issues, it's strange you have that issue with a good machine. Maybe it can be linked to some internet slowdown. I haven't been able to find out how to set up this option in the plugin.

About the pac4j-core in version 1.7.0, it's meant on purpose as we wanted to ensure backward compatibility. All new 1.7.x versions should work with the core API (developed in the 1.7.0).

To reproduce it, I start the demo, call http://localhost:8080, click on "Protected url by SAML2 : saml2/index.jsp", login in the SAML IdP provider and after that, I'm redirected back to the application and that's where I get the ClassNotFoundException if I don't have the xalan jar.

mmoayyed commented 9 years ago

I think the plugin has a setting for "systemSettings". We may be able to pass arguments there.

I also found this issue with 1.7: https://github.com/pac4j/pac4j/pull/181

Was able to duplicate the error. Will look into it next.

mmoayyed commented 9 years ago

Pushed further commits into https://github.com/pac4j/pac4j/pull/181 and with those changes, I can get the demo to work correctly. You were perfectly right; xalan needed to be added to the saml module.

mmoayyed commented 9 years ago

This is what I see at the end of the demo test:

`` profile : | id: _e5219b8f6dab362ea2d01962a16d0703 | attributes: {urn:oid:1.3.6.1.4.1.5923.1.1.1.10=[T/R6FIT1n+qhRl8OZIHc1cLirk0=], urn:oid:1.3.6.1.4.1.5923.1.1.1.1=[Member, Staff], urn:oid:0.9.2342.19200300.100.1.1=[myself], urn:oid:2.5.4.3=[Me Myself And I], urn:oid:1.3.6.1.4.1.5923.1.1.1.6=[myself@testshib.org], urn:oid:2.5.4.20=[555-5555], urn:oid:1.3.6.1.4.1.5923.1.1.1.9=[Member@testshib.org, Staff@testshib.org], urn:oid:1.3.6.1.4.1.5923.1.1.1.7=[urn:mace:dir:entitlement:common-lib-terms], urn:oid:2.5.4.42=[Me Myself], urn:oid:2.5.4.4=[And I]} | roles: [] | permissions: [] | isRemembered: false |

leleuj commented 9 years ago

Yes, it works now. I'm releasing pac4j v1.7.1...

leleuj commented 9 years ago

Just opened a small PR on CAS to upgrade to pac4j v1.7.1 and avoid any incompatibility issues: https://github.com/Jasig/cas/pull/1029