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

Support the authorization code flow for Server Web Apps #4480

Closed
sberyozkin opened this issue Oct 9, 2019 · 24 comments
Closed

Support the authorization code flow for Server Web Apps #4480

sberyozkin opened this issue Oct 9, 2019 · 24 comments
Labels
area/oidc kind/enhancement New feature or request
Milestone

Comments

@sberyozkin
Copy link
Member

Description
The OIDC adapter has to be able to authenticate the users accessing the server web apps/confidential clients using the best practice code flow.
Stian recommends to distinguish between the bearer only and code flow modes using very user friendly property values something like:

# the adapter just checks the token with 401/403
mode=authorization-only

# the adapter will redirect when the user is not authenticated and possibly authorize as well based on the roles in the IdToken, while also letting the web app forward the access tokens further if needed
 
mode=web

etc. @stianst, @pedroigor FYI.

@sberyozkin sberyozkin added kind/enhancement New feature or request area/oidc labels Oct 9, 2019
@sberyozkin
Copy link
Member Author

Pedro, we will need to test that the static resources are accessible as part of it, I can wire in the test for sure :-)

@pedroigor
Copy link
Contributor

@sberyozkin, I'll be working on this one.

I'm not sure about yet how to indicate whether or not the app should start the code dance or just accept bearer tokens.

Meanwhile, I'll be working on making the necessary changes to get code flow working.

@pedroigor
Copy link
Contributor

@sberyozkin I guess we should leverage Vert.x OAuth2 in order to implement code flow in the extension, right?

@pedroigor
Copy link
Contributor

pedroigor commented Oct 9, 2019

@sberyozkin, I manage to get something work for code flow. But I have some questions:

  • In order to leverage code flow from Vert.x I'm invoking OAuth2Auth#authenticate so that we are able to both redirect to the IdP as well as handle the authorization response (with the code param). That means async invoking the IdentityProviderManager once the callback used to call the OAuth2Auth#authenticate is done.

  • I would be forwarding some parameters, if available as query parameters when reaching the app, to the IdP. For instance: scope, prompt, login_hint, etc. Starting by those we currently support in Keycloak. Is that OK?

  • We need to validate the state sent to the IdP when making the authorization request. To validate it later once we get an authorization response, I'll be setting a cookie which value is contains the state so that we can compare.

@sberyozkin
Copy link
Member Author

sberyozkin commented Oct 10, 2019

@pedroigor Hi Pedro, it all sounds good, thanks; I mean, we can tweak/optimize things going forward if something proves a bit sub optimal :-) Please consider to have a dedicated config group for the various redirect properties.

As per choosing between the bearer-only and the code flow, as per our discussion with Stian, lets start with a property such as mode and set it to either authorization (=> less technical compared to bearer-only) or web (=> code flow). May be even more user friendlier names can be provided but we can finalize before 0.25.0

@sberyozkin
Copy link
Member Author

Hi @pedroigor we have a priority issue for 0.25.0, #4495 as the 0.24.0 users are now starting to feel some pain due to the missing configuration :-). If you reckon you can finalize this one quickly enough then please do a PR, but otherwise lets work on #4495 (may be I can help)

@pedroigor
Copy link
Contributor

pedroigor commented Oct 10, 2019

@sberyozkin, I should be able to deliver this quickly, if tests allow me to do so :)

So far, I have the authorization code flow working by leveraging Vert.x OAuth2. Where a cookie is set by the extension to create a session for the user after successful authentication. Where session time on the application is bound to the ID Token lifetime.

This covers pretty much the basics. The missing points would be:

  • Tests. I think I'll need something like HTMLUnit to better write tests for this flow. Like using the authorization URL to redirect users, fill the login form, deal with consent (if necessary), etc.

  • Decide on how we would indicate that the app accepts only bearer or code flow (ongoing discussions)

  • Decide if we want to allow users to automatically refresh tokens so that the end-user is not redirected when the ID Token expires (SSO session lifetime may be different than ID Token and usually OPs update session time during refresh)

  • Support for forwarding query parameters such as scope, prompt, ui_locales, and login_hint, to the OP when starting the code flow by reaching the authorization endpoint. There is also some KC specific parameters that could be passed, which we could allow users to specify via configuration.

  • Logout

I think that once we do above, we're pretty much done. Being Logout what should take a little more time as it should also involve changes to Keycloak.

I can send a PR with what I have so far, without tests (see #1 above), if you want to.

@sberyozkin
Copy link
Member Author

sberyozkin commented Oct 14, 2019

@pedroigor Hi Pedro, thanks, I've actually missed this comment, sorry; IMHO, tests and a property for choosing between the type of the flow, plus some minor tweaks around making both types of tokens available would be all we need for a start to get it going; we can then agree on some follow-up tasks alongside what you've suggested, I'd say, having some basic logout support afterwards would be the 1st issue to sort out so that the users could have a complete experience :-)

@tassadar81
Copy link

Hello, do you have any plans about this issue? I'm facing now this topic for apps to be written in the company i'm working so it will be very useful to know a roadmap about is it's already planned.
Thanks

@sberyozkin
Copy link
Member Author

Yes, Pedro has already done a lot of work around it, we just have some higher priority issues to deal with. Stay tuned :-)

@sberyozkin sberyozkin added this to the 1.0.0 milestone Oct 24, 2019
@pedroigor
Copy link
Contributor

@sberyozkin I've talked with Stian about the missing configuration bits and we agreed about these points:

  • The client type values that we'll use to decide whether or the application supports bearer or code are: client and resource-server. Where client means a client acting on behalf of the user and interacting with a resource server (as per spec definition). The resource-server type means the application is bearer protected.

  • We figure out that the basis for getting standard logout working are already provided by Keycloak. We thought we were missing some bits, but what we have is sufficient to support logout in standard fashion.

  • In order to invalidate the session (and allow multiple instances/pods of a same application) based on the ID_Token carried by the "session" cookie, we are going to introspect it on a per-request basis based on a cache to avoid unnecessary requests to the server. We are going to have two basic configurations for this cache: max-entries and max-age. The max-entries refers to the maximum number of cached entries. The max-age how much a entry should live in the cache. If a ID_Token is not in cache that would cause a round trip to the introspection endpoint.

  • In order to allow users to pass additional parameters to the authorization request, prior to start de code flow, we are going to provide a hook (probably a CDI bean) so that users can add these parameters and send them to the OP.

I should be working with these changes now. Please, let me know if you have any consideration.

@sberyozkin
Copy link
Member Author

@pedroigor, and @stianst Thanks for the update.
So if it is a 'client' then it is the code flow ? And resource-server - the bearer token receiver ?
I'd only say that the 'client' may choose to forward the access token but may not be interested, all it wants is to interact with the user, it just happens the code flow will return the id and access tokens.

I'm not sure these particular qualifiers make it less technical for the users which is what Stian asked for originally, and is a good idea. To me 'client' means this resource server acts as an OIDC client which is fairly technical. And as I said, this 'client' can be just well, a plain 'resource-server' which only needs an ID token, not worrying about the access token.

I propose for us to think a bit more about these names ?

Speaking about the code flow, @pedroigor, as I said earlier, the PR now only makes the ID Token available but we need to make sure the access token is also available.

re the code flow properties, asking the users to create a bean just to set few extra scopes seems complex. If the users can add a lot then they should have this option, but I suggest we start with a configuration group for the redirect properties, example, let the users set a comma separated list of the scopes, and we can slowly grow that group.
thanks

@sberyozkin
Copy link
Member Author

sberyozkin commented Oct 24, 2019

@pedroigor I did a quick code a couple of weeks back. But it is not relevant now I guess. My plan was to find out how to get both ID and access tokens from Vertx

@stianst
Copy link
Contributor

stianst commented Oct 24, 2019

I'm not 100% convinced about client and resource-server either, but struggling to come up with good names. What about app and service? Or frontend and backend?

@pedroigor
Copy link
Contributor

I would go for what we know is a well-known type/concept which can be referenced. In fact, client is more related to OAuth where relaying-party is more related to OIDC and authentication.

