vercel / next.js

The React Framework
https://nextjs.org
MIT License
125.4k stars 26.78k forks source link

Can't restrict pages routed through next-router with next/link. #4029

Closed MatthiasMargot closed 6 years ago

MatthiasMargot commented 6 years ago

Expected Behavior

I am building a wrapper middleware in express to restrict an entire next application from the public without a password and a username. It should catch every get request that's coming through, check for a logged-in boolean on a session found with an id from a session-cookie.

Current Behavior

It works as expected for every normal browser http get request, however it does not work for pages served through the next-router. I noticed this because I had reused a logo component on the restriction page which simply linked to '/' and even though the request went through the middleware and the correct code was run (basic if else with a logged-in boolean) it did serve the homepage.

Steps to Reproduce (for bugs)

const express = require('express')
const next = require('next')

const app = next({
    dir: '.',
    dev: process.env.NODE_ENV !== 'PRODUCTION'
})

const handle = app.getRequestHandler()
const server = express()

/* HARDCODED VALIDATE BOOLEAN */
const authenticated = false

app.prepare()

.then(() => {
    /*
        Page restriction middleware
    */
    server.get('*', (req, res) => {
        console.log(req.path)
        if (!authenticated) {
            console.log('Not authenticated.')
            return app.render(req, res, '/restriction')
        } else {
            console.log('Authenticated.')
            return handle(req, res)
        }

    })

    const PORT = process.env.PORT || process.env.port || 3000
    server.listen(PORT)

})

This code should never log 'Authenticated' much less actually serve the requested resource right? Here's the thing, it never logs 'Authenticated' but it does serve any page you want if it was requested with next/link.

Front-end to reproduce:

index.js

export default () =>
  <div>Restricted page. You cant see this without bypassing the express routing middleware.</div>

restriction.js

import Link from 'next/link'
export default () =>
    <Link href="/"><button>I am the magic resctriction bypasser.</button></Link>

What's sort of funny about this is that you can click on that Link in the restriction page and then refresh the page and the middleware will render the restriction page again.

egorovli commented 6 years ago

You have to check client's permission both server- and client-side. Once the page is rendered, Express won't serve routes followed via next/link.

egorovli commented 6 years ago

For your particular case:

import React, { Component } from 'react'
import Router from 'next/router'

export default class Restricted extends Component {
  static getInitialProps = async ctx => {
    // pseudo-code which you'll have to implement: make an /api call or extract data from JWT, etc.
    const { isAuthenticated } = await checkUserPermissions(ctx)
    return { isAuthenticated }
  }

  render = () => {
    if (!this.props.isAuthenticated) {
      Router.push('/login')
      return
    }

    return (
      <div>Restricted page. You cant see this without bypassing the express routing middleware.</div>
    )
  }
}

Note that you can only do client-side transitions with next/router, so you'll have to implement server-side logic as well.

MatthiasMargot commented 6 years ago

So then, how would I go about restricting the application as a whole? Because the example you gave only restricts whatever URL the 'Restricted' Component maps to. If it's only a single page I have to restrict that's fine but the only way to do this that I can see with this is having a HOC that does this for every component in particular which doesn't really make sense because that sound a lot more like a cherry picked approach on which pages to restrict

2 Questions:

I simply need to have this functionality: Nodody can see any of the sites content without going through an entry gate. Restricting every page in particular does not sound like the correct approach, not even with a HOC that does this.

egorovli commented 6 years ago

From what I've learned, you can't have a wrapping component for the entire app and shared by all pages. There is _document.js, but you can't put your business logic there.

So for instance if you want to have a common layout across multiple pages, you'll have to introduce a separate component and wrap each page in it (see here https://github.com/zeit/next.js/wiki/Global-styles-and-layouts).

So with your case I would go with either a separate component or a HOC. You don't have to cherry-pick your restricted pages, since you can embed the path checking logic inside of this component/HOC, but you'll have to include it everywhere.

./decorators/withUserCheck.jsx

import React, { Component } from 'react'
import PropTypes from 'prop-types'
import { wrapDisplayName } from 'recompose'
import hoistStatics from 'hoist-non-react-statics'
import { checkIfUserIsAuthenticated } from '../utils'

const withUserCheck = WrappedComponent => {
  class ComponentWithUserCheck extends Component {
    static displayName = wrapDisplayName(WrappedComponent, 'withUserCheck')

    static WrappedComponent = WrappedComponent

    static propTypes = {
      wrappedComponentRef: PropTypes.func
    }

    static getInitialProps = async ctx => {
      const wrappedComponentProps = typeof WrappedComponent.getInitialProps === 'function'
        ? await WrappedComponent.getInitialProps(ctx)
        : {}

      // Pseudo-function which you will have to write yourself.
      // Read about what you can extract from ctx in getInitialProps() here:
      // https://github.com/zeit/next.js#fetching-data-and-component-lifecycle
      // You can redirect via Express'es res.redirect() here if this is a
      // server call (check ctx.req and ctx.res) or with next/router if it
      // is a client-side call.
      const isAuthenticated = checkIfUserIsAuthenticated(ctx)
      return { ...wrappedComponentProps, isAuthenticated }
    }

    render = () => {
      const { wrappedComponentRef, ...restProps } = this.props

      return (
        <WrappedComponent {...restProps} ref={wrappedComponentRef} />
      )
    }
  }

  return hoistStatics(ComponentWithUserCheck, WrappedComponent)
}

export default withUserCheck

./pages/some-page.jsx

import React, { Component } from 'react'
import withUserCheck from '../decorators/withUserCheck'

@withUserCheck
export default class SomeComponent extends Component {
  render = () => {
    return (
      <div>
        <h1>Hello</h1>
        <p>This page may be restricted or may be not. The decorator should decide.</p>
      </div>
    )
  }
}

Check https://github.com/zeit/next.js#fetching-data-and-component-lifecycle for what you can extract from ctx in getInitialProps().