Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improves with-cookie-auth example #10062

Closed
wants to merge 5 commits into from
Closed

Improves with-cookie-auth example #10062

wants to merge 5 commits into from

Conversation

VictorAssis
Copy link
Contributor

Access user data (token in this case) on the header or other Layout components its a real necessity.

To make this more easily, I created a HOC called withAuth that get the user from the cookie and enable that in pageProps and in the context of the pages.

To exemplify the use of HOC, I added in login page a message warning the user that he is already logged in and in the header I show the token and the logout button only if the user is logged.

@ijjk
Copy link
Member

ijjk commented Jan 13, 2020

Stats from current PR

Default Server Mode
General
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
buildDuration 13.9s 14s ⚠️ +79ms
nodeModulesSize 49 MB 49 MB
Client Bundles (main, webpack, commons)
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
main-HASH.js gzip 5.11 kB 5.11 kB
webpack-HASH.js gzip 746 B 746 B
4952ddcd88e7..54d3.js gzip 4.68 kB 4.68 kB
commons.HASH.js gzip 4.06 kB 4.06 kB
de003c3a9d30..9881.js gzip 13.7 kB 13.7 kB
framework.HASH.js gzip 39.1 kB 39.1 kB
Overall change 67.5 kB 67.5 kB
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
main-HASH.module.js gzip 4.18 kB 4.18 kB
webpack-HASH..dule.js gzip 746 B 746 B
4952ddcd88e7..dule.js gzip 5.56 kB 5.56 kB
de003c3a9d30..dule.js gzip 12.5 kB 12.5 kB
framework.HA..dule.js gzip 39.1 kB 39.1 kB
Overall change 62.1 kB 62.1 kB
Legacy Client Bundles (polyfills)
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
polyfills-HASH.js gzip 4.76 kB 4.76 kB
Overall change 4.76 kB 4.76 kB
Client Pages
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_app.js gzip 1.33 kB 1.33 kB
_error.js gzip 4.07 kB 4.07 kB
hooks.js gzip 779 B 779 B
index.js gzip 222 B 222 B
link.js gzip 2.9 kB 2.9 kB
routerDirect.js gzip 283 B 283 B
withRouter.js gzip 282 B 282 B
Overall change 9.87 kB 9.87 kB
Client Pages Modern
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_app.module.js gzip 757 B 757 B
_error.module.js gzip 3.06 kB 3.06 kB
hooks.module.js gzip 371 B 371 B
index.module.js gzip 212 B 212 B
link.module.js gzip 2.47 kB 2.47 kB
routerDirect..dule.js gzip 273 B 273 B
withRouter.m..dule.js gzip 272 B 272 B
Overall change 7.41 kB 7.41 kB
Client Build Manifests
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_buildManifest.js gzip 61 B 61 B
_buildManife..dule.js gzip 61 B 61 B
Overall change 122 B 122 B
Rendered Page Sizes
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
index.html gzip 1.02 kB 1.02 kB
link.html gzip 1.03 kB 1.03 kB
withRouter.html gzip 1.01 kB 1.01 kB
Overall change 3.06 kB 3.06 kB

Serverless Mode
General
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
buildDuration 14.5s 14.6s ⚠️ +54ms
nodeModulesSize 49 MB 49 MB
Client Bundles (main, webpack, commons)
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
main-HASH.js gzip 5.11 kB 5.11 kB
webpack-HASH.js gzip 746 B 746 B
4952ddcd88e7..54d3.js gzip 4.68 kB 4.68 kB
commons.HASH.js gzip 4.06 kB 4.06 kB
de003c3a9d30..9881.js gzip 13.7 kB 13.7 kB
framework.HASH.js gzip 39.1 kB 39.1 kB
Overall change 67.5 kB 67.5 kB
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
main-HASH.module.js gzip 4.18 kB 4.18 kB
webpack-HASH..dule.js gzip 746 B 746 B
4952ddcd88e7..dule.js gzip 5.56 kB 5.56 kB
de003c3a9d30..dule.js gzip 12.5 kB 12.5 kB
framework.HA..dule.js gzip 39.1 kB 39.1 kB
Overall change 62.1 kB 62.1 kB
Legacy Client Bundles (polyfills)
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
polyfills-HASH.js gzip 4.76 kB 4.76 kB
Overall change 4.76 kB 4.76 kB
Client Pages
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_app.js gzip 1.33 kB 1.33 kB
_error.js gzip 4.07 kB 4.07 kB
hooks.js gzip 779 B 779 B
index.js gzip 222 B 222 B
link.js gzip 2.9 kB 2.9 kB
routerDirect.js gzip 283 B 283 B
withRouter.js gzip 282 B 282 B
Overall change 9.87 kB 9.87 kB
Client Pages Modern
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_app.module.js gzip 757 B 757 B
_error.module.js gzip 3.06 kB 3.06 kB
hooks.module.js gzip 371 B 371 B
index.module.js gzip 212 B 212 B
link.module.js gzip 2.47 kB 2.47 kB
routerDirect..dule.js gzip 273 B 273 B
withRouter.m..dule.js gzip 272 B 272 B
Overall change 7.41 kB 7.41 kB
Client Build Manifests
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_buildManifest.js gzip 61 B 61 B
_buildManife..dule.js gzip 61 B 61 B
Overall change 122 B 122 B
Serverless bundles
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_error.js gzip 77.8 kB 77.8 kB
hooks.html gzip 1.05 kB 1.05 kB
index.js gzip 78 kB 78 kB
link.js gzip 80.4 kB 80.4 kB
routerDirect.js gzip 78.1 kB 78.1 kB
withRouter.js gzip 78.1 kB 78.1 kB
Overall change 393 kB 393 kB

Commit: 5e14936

@ijjk
Copy link
Member

ijjk commented Jan 13, 2020

Stats from current PR

Default Server Mode
General
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
buildDuration 13.8s 14s ⚠️ +226ms
nodeModulesSize 49 MB 49 MB
Client Bundles (main, webpack, commons)
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
main-HASH.js gzip 5.11 kB 5.11 kB
webpack-HASH.js gzip 746 B 746 B
4952ddcd88e7..54d3.js gzip 4.68 kB 4.68 kB
commons.HASH.js gzip 4.06 kB 4.06 kB
de003c3a9d30..9881.js gzip 13.7 kB 13.7 kB
framework.HASH.js gzip 39.1 kB 39.1 kB
Overall change 67.5 kB 67.5 kB
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
main-HASH.module.js gzip 4.18 kB 4.18 kB
webpack-HASH..dule.js gzip 746 B 746 B
4952ddcd88e7..dule.js gzip 5.56 kB 5.56 kB
de003c3a9d30..dule.js gzip 12.5 kB 12.5 kB
framework.HA..dule.js gzip 39.1 kB 39.1 kB
Overall change 62.1 kB 62.1 kB
Legacy Client Bundles (polyfills)
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
polyfills-HASH.js gzip 4.76 kB 4.76 kB
Overall change 4.76 kB 4.76 kB
Client Pages
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_app.js gzip 1.33 kB 1.33 kB
_error.js gzip 4.07 kB 4.07 kB
hooks.js gzip 779 B 779 B
index.js gzip 222 B 222 B
link.js gzip 2.9 kB 2.9 kB
routerDirect.js gzip 283 B 283 B
withRouter.js gzip 282 B 282 B
Overall change 9.87 kB 9.87 kB
Client Pages Modern
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_app.module.js gzip 757 B 757 B
_error.module.js gzip 3.06 kB 3.06 kB
hooks.module.js gzip 371 B 371 B
index.module.js gzip 212 B 212 B
link.module.js gzip 2.47 kB 2.47 kB
routerDirect..dule.js gzip 273 B 273 B
withRouter.m..dule.js gzip 272 B 272 B
Overall change 7.41 kB 7.41 kB
Client Build Manifests
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_buildManifest.js gzip 61 B 61 B
_buildManife..dule.js gzip 61 B 61 B
Overall change 122 B 122 B
Rendered Page Sizes
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
index.html gzip 1.02 kB 1.02 kB
link.html gzip 1.03 kB 1.03 kB
withRouter.html gzip 1.01 kB 1.01 kB
Overall change 3.06 kB 3.06 kB

