someone1 / hydra-gcp

A package used to bootstrap ory/hydra utilizing things like the IAM API for signing and Datastore for storage
Apache License 2.0
4 stars 0 forks source link

Status? #1

Open HIRANO-Satoshi opened 6 years ago

HIRANO-Satoshi commented 6 years ago

Hi Prateek,

I'm looking for a high performance OIDC PR for GCP. Your work seems great.

I have some questions. Is it ready to use? You contributed some PRs to hydra. Have you reflected them to this work?

What client library for JS are you using with this? Does silent refresh work?

someone1 commented 6 years ago

I haven't updated the datastore plugin with the latest changes to hydra (I think its updated to beta 7) which is on my to-do list for the next week or two. I have a test setup running on my production environment for the hydra 0.11 release which is running fine but if you're not in a hurry I suggest waiting until the 1.0 hydra release which includes many new features, including proper support for silent refresh.

For the JS library, I'm using AppAuth-JS and extended it a bit for my use-cases (including silent refresh). The project itself provides the basic plumbing but unfortunately batteries are not included. Luckily, it isn't too much trouble to implement the remaining parts.

I'll update here once I update my plugin to work with the latest hydra release and try it in my production environment.

HIRANO-Satoshi commented 6 years ago

Okay, thanks. I will wait and I would like to contribute to it someday.

I'll use only the datastore part and won't use IAM. To use IAM, users need to have Google acounts, right?

I've read a lot of arguments about secure token refresh for public clients. Maybe auth code flow (with PKCE) would be best for public apps with silent refresh using httpOnly cookies instead of use of refresh tokens.

Yesterday I setup Hydra and AppAuth-JS and confirmed they work together. Next I need silent refresh in AppAuth. I would appreciate if you could share your version of AppAuth.

Do you think AppAuth-JS accepts silent refresh if you send a PR?

someone1 commented 6 years ago

The IAM API is used to sign the JWT access tokens. Currently, Hydra prefers to manage its own keys and only recently added support for using JWT vs Opaque access tokens. I prefer to use a managed service (such as the IAM API or possibly a KMS) to manage my keys/secrets. IAM was simple enough for me to add since I wrote other projects to interface with it.

You're users do not need Google accounts if you want to use the IAM features - its only used to sign tokens, but with hydra, your login/consent apps handle user login/management and tell hydra to issue a token or not. To be clear, you can have users sign-up/login with any OpenID Connect (e.g. Facebook, Twitter, Google, etc.), local user management, SAML, etc. system you want on your end, and leverage hydra + GCP to manage Oauth2 clients/tokens leveraging Cloud Datastore for the database and the IAM API for key management (you never know the private key and its rotated regularly).

For the AppAuth-JS silent refresh bit - it's not written in a way that can be shared/pluggable and since I'm waiting for the hydra 1.0 release before deploying my application to production, I can't vouch for its accuracy or compatibility. Here is the high level of what I did to get "silent" refreshes:

AppAuth-JS rejected any request to add more functionality like Silent Refresh or session management, so I don't think we'll get this in the core functionality.

Some relevant Code bits (this is just some cut/paste from my code, not sure it'll compile and its missing a few things I'm sure):

import {
  AuthorizationRequest,
  AuthorizationNotifier,
  RedirectRequestHandler,
  LocalStorageBackend,
  AuthorizationNotifier,
  LocationLike,
  BasicQueryStringUtils,
  cryptoGenerateRandom,
} from '@openid/appauth';

     const sessionIframe = window.document.createElement('iframe');
     sessionIframe.id = 'myiFrameForSilentRenew';
     sessionIframe.style.display = 'none';
     window.document.body.appendChild(sessionIframe);

    const LONGER_RANDOM_BYTES = 8;
    const longerRandomGenerator = () => {
      return getRandomString(LONGER_RANDOM_BYTES);
    };

    const storage = new LocalStorageBackend(window.sessionStorage);

    const request = new AuthorizationRequest(
              <clientid>,
              <static_callback_html_redirect_uri>,
              scopes.join(' '),
              AuthorizationRequest.RESPONSE_TYPE_CODE,
              getRandomString(128),
              { access_type: 'online', prompt: 'none' },
            );

  const authorizationHandler = new RedirectRequestHandler(
              storage,
              new BasicQueryStringUtils(),
              sessionIframe.contentWindow.location,
              longerRandomGenerator,
            );
   const notifier = new AuthorizationNotifier();
  // Setup Notifier and message listener
  const recieveMessage = (event: MessageEvent) => { 
   // Make sure the response is from the same origin and contains the response code or handle errors
  // Then create a new RedirectRequestHandler and call `completeAuthorizationRequestIfPossible`
   }
  const eventOpts: AddEventListenerOptions = {
     passive: true,
  };
  window.addEventListener('message', recieveMessage, eventOpts);
  authorizationHandler.setAuthorizationNotifier(notifier);
  authorizationHandler.performAuthorizationRequest(configuration, request);
HIRANO-Satoshi commented 6 years ago

Thanks so much! It would be great help for me and others.

I understand. The IAM manages secret keys for JWT for you and expires access tokens in one hour, so you need silent refresh. Resource servers must run on GCP and use your gcp-jwt-go for validation.

It would be useful. How about performance? Is it fast enough? throughput?

I think validation of an access token in a resource server could be done by just fetching the session information for the access token from Datastore. Since we don't need to call token introspection API, it's fast. That should work for JWT tokens and we don't need to call IAM API, right?

The silent refresh is the matter of usage of AppAuth-JS and you didn't modify it, right?

You do performAuthorizationRequest() repeatedly, before an access token expires. Or even after the access token expired, because Hydra gives you a long life cookie?

someone1 commented 6 years ago

There is no requirement to run on GCP, you can run on any platform you wish, though since this is meant to leverage GCP services it will be most performant if you run it there.

I have not done any benchmarking, though I'd expect it to be similar in performance to Hydra with the overhead of the IAM API for signing tokens. Now that their Cloud KMS supports asymmetric keys, I may switch from the IAM API to that though there's a lot to be considered before doing so - I believe this API is highly performant and shouldn't come with any soft limits the IAM API may have.

Yes you can try and access the Datastore directly, however, I personally don't do this nor do I recommend doing so. Since this is a JWT, you can validate the tokens signature using my gcp-jwt-go package which keeps the public certs in memory and validates fairly quickly. This validates the JWT is properly signed (thus was once authorized) but does NOT guarantee that the token was not revoked. If you need to validate that a token is not revoked on every request, then you can use the token introspection API from hydra or directly access datastore, though keeping in mind the API from hydra will be more stable. Depending on your needs, you can keep the lifetime of an access token low (e.g. 5m) so that you limit your exposure if you don't check for revoked tokens on every request the token.

In terms of silent refresh in AppAuth-JS, I'll admit that I haven't fully tested this since hydra is still working towards a 1.0 release and will manage consent cookies and such for us. It should work somewhat as follows:

  1. Upon first load of the application, a silent refresh is requested and immediately rejected
  2. The application registers the rejection and performs a normal redirect flow for login
  3. The user logs in and gives consent for the application and is redirected to the application
  4. The application performs a PKCE code exchange and gets an Access Token
  5. The application performs a silent refresh prior to the Access Token expiration to renew it

I've given some code/hints on how to implement a silent refresh handler with AppAuth-JS, which out of the box does not provide this implementation, though it should support it without modification. You can see how other packages handle silent refresh (angular-oidc is a popular choice - though it doesn't support PKCE) so you can get an idea of how to use AppAuth-JS to implement your own.

HIRANO-Satoshi commented 6 years ago

Thanks much for explanation!!

If I use your gcp-jwt-go for validation, users could access our server after they singed out for five minutes, for example. If you do that in your app, is that safe?

I understood the silent refresh story. I added silent refresh, implicit/hybrid flow, login popup window/tab, Cordova and so on to the example app of AppAuth. I think it covers most use cases.

https://github.com/HIRANO-Satoshi/AppAuth-JS/blob/popup-redesign/src/app/index_unified.ts

I almost wrote unified_authorization_handler.ts as a subclass of redirect_auth_handler.ts.

Do you think they accept if I complete it (with test) and send this as a PR? I'm not confident in this case. I managed to keep existing code almost unchanged, though.

2018-09-15 14 50 07

someone1 commented 6 years ago

I've updated my gcp-jwt-go package to implement Cloud KMS, rewrote it as it was clumsy and not optimized as well as it could have been, and added performance to the readme. Obviously since it's using a 3rd party to sign tokens, it will be slower than any local/native signing mechanism, but it is much more secure.

As far as short-lived Access Tokens go: the validation of tokens isn't usually too expensive, and I will try and get performance numbers regarding basic signing/introspection requests for hydra-gcp so you'd have a better idea of the latency, but I personally do introspection requests on every request. There are many strategies to optimize for this - such as a blacklist check vs a full introspection request where the blacklist can be local to your API server (if using a single server) or a redis cache (the blacklist item only needs to live as long as the token is valid for!). If your app discards the Access Token after logout, the only real potential issue you're trying to protect against here is that the token doesn't exist elsewhere (leaked?) before its expired which may be another issue altogether.

Your work seems great! I'm not sure about them accepting a PR, the author explicitly said the library was just meant as basic plumbing. That said - I haven't had a chance to look through your code completely but I assume you tested it and it works? You can maybe release a library on NPM or something but it does seem as if some of your code was specific to your use case (e.g. the material snackbar addition).

someone1 commented 6 years ago

FYI - Some performance numbers can be found here for hydra gcp: https://github.com/someone1/hydra-gcp/tree/master/benchmarks

Since this is targeted toward Hydra 1.0 which is still in beta, I won't say this is "production" ready - but if you're in the development phase, you can start trying this out and report any bugs you encounter here. I think there is opportunity to make this even faster by utilizing Cloud KMS which I'll be exploring.

HIRANO-Satoshi commented 6 years ago

Thanks, Prateek. Very interesting!

For introspection PostgreSQL is the actual benchmark which consumes 0.068 in most cases. Datastore+IAM requires 0.109 in typical cases.

You think 0.109 sec has impact to user experiences, don't you? It is not small.

Datastore+opaque says the Datastore is much slower than postgreSQL. We could shorten that by either caching of Datastore or the Datastore shortcut from RP.

Datastore clients with caching capability such as nds and Goon do not work on GAE flexible and GCE yet. But we don't need such big library. How about just use Google's Memorystore API here?

func (f *FositeDatastoreStore) findSessionBySignature(...
    if err := f.client.Get(ctx, key, &d); err == datastore.ErrNoSuchEntity {

A short lived on-memory cache could improve the performance for repetitive calls to RP drastically (as NDB does). We can invalidate the cache by deleteSession() and revokeSession(). Even only the on-memory cache would be much better.

I haven't tried your new version yet. Thanks for the new release. I will use it.

I spent time to improve my fork of AppAuth and implement our login provider. I almost finished. Silent refresh is working beautifully. Thanks so much!

I was really disappointed when Hydra showed the following error. Our app is a public app. I resolved it using login_hint and now it's working.

The Authorization Server requires End-User consent" error=consent_required hint="OAuth 2.0 Client is marked public and requires end-user consent, but \"prompt=none\" was requested.

HIRANO-Satoshi commented 6 years ago

To speed up token validation we want to do validation on our resource server locally. But there would be time lag between logout and the lifetime of an access token.

If our client teaches logout to our resource server too, there is no time lag. No introspection API call, no Datastore access, and no IAM access. 0.0464 secs. Very fast.

BTW, I think we could use Google's App Identity Service to sign besides IAM. I'm not very sure.

someone1 commented 6 years ago

We can't sign using the Identity Service since hydra won't run on AppEngine Standard. With the recent release of go 1.11 for AppEngine we may get cgo compatibility and make this possible but the Identity Service API is on its way out so you're better off using something like IAM or CloudKMS though we can explore this if hydra ever runs on AppEngine standard.

I have a PR for qedus/nds to add support for redis, we'll see if it makes a difference in the benchmarks to have a cache at the datastore level (it should) once I'm done and it's been merged (if it ever gets merged).