search

eXist-db / existdb-saml

XQuery module that implements SAML v2 single sign-on
GNU Lesser General Public License v2.1
4 stars 3 forks source link

[BUG] No version for EXPath Crypto dependency #8

Closed adamretter closed 11 months ago

adamretter commented 1 year ago

The existdb-saml EXPath Package (including the latest version 1.6.3) does not declare the version of the EXPath Crypto module that it depends on.

The expath-pkg.xml in the XAR file, only contains this:

<package xmlns="http://expath.org/ns/pkg" name="http://exist-db.org/xquery/exsaml" abbrev="existdb-saml" version="1.6.3" spec="1.0">
    <title>existdb-saml</title>
    <dependency package="http://expath.org/ns/crypto"/>
    <dependency processor="http://exist-db.org" semver-min="3.2.0"/>
</package>

The dependency on http://expath.org/ns/crypto should declare at least a semver-min.

There have been lots of differing version of the EXPath Crypto module (published for eXist-db), including crypto versions: 6.0.1, 6.0.0, 5.3.0, 1.0.0, 0.7, 0.6, 0.5, 0.3.5, and 0.3.4.

The existdb-saml module declares that it requires eXist-db version 3.2.0 or later. The version of EXPath Crypto for eXist-db 3.2.0 (i.e. 0.3.5) however does not appear to work with the existdb-saml module on eXist-db 6.1.0. See https://exist-db.org/exist/apps/public-repo/packages/crypto?eXist-db-min-version=3.2.0

Questions

  1. Which is the version that is meant to be used by existdb-saml please?
  2. Does the semver-min of eXist-db used by the existdb-saml module need to be updated perhaps?
adamretter commented 1 year ago

I have also just tested the following which also appears to fail:

I get the following error:

Caused by: org.exist.xquery.XPathException: exerr:ERROR error found while loading module exsaml: Error while loading module xmldb:///db/apps/existdb-saml/content/exsaml.xqm: error found while loading module crypto: Cannot find module class from EXPath repository: org.expath.exist.crypto.ExistExpathCryptoModule
    at org.exist.repo.ExistRepository.getModule(ExistRepository.java:164)
    at org.exist.repo.ExistRepository.resolveJavaModule(ExistRepository.java:140)
    at org.exist.xquery.XQueryContext.resolveInEXPathRepository(XQueryContext.java:509)
    at org.exist.xquery.XQueryContext.importModule(XQueryContext.java:2450)
    at org.exist.xquery.parser.XQueryTreeParser.importDecl(XQueryTreeParser.java:6353)
    at org.exist.xquery.parser.XQueryTreeParser.prolog(XQueryTreeParser.java:5367)
    at org.exist.xquery.parser.XQueryTreeParser.libraryModule(XQueryTreeParser.java:4047)
    at org.exist.xquery.parser.XQueryTreeParser.module(XQueryTreeParser.java:3878)
    at org.exist.xquery.parser.XQueryTreeParser.xpath(XQueryTreeParser.java:3659)
    at org.exist.xquery.XQueryContext.compileModule(XQueryContext.java:2666)
    at org.exist.xquery.XQueryContext.compileOrBorrowModule(XQueryContext.java:2607)
    at org.exist.xquery.XQueryContext.importModuleFromLocation(XQueryContext.java:2536)
    at org.exist.xquery.XQueryContext.importModule(XQueryContext.java:2471)
    at org.exist.xquery.parser.XQueryTreeParser.importDecl(XQueryTreeParser.java:6353)
    at org.exist.xquery.parser.XQueryTreeParser.prolog(XQueryTreeParser.java:5367)
    at org.exist.xquery.parser.XQueryTreeParser.mainModule(XQueryTreeParser.java:4059)
    at org.exist.xquery.parser.XQueryTreeParser.module(XQueryTreeParser.java:4004)
    at org.exist.xquery.parser.XQueryTreeParser.xpath(XQueryTreeParser.java:3659)
    at org.exist.xquery.XQuery.compile(XQuery.java:251)
    at org.exist.xquery.XQuery.compile(XQuery.java:180)
    at org.exist.xquery.XQuery.compile(XQuery.java:138)
    at org.exist.http.urlrewrite.XQueryURLRewrite.runQuery(XQueryURLRewrite.java:671)
    at org.exist.http.urlrewrite.XQueryURLRewrite.service(XQueryURLRewrite.java:241)
    ... 37 more
Caused by: java.lang.ClassNotFoundException: org.expath.exist.crypto.ExistExpathCryptoModule
    at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:445)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:587)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
    at java.base/java.lang.Class.forName0(Native Method)
    at java.base/java.lang.Class.forName(Class.java:467)
    at org.exist.repo.ExistRepository.getModule(ExistRepository.java:154)
    ... 59 more

I believe the additional error in this comment may instead be related to https://github.com/eXist-db/exist/issues/4901

chakl commented 1 year ago

Which is the version that is meant to be used by existdb-saml please?

The latest EXPath crypto module v6.0.1

Does the semver-min of eXist-db used by the existdb-saml module need to be updated perhaps?

Definitely. This was planned once the crypto-module renaming and re-versioning would settle. I guess crypto-6.0.1 is stable for long enough now.

There have been lots of differing version of the EXPath Crypto module (published for eXist-db), including crypto versions: 6.0.1, 6.0.0, 5.3.0, 1.0.0, 0.7, 0.6, 0.5, 0.3.5, and 0.3.4.

Yes. The library had to deal with this mess since crypto-0.3.x

chakl commented 1 year ago

I have also just tested the following which also appears to fail: eXist-db 6.1.0-SNAPSHOT existdb-saml 1.6.3 EXPath Crypto 6.0.1

I had installed a test system (that I no longer have access to) with exactly these versions, and I have not seen the error you posted. Quite frankly, I have no idea.

I maintain other systems that run exist v6.2.0 and crypto v6.0.1. Unfortunately, they forked existdb-saml and introduced an incompatble version numbering. But a quick look at the code suggests the code that does crypto import and crypto usage is not different from existdb-saml v1.6.x.

existdb-saml 1.6.x is expected to work with exist6 and crypto-6.0.1.

I very vaguely remember a confusing error like this when I installed and upgraded various crypto-lib versions in a row for testing. I suspected some confusion with package management, restarted with a fresh exist, and never bothered to debug this.

adamretter commented 1 year ago

existdb-saml 1.6.x is expected to work with exist6 and crypto-6.0.1.

@chakl Thanks, To clarify... would that explicitly be eXist-db 6.0.0 then, or if not, which newer version please?

chakl commented 1 year ago

I'd say >=5.3.0, you know that release better than me

joewiz commented 1 year ago

I had thought that expath-crypto-module 6.0.1 required eXist 6, but I see from the pom that the dependency is actually eXist 5.3.0+: https://github.com/eXist-db/expath-crypto-module/blob/main/pom.xml#L97. But then again this issue says there are problems with 5.3.0: https://github.com/eXist-db/expath-crypto-module/issues/64. Hmm...

chakl commented 1 year ago

Hmm.. when following your link https://github.com/eXist-db/expath-crypto-module/issues/64, it seems to refer to tests with exist v5.4.x versions. Tests failed with incompatible types: org.exist.storage.serializers.Serializer cannot be converted to com.evolvedbinary.j8fu.function.ConsumerE<com.evolvedbinary.j8fu.function.ConsumerE<org.exist.storage.serializers.Serializer,java.io.IOException>,java.io.IOException>

I don't think this 5.4.x release was put into production anywhere.

chakl commented 1 year ago

Closing this. Not a bug.

Not specifying a semver-min attribute for the crypto dependency implies "use latest available version", which is correct. An upcoming maintenance release will be more explicit on the expected crypto version.

@adamretter If you have more evidence of incorrect behavior, please open another issue.

adamretter commented 1 year ago

Not specifying a semver-min attribute for the crypto dependency implies "use latest available version"

Agreed.

which is correct

Well not always actually! It depends on which version of eXist-db you have. The Crypto module has a minimum supported version of eXist-db too! So the "latest" version of the Crypto module for eXist-db 6.1.0 will not be the same as the latest version of the Crypto module for previous versions of eXist-db.

So for example. The existdb-saml module will almost certainly not work on eXist-db 3.2.0 with the version of the Crypto module which supports eXist-db 3.2.0. That is to say that the following from the existdb-saml module's expath-pkg.xml file is incorrect and needs to be corrected:

<dependency package="http://expath.org/ns/crypto"/>
<dependency processor="http://exist-db.org" semver-min="3.2.0"/>
chakl commented 1 year ago

@adamretter I hear you. As stated in the last Close these issues will be addressed in an upcoming maintenance release.

So for example. The existdb-saml module will almost certainly not work on eXist-db 3.2.0

Probably not, but you're fabricating a fictional scenario. You may want to hire consultants who know eXist-db and SAML to address this, if very old eXist-db need to be modernized.

Re-Closing

adamretter commented 1 year ago

but you're fabricating a fictional scenario.

I am afraid I don't understand. How is this fictional? The fact of the matter is that the existdb-saml application declares compatibility with eXist-db 3.2.0 but it is not compatible.

You may want to hire consultants who know eXist-db and SAML to address this

We are those consultants!

We will send a PR to fix this problem, and that will close this issue.