vulcan-next icon indicating copy to clipboard operation
vulcan-next copied to clipboard

Introduce "Public until proven Private" pattern and deprecate "getServerSideProps as middleware" pattern

Open eric-burel opened this issue 3 years ago • 7 comments

withPrivateAccess is a monster that should be simplified.

Specifically, using getServerSideProps just for redirection should be considered an anti-pattern (at least if you have no other valid reason to do SSR in the first place), as it brings all sort of unexpected issues.

We should provide an alternative to secure application interface shell rendering, like a preset reverse proxy, a custom server, or wait for Next to introduce middlewares on its custom server. But that may not be mandatory, given that an SPA if for example not really able to do that in the first place (all code is sent to the user even before being logged in).

eric-burel avatar Oct 09 '20 15:10 eric-burel

How Vercel dashboard is implementing this pattern: https://github.com/vercel/next.js/discussions/10724#discussioncomment-720

They don't try to prevent the initial rendering of the page shell, you can notice it by opening vercel.com/dashboard without being logged in, you'll see the page skeleton.

They add an inline script AFTER the page is statically built, outside of React. This script is in charge of preventing the page to load even after being statically built (except if you disable JS).

eric-burel avatar Oct 13 '20 15:10 eric-burel

I've looked more in detail what they do on vercel.com. Basically, the first thing the html does is put a huge white panel in front of the whole page, check if there is an authorization cookie on the document object. If there is, then remove the white panel, if not then replace the location with the login page (front-end redirection). This is done in vanilla JS, everything is inlined in the html head.

Here's the code that handles this on the vercel.com frontend. It's in first position inside the head of the statically rendered html:

<style>
  body::before {
    content:'';
    display:block;
    position:fixed;
    width:100%;
    height:100%;
    top:0;
    left:0;
    background:var(--geist-background);
    z-index: 99999
  }
  .render body::before {
    display:none
  }
</style>
<noscript>
  <style>
    body::before {
      content:none
    }
  </style>
</noscript>
<script>
  if (!document.cookie || document.cookie.indexOf('authorization=') === -1) {
    location.replace('/login?next=' + encodeURIComponent(location.pathname))
  } else {
    document.documentElement.classList.add('render')
  }
</script>

Apollinaire avatar Oct 14 '20 09:10 Apollinaire

Similar to @Apollinaire's investigations & Vercel's approach, here's how I have achieved this pattern wholly client-side:

  1. Inject an inline script in the statically rendered page, ensuring the browser executes it before React loads / rehydrates.
  2. That script checks if the user might be logged in.
  3. If they might be, then we do nothing in the inline script and let the regular client side auth check happen.
  4. If they are definitely not logged in, then we set a style on the html to hide it (display: none) and immediately redirect them to /login .

This prevents the "flash of layout shell" for users who are definitely not logged in.

