Skip to content

Update to oauth2-oidc-sdk:6.2 #6101

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

Closed
jzheaux opened this issue Nov 15, 2018 · 16 comments
Closed

Update to oauth2-oidc-sdk:6.2 #6101

jzheaux opened this issue Nov 15, 2018 · 16 comments
Labels
in: build An issue in the build in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Milestone

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Nov 15, 2018

Summary

We need to update to Nimbus OAuth2 Oidc Sdk 6.2. This can be done by updating dependency-management.gradle.

@jzheaux jzheaux added in: build An issue in the build type: enhancement A general enhancement in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) labels Nov 15, 2018
@jzheaux jzheaux added this to the 5.2.0.M1 milestone Nov 15, 2018
@hellosatish
Copy link

Hi @jzheaux
i would like to contribute to this one.

@jzheaux
Copy link
Contributor Author

jzheaux commented Nov 16, 2018

Sounds great, @hellosatish, it's yours!

@hellosatish
Copy link

@jzheaux i forked the repo again and then used "gradle clean build integrationTest" (without any changes) but tests are failing for ACL again
erro

@jzheaux
Copy link
Contributor Author

jzheaux commented Nov 16, 2018

@hellosatish Let's try two things:

First, make sure that your command is actually "./gradlew clean build integrationTest" if you are on Mac or Linux or "gradlew.bat clean build integrationTest" if you are on Windows. The reason for this is it makes sure that it uses the version of Gradle that Spring Security needs.

Second, if that still fails, then go ahead and let's see if we can find a pattern in the test failures. Do you see something in common about them? The build is passing on our build machines and locally for me, so I imagine that we'll see a common thread on your machine.

@hellosatish
Copy link

hellosatish commented Nov 16, 2018

@jzheaux i am on windows. and my gradle vesion is 4.4.1.
All tests fail with same cause
org.mockito.exceptions.base.MockitoException
Caused by: java.lang.IllegalArgumentException
Attached here is the complete log for gradle command execution.
logs.txt

I have pushed the code to forked repo. if you could check that builds for you locally.
commit

@jzheaux
Copy link
Contributor Author

jzheaux commented Nov 16, 2018

The report will actually give you a bit more information, @hellosatish. If you click on one of those links on the test failures report, what is the full stack trace?

I just double-checked on a Windows machine and was able to build on Java 8. What version of Java are you using? You can do java -version from the command line.

Also, you can try this for a cleaner environment: gradlew.bat --refresh-dependencies clean build integrationTest (again, make sure you are running gradlew.bat, not gradle)

@hellosatish
Copy link

Hi @jzheaux i used the command .\gradlew.bat --refresh-dependencies clean build integrationTest but it fails again. However i have made the changes and pushed to the repository. Could you please see if my repository runs for you locally.
and i am running on java 8. attached here the logs for your reference.
java_version
command
exception.txt

@jzheaux
Copy link
Contributor Author

jzheaux commented Nov 18, 2018

I pulled your changes locally, and, yes, it builds fine for me.

I have not seen that exception before which you got in your build; however, this StackOverflow post sounds interesting: https://stackoverflow.com/questions/37527038/mockito-object-is-not-an-instance-of-declaring-class

I wonder if maybe you have two versions of JDK 8 installed? I have seen in the past the Windows %PATH% pointing to one JDK and %JAVA_HOME% pointing to another.

@hellosatish
Copy link

Hi @jzheaux Thanks for pointing to link. i have my both variables set to JDK 8 only (attached here for reference). However ill upgrade the VM.
Meanwhile if the commit looks good, then what should be the next step?

java_home
path

@jzheaux
Copy link
Contributor Author

jzheaux commented Nov 19, 2018

@hellosatish The next step is to squash your commits so that you are only sending over one commit to Spring Security (right now your repo has three).

Once there is just one commit, then you can submit a pull request.

I've found rebase to be the easiest way to manage squashing in general. Let me know if you run into difficulties, and I'll be happy to help.

@hellosatish
Copy link

hellosatish commented Nov 19, 2018

Hi @jzheaux
i tried squashing my commits and can see only one commit,
Looking at the commit message. i think i merged that with other commit. Please do let me know if there is a way to correct that or should i create a new fork and commit changes again.
of if this is correct then what is the next step

@jzheaux
Copy link
Contributor Author

jzheaux commented Nov 19, 2018

Yes, I've made that mistake before myself.

Try this: https://stackoverflow.com/questions/6217156/break-a-previous-commit-into-multiple-commits

hellosatish pushed a commit to hellosatish/spring-security that referenced this issue Nov 20, 2018
@hellosatish
Copy link

hellosatish commented Nov 20, 2018

Hi @jzheaux thanks for you help.
I spent a lot of time sqaushing/splitting the commits and could not get that to one commit message.
So i started with clean slate and have created a single commit. If this looks good then do let me know should i create a pull request? and anything specific i need to consider while creating the request.

@jzheaux
Copy link
Contributor Author

jzheaux commented Nov 20, 2018

Yes, you can go ahead and create a pull request. Thanks!

@hellosatish
Copy link

@jzheaux Thanks for merging this pull request and specially for your continued help on the issues i faced. I would be happy to help if there are more issues i can contribute to.

@jzheaux
Copy link
Contributor Author

jzheaux commented Nov 21, 2018

For sure! Feel free to pick up something that looks interesting. You might consider #5834

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: build An issue in the build in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants