AntonEmery / chord-app

React app for notating guitar chords
2 stars 1 forks source link

document.cookie does not contain substring connect.sid= #78

Closed XiXiaPdx closed 3 years ago

XiXiaPdx commented 3 years ago

Currently, the server is successsfully setting the cookie for api.chord-app.com.
The cookie is in chrome, just visit cookies under security preferences and search for api.chord-app.com

But, although the cookie is in the browser, it is not being stored in document.cookie.

Auth.js in the client depends on finding this cookie to allows access to private routes. Since we can't find one, we dont' have access to private routes. App.js has all the routes that are private, like /chordsheets

https://github.com/AntonEmery/chord-app/blob/69732c5b84f9928b70db0a48edc7fd88f69c01c8/src/client/src/Auth.js#L6

Acceptance Criteria (AC):

cookie from browser is stored in document.cookie

XiXiaPdx commented 3 years ago

@AntonEmery Is this secure?

Document.cookie can be different than the actual cookie the server set in the browser.
User would have access to private routes even though their cookie is not === to server cookie.
The only check here is existence, not equality

AntonEmery commented 3 years ago

Yea technically someone could type in the console document.cookie = 'connect.sid=' and then have access to /chordsheets, for instance. But there would not be any user data unless someone else was still logged in to the app, in which case they would have access to that route anyways. So for this use case it seemed fine.

AntonEmery commented 3 years ago

We should probably change that domain value to an env variable, so it will work locally and in prod.

https://github.com/AntonEmery/chord-app/blob/f03890d395e4e05dab66198deaf007d32b8cd2b8/src/server/app.js#L39

AntonEmery commented 3 years ago

We can disregard my above comment, the app works locally with the domain set to chord-app.com

Closing this issue.