Using Keycloak/OKTA with jwt instead of sessions

yelhouti commented 5 years ago

Overview of the feature request

I would like to be able to use jwt instead of sessions when working keycloak.

Motivation for or Use Case

Using jwt instead of sessions has many advantages including having stateless micro-services. I don't think you guys need more convincing, since UAA is staeless.

There is a security problem with how OAuth2 is implemented today (as discussed here: https://github.com/jhipster/generator-jhipster/issues/6941#issuecomment-424038200), and while fixing it I would prefer to do things the right way.

Related issues or PR

It have been discussed in some comments on other issues, but I prefer having a new issue that either fixes the concern, or explain the choices to other users.

[X] Checking this box is mandatory (this is just to show you read everything)

PierreBesson commented 5 years ago

We don't plan to do this as there was specific reasons why it was decided to do stateful Oauth2 instead of stateless, see this twitter thread: https://twitter.com/avdev4j/status/1087652807505780736

As for the sign out problem you talk about, it should be fixed in the last 5.8 release : https://github.com/jhipster/generator-jhipster/pull/8757

yelhouti commented 5 years ago

@PierreBesson I'm talking about security problems, not the logout ones. Also, if someone manages to access the token localStorage (we can also use sessionStoarge by the way) he could also access the sessionCookie, so no difference there. Any other reason?

PierreBesson commented 5 years ago

It seems you know what you are talking about so we should discuss this more. I'm reopening the issue. ping @mraible.

mraible commented 5 years ago

The session cookie is sent with an httpOnly flag (if it’s not, it should be) that prevents JavaScript from being able to read it.

The problem with moving to implicit flow (besides that it’s not as secure) is that we’d have to maintain a lot more code on the client apps. Implicit flow + PKCE offers a more secure solution, but will carry the same maintenance burden on the client code.

On Jan 26, 2019, at 04:04, Pierre Besson notifications@github.com wrote:

Reopened #9120.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

yelhouti commented 5 years ago

@mraible even if httpOnly flag is used the attacker could send any request through my browser when using sessions, which opens to attacks like XCRF. Also I'm not suggesting using implicite flow, but hybrid flow (which doesn't require a client secret, and allows for refresh tokens on the front end), implicit flow is such a hassle since you spend time redirecting to retrieve a new access token.

mraible commented 5 years ago

@yelhouti If you want to create a pull request, I'll be happy to review the code.

yelhouti commented 5 years ago

@mraible can I first create project, we agree on what is generated, then merge? Also, I didn't mention that PKCE used to be a necessity for iOS mobile apps, since older iOS versions allow for multiple apps using the same URL (scheme) now (since iOS 10 I think) you can use https with domain verification, so not a problem anymore.

mraible commented 5 years ago

Here's the process I typically use when modifying JHipster:

Generate a new app, make the necessary changes, get it all working. Check into source control.
For the generator-jhipster project, make changes to templates, and iterate until I can generate the same project as the final result of step 1.
Create pull request with necessary changes. Fix any test failures caught by CI.

My co-worker @aaronpk submitted a new draft for OAuth 2.0 browser-based apps that recommends authorization code flow + PKCE.

We don't support CORS on our /authorize endpoint just yet, so it's not possible to do PKCE in a browser with Okta today. We hope to fix that in the next couple of months. I'm not sure about Keycloak.

yelhouti commented 5 years ago

Keycloak support CORS, they also have valid redirect URIs which might be required. What I will do is step one, validate with you, then do the others steps.

avdev4j commented 5 years ago

I am very interested in this feature @yelhouti Feel free to fork the generator and open a pull request (even with WIP status). We could probably help you if it's necessary. 👍

yelhouti commented 5 years ago

First I will create the POC with a monolith project (it's almost done) then I will contact you for wokring on the generator. I'll share my code with you @avdev4j very soon. feel free to talk to me on gitter so we can better organise.

VinodAnandan commented 5 years ago

Hi,

I agree with the fact that a long lived insecure session has more security implications compared to the short lived tokens.

As HTTP is stateless, the protocol session was mainly used to identify the request. The OpenID connect enables the Identity, Authentication, Authorization. If OIDC is already utilized for IAM, then there is no need of using additional session handling. In-fact it may result in a more complicated architecture as well as possible security and operational issues. Session is not a good friend for the microservice architecture.

To implement session management securely - https://www.owasp.org/index.php/Session_Management_Cheat_Sheet

There are several implementations of SPA with PKCE

https://damienbod.com/2019/01/09/securing-angular-applications-using-the-openid-connect-code-flow-with-pkce/

https://github.com/damienbod/angular-auth-oidc-client

https://github.com/IdentityModel/oidc-client-js

VinodAnandan commented 5 years ago

Vue.js : https://damienbod.com/2019/01/29/securing-a-vue-js-app-using-openid-connect-code-flow-with-pkce-and-identityserver4/

yelhouti commented 5 years ago

Here my version with angular and keycloak: https://github.com/elhoutico/jhipster-keycloak-stateless There are still few things todo (check the TODOs) and help is appreciated

yelhouti commented 5 years ago

@VinodAnandan @mraible or anyone, can you tell me if PKCE is needed in any other case than the one I mentionned (with iOS) to see how important it is to implement. Thanks

mraible commented 5 years ago

@yelhouti Can you provide a brief summary of what you did to get things working? I see you're using keycloak-js. Does this store the access token in local or session storage? If so, I'm not in favor of this change as it reduces security.

yelhouti commented 5 years ago

@mraible keylcoak-js doesn't use storage for storing the tokens, also I'm planning to use session storage for my implementation that replaces keycloak-js to make it work with Okta and solve other problems. can you tell me how an attacker that manages to get access to my machine/browser and steal the token from storage, not able to use the cookie (I can show how the opposite is true meaning cookies are less secure).

What I did is:

get/refresh the token in the front end and send it with each request
verify the tokens signature and audience using the private key instead of and endpoint using the new OIdC support by spring security.
some other minor stuff

mraible commented 5 years ago

My colleague, @rdegges, wrote a post about what's wrong with using local storage, which I believe applies to session storage as well. The most common way that attackers can get access to your local storage is via 3rd party scripts. Of course, a developer probably won't add 3rd party scripts, but as soon as marketing gets involved, some might be added.

For cookies, you can use an httpOnly flag to prevent JavaScript from having access to them.

I think it's OK to add what you're implementing to JHipster, but I don't think it should be the default option. I think we should prompt the user to choose between authorization code flow (the current setup) and implicit flow. Then they can make the security decision themselves, and we can warn them that implicit flow is less secure.

If we do add support for implicit flow, we should also enable a CSP by default that says "only scripts from this app" are allowed.

yelhouti commented 5 years ago

@mraible, With all due respect to your colleague, if some javascript can be used to access storage, it can be used to send any request using the cookie without retrieving it, it might be less convenient for the attacker but for me it's the same. Also, using cookies opens you to more attacks like XSRF, where a script can execute GET requests using your cookie without even injecting malicious code, getting access to your browser or attacking the server, but only abusing the trust the server has in your browser (in his own website, the attacker can send GET requests to yours if both are open inside the same browser, and use your cookies in the process). Also open to discuss the matter with your colleague if his interested. By the way I read the article, I agree with most point except using sessionStorage for JWT, which for fairness he doesn't say to avoid at all cost. Here is OWASP take on the subject: https://www.owasp.org/index.php/JSON_Web_Token_(JWT)_Cheat_Sheet_for_Java#Token_storage_on_client_side And is exactly what I'm planning to do.

mraible commented 5 years ago

That's good info @yelhouti, thanks for the link!

Another thought I had today is that this is already implemented in Ionic for JHipster. When you create an Ionic project, it sets up a resource server on the JHipster side and uses implicit flow. In the next version, I'll upgrade to Ionic 4 and try adding PKCE support. Since you can use Ionic for PWAs, and it supports Angular, React, and Vue in v4, that could be an option.

ruddell commented 5 years ago

@yelhouti I think you are forgetting about the CORS and XSRF protection in JHipster

CORS prevents an attacker from making a request to your backend unless you specifically add their domain to the authorized origins.
The XSRF token is extracted from a cookie value and added as request header to any PUT/POST requests. This also prevents an outside site/script from making a valid request that changes data (other domains don't have direct access to your domain's cookies, so can't extract the value).

In the link you posted, it addresses this in the section notes:

Note: It's also possible to implements the authentication service in a way that the token is issued within a hardened cookie, but in this case, a protection against Cross-Site Request Forgery attack must be implemented.

Note that I'm not trained in security so others will probably know better.

VinodAnandan commented 5 years ago

Hi,

In the implicit flow, the token exchange is happening via GET requests and it will result into the token getting logged in different servers, client side, etc. An abuser who gets access to this token can easily impersonate the user.

Using PKCE, the client app only receives the authorization code via the GET request and it's exchanged via a POST request with the token endpoint to obtain tokens. Even if the abuser gets the authorization code, it will be protected with PKCE (Proof Key for Code Exchange ).

yelhouti commented 5 years ago

First of all guys, thank you all for your comments, I really appreciate it. @mraible I will definitely check the cods and see if we can reuse/improve it. Also, since Ionic 4, a simple blueprint might help generate the Ionic project if stateless OIdC support is added. @ruddell I didn't forget about the protections, these are complimentary: If you follow the linked you mentioned, about using cookies it says on protecting against XSRF: The following list assumes that you are not violating RFC2616, section 9.1.1, by using GET requests for state changing operations. Which I can tell you many people use (example: mark message as seen when user X GET's it) to summarize XSRF tokens protect only POTS/PUT and XHR requests when implemented correctly, GET request can't be protected that way since (your second point) they can circumvent CORS protection. If you want more information on how or to discuss the matter more in dept please DM me on gitter it would be a pleasure. (Edit: experience is often more valuable that training, and we are all human who forget things, so thanks for helping :) ). @VinodAnandan , as I tried to explain I'm planning to use hybrid flow instead implicit flow which doesn't not have the problem of GET params. for PKCE, the RFC (https://tools.ietf.org/html/rfc7636#section-1) says it's used to avoid attacks "within a communication path not protected by Transport Layer Security (TLS), such as inter-application communication within the client's operating system" this is not possible with browser and the only thing I can think of it resembles to is "scheme squatting" for iOS which is not a problem anymore if you use universal links/deeplinks (https://developer.apple.com/ios/universal-links/) available since iOS 9. Am I missing a point or there is no need for PKCE anymore in our use case.

mraible commented 5 years ago

@yelhouti My colleague, @aaronpk, proposed a new spec for OAuth 2.0 that recommends PKCE for browser-based apps. See OAuth 2.0 for Browser-Based Apps for more information. From its overview section:

For authorizing users within a browser-based application, the best current practice is to

o Use the OAuth 2.0 authorization code flow with the PKCE extension o Require the OAuth 2.0 state parameter o Recommend exact matching of redirect URIs, and require the hostname of the redirect URI match the hostname of the URL the app was served from o Do not return access tokens in the front channel

Previously it was recommended that browser-based applications use the OAuth 2.0 Implicit flow. That approach has several drawbacks, including the fact that access tokens are returned in the front-channel via the fragment part of the redirect URI, and as such are vulnerable to a variety of attacks where the access token can be intercepted or stolen. See Section 7.8 for a deeper analysis of these attacks and the drawbacks of using the Implicit flow in browsers, many of which are described by [oauth-security-topics].

Instead, browser-based apps can perform the OAuth 2.0 authorization code flow and make a POST request to the token endpoint to exchange an authorization code for an access token, just like other OAuth clients. This ensures that access tokens are not sent via the less secure front-channel, and are only returned over an HTTPS connection initiated from the application. Combined with PKCE, this enables the authorization server to ensure that authorization codes are useless even if intercepted in transport.

yelhouti commented 5 years ago

EDIT: this comment doesn't serve any purpose anymore :)

VinodAnandan commented 5 years ago

Hi,

I will explain the other situations when the authorization code can be leaked and abused. Between a public client and an Identity Provider Server, there can be many other servers and devices ( Load balancer, Firewalls, WebServers, app servers, etc.). Most of these will log (e.g: access_log) the URL parameters including the public client (browser history).If an abuser gets the authorization_code from any of these sources, he can send it to the Identity Provider and obtain the tokens to impersonate the user.

For a PKCE flow, along with the initial request, the public client will send a code_challenge/hash of the random string. And when the public client will request the tokens with an authorization code, it must provide the code_verifier (random string) otherwise the tokens won't be issued.

Authorization code and refresh code is really sensitive and it needs more security. Silent renewal is a secure replacement of using refresh token SPA.

All the information mentioned above can be implemented in an Angular SPA and it's explained in the following link.

https://damienbod.com/2019/01/09/securing-angular-applications-using-the-openid-connect-code-flow-with-pkce/

If you need further details, please let me know.

yelhouti commented 5 years ago

@VinodAnandan authorization_code can't be used twice, and if you have SSL from your keycloak server the browser/app there is no way it can be retreived. I understand exactly how PKCE works, what I don't see is the need for it when you have HTTPS. (event query parameters are intercepted when you use HTTPS). Any way, If you guys insiste on adding PKCE, even if I don't agree I will oblige, it's 10 more lines of code, I will live :) . But thanks for confirming my suspicions. Also I found this: https://github.com/Hitachi/contributions/wiki/Description-of-RFC7636-for-keycloak that says OIdC protects against the same attacks using nonce...

yelhouti commented 5 years ago

Also, I found this lib https://github.com/IdentityModel/oidc-client-js that is certified by openid https://openid.net/developers/certified/ Cherry on the cake, they support PKCE...

mraible commented 5 years ago

There's also AppAuth for JS from the OpenID project itself. I've used it with Electron in the past.

VinodAnandan commented 5 years ago

@yelhouti the abuser may have access to the data in the URL from any source mentioned before (SSL/TLS can't prevent all data leakage). It's not a safe practice to share sensitive data via the URL.

The beauty of PKCE is that it will help to authenticate a public client without a public client credentials. The code_challenge and code_verifier will indirectly authenticate the public client.

Also, the silent renewal is very important. It will help to prevent storing/processing of very very sensitive refresh token in the browser.

The following angular-auth-oidc-client is also a Certified OpenID Connect Implementation. They have implemented PKCE with silent renewal.

https://github.com/damienbod/angular-auth-oidc-client

More details about the implementation can be found on the below URL.

https://damienbod.com/2019/01/09/securing-angular-applications-using-the-openid-connect-code-flow-with-pkce/

yelhouti commented 5 years ago

@VinodAnandan I agree about the URL part even though I can't find any modern reverse proxy/server that logs query param by default. stupid servers by the way can also log post params.

Your lib sounds pretty cool, keycloak-js adapter uses a similar iframe mechanism. I will definately give it a chance since I will be starting with angular part.

Thanks for sharing

yelhouti commented 5 years ago

I'm pretty happy with the current version at https://github.com/elhoutico/jhipster-keycloak-stateless please @mraible give it a look so I can start working on the generator. For the anecdote, there was a bug blocking me in the library yesterday, after I understood what it was it was fixed today. So it all worked great. By the way, great lib @VinodAnandan . My implementation can be much improved to show time remaining before token expiration + a button to refresh..., all easy to do thanks to the lib.

yelhouti commented 5 years ago

@mraible are you ok with the changes, and can I start working the generator please?

Thanks

VinodAnandan commented 5 years ago

@yelhouti, I am happy that I could convince you about the benefits of PKCE as well as some security issues with the implicit flow. If you want to discuss more, please let me know

mraible commented 5 years ago

@yelhouti I took a quick look at your example project and figured out what you changed by looking at the commits. It looks like there are some Keycloak settings hardcoded in the client. With the current implementation, it's easy to switch OAuth providers using properties. With this setup, it looks like developers will need to modify app.module.ts to specify their settings. Is there any way to implement so it's easier?

You'll need to implement things for the React implementation too.

yelhouti commented 5 years ago

@VinodAnandan to tell you the truth, I'm not convinced at all about PKCE but since it's in the lib why not. For Implicit flow It was clear that Implicit flow has many security and practicality drawbacks so it was a no go. On the other hand the angular lib you pointed at was very helpful :)

yelhouti commented 5 years ago

@mraible Indeed we can put stuff in a app.constants.ts (which I forgot to clean by the way). for the react part, do you know anyone that could help :) ?

pascalgrimaud commented 5 years ago

I'd suggest to wait @jdubois and @deepu105 before starting to implement it into the core, as it will add one additionnal option.

Another solution: what it could be done too, is the blueprint. Same option, excepting for authenticationType.

deepu105 commented 5 years ago

I'm not in favour of adding another auth option to the mix, unless @mraible says that the value outweigh the complexity, as it will complicate the templates. So I'll also suggest you to try it as a blueprint

deepu105 commented 5 years ago

Or if this option is better than current then replace it. I'm not in favour of providing 2 options for oauth in questions coz

It will increase our maintenance burden
It will confuse most normal users as they may not understand the differences clearly
If we are gonna say one option is less secure than other in question then no point providing the less secure option

So I suggest you guys come to an agreement about which is a better implementation and go with that instead of providing options or do it as a blueprint

yelhouti commented 5 years ago

Totally Agree with @deepu105 , also IMHO, once we have proper support for OIdC, we should tweek Jhipster's UAA implementation to use the same client code for authentication, this would fix some security issues that could happen with the OAuth2. @mraible what do you think?

mraible commented 5 years ago

I'm not convinced this is a better solution than the current one. It seems to require more code on the client, which will cause a maintenance burden. If it's possible to fix the small security issue you mentioned, I'd rather go that route.

yelhouti commented 5 years ago

@mraible Here is why I think this solution is better:

fixing the issue that a mentioned (which should clearly be done), doesn't protect against other threats that are inherent in the session paradigm (CSRF attacks...)
it's stateless so from our server/microservice stand point so scally horizantally is easy/efficient
sessions don't work OOTB with mobile applications...
Keycloak receives at most one request per user request instead of one or more per microservice. ...

I'm happy if you can change my mind, since this involves a lot of work for me, (that I'm doing any way for my customers ) :p

deepu105 commented 5 years ago

Since @mraible is the stream lead of OIDC as well as the original contributor of this part I'll leave the final decision to him.

mraible commented 5 years ago

@yelhouti We currently have CSRF protection with both React and Angular, so I'm not sure what you mean in your first point. Do you have any code or a test that shows what we have is vulnerable?

Yes, it's stateless, but have you done performance tests to ensure it scales better? In theory, it might, but I'd like to see some hard numbers. You can always use Spring Session with Redis, which will likely solve performance issues.

For mobile, both Ignite JHipster and Ionic for JHipster setup a resource server. You can see the one for Ionic here.

I'm sorry for being stubborn about this. In my time at Okta, I've learned a lot about OAuth 2.0 and OIDC. From what all my co-workers tell me, authorization code flow on the server is the most secure implementation. I don't want to reduce security. I can see this as an add-on, but not a replacement.

If you feel really strongly about this, I'd suggest you implement it as a module, or volunteer to take over the stream lead for OAuth/OIDC in JHipster. I'd be happy to give up the maintenance burden. ;)

yelhouti commented 5 years ago

for the first point, you can't protect against CSRF for GET requests with side effects since cookire are sent automatically by the browser even if used from another domain (which I think is still the case for some backward compatibility reasons), explained here: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet . For the redis solution having a consensus between all the nodes has a cost that increase exponentially with each new node, this same bottleneck is present when you want to scale keycloak/Okta or it's DB for SSO reasons..., and it would just a waste of resources to to have to deal with the same problem twice. Ionic is web and I don't know about Ignire, but what I know is that the Http client in native Android apps doesn't handle cookies OOTB. @mraible no need to apologize I'm stubborn my self :D , and you and your coworkers are right, authorization code flow on the server is the most secure solution but:

It only works with clients that can handle redirects and auto posting forms (native mobile http client can't).
It doesn't scale well.

It would be an honor for me to take the lead for OAuth/OIdc in JHipster if the core team agrees :) .

mraible commented 5 years ago

@yelhouti Can you please confirm your current example works with Okta? You can signup for a developer account at https://developer.okta.com/signup/.

yelhouti commented 5 years ago

@mraible sure

yelhouti commented 5 years ago

@mraible I tried chaging the CORS configs... didn't manage to have CORS header added to response from token endpoit: Access to XMLHttpRequest at 'https://dev-190319.okta.com/oauth2/v1/token' from origin 'http://localhost:8080' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. Once this is fixed, It will work. It seems, you already now that: https://devforum.okta.com/t/cors-headers-for-oauth2-v1-token/355/6 :p Also implicit flow doesn't work well with angular because of the # used also for URLs, but I'm sure I can make it work if I tweak a bit... Yep, can easily be solved using: https://github.com/damienbod/angular-auth-oidc-client/issues/132 @deepu105 are you sure you don't just want to add a new stateless Keycloak/OKTA option at this point to keep backward compatibility. as i'm sure people will complain if we replace an option we already had, even if it's for a better one?

mraible commented 5 years ago

@yelhouti Can you try using https://dev-190319.okta.com/oauth2/default/v1/token as your base URL instead? If that doesn't work, make sure http://localhost:8080 is listed as a Trusted Origin in API > Trusted Origins.

If it still doesn't work, I believe it's because we don't allow CORS to our token endpoint. It works in mobile and Electron apps because they don't send an origin header.

jhipster / generator-jhipster