remix-run / react-router

Declarative routing for React
https://reactrouter.com
MIT License
52.92k stars 10.25k forks source link

Routes onEnter Authentication in Server-Side Rendering #2447

Closed Neta-Alon closed 8 years ago

Neta-Alon commented 8 years ago

Hey guys,

Following the example the docs provided, I encountered a problem when using server-side rendering.

If someone were to travel to ExampleSite.com/dashboard the server would still evaluate the function requireAuth in the onEnter hook. Since it is run in the server, it obviously won't find any token stored in the localStorage.

I can implement a check in requireAuth to see if the code is ran in the server, for instance:

function isClient() {
   return typeof window != 'undefined' && window.document;
}
function requireAuth(nextState, replaceState) {
  if (isClient())
    if (!auth.loggedIn())
      replaceState({ nextPathname: nextState.location.pathname }, '/login')
}

(from StackOverflow question)

Or I can use the internal React checks:

//from react/node_modules/fbjs/lib/ExecutionEnvironment
var canUseDOM = !!(
  (typeof window !== 'undefined' &&
  window.document && window.document.createElement)
);

(from StackOverflow Question)

But these all seem like bad practices - They complicate the code a tad and obscure it, and may have security issues (someone at StackOverflow mentioned you can inject the window.document element).

I know this isn't a big problem, but I was wondering if there's a better way to do it, and if not, maybe the example in the React-Router docs should include it for future visitors.

Thanks

taion commented 8 years ago

For questions and support, please visit our channel on Reactiflux or Stack Overflow. The issue tracker is exclusively for bug reports and feature requests.

Neta-Alon commented 8 years ago

I don't know if you've read all the way through, but this is at least a problem in the docs since 90% of the React apps I saw use server-rendering. Could you give your mind on the subject?

I'll post this in Stack Overflow as well.

knowbody commented 8 years ago

I do agree with @Neta-Alon we should document that.

taion commented 8 years ago

The problem is that there's not really a single generic answer.

What you really want to do on the server is to replace the logic to e.g. check a cookie (you don't want to skip the auth logic and show the user something that user isn't permissioned for, or else just not render any auth-required content).

But this then becomes specific to the server in question, &c. It would be appropriate maybe for a full-fledged boilerplate, but I think it'd be quite lengthy for a single example. Maybe just add a sentence or two to describe what to do in principle?

Neta-Alon commented 8 years ago

@taion - That's what I had in mind, too. I know it threw me off as a first timer using Node.js, Express and React.

taion commented 8 years ago

It's tricky and I sympathize. It's just that a full-fledged example is going to be quite involved, and would have to e.g. include things like Express, which we can't assume that everyone would use (v Koa or Hapi or something).

ryanflorence commented 8 years ago

requireAuth has to work on your server and client differently, we aren't in the business of prescribing a solution, but I hope somebody can make a really robust example eventually.

nickhudkins commented 8 years ago

Hi! I'd like to beat a dead horse a bit here because I have been doing a lot of thinking about server side rendering / client side rendering and auth and how they all play nicely together.

A few initial thoughts:

1.) Why do we care about server side rendering?

2.) Sessions suck, JWTs dont.

Those are all of my initial thoughts.

While this ends up being a solution OUTSIDE of react-router, perhaps it is a worthwhile pattern to use as a way to get around the PITA that is creating a server and client implementation for auth.

Here is example (working) code:

  const renderPage = (res, content) => {
    res.status(200).send(`
        <!doctype html>
        <html>
          <head>
            <title>React Router Example</title>
          </head>
          <body>
            <div id='root'>${content ? content : ''}</div>
            <script src="/static/app.bundle.js"></script>
          </body>
        </html>
    `);
  };

  const skipServerRender = (req, res) => {
    renderPage(res, null);
  };

  app.get('/auth-required', skipServerRender); // A route that requires auth.

  // Server rendered ReactRouter!
  app.get('*', function(req, res) {
    match({ routes, location: req.url }, (err, redirectLocation, renderProps) => {
      if (err) {
        res.status(500).send(err.message);
      } else if (redirectLocation) {
        res.redirect(302, redirectLocation.pathname + redirectLocation.search);
      } else if (renderProps) {
        const renderedContent = renderToString(<RoutingContext {...renderProps} />);
        renderPage(res, renderedContent);
      } else {
        res.status(404).send('Not found');
      }
    });
  });

The idea of "skipping" a render is taken from Cycle.js , thanks @staltz!

I think the API for this could be improved as it does feel sort of crummy catching routes back in server-land rather than letting React Router handle all routing.

Am I nuts? If the idea of skipping rendering server side based on a route prop sounds good let me know and I can work on a PR. Otherwise I'll leave this as a user-land problem and call it a day.

taion commented 8 years ago

JWTs don't preclude server-side rendering.

The approach you take is to stick the JWT into a cookie; use isomorphic logic on both client and server but source the token from the request cookie and make requests on behalf of the user when running on the server.

SEO is less relevant for login-gated content but a correct isomorphic auth implementation should really only have a branch for where the user token comes from. There's nothing really to fight with here.

nickhudkins commented 8 years ago

Foot In Mouth.

ghost commented 7 years ago

Hey guys, This is actually a problem that I was coming across while using JWT based authentication with auth0 non-interactive client. I have a React Express based universal environment but the jwt authentication is from server to server only (non-interactive clients) So what I am doing is I am calling a standalone API which returns data only if authenticated. Now I am unable to use the conditional rendering in case of server-side for the states of auth and non-auth

This is my setup with react-router for universal environment

app.get('*', (req, res) => {
  if (__DEV__) webpackIsomorphicTools.refresh();
  const history = createHistory();
  const store = configureStore(history);
  const renderHtml = (store, htmlContent) => { // eslint-disable-line no-shadow
    const html = renderToStaticMarkup(<Html store={store} htmlContent={htmlContent} />);
    return `<!doctype html>${html}`;
  };
  // Load data on server-side
  const loadBranchData = () => {
    const promises = [];
    routes.some((route) => {
      const match = matchPath(req.url, route);
      if (match && route.loadData) promises.push(route.loadData(store.dispatch, match.params));
      return match;
    });
    return Promise.all(promises);
  };
  loadBranchData()
    .then(() => {
      // Setup React-Router server-side rendering
      const routerContext = {};
      const htmlContent = renderToString(
        <Provider store={store}>
          <StaticRouter location={req.url} context={routerContext}>
            <App />
          </StaticRouter>
        </Provider>,
      );
      // Check if the render result contains a redirect, if so we need to set the specific status and redirect header and end the response
      if (routerContext.url) {
        res.status(301).setHeader('Location', routerContext.url); res.end(); return;
      }
      // Checking is page is 404
      const status = routerContext.status === '404' ? 404 : 200;
      // Pass the route and initial state into html template
      res.status(status).send(renderHtml(store, htmlContent));
    })
    .catch((err) => {
      res.status(404).send('Not Found :(');
      console.error(`==> 😭  Rendering routes error: ${err}`);
    });
});

And this is what standard express does where the api works on routing properly

app.get('/', function(req, res){
  res.render('index');
})
app.get('/movies', getAccessToken, function(req, res){
  request
    .get('http://localhost:8080/movies')
    .set('Authorization', 'Bearer ' + req.access_token)
    .end(function(err, data) {
      if(data.status == 403){
        res.send(403, '403 Forbidden');
      } else {
        var movies = data.body;
        res.render('movies', { movies: movies} );
      }
    })
})

Any help shall be appreciated greatly! Thank you so much for your attention