Skip to content

Provide JDBC implementation of OAuth2AuthorizationService #304

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

Merged
merged 1 commit into from
Jun 15, 2021

Conversation

ghost
Copy link

@ghost ghost commented May 24, 2021

Add new JDBC implementation of the OAuth2AuthorizationService

Closes gh-245

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 24, 2021
@rlewczuk
Copy link
Contributor

Regarding build errors on windows-latest 11/12, make sure you don't use .equals() when comparing Instant objects, instead check for some range, eg. +/0 few milliseconds or so.

@ghost ghost force-pushed the gh-245 branch 2 times, most recently from 0a559dc to 7a52ec8 Compare May 25, 2021 09:34
@ghost
Copy link
Author

ghost commented May 25, 2021

Regarding build errors on windows-latest 11/12, make sure you don't use .equals() when comparing Instant objects, instead check for some range, eg. +/0 few milliseconds or so.

Hi @rlewczuk . As I'm using the equals method from Ouath2Authorization (which is delegating the call to AbstractOAuth2Token) I can't modify it. The fix that I implemented was to truncate the Instant when creating the tokens: Instant.now().truncatedTo(ChronoUnit.MILLIS)). That's because as of Java 9, Instant.now() has nanosecond resolution and the DB Timestamp has millisecond resolution.

@ghost
Copy link
Author

ghost commented May 25, 2021

Hi @jgrandja . The implementation is ready for review.

@ghost ghost force-pushed the gh-245 branch from 7a52ec8 to d18a5dd Compare June 4, 2021 13:15
@jgrandja jgrandja added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 8, 2021
@jgrandja jgrandja added this to the 0.1.2 milestone Jun 8, 2021
@jgrandja
Copy link
Collaborator

jgrandja commented Jun 8, 2021

@ovidiupopa91

FYI, @sjohnr is a new member on our team and he will be handling the review of this PR. We are targeting this for the 0.1.2 release (Jul 8) so @sjohnr will be providing initial feedback soon.

@ghost
Copy link
Author

ghost commented Jun 8, 2021

@jgrandja / @sjohnr awesome!

Copy link
Member

@sjohnr sjohnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @ovidiupopa91, thanks for your work on this! Just so you know, I'm about to head out of town for the week, but wanted to give a bit of feedback sooner than later. This feedback focuses on the schema. I'll do a more thorough review of the code early next week hopefully.

See feedback inline.

Add new JDBC implementation of the OAuth2AuthorizationService

Closes spring-projectsgh-245
@ghost ghost force-pushed the gh-245 branch from 095acb5 to 632c2cf Compare June 15, 2021 07:03
Copy link
Member

@sjohnr sjohnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!

@sjohnr sjohnr merged commit 552751b into spring-projects:main Jun 15, 2021
@ghost ghost deleted the gh-245 branch June 16, 2021 09:14
@ghost
Copy link
Author

ghost commented Jun 16, 2021

@sjohnr awesome, thank you!

Let me know when there will be any planned feature/task that I could start looking at (for the next release)

@jgrandja
Copy link
Collaborator

Thank you @ovidiupopa91 !

Would you be interested in adding the JDBC implementation for the recently added OAuth2AuthorizationConsentService in gh-283 ?

@ghost
Copy link
Author

ghost commented Jun 16, 2021

hi @jgrandja . Sure, I can do it. Do we have an issue raised for this one? (to reference it in the branch name)
I can create it if not already created.

@jgrandja
Copy link
Collaborator

@ovidiupopa91 I just logged gh-313. Can you ask for it on there so I can assign to you. Thanks!

sjohnr pushed a commit that referenced this pull request Jun 16, 2021
sjohnr pushed a commit to sjohnr/spring-authorization-server that referenced this pull request Jun 17, 2021
sjohnr pushed a commit that referenced this pull request Jun 18, 2021
jgrandja added a commit that referenced this pull request Jun 23, 2021
@sjohnr sjohnr added the status: duplicate A duplicate of another issue label Jun 28, 2021
sjohnr pushed a commit to sjohnr/spring-authorization-server that referenced this pull request Jun 29, 2021
sjohnr pushed a commit that referenced this pull request Jun 29, 2021
jgrandja added a commit that referenced this pull request Jul 9, 2021
jgrandja added a commit that referenced this pull request Nov 30, 2021
doba16 pushed a commit to doba16/spring-authorization-server that referenced this pull request Apr 21, 2023
doba16 pushed a commit to doba16/spring-authorization-server that referenced this pull request Apr 21, 2023
doba16 pushed a commit to doba16/spring-authorization-server that referenced this pull request Apr 21, 2023
doba16 pushed a commit to doba16/spring-authorization-server that referenced this pull request Apr 21, 2023
doba16 pushed a commit to doba16/spring-authorization-server that referenced this pull request Apr 21, 2023
doba16 pushed a commit to doba16/spring-authorization-server that referenced this pull request Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide JDBC implementation of OAuth2AuthorizationService
5 participants