Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Docs/security in gatsby #22395
Docs/security in gatsby #22395
Changes from 20 commits
f5434f0
e1afad0
b1e2b7d
6f7c5c2
a0e707a
06ad16d
a2a85e7
b271cb6
c26b54c
f90037c
fcdabee
664d376
8c838da
b15df2a
9c3abbe
9dade72
7339fa4
194c7d8
344d2fd
1e52d47
88039d3
5c12797
d1481fa
deb6aaa
cf87593
938df61
77cd189
7aa4da9
d7fe95a
090097b
406837e
3b81651
1640a70
52e4265
aad2dcb
65a3a41
84238b6
d37dc36
d315812
95acf5c
820047c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be re written it's pretty confusing. https://en.wikipedia.org/wiki/Cross-site_request_forgery has some good explanations you can quote from. If you do choose to fix what you have I think it would still be helpful to link out to a more complete resource like Wikipedia for those interested in diving deeper than the quick overviews we're providing here (this applies to XSS as well).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In these past few days, I'm exploring more in the topic in CS253 - Web Security classes from Stanford. I'll rewrite this section, making it simple, doing some quotes and linking these resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand the middle sentence. Is the goal to explain that this is true for unauthenticated AND authenticated users?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, it's confusing. It only works if you are authenticated to the original website. I'm going to rewrite this section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You "copy the site" example above could be fixed by simply scoping your cookies to a certain domain. Make sure you info is accurate (I'm no expert here either) and complete (aka. solve every problem you present, and do so correctly). Again, I think since security is such a deep and nuanced topic it'll be to your benefit to keep these short and simple then link out to more complete resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on further thought...Based on how this info is presented Tokens seem to be "THE" solution. I'm guessing they are just one, of many solutions. Present it as such, "On way CSRF attacks can be mitigated..., .... To read more on this topic check out....", then link out to more in depth resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sry I just realized you do have the Same-site cookies example...Fix you headings please The "How Can it be prevented" should probably be a ### header and then each example be #### header. Optionally, Instead of presenting several problems then several solutions, Give one problem, and its solution, then another and its solution...etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, sorry about that! I think that is good we are learning things while doing this. I'm gonna rewrite this section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to be sorry. you're doing great work, just trying to give helpful feedback. thanks and keep it up. I've already learned a thing or two.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe needs an example. not totally understanding how/why this works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, totally. I add a more clear example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is still outstanding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This covers build variables well but not client side keys. Not sure if the note is intended to cover it cause the language is a bit confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this note does not cover this part. What do you think it should be here regarding client-side keys? encryption? known methods, e.g. JWT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this doc should be a starting point. so defining the issue and maybe an example of how to solve then (again) links out to more thorough resources (like Gatsby's docs on keys and env variables).