Serverless Mode
General
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
buildDuration 14.3s 14.6s ⚠️ +265ms
nodeModulesSize 49 MB 49 MB
Client Bundles (main, webpack, commons)
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
main-HASH.js gzip 5.11 kB 5.11 kB
webpack-HASH.js gzip 746 B 746 B
4952ddcd88e7..54d3.js gzip 4.68 kB 4.68 kB
commons.HASH.js gzip 4.06 kB 4.06 kB
de003c3a9d30..9881.js gzip 13.7 kB 13.7 kB
framework.HASH.js gzip 39.1 kB 39.1 kB
Overall change 67.5 kB 67.5 kB
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
main-HASH.module.js gzip 4.18 kB 4.18 kB
webpack-HASH..dule.js gzip 746 B 746 B
4952ddcd88e7..dule.js gzip 5.56 kB 5.56 kB
de003c3a9d30..dule.js gzip 12.5 kB 12.5 kB
framework.HA..dule.js gzip 39.1 kB 39.1 kB
Overall change 62.1 kB 62.1 kB
Legacy Client Bundles (polyfills)
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
polyfills-HASH.js gzip 4.76 kB 4.76 kB
Overall change 4.76 kB 4.76 kB
Client Pages
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_app.js gzip 1.33 kB 1.33 kB
_error.js gzip 4.07 kB 4.07 kB
hooks.js gzip 779 B 779 B
index.js gzip 222 B 222 B
link.js gzip 2.9 kB 2.9 kB
routerDirect.js gzip 283 B 283 B
withRouter.js gzip 282 B 282 B
Overall change 9.87 kB 9.87 kB
Client Pages Modern
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_app.module.js gzip 757 B 757 B
_error.module.js gzip 3.06 kB 3.06 kB
hooks.module.js gzip 371 B 371 B
index.module.js gzip 212 B 212 B
link.module.js gzip 2.47 kB 2.47 kB
routerDirect..dule.js gzip 273 B 273 B
withRouter.m..dule.js gzip 272 B 272 B
Overall change 7.41 kB 7.41 kB
Client Build Manifests
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_buildManifest.js gzip 61 B 61 B
_buildManife..dule.js gzip 61 B 61 B
Overall change 122 B 122 B
Serverless bundles
zeit/next.js canary VictorAssis/next.js improve-cookie-example Change
_error.js gzip 77.8 kB 77.8 kB
hooks.html gzip 1.05 kB 1.05 kB
index.js gzip 78 kB 78 kB
link.js gzip 80.4 kB 80.4 kB
routerDirect.js gzip 78.1 kB 78.1 kB
withRouter.js gzip 78.1 kB 78.1 kB
Overall change 393 kB 393 kB

Commit: 758b14b

@lfades
Copy link
Member

lfades commented Jan 13, 2020

"Access user data (token in this case) on the header or other Layout components its a real necessity." - That's true, but what you need is the data, not the token.

A custom _app will affect all pages in the app, you may not need auth info for all pages, therefore we don't recommend doing this, and specially if _app has getInitialProps, which will turn all pages into serverless functions.

@VictorAssis Thank you for taking the time of creating a PR. For the reasons above I'm going to close it.

The example should remove the token from the client code, so it would only be available for API calls, and won't be accessible through the client js. Meaning that the current example is not very secure, and this doesn't help.

@lfades lfades closed this Jan 13, 2020
@VictorAssis
Copy link
Contributor Author

VictorAssis commented Jan 13, 2020

"That's true, but what you need is the data, not the token." - This is also true. I did not implement the login by saving other user data in cookies because I thought it might get too complex for the example, but I can change it here.

"A custom _app will affect all pages in the app, you may not need auth info for all pages, therefore we don't recommend doing this, and specially if _app has getInitialProps, which will turn all pages into serverless functions." - I understand that, but if a site has a user info in the header and use the layout in all pages, i think that dont exist way to avoid this. Im right?

Thinking of what you said, if I return the Layout component to the pages, use the HOC only in pages with Layout component and save other user data in token, do I solve these problems?

Sorry for insist @lfades , i'm having a great experience with Next Js and i want to contribute.

@lfades
Copy link
Member

lfades commented Jan 14, 2020

@VictorAssis No problem, Is nice to see that you're interested in fixing the example, here's what you can do:

  • Have the Layout and auth HOC in the page, and remove _app.
  • Remove the token from the browser, so in the server you would use cookie to get the cookie and return the data, and in the client you would to a fetch to an API endpoint that returns the data (use typeof window === 'undefined' to know if it's the server or the browser inside getInitialProps).

@VictorAssis
Copy link
Contributor Author

@lfades I'm having some questions on how to remove the token from client. When you refer to having the token on the server, do you mean api endpoints or getInitialProps?

I imagine these two scenarios:

1. Access cookies only in api

When the api receive a request in login endpoint, then she save the token in the cookies and return a flag of success to the client. The profile endpoint directly access the token to get the data.

The withAuth needs to consult the profile endpoint always (to get user info for header) or consult once and save the data in localstorage.

To logout its necessary to create another endpoint who clean the cookie from api.

The authorization header would not be necessary in this case.

2. Api doesn't change

Here I am having trouble figuring out a way to not access the token on the client.

Today, the login function in auth.js is responsible for saving the cookie and it runs on the client side. This is a problem?

If not, withAuth's getInitialProps will check if it is on the client or server side for API call data or directly from the cookie. In this way, will also save user information in the cookie.

And to logout, will we remove the cookie on the client (as it works today) or will we create an exit page that will remove cookies on the server?

Thanks for help!

@lfades
Copy link
Member

lfades commented Jan 20, 2020

@VictorAssis You are right, API routes would be needed for login/logout, and only the API routes will have access to the cookie, which should never be sent to the client, you may want to read the converstaion here: #9913

@vercel vercel locked as resolved and limited conversation to collaborators Jan 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants