fix(auth): update auth to use localStorage#540
Conversation
|
@yuvvantalreja is attempting to deploy a commit to the Dominik Moritz's projects Team on Vercel. A member of the Team first needs to authorize it. |
| this.app.use(passport.session()); | ||
|
|
||
| // Handle preflight OPTIONS requests explicitly | ||
| this.app.options('*', (req, res) => { |
There was a problem hiding this comment.
Why do we need to handle preflight? Is that needed for something in auth?
There was a problem hiding this comment.
yeah I was getting a bunch of CORS issues without them
There was a problem hiding this comment.
I see let's say in the comment that it's needed for cors
| res.header('Access-Control-Allow-Origin', origin); | ||
| } | ||
|
|
||
| res.header('Access-Control-Allow-Methods', 'GET,PUT,POST,DELETE,OPTIONS'); |
| res.header('Access-Control-Allow-Origin', origin); | ||
| } | ||
|
|
||
| res.header('Access-Control-Allow-Methods', 'GET,PUT,POST,DELETE,OPTIONS'); |
|
|
||
| const dataString = JSON.stringify(userInfo); | ||
| const signature = crypto | ||
| .createHmac('sha256', process.env.SESSION_SECRET || 'vega-editor-secret') |
There was a problem hiding this comment.
maybe it's better not to have a fallback so we crash if this is not defined
|
What kind of GitHub auth token are we giving to the front end? I want to make sure it has minimal permissions so that it can't be abused to do many other things. |
Its a personal access token that is received as part of the OAuth workflow. We can set the token to the gist scope which only allows the token to be used for CRUD operations on public/private gists belonging to the user. I was also deciding to implement some additional security features, mainly implementing a per-gist authorization system that only allows modification of specific gists, along with rate limiting read/write operations on gists. Do let me know if this is something reasonable to explore. |
|
Would the rate limiting mean we have to proxy requests? I'd like to keep the backend lean. |
I see, that would indeed complicate our backend. |
|
Deployment failed with the following error: |
|
I see this is marked as draft. Is this ready for a review? |
| import express from 'express'; | ||
| import session from 'express-session'; | ||
| import passport from 'passport'; | ||
| import redis from 'redis'; |
|
Moved to #543. You have write access. |
2 participants
Problem
The application was experiencing authentication issues specifically in Safari:
Solution
Implemented a token-based authentication system as a workaround:
Added localStorage-based authentication token storage:
Updated backend auth controller: