Skip to content

The current csrftoken parser breaks on multiple crsftokens #783

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

The current csrftoken parser breaks on multiple crsftokens #783

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6c8d4a3
The current csrftoken parser breaks on multiple crsftokens
allen-munsch Sep 24, 2019
e33523e
Use var instead of let, add var for value
allen-munsch Sep 24, 2019
9710d00
Remove duplicate code
allen-munsch Sep 24, 2019
3139124
add check for cookie
allen-munsch Sep 24, 2019
f118de7
update order of cookie and header adding
allen-munsch Sep 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 61 additions & 45 deletions graphene_django/static/graphene_django/graphiql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,41 @@
(function() {

// Parse the cookie value for a CSRF token
var csrftoken;
var cookies = ('; ' + document.cookie).split('; csrftoken=');
if (cookies.length == 2)
csrftoken = cookies.pop().split(';').shift();

var cookies = document.cookie.split(';').reduce((p, c) => {
var x = {};
var key = c.split('=')[0].trim();
var value = c.split('=')[1]
x[key] = p[key] ? p[key].concat([value]) : [value];
return Object.assign(p, x);
}, {});
var headers = {
Accept: 'application/json',
'Content-Type': 'application/json'
};
if (cookies.csrftoken && cookies.csrftoken.length) {
headers['X-CSRFToken'] = cookies.csrftoken.pop();
}
// Collect the URL parameters
var parameters = {};
window.location.hash.substr(1).split('&').forEach(function (entry) {
var eq = entry.indexOf('=');
if (eq >= 0) {
parameters[decodeURIComponent(entry.slice(0, eq))] =
decodeURIComponent(entry.slice(eq + 1));
}
});
window.location.hash
.substr(1)
.split('&')
.forEach(function(entry) {
var eq = entry.indexOf('=');
if (eq >= 0) {
parameters[decodeURIComponent(entry.slice(0, eq))] = decodeURIComponent(entry.slice(eq + 1));
}
});
// Produce a Location fragment string from a parameter object.
function locationQuery(params) {
return '#' + Object.keys(params).map(function (key) {
return encodeURIComponent(key) + '=' +
encodeURIComponent(params[key]);
}).join('&');
return (
'#' +
Object.keys(params)
.map(function(key) {
return encodeURIComponent(key) + '=' + encodeURIComponent(params[key]);
})
.join('&')
);
}
// Derive a fetch URL from the current URL, sans the GraphQL parameters.
var graphqlParamNames = {
Expand All @@ -39,27 +54,31 @@

// Defines a GraphQL fetcher using the fetch API.
function graphQLFetcher(graphQLParams) {
var headers = {
'Accept': 'application/json',
'Content-Type': 'application/json'
};
if (csrftoken) {
headers['X-CSRFToken'] = csrftoken;
function getFetch(headers) {
return fetch(fetchURL, {
method: 'post',
headers: headers,
body: JSON.stringify(graphQLParams),
credentials: 'include'
});
}
return fetch(fetchURL, {
method: 'post',
headers: headers,
body: JSON.stringify(graphQLParams),
credentials: 'include',
}).then(function (response) {
return response.text();
}).then(function (responseBody) {
try {
return JSON.parse(responseBody);
} catch (error) {
return responseBody;
}
});
return getFetch(headers)
.then(function(response) {
console.log(headers);
return response.text();
})
.then(function(responseBody) {
try {
return JSON.parse(responseBody);
} catch (error) {
if (cookies.csrftoken && cookies.csrftoken.length) {
headers['X-CSRFToken'] = cookies.csrftoken.pop();
console.log('retry', headers)
return getFetch(headers);
}
Copy link
Member

Choose a reason for hiding this comment

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

There could be other reasons why the GraphQL endpoint doesn't return valid JSON so always assuming it's to do with a csrf token issue doesn't feel right.

Copy link
Contributor Author

@allen-munsch allen-munsch Dec 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@jkimbo see: https://stackoverflow.com/questions/576535/cookie-path-and-its-accessibility-to-subfolder-pages

From what I understand, the cookie sub path /2/m/2 would have both it's subpath cookie and the root path cookie. /.

The two paths have different database multitenancy requirements. It would probably be more accurate to read the cookie path directly, instead of just reading, checking them all?

Copy link
Member

Choose a reason for hiding this comment

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

Yes I understand that cookies can have different paths but looking at the Django source code the csrf cookie should be set to a path that is defined in the settings: https://github.com/django/django/blob/5a68f024987e6d16c2626a31bf653a2edddea579/django/middleware/csrf.py#L191

So I don't think it's possible to have 2 csrf cookies at the same time and I'm wondering how that came about?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

endpoints per multi tenancy. localhost:8000/:multitenantid/endpoints/here so multiple users can get different csrftoken cookies on a single machine ... it's not the default of django, but it is a real use case.

Copy link
Member

@jkimbo jkimbo Jan 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Ah I didn't realise that was possible! Could you not host the graphql endpoint per tenant as well or those that not work?

return responseBody;
}
});
}
// When the query and variables string is edited, update the URL bar so
// that it can be easily shared.
Expand All @@ -80,20 +99,17 @@
}
var options = {
fetcher: graphQLFetcher,
onEditQuery: onEditQuery,
onEditVariables: onEditVariables,
onEditOperationName: onEditOperationName,
query: parameters.query,
}
onEditQuery: onEditQuery,
onEditVariables: onEditVariables,
onEditOperationName: onEditOperationName,
query: parameters.query
};
if (parameters.variables) {
options.variables = parameters.variables;
}
if (parameters.operation_name) {
options.operationName = parameters.operation_name;
}
// Render <GraphiQL /> into the body.
ReactDOM.render(
React.createElement(GraphiQL, options),
document.body
);
ReactDOM.render(React.createElement(GraphiQL, options), document.body);
})();