-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add in-memory implementation for OAuth2AuthorizationService #71
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
Add in-memory implementation for OAuth2AuthorizationService #71
Conversation
.../main/java/org/springframework/security/oauth2/server/authorization/OAuth2Authorization.java
Outdated
Show resolved
Hide resolved
fea0565
to
34c895d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @watsta. Please see my review comments.
.../main/java/org/springframework/security/oauth2/server/authorization/OAuth2Authorization.java
Show resolved
Hide resolved
...ringframework/security/oauth2/server/authorization/AuthorizationCodeOAuth2Authorization.java
Outdated
Show resolved
Hide resolved
.../main/java/org/springframework/security/oauth2/server/authorization/OAuth2Authorization.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/springframework/security/oauth2/server/authorization/TokenContainer.java
Outdated
Show resolved
Hide resolved
...org/springframework/security/oauth2/server/authorization/AccessTokenOAuth2Authorization.java
Outdated
Show resolved
Hide resolved
@jgrandja I updated the PR, let me know if it's headed in the right direction. Thanks! |
...springframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationService.java
Outdated
Show resolved
Hide resolved
...springframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationService.java
Outdated
Show resolved
Hide resolved
.../main/java/org/springframework/security/oauth2/server/authorization/OAuth2Authorization.java
Outdated
Show resolved
Hide resolved
...ngframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationServiceTest.java
Outdated
Show resolved
Hide resolved
@jgrandja I updated the PR, let me know of any shortcomings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @watsta. Please see the additional review comments.
Also, before you push the next updates, can you please rebase on master and then force push. We use the git rebase workflow instead of merging.
...springframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationService.java
Outdated
Show resolved
Hide resolved
...springframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationService.java
Outdated
Show resolved
Hide resolved
...springframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationService.java
Outdated
Show resolved
Hide resolved
* Creates an {@link InMemoryOAuth2AuthorizationService}. | ||
*/ | ||
public InMemoryOAuth2AuthorizationService() { | ||
this(new CopyOnWriteArrayList<>()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass Collections.emptyList()
instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied, though I think this defeats the purpose of this class, you won't be able to save authorizations into this in-memory store. Is that fine?
...springframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationService.java
Outdated
Show resolved
Hide resolved
.../main/java/org/springframework/security/oauth2/server/authorization/OAuth2Authorization.java
Outdated
Show resolved
Hide resolved
.../main/java/org/springframework/security/oauth2/server/authorization/OAuth2Authorization.java
Outdated
Show resolved
Hide resolved
.../main/java/org/springframework/security/oauth2/server/authorization/OAuth2Authorization.java
Outdated
Show resolved
Hide resolved
.../main/java/org/springframework/security/oauth2/server/authorization/OAuth2Authorization.java
Outdated
Show resolved
Hide resolved
...ngframework/security/oauth2/server/authorization/InMemoryOAuth2AuthorizationServiceTest.java
Outdated
Show resolved
Hide resolved
437ad78
to
20529dd
Compare
@jgrandja see my reply, PR is also updated |
aef346f
to
a243904
Compare
a243904
to
7a6a316
Compare
Thanks for all the updates @watsta ! FYI, I added a polish commit to get this merged. This is now in master. |
Fixes gh-43
Haven't provided unit tests yet,I am unsure about a few things, see comments.