@pedroigor
Copy link
Contributor

re the code flow properties, asking the users to create a bean just to set few extra scopes seems complex. If the users can add a lot then they should have this option, but I suggest we start with a configuration group for the redirect properties, example, let the users set a comma separated list of the scopes, and we can slowly grow that group.

This is because the decision to include a scope or any other property can be done dynamically based on requests.

I also thought about defining them, statically, in application.properties, but not so flexible.

@stianst
Copy link
Contributor

stianst commented Oct 24, 2019

Setting code flow properties is really something that has to be supported in a programmatic way. Scope, prompt, max_age, and IdP specific things are most often set dynamically at runtime and not statically in a config file.

@stianst
Copy link
Contributor

stianst commented Oct 24, 2019

For names of app types the general idea is that someone can secure their apps without knowing all that much about oidc. Names like client, relaying party and resource server are a bit confusing imo. Perhaps we should ask some non-security guys?

@pedroigor
Copy link
Contributor

Yeah, that could help. Whatever the person says we use :)

@stianst
Copy link
Contributor

stianst commented Oct 24, 2019 via email

@sberyozkin
Copy link
Member Author

sberyozkin commented Oct 25, 2019

@pedroigor @stianst thanks;

Re the names, I was thinking about the 'client' on the plane yesterday and I just could not sleep because of it :-). I was going to suggest authorization and web but then I had doubts again.
Indeed, marking a web application as the client if all what is required is to get the human users authenticating into it, can be confusing unless someone has read the OAuth2/OIDC specs. Technically it is correct but the concept of the web app being a 'client' for the purpose of protecting this very same web application can be confusing.

Something like 'service' and '(web)app', which is what Stian suggested reads better to me, I just prototyped '(web)'. 'service' is something which reads as if the interaction with a human user is not of the main concern as far as the security is concerned. While '(web)app' does imply somehow some HTML will be there :-).

So may be service and web-app ? The former is about just checking the tokens and returning 401/403, the latter can scale to either of the server web app cases Stian went through (1) just get the user authenticate with IdToken, 2) in addition to 1) make a remote call with the access token on behalf of the user).

Or completely alternatively, switch back to the flow types: bearer-only (which KC users in particular already know about), and now authorization-code ? And possibly follow up with some other flow when considered acceptable (once the OAuth2 token binding is more widely supported, etc) ?

What do you think ? thanks

@sberyozkin
Copy link
Member Author

@stianst @pedroigor sure, letting the users customize with the bean will meet all the variations, I realized afterwards trying to configure it all can be complex in its own way; we can still offer a configuration group for some straightforward cases, and make sure both pieces of the info (custom bean provided and the config-provided) add to a common set of the redirect parameters

@stianst
Copy link
Contributor

stianst commented Oct 25, 2019 via email

@pedroigor
Copy link
Contributor

@stianst @sberyozkin Deal.

pedroigor added a commit to pedroigor/quarkus that referenced this issue Oct 25, 2019
pedroigor added a commit to pedroigor/quarkus that referenced this issue Oct 25, 2019
pedroigor added a commit to pedroigor/quarkus that referenced this issue Oct 28, 2019
pedroigor added a commit to pedroigor/quarkus that referenced this issue Oct 28, 2019
pedroigor added a commit to pedroigor/quarkus that referenced this issue Oct 28, 2019
pedroigor added a commit to pedroigor/quarkus that referenced this issue Oct 28, 2019
pedroigor added a commit to pedroigor/quarkus that referenced this issue Oct 28, 2019
stuartwdouglas pushed a commit to pedroigor/quarkus that referenced this issue Oct 28, 2019
stuartwdouglas pushed a commit to pedroigor/quarkus that referenced this issue Oct 29, 2019
stuartwdouglas added a commit that referenced this issue Oct 29, 2019
[fixes #4480] - Initial Code Flow Support
@gsmet gsmet modified the milestones: 1.0.0, 0.27.0 Nov 3, 2019
mmusgrov pushed a commit to mmusgrov/quarkus that referenced this issue Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/oidc kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants