react-keycloak / react-keycloak

React/React Native/NextJS/Razzle components for Keycloak
MIT License
560 stars 131 forks source link

Support SSR with Razzle #51

Closed panz3r closed 4 years ago

panz3r commented 4 years ago

Add support for app server-rendered using Razzle.

monowai commented 4 years ago

Thanks for opening this.

@react-keycloak/nextjs extension is largely pure React so it should be good to use across any SSR approach. appWithKeyCloak, which is the only class to import from NextJS, is there to support state of certain key variables including the keycloak object.

Perhaps enhancing @react-keycloak/web to support plugable storage mechanisms - including NextJS and maybe favouring localstorage - to persist and retrieve key variables to use in the initialisation of keycloak? Granted this won't solve the problem of reinitialising the keycloak, but it would improve user experience when it comes to reloading

panz3r commented 4 years ago

Hi @monowai ,

Thanks again for your feedback and patience.

I just released an initial implementation of the adapter for razzle which I named @react-keycloak/razzle, the package is available on npm.

The documentation for the module is also available here.

At the moment TS typings are not yet available, I'll work to add them as soon as possibile.

Let me know if this helps you and works as expected, I made some test with the Razzle app available under examples/razzle-app folder of this repo.

monowai commented 4 years ago

Cheens @panz3r - I managed to get things up and running and it appears to be behaving as expected.

Be great to get the typings in place. Awesome work!

panz3r commented 4 years ago

Hi @monowai ,

Thanks for your feedback.

As of version 1.0.0-alpha.4 of @react-keycloak/razzle some TS typings are included.

Let me know if this works for you and if I can close this issue.

monowai commented 4 years ago

Cheers! There are a few issues; I can separate if you prefer ?

  1. On the typescript front, ServerPersistors & SSRKeycloakProvider reports "only refers to a type but is being used as a value
  2. It seems you have to have at least one component use {{withKeycloak}} before you can reliably use the hooks (documentation)
  3. Using the default config, refreshing the page seems to break the KC LoginIFrame feature. keycloak.js determines that event.data ===“changed” and clears down the token; Things then seem to get out of sync with react-keycloak as the token gets lost. Disabling LoginIFrame stops the page refreshing every 5 seconds and doesn't seem to have any adverse issues.
    <SSRKeycloakProvider
      keycloakConfig={keycloakConfig}
      persistor={ClientPersistors.Cookies}
      initConfig={{ checkLoginIframe: false }}
    >
panz3r commented 4 years ago

Hi @monowai ,

Thanks for your quick feedback and the details.

  1. On the typescript front, ServerPersistors & SSRKeycloakProvider reports "only refers to a type but is being used as a value

This should be fixed on master, I'm yet to release a new version with the updated typings.

  1. It seems you have to have at least one component use {{withKeycloak}} before you can reliably use the hooks (documentation)

This sounds really strange, the 2 methods withKeycloak and useKeycloak contains the exact same logic so that they are idempotent. What do you mean by use reliably?

  1. Using the default config, refreshing the page seems to break the KC LoginIFrame feature. keycloak.js determines that event.data ===“changed” and clears down the token; Things then seem to get out of sync with react-keycloak as the token gets lost. Disabling LoginIFrame stops the page refreshing every 5 seconds and doesn't seem to have any adverse issues.

I made some testing about this with the razzle-app example code and couldn't verify the issue. Can you give me more details about the setup and environment you discovered this issue with?

Thanks again for your feedbacks and support.

monowai commented 4 years ago

Skip my point 2 - I believe I misreported that - my bad.

Point 3, the example app works fine. I pretty much use functional component and hooks exclusively. Within an effect, the hook is part of the effects dependencies (hooks exhaustive depends rule). Not sure if it's related to hooks or this cookie issue:

A cookie associated with a cross-site resource at http://keycloak/ was set without theSameSiteattribute. It has been blocked, as Chrome now only delivers cookies with cross-site requests if they are set with

I'll try and reproduce on the example app which might be easier than explaining how to start my stack

panz3r commented 4 years ago

Hi @monowai ,

That cookie issue might be the actual cause of your issue. As stated on keycloakrelease notes for version 8.0.2 (available here)

starting with this version, the Javascript adapter will only be fully functional when using the SSL / TLS connection on the Keycloak side.

Looking at the error thrown by Chrome I'd suppose you are exposing your keycloak server over http hence the cookies required by Keycloak are getting blocked by Chrome (this started happening as of version 80).

I hope this helps you with the issue.

Let me know and thanks again for your help and patience.

monowai commented 4 years ago

Issue is reproducible in Chrome but works in Safari. The gist below works for both

In the example app using Chrome

Here's a gist with my changes as a patch (includes a bit of prettier+eslint).

panz3r commented 4 years ago

Hi @monowai ,

The issue lies within keycloak-js itself, the behaviour that's implemented in Chrome will soon be updated also on Safari, Firefox and Edge, so the only solution is to follow Keycloak recommended setup and serve keycloak authentication pages over https (so that cookies can work as expected), if that's not possible you can try to downgrade keycloak-js to version 8.0.1 (before the changes to cookies were introduced) but I wouldn't recommend this.

As for @react-keycloak/razzle the cookies that are being set are used to keep the authentication state (by saving current tokens) in sync between client- and server-side, otherwise you'd see lots of pre-rendering warnings. Unfortunately an implementation using localStorage is not viable since it would only work on the client-side (the only simple way to share data between client and server is to use cookies). This cookies should be unaffected by the new security rules introduced by Chrome since they are used only on the same domain of the page, whilst keycloak cookies are shared between keycloak auth domain and localhost (during development).

monowai commented 4 years ago

Hi @panz3r - My reading has got me to the same conclusion. This Jira Issue confirms the issue and disabling iFrame as the workaround. This issue tracks the enhancement work they have yet to undertake.

I reckon we can close this issue when the last of the typescript bindings are in a release. You've been awesome!

panz3r commented 4 years ago

Hi @monowai ,

Thanks for your feedback and for the additional info.

I recently released new versions of @react-keycloak packages which includes a new version of @react-keycloak/razzle with updated TS typings.

Let me know if you find any issue with the latest version.

monowai commented 4 years ago

hi hi - sorry for the delay. Still getting typescript errors:

ServerPersistors.ExpressCookies...  // property ExpressCookes does not exist..
<SSRKeycloakProvider keycloakConfig={keycloakConfig}... > // no overload matches this call. 
panz3r commented 4 years ago

Hi @monowai ,

Thanks again for your feedback and for the patience.

I'm currently working on the building configuration of the whole repo so I'm a bit busy atm, I'll look into the TS typings for @react-keycloak/razzle as soon as possibile.

panz3r commented 4 years ago

Hi @monowai ,

Sorry for the delay, in the latest versions of @react-keycloak/razzle TS typings should be fine.

I released 2 separate versions v1.2.1 and v2.0.0, the latter has supports for keycloak-js version 9.0.2 and later because of a change in their TS typings (the changes occurred from v9.0.2 and they became incompatible with the previous version).

Thanks again for your patience and let me know if you find any issue with the latest version.

monowai commented 4 years ago

Looks like this can now be close awesome work sir. Hope you can your family are keeping well

panz3r commented 4 years ago

Thanks @monowai for your support in testing this out.