XSRF plugin for Angular #207
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maramihali
force-pushed
the
maramihali-angular-xsrf
branch
from
dc7e336
to
8e99b57
Compare
empijei
reviewed
Oct 14, 2020
Comment on lines
117
to
121
| wantHeader: map[string][]string{ | ||
| "Content-Type": {"text/plain; charset=utf-8"}, | ||
| "X-Content-Type-Options": {"nosniff"}, | ||
| }, | ||
| wantBody: "Forbidden\n", |
There was a problem hiding this comment.
These assertions feel like we are testing the framework rather than just the plugin. I would personally just test for status here. If you want to be thorough maybe also body, but the header check I think should only be a "I added a cookie or not", which would allow you to merge this test with the one above.
There was a problem hiding this comment.
Right, but we tested for the headers everywhere and I mostly did it for uniformity across tests. I suggest removing these checks, if you think they are not needed, in a follow-up PR.
maramihali
added
the
hacktoberfest-accepted
maramihali
force-pushed
the
maramihali-angular-xsrf
branch
from
d04794f
to
8eb6872
Compare
kele
requested changes
Oct 14, 2020
added 2 commits
October 15, 2020 12:21
… adding the OnError methods
…ne and created file that contains helper functions used by both. Refactored some logic according to code review
|
It seemed that it correctly moved everything besides one file (because we kept |
…enCookieName and TokenHeaderName to their default values as indicated by the documentation
maramihali
force-pushed
the
maramihali-angular-xsrf
branch
from
8eb6872
to
c399974
Compare
kele
reviewed
Oct 15, 2020
kele
requested changes
Oct 15, 2020
maramihali
force-pushed
the
maramihali-angular-xsrf
branch
from
c0f24b9
to
a2d86a0
Compare
kele
requested changes
Oct 16, 2020
kele
requested changes
Oct 16, 2020
maramihali
force-pushed
the
maramihali-angular-xsrf
branch
from
ca34450
to
6f29698
Compare
kele
approved these changes
Oct 16, 2020
empijei
suggested changes
Oct 19, 2020
added 2 commits
October 19, 2020 10:26
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #194
Created a separate plugin that provides protection against XSRF in Angular. More details can be found here: https://docs.angularjs.org/api/ng/service/$http#cross-site-request-forgery-xsrf-protection
Changed the directory tree to include the two XSRF plugins in the same directory with the functionality used by both placed in a separate file.