You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2022/03/26 15:44:06 UTC
[GitHub] [cxf] reta opened a new pull request #927: Merge Opensaml4 to Jakarta EE branch
reta opened a new pull request #927:
URL: https://github.com/apache/cxf/pull/927
Merge Opensaml4 to Jakarta EE branch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] jimma commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
jimma commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1082695892
@coheigea This can be resolved by @reta's another change https://github.com/apache/cxf/pull/929.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on a change in pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #927:
URL: https://github.com/apache/cxf/pull/927#discussion_r838004445
##########
File path: rt/rs/security/oauth-parent/oauth2-saml/src/main/java/org/apache/cxf/rs/security/oauth2/saml/SamlOAuthValidator.java
##########
@@ -144,7 +145,7 @@ private void validateSubjectConfirmation(Message m,
SubjectConfirmationData subjectConfData) {
if (subjectConfData == null) {
if (!subjectConfirmationDataRequired
- && cs.getNotOnOrAfter() != null && !cs.getNotOnOrAfter().isBeforeNow()) {
+ && cs.getNotOnOrAfter() != null && !cs.getNotOnOrAfter().isBefore(Instant.now())) {
Review comment:
@coheigea could you please address / clarify ^^ , thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1082484227
@coheigea how to build hint:
```
mvn install -DskipITs=true -DskipTests=true -pl '!:cxf-rt-transports-jms,!:cxf-rt-transports-websocket,!:cxf-rt-rs-service-description-swagger,!:cxf-rt-rs-service-description-microprofile-openapi,!:cxf-java2swagger-plugin,!:cxf-wadl2java-plugin'
```
There dependency issues with `org.apache.wss4` since some modules are not present in 3.0.0-SNAPSHOT:
```
[ERROR] Failed to execute goal on project cxf-rt-security-saml: Could not resolve dependencies for project org.apache.cxf:cxf-rt-security-saml:bundle:4.0.0-SNAPSHOT: Failure to find org.apache.wss4j:wss4j-ws-security-dom:jar:3.0.0-SNAPSHOT in https://repo.spring.io/milestone/ was cached in the local repository, resolution will not be reattempted until the update interval of spring.milestone has elapsed or updates are forced -> [Help 1]
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] jimma edited a comment on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
jimma edited a comment on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1080423204
@coheigea These compilation failures are caused by the jakarta dependencies are missing. I locally excluded these modules and code/test in pom file like : https://github.com/jimma/cxf/commit/751f91e2c5ba53537720c1a282493010f14000ae
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] jimma commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
jimma commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1082721892
@coheigea It looks like the rs artifacts are not compiled. Please try to run `mvn clean install -Pfastinstall` under rt/rs to see if it helps.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1083028602
> @coheigea I'll work with @reta this week to add a "jakarta" profile or something else to get make the build work. At least make the -Pfastinstall profile work.@reta WDYT?
Oh, if `jakarta` profile is active by default, but if `-P` specified, it should be added as well: `-Pfastinstall,jakarta`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] jimma commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
jimma commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1080423204
@coheigea These compilation failures are caused by the jakarta dependencies are missing. I simply excluded these modules and code/test in pom file like : https://github.com/jimma/cxf/commit/751f91e2c5ba53537720c1a282493010f14000ae
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] coheigea commented on a change in pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
coheigea commented on a change in pull request #927:
URL: https://github.com/apache/cxf/pull/927#discussion_r839380054
##########
File path: rt/rs/security/oauth-parent/oauth2-saml/src/main/java/org/apache/cxf/rs/security/oauth2/saml/SamlOAuthValidator.java
##########
@@ -144,7 +145,7 @@ private void validateSubjectConfirmation(Message m,
SubjectConfirmationData subjectConfData) {
if (subjectConfData == null) {
if (!subjectConfirmationDataRequired
- && cs.getNotOnOrAfter() != null && !cs.getNotOnOrAfter().isBeforeNow()) {
+ && cs.getNotOnOrAfter() != null && !cs.getNotOnOrAfter().isBefore(Instant.now())) {
Review comment:
@andrei-ivanov The previous code used https://www.joda.org/joda-time/apidocs/org/joda/time/DateTime.html#DateTime-- which uses the default time zone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1083036803
> @reta With the above command and this patch, I still see:
>
> [ERROR] /Users/coheigea/src/apache/cxf/rt/transports/http/src/main/java/org/apache/cxf/transport/http/osgi/ServletExporter.java:[125,24] cannot access javax.servlet.Servlet [ERROR] class file for javax.servlet.Servlet not found
@coheigea just merged https://github.com/apache/cxf/pull/929 to fix that, thanks @jimma !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] jimma commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
jimma commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1082768644
@coheigea I'll work with @reta this week to add a "jakarta" profile or something else to get make the build work. At least make the -Pfastinstall profile work.@reta WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] coheigea commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
coheigea commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1082710596
Thanks @jimma that's better. I get a compilation failure in the ws-security systests:
[ERROR] /Users/coheigea/src/apache/cxf/systests/ws-security/src/test/java/org/apache/cxf/systest/ws/httpget/HTTPGetTest.java:[113,19] cannot access javax.ws.rs.client.InvocationCallback
[ERROR] class file for javax.ws.rs.client.InvocationCallback not found
And in the STS systests:
[ERROR] /Users/coheigea/src/apache/cxf/services/sts/systests/basic/src/test/java/org/apache/cxf/systest/sts/rest/STSRESTTest.java:[766,39] incompatible types: javax.ws.rs.core.Response cannot be converted to jakarta.ws.rs.core.Response
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] coheigea commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
coheigea commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1082728919
@jimma cxf-rt-rs-client doesn't compile.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] jimma commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
jimma commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1083045578
> > @coheigea I'll work with @reta this week to add a "jakarta" profile or something else to get make the build work. At least make the -Pfastinstall profile work.@reta WDYT?
>
> Oh, the `jakarta` profile is active by default, but if `-P` specified, it should be added as well: `-Pfastinstall,jakarta`
@reta I mean if it's possible that we can exclude these modules with `jakarta` profile instead of using a command line flag.
Then the `-Pfastinstall,jakarta` can work to build the artifacts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1083118163
> > > @coheigea I'll work with @reta this week to add a "jakarta" profile or something else to get make the build work. At least make the -Pfastinstall profile work.@reta WDYT?
> >
> >
> > Oh, the `jakarta` profile is active by default, but if `-P` specified, it should be added as well: `-Pfastinstall,jakarta`
>
> @reta I mean if it's possible that we can exclude these modules with `jakarta` profile instead of using a command line flag. Then the `-Pfastinstall,jakarta` can work to build the artifacts.
Ah I see, yeah, we can do that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] andrei-ivanov commented on a change in pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
andrei-ivanov commented on a change in pull request #927:
URL: https://github.com/apache/cxf/pull/927#discussion_r839449906
##########
File path: rt/rs/security/oauth-parent/oauth2-saml/src/main/java/org/apache/cxf/rs/security/oauth2/saml/SamlOAuthValidator.java
##########
@@ -144,7 +145,7 @@ private void validateSubjectConfirmation(Message m,
SubjectConfirmationData subjectConfData) {
if (subjectConfData == null) {
if (!subjectConfirmationDataRequired
- && cs.getNotOnOrAfter() != null && !cs.getNotOnOrAfter().isBeforeNow()) {
+ && cs.getNotOnOrAfter() != null && !cs.getNotOnOrAfter().isBefore(Instant.now())) {
Review comment:
Well, maybe that was incorrect too, that's why I was hoping someone who knows better can verify
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] coheigea commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
coheigea commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1080352632
@reta When I download this patch and apply it to the CXF-8371 branch I see a lot of compilation errors, e.g. in rt-transports-jms and transports-http. Am I doing something incorrectly? When I run the tests in cxf-rt-ws-security I see errors like:
java.lang.NoClassDefFoundError: javax/xml/ws/WebServiceException
at org.apache.cxf.ws.security.wss4j.saml.StaxToDOMSamlTest.createService(StaxToDOMSamlTest.java:619)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1080570595
> @reta When I download this patch and apply it to the CXF-8371 branch I see a lot of compilation errors, e.g. in rt-transports-jms and transports-http. Am I doing something incorrectly? When I run the tests in cxf-rt-ws-security I see errors like:
Yeah, we are slowly progressing but the branch is not fully buildable just yet (the jms was fixed by https://github.com/apache/cxf/pull/919 but we reverted that), excluding problematic modules would be a way to go for now (as @jimma suggested here [1]). I will share the command line today in the description for [2] how to build with modules excluded, sorry for that.
[1] https://github.com/apache/cxf/pull/927#issuecomment-1080423204
[2] https://github.com/apache/cxf/pull/912
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] andrei-ivanov commented on a change in pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
andrei-ivanov commented on a change in pull request #927:
URL: https://github.com/apache/cxf/pull/927#discussion_r835779197
##########
File path: rt/rs/security/oauth-parent/oauth2-saml/src/main/java/org/apache/cxf/rs/security/oauth2/saml/SamlOAuthValidator.java
##########
@@ -144,7 +145,7 @@ private void validateSubjectConfirmation(Message m,
SubjectConfirmationData subjectConfData) {
if (subjectConfData == null) {
if (!subjectConfirmationDataRequired
- && cs.getNotOnOrAfter() != null && !cs.getNotOnOrAfter().isBeforeNow()) {
+ && cs.getNotOnOrAfter() != null && !cs.getNotOnOrAfter().isBefore(Instant.now())) {
Review comment:
should there be an explicit timezone be specified instead of using the JVM default?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1079719632
@jimma fyi :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] coheigea commented on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
coheigea commented on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1082689345
@reta With the above command and this patch, I still see:
[ERROR] /Users/coheigea/src/apache/cxf/rt/transports/http/src/main/java/org/apache/cxf/transport/http/osgi/ServletExporter.java:[125,24] cannot access javax.servlet.Servlet
[ERROR] class file for javax.servlet.Servlet not found
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta edited a comment on pull request #927: Merge Opensaml4 to Jakarta EE branch
Posted by GitBox <gi...@apache.org>.
reta edited a comment on pull request #927:
URL: https://github.com/apache/cxf/pull/927#issuecomment-1083028602
> @coheigea I'll work with @reta this week to add a "jakarta" profile or something else to get make the build work. At least make the -Pfastinstall profile work.@reta WDYT?
Oh, the `jakarta` profile is active by default, but if `-P` specified, it should be added as well: `-Pfastinstall,jakarta`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org