Skip to content
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

Update fetch to pass cookie to server #63

Merged
merged 1 commit into from
Jul 9, 2015
Merged

Update fetch to pass cookie to server #63

merged 1 commit into from
Jul 9, 2015

Conversation

abbycabs
Copy link
Member

@abbycabs abbycabs commented Jul 1, 2015

Change made from discussion with @wjrsimpson & @rcpeters on project call. Looks like we just needed to update some config to get the cookie passed!

* check session for correct ORCID
* otherwise, 403
* issue #62
@abbycabs abbycabs added the review label Jul 1, 2015
@@ -23,7 +23,8 @@ var Issue = React.createClass({
var url = '/papers/' + path[path.length-2] + '/' + path[path.length-1] + '/users/' + orcid + '/badges/' + badge;

fetch(url, {
method: 'post'
method: 'post',
credentials: 'same-origin'
Copy link
Member Author

Choose a reason for hiding this comment

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

@abbycabs
Copy link
Member Author

abbycabs commented Jul 6, 2015

@wjrsimpson -- do you have time to review? This should answer your security concerns from our last call

@wjrsimpson
Copy link
Contributor

Hi Abi,

Yes, I reviewed and did some basic testing locally.

I tried to create a badge using curl

a) without cookies

and

b) with cookies but a different ORCID in the URL

and got a 403 response for both.

So, this addresses the concern I raised in the call that we needed to make sure that badge creation can't be spoofed easily.

I need to mention some caveats, and make sure I am not misleading anyone.

i) I am not an expert in React or node
ii) I am not a security expert
iii) I haven't conducted a full security audit of the app

Great! Thanks, Abi.

@abbycabs
Copy link
Member Author

abbycabs commented Jul 9, 2015

Yay, thanks @wjrsimpson -- we should maybe add these tests to our test suite. I'll make an issue.

abbycabs added a commit that referenced this pull request Jul 9, 2015
Update fetch to pass cookie to server
@abbycabs abbycabs merged commit a1a46e8 into master Jul 9, 2015
@abbycabs abbycabs deleted the orcid-security branch July 27, 2015 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants