Skip to content
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

Provide reactive implementation of AuthorizedClientServiceOAuth2AuthorizedClientManager #7569

Closed
jgrandja opened this issue Oct 25, 2019 · 8 comments

Comments

@jgrandja
Copy link
Collaborator

@jgrandja jgrandja commented Oct 25, 2019

We should provide a reactive implementation equivalent of AuthorizedClientServiceOAuth2AuthorizedClientManager.

@ankurpathak

This comment has been minimized.

Copy link
Contributor

@ankurpathak ankurpathak commented Oct 27, 2019

@jgrandja I would like to take this one.

@jgrandja

This comment has been minimized.

Copy link
Collaborator Author

@jgrandja jgrandja commented Oct 28, 2019

Thank you @ankurpathak. Let me know if you have any questions.

ankurpathak added a commit to ankurpathak/spring-security that referenced this issue Oct 31, 2019
…entManager

ReactiveOAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager is reactive
version of AuthorizedClientServiceOAuth2AuthorizedClientManager

Fixes: spring-projectsgh-7569
@ankurpathak

This comment has been minimized.

Copy link
Contributor

@ankurpathak ankurpathak commented Oct 31, 2019

@jgrandja Here is early pull request for this issue, just to understand if I am on right path. Expect your feedback.
#7589

ankurpathak added a commit to ankurpathak/spring-security that referenced this issue Oct 31, 2019
…entManager

ReactiveOAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager is reactive
version of AuthorizedClientServiceOAuth2AuthorizedClientManager

Fixes: spring-projectsgh-7569
@jgrandja

This comment has been minimized.

Copy link
Collaborator Author

@jgrandja jgrandja commented Oct 31, 2019

@ankurpathak I took a look at #7589 and yes this is the implementation I was expecting.

ankurpathak added a commit to ankurpathak/spring-security that referenced this issue Nov 1, 2019
…entManager

ReactiveOAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager is reactive
version of AuthorizedClientServiceOAuth2AuthorizedClientManager

Fixes: spring-projectsgh-7569
ankurpathak added a commit to ankurpathak/spring-security that referenced this issue Nov 1, 2019
…entManager

ReactiveOAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager is reactive
version of AuthorizedClientServiceOAuth2AuthorizedClientManager

Fixes: spring-projectsgh-7569
ankurpathak added a commit to ankurpathak/spring-security that referenced this issue Nov 1, 2019
…entManager

ReactiveOAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager is reactive
version of AuthorizedClientServiceOAuth2AuthorizedClientManager

Fixes: spring-projectsgh-7569
@ghostd

This comment has been minimized.

Copy link
Contributor

@ghostd ghostd commented Nov 1, 2019

HI,

Can we expect a sample (in the repo or in the official documentation or elsewhere) showing how to combine those classes to run a batch (or some code without web context) with OAuth ?

Regards

ankurpathak added a commit to ankurpathak/spring-security that referenced this issue Nov 2, 2019
…entManager

ReactiveOAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager is reactive
version of AuthorizedClientServiceOAuth2AuthorizedClientManager

Fixes: spring-projectsgh-7569
@ankurpathak

This comment has been minimized.

Copy link
Contributor

@ankurpathak ankurpathak commented Nov 2, 2019

@jgrandja Here is complete pull request with tests. I have some confusion on name of class and instance variables. So lets agree with some commons on them and if it needs any modification do let me know.

@frzme

This comment has been minimized.

Copy link

@frzme frzme commented Nov 11, 2019

I think it is rather unfortunate that this feature which supported in spring security 5.1 is being treated as an addition. I think it should be treated as an regression and therefore fixed within the scope of the 5.2 release and not postponed till 5.3. Using the "wrong" (previously working) DefaultClientManager fails transparently - for users it looks like it might be working, it's just re-requesting oauth tokens with every request, which makes it rather hard to spot the error.

I'm fine with a natively supported alternate configuration but hope that it can make it to 5.2.x.

@jgrandja

This comment has been minimized.

Copy link
Collaborator Author

@jgrandja jgrandja commented Nov 11, 2019

@frzme Please see comment

ankurpathak added a commit to ankurpathak/spring-security that referenced this issue Nov 25, 2019
…entManager

ReactiveOAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager is reactive
version of AuthorizedClientServiceOAuth2AuthorizedClientManager

Fixes: spring-projectsgh-7569
ankurpathak added a commit to ankurpathak/spring-security that referenced this issue Nov 25, 2019
…entManager

ReactiveOAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager is reactive
version of AuthorizedClientServiceOAuth2AuthorizedClientManager

Fixes: spring-projectsgh-7569
ankurpathak added a commit to ankurpathak/spring-security that referenced this issue Nov 27, 2019
…entManager

ReactiveOAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager is reactive
version of AuthorizedClientServiceOAuth2AuthorizedClientManager

Fixes: spring-projectsgh-7569
philsttr added a commit to philsttr/spring-security that referenced this issue Dec 5, 2019
philsttr added a commit to philsttr/spring-security that referenced this issue Dec 5, 2019
…review feedback

Rename OAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager to AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager.

Handle empty mono returned from contextAttributesMapper.

Handle empty map returned from contextAttributesMapper.

Fix DefaultContextAttributesMapper so that it doesn't access ServerWebExchange.

Fix unit tests so that they pass.

Use StepVerifier in unit tests, rather than .subscribe().

Addresses review comments of spring-projectsgh-7589

Fixes spring-projectsgh-7569
@jgrandja jgrandja modified the milestones: 5.3.x, 5.3.0.M1 Dec 6, 2019
philsttr added a commit to philsttr/spring-security that referenced this issue Dec 10, 2019
Rename OAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager to AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager.

Handle empty mono returned from contextAttributesMapper.

Handle empty map returned from contextAttributesMapper.

Fix DefaultContextAttributesMapper so that it doesn't access ServerWebExchange.

Fix unit tests so that they pass.

Use StepVerifier in unit tests, rather than .subscribe().

Fixes spring-projectsgh-7569
@jgrandja jgrandja self-assigned this Dec 10, 2019
@jgrandja jgrandja closed this in c29309d Dec 10, 2019
jgrandja added a commit that referenced this issue Dec 10, 2019
Rename OAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager to AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager.

Handle empty mono returned from contextAttributesMapper.

Handle empty map returned from contextAttributesMapper.

Fix DefaultContextAttributesMapper so that it doesn't access ServerWebExchange.

Fix unit tests so that they pass.

Use StepVerifier in unit tests, rather than .subscribe().

Fixes gh-7569
jgrandja added a commit that referenced this issue Dec 10, 2019
…entManager

ReactiveOAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager is reactive
version of AuthorizedClientServiceOAuth2AuthorizedClientManager

Fixes: gh-7569
jgrandja added a commit that referenced this issue Dec 10, 2019
Rename OAuth2AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager to AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager.

Handle empty mono returned from contextAttributesMapper.

Handle empty map returned from contextAttributesMapper.

Fix DefaultContextAttributesMapper so that it doesn't access ServerWebExchange.

Fix unit tests so that they pass.

Use StepVerifier in unit tests, rather than .subscribe().

Fixes gh-7569
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.