Skip to content

Conversation

@Karyum
Copy link
Collaborator

@Karyum Karyum commented May 27, 2018

weird last time i checked the repo the security problem was there now nothing... 😅
rel #115

@Karyum Karyum requested a review from m4v15 May 27, 2018 20:31
@m4v15
Copy link
Collaborator

m4v15 commented May 28, 2018

Yeah I dismissed it as not a problem because looking into it I think it's fine.

Copy link
Collaborator

@m4v15 m4v15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really changes just a question to double check things

if (access) {
verifyToken(req.cookies)
.then(() => next())
.catch(err =>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't it like this originally? Why did it get changed - if there was a reason, will changing it back maybe break something else?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as in, we had it doing a redirect which then got changed into a res.send at some point

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not actually sure why it got changed i don't really remember, but i do know that we were doing res.send just cause we had the form's validation or Axios which is doing the request to the server and not the the browser

@m4v15 m4v15 removed their assignment Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants