You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We are using oauth2 to authenticate against our custom OpenID Connect identity provider.
We need to pass some additional parameters when requesting tokens.
The way we did this so far is
added parameters we need as properties to oauth2_config
oauth2_config::_request_token does append_query() for our additional properties
May be passing additional parameters when requesting tokens could be done in a better more generic way and be added to the cpprestsdk?
Ideally you would add a pipeline stage before the OAuth handler is added. We don't have a nice API to do that, but you should be able to accomplish it by not setting the oauth2_config object inside the http_client_config and instead creating your own OAuth pipeline stage (should be as easy as copy-pasting the single relevant line in http_client::http_client(). Since you then control when the OAuth stage gets added, you can add your stage before it and thereby modify any outgoing OAuth requests.
Hi @ras0219-msft,
Not sure we are speaking about same thing...
Isn't it so that OAuth pipeline stage you are speaking is about sending http requests with the token acquired via OAuth already?
I was wandering about OAuth2 flow itself. We need to send additional parameters to the OAuth2 Identity Provider when initiating e.g. auth code grant flow.
It turns out I misunderstood your question. Yes, you're right, there is no way to do what you need currently. I totally agree it would be great to add a way to do this.
I do think that pipeline stages are the right way to do this; you will just need a way to add a pipeline stage to the OAuth2 internal http_client. Something resembling the current http_client interface, but on the oauth2_config object instead.
Alternatively, the oauth2_config could receive the whole http_client to use. This would enable total customization of the request pipeline, but would take more consideration to design and implement correctly.
Using pipeline stages sounds a little bit overkill to me in this case.
Another approach to introduce the extensibility point in oauth2_config::__request_token(uri_builder& request_body_ub) is to a generic map as an additional property to oauth2_config:
Then user can add any number of additional parameters to send with the request to the token endpoint and oauth2_config::__request_token() could just add them all with request_body_ub.append_query() in addition to standard parameters.
I did not mention this is the original message but we need a similar mechanism for building oauth2_config::build_authorization_uri(bool generate_state). E.g. we need to pass a lang=xx param to the browser to define a language to be used for the sign-in form.
Pipeline stages are certainly more powerful than you need at this moment. However, they do solve the entire class of problems similar to yours but slightly different (I need to remove a given header from the request, I need to redirect the API only in some circumstances, etc). I see pipeline stages as an appropriate solution when another opaque object owns the requests, but you need to perform some tweak.
In addition to a low level pipeline based solution, it might be reasonable to have an API like oauth2_config::append_query(string_t, string_t). My hesitation there is that it would be duplicating APIs that exist elsewhere, which is almost always a sign of poor design (we already have uri_builder::append_query, why is oauth2_config acting like a uri_builder?). Perhaps the oauth2_config should actually accept an entire uri_builder object instead of the current stringly-typed auth_endpoint. All current uses of auth_endpoint are to immediately pass it into a uri_builder which certainly suggests it shouldn't be a string_t in the first place.
In the case of build_authorization_uri, I think the cleanest solution is to deprecate it and add another api (say just authorization_uri) that either returns a uri_builder object directly or takes one as a by-ref out parameter. This would make it easy to modify any parameters of the URI and makes less assumptions about how the application wants to represent its own data. The existing build_authorization_uri would be reduced to calling the new overload and then calling to_string() on the result.
For our 3.0 release, I have been working on an overhaul to the OAuth2 APIs. As part of the overhaul, I added an API for Extension Grants, which I think satisfy your requirements above. As I posted in your other thread, I'd appreciate any feedback you have on the other proposed changes as well.
Activity
ras0219-msft commentedon May 10, 2016
I think pipeline stages might be appropriate for your use case here. Our current oauth support is implemented through a pipeline stage: https://github.com/Microsoft/cpprestsdk/blob/54d7fcdac2a2e4621d6af5c42140c0986c8d8334/Release/src/http/client/http_client.cpp#L388.
Ideally you would add a pipeline stage before the OAuth handler is added. We don't have a nice API to do that, but you should be able to accomplish it by not setting the
oauth2_config
object inside thehttp_client_config
and instead creating your own OAuth pipeline stage (should be as easy as copy-pasting the single relevant line inhttp_client::http_client()
. Since you then control when the OAuth stage gets added, you can add your stage before it and thereby modify any outgoing OAuth requests.alot1 commentedon May 10, 2016
Hi @ras0219-msft,
Not sure we are speaking about same thing...
Isn't it so that OAuth pipeline stage you are speaking is about sending http requests with the token acquired via OAuth already?
I was wandering about OAuth2 flow itself. We need to send additional parameters to the OAuth2 Identity Provider when initiating e.g. auth code grant flow.
Or did I misunderstood your suggestion?
ras0219-msft commentedon May 10, 2016
It turns out I misunderstood your question. Yes, you're right, there is no way to do what you need currently. I totally agree it would be great to add a way to do this.
I do think that pipeline stages are the right way to do this; you will just need a way to add a pipeline stage to the OAuth2 internal http_client. Something resembling the current
http_client
interface, but on theoauth2_config
object instead.Alternatively, the
oauth2_config
could receive the wholehttp_client
to use. This would enable total customization of the request pipeline, but would take more consideration to design and implement correctly.Would either of these work for you?
alot1 commentedon May 11, 2016
Using pipeline stages sounds a little bit overkill to me in this case.
Another approach to introduce the extensibility point in
oauth2_config::__request_token(uri_builder& request_body_ub)
is to a generic map as an additional property tooauth2_config
:Then user can add any number of additional parameters to send with the request to the token endpoint and
oauth2_config::__request_token()
could just add them all withrequest_body_ub.append_query()
in addition to standard parameters.I did not mention this is the original message but we need a similar mechanism for building
oauth2_config::build_authorization_uri(bool generate_state)
. E.g. we need to pass alang=xx
param to the browser to define a language to be used for the sign-in form.[-]Additional parameters with requesting OAuth2 tokens[/-][+]Additional parameters when requesting OAuth2 tokens[/+]ras0219-msft commentedon May 12, 2016
Pipeline stages are certainly more powerful than you need at this moment. However, they do solve the entire class of problems similar to yours but slightly different (I need to remove a given header from the request, I need to redirect the API only in some circumstances, etc). I see pipeline stages as an appropriate solution when another opaque object owns the requests, but you need to perform some tweak.
In addition to a low level pipeline based solution, it might be reasonable to have an API like
oauth2_config::append_query(string_t, string_t)
. My hesitation there is that it would be duplicating APIs that exist elsewhere, which is almost always a sign of poor design (we already haveuri_builder::append_query
, why isoauth2_config
acting like auri_builder
?). Perhaps theoauth2_config
should actually accept an entireuri_builder
object instead of the current stringly-typedauth_endpoint
. All current uses ofauth_endpoint
are to immediately pass it into auri_builder
which certainly suggests it shouldn't be astring_t
in the first place.In the case of
build_authorization_uri
, I think the cleanest solution is to deprecate it and add another api (say justauthorization_uri
) that either returns auri_builder
object directly or takes one as a by-ref out parameter. This would make it easy to modify any parameters of the URI and makes less assumptions about how the application wants to represent its own data. The existingbuild_authorization_uri
would be reduced to calling the new overload and then callingto_string()
on the result.ras0219-msft commentedon Jun 25, 2016
For our 3.0 release, I have been working on an overhaul to the OAuth2 APIs. As part of the overhaul, I added an API for Extension Grants, which I think satisfy your requirements above. As I posted in your other thread, I'd appreciate any feedback you have on the other proposed changes as well.
The Extension Grant is at
https://github.com/Microsoft/cpprestsdk/blob/192541718bdae3298cfcc06bd17d11d22a8cbbb1/Release/include/cpprest/oauth2.h#L453
alot1 commentedon Jun 25, 2016
Reading the code looks very good! I will give it a try and let you know the results.
alot1 commentedon Jun 28, 2016
Tried to build the oauth2-refactor-2 branch on OS X and on Windows, but did not succeed: there are compilation errors.
pinnekamp commentedon May 7, 2019
Is that branch still available somewhere? It does not seem to be merged.