For users who might be logged in, they'll always see the layout shell while we trigger a client-side API request to confirm if they're logged in or not (ie; to check their session token/JWT stored locally isn't expired).. If they're not logged in, they're redirected to /login.

You can use this component somewhere in your page to do the check:

let fastLoginRedirectInjected = false;
export const FastLoginRedirect = () => {
  // Only need to inject this helper at most once
  if (fastLoginRedirectInjected) {
    return null;
  } else {
    fastLoginRedirectInjected = true;
    return (
      <Head>
        <script
          dangerouslySetInnerHTML={{
            // When not logged in, we want to avoid the user having to wait for the
            // whole JS bundle to download before doing a redirect to the login page.
            // Why not do it server side? Because all pages are statically
            // generated. Adding a server execution would add extra time.
            // For the script to function, it relies on being injected early.
            __html: `
(function() {
  try {
    ${/* from https://stackoverflow.com/a/25490531/473961 */ ''}
    function getCookieValue(name) {
      const b = document.cookie.match('(^|;)\\s*' + name + '\\s*=\\s*([^;]+)');
      return b ? b.pop() : '';
    }
    if (getCookieValue('${JWT_COOKIE}').length > 0) {
      ${/* The user _might be_ authed (cookie could still be expired, etc) */ ''}
      const nextUrl = window.location.pathname + window.location.search;
      ${/* Hide the page while redirecting */ ''}
      document.querySelector('html').style.display = 'none';
      window.location.replace('/login?next_url=' + encodeURIComponent(nextUrl));
    }
  } catch { }
})();
        `,
          }}
        />
      </Head>
    );
  }
}

There's a further consideration for non-JS users which Vercel appears to be handling, but my script above does not handle (given we're relying on a client-side API call anyway, it's safe to assume the user has JS).

jesstelford avatar Oct 14 '20 20:10 jesstelford

Coming back at the idea of a reverse proxy:

It seems that to fully exploit Next capabilities, it's better not to try to do everything within the framework, but think "out-of-the-box", literally. Next does not expose the server voluntarily: this way, you are stuck in the "pit-of-success" and you barely have a way to write a slow, bad application. There are no middlewares either, same reason. The only way you can alter "req/res", is using getServerSideProps/getInitialProps => there is props in the name for a good reason, those are not middleware either but data fetching method. Next allows to do some redirections from there, but that's a convenience. You should never use those methods just for redirecting, that's an anti-pattern.

Yet, we still need to do some server-side routing if we want a fast, good experience. What's the best approach then? Using the right tool for the right job. A reverse proxy, load balancer or whatever gateway.

Problem: we are all front-end developers, at best full-stack: we don't know sh*t about that stuff. I've tried to read nginx documentation, sincerely. This how you redirect port 80 to port 3000:

server {
    listen   80;
    server_name  p3000;
    location / {
        proxy_pass http://0.0.0.0:3000;
        include /etc/nginx/proxy_params;
    }
}

image And good luck to set it correctly, modifying some mysterious files with Vim from SSH and using sytemctl commands.

sudo cp /etc/nginx/sites-available/default /etc/nginx/sites-available/default.backup
sudo nano /etc/nginx/sites-available/default

This is not acceptable given the effort we make at Vulcan to make web development accessible. Meteor Up did manage this complexity for us, hopefully, and we would need to find a way to get the same user experience with Next.

Proposal for client-side routing

  • Just do redirects as usual, using useEffect + router.push. I've updated my Stack Overflow answer to reflect that.
  • In a future iteration of Vulcan Next, we may add the nice hack that prevents flashing the content if you have no token, for convenience

That's sufficient for 99% of use cases. If you belong to the 1% that want to have nice 301 responses when the user accesses a private page while not being logged in:

Proposal for server-side routing

  • If you fit into Next common use cases like i18n management and basic redirection, if you are happy with getServerSideProps, stick to that. If you are in the 0.1% of use cases left...
  • We are able to create a sitemap automatically
  • Which means we are able to automatically get all the possible routes/pages of a Next app (thank you file-based routing!)
  • Next and Storybook are using "export" based patterns to expose information. This allows Next to parse your getServerSideProps with an isomorphic approach, and Storybook stories to be reused in 3rd party tools like Figma.
  • So, if we follow this logic, we could export an object describing the page redirection rules, in a declarative format
  • From there, we could automatically build some rules for a proxy, like Nginx or abstractions over Nginx that handles complex redirection patterns.
  • We could also create rules for common hosts like AWS etc.. I am yet to find an equivalent in Vercel however.

This sounds complicated but the biggest advantage is that you can keep your Next app completely static. You deport the complexity of request processing to tools even more powerful than Node, specialized in those uses cases, and also less costly.

eric-burel avatar Apr 20 '21 17:04 eric-burel

Redirection documentation seems to have been updated, making this even more relevant: https://nextjs.org/docs/api-reference/next.config.js/redirects#header-cookie-and-query-matching

Now you can do some matching on the request cookies and header to do redirections. If you add a server in charge of setting the headers in the equation, you get the perfect architecture for efficient server redirections.

It allows an awesome separation of concern: the proxy sets headers and cookies, the front-end team configure redirections in Next based on them.

It feels like the missing piece would be a simple, JS framework focused only on request processing (basically, checking that you are auth and setting relevant header), that prepares the request for Next. Then the redirection setup let's the frontend team do their job.

eric-burel avatar Apr 27 '21 08:04 eric-burel

Related: https://blog.vulcanjs.org/lets-bring-the-jamstack-to-saas-introducing-rainbow-rendering-ad1834fe62ff The "Unicorn Architecture" is a more generic variation of "Public is the new Private" pattern, that also includes multi-tenancy in addition to solving the server-side redirection problem.

eric-burel avatar Jun 11 '21 07:06 eric-burel

Nice example: how next-themes handles this scenario for theming: https://github.com/pacocoursey/next-themes/issues/67#issuecomment-994919398

eric-burel avatar Dec 15 '21 16:12 eric-burel