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

Add support for CSRF tokens #5055

Merged
merged 17 commits into from
Aug 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .circleci/docker-compose.cypress.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ services:
REDASH_REDIS_URL: "redis://redis:6379/0"
REDASH_DATABASE_URL: "postgresql://postgres@postgres/postgres"
REDASH_RATELIMIT_ENABLED: "false"
REDASH_ENFORCE_CSRF: "true"
scheduler:
build: ../
command: scheduler
Expand Down
6 changes: 6 additions & 0 deletions client/app/services/axios.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import axiosLib from "axios";
import { Auth } from "@/services/auth";
import qs from "query-string";
import Cookies from "js-cookie";

export const axios = axiosLib.create({
paramsSerializer: params => qs.stringify(params),
Expand All @@ -14,6 +15,11 @@ axios.interceptors.request.use(config => {
const apiKey = Auth.getApiKey();
if (apiKey) {
config.headers.Authorization = `Key ${apiKey}`;
} else {
const csrfToken = Cookies.get("csrf_token");
if (csrfToken) {
config.headers.common["X-CSRF-TOKEN"] = csrfToken;
}
}

return config;
Expand Down
32 changes: 24 additions & 8 deletions client/cypress/cypress.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,37 @@
/* eslint-disable import/no-extraneous-dependencies, no-console */
const { find } = require("lodash");
const atob = require("atob");
const { execSync } = require("child_process");
const { post } = require("request").defaults({ jar: true });
const { get, post } = require("request").defaults({ jar: true });
const { seedData } = require("./seed-data");
var Cookie = require("request-cookies").Cookie;

const baseUrl = process.env.CYPRESS_baseUrl || "http://localhost:5000";

function seedDatabase(seedValues) {
const request = seedValues.shift();
const data = request.type === "form" ? { formData: request.data } : { json: request.data };
get(baseUrl + "/login", (_, { headers }) => {
const request = seedValues.shift();
const data = request.type === "form" ? { formData: request.data } : { json: request.data };

post(baseUrl + request.route, data, (err, response) => {
const result = response ? response.statusCode : err;
console.log("POST " + request.route + " - " + result);
if (seedValues.length) {
seedDatabase(seedValues);
if (headers["set-cookie"]) {
const cookies = headers["set-cookie"].map(cookie => new Cookie(cookie));
const csrfCookie = find(cookies, { key: "csrf_token" });
if (csrfCookie) {
if (request.type === "form") {
data["formData"] = { ...data["formData"], csrf_token: csrfCookie.value };
} else {
data["headers"] = { "X-CSRFToken": csrfCookie.value };
}
}
}

post(baseUrl + request.route, data, (err, response) => {
const result = response ? response.statusCode : err;
console.log("POST " + request.route + " - " + result);
if (seedValues.length) {
seedDatabase(seedValues);
}
});
});
}

Expand Down
41 changes: 30 additions & 11 deletions client/cypress/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,36 @@ import "@percy/cypress"; // eslint-disable-line import/no-extraneous-dependencie

const { each } = Cypress._;

Cypress.Commands.add("login", (email = "admin@redash.io", password = "password") =>
cy.request({
url: "/login",
method: "POST",
form: true,
body: {
email,
password,
},
})
);
Cypress.Commands.add("login", (email = "admin@redash.io", password = "password") => {
let csrf;
cy.visit("/login");
cy.getCookie("csrf_token")
.then(cookie => {
if (cookie) {
csrf = cookie.value;
} else {
cy.visit("/login").then(() => {
cy.get('input[name="csrf_token"]')
.invoke("val")
.then(csrf_token => {
csrf = csrf_token;
});
});
}
})
.then(() => {
cy.request({
url: "/login",
method: "POST",
form: true,
body: {
email,
password,
csrf_token: csrf,
},
});
});
});

Cypress.Commands.add("logout", () => cy.visit("/logout"));
Cypress.Commands.add("getByTestId", element => cy.get('[data-test="' + element + '"]'));
Expand Down
77 changes: 40 additions & 37 deletions client/cypress/support/redash-api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@

const { extend, get, merge, find } = Cypress._;

const post = options =>
cy
.getCookie("csrf_token")
.then(csrf => cy.request({ ...options, method: "POST", headers: { "X-CSRF-TOKEN": csrf.value } }));

export function createDashboard(name) {
return cy.request("POST", "api/dashboards", { name }).then(({ body }) => body);
return post({ url: "api/dashboards", body: { name } }).then(({ body }) => body);
}

export function createQuery(data, shouldPublish = true) {
Expand All @@ -21,10 +26,10 @@ export function createQuery(data, shouldPublish = true) {
);

// eslint-disable-next-line cypress/no-assigning-return-values
let request = cy.request("POST", "/api/queries", merged).then(({ body }) => body);
let request = post({ url: "/api/queries", body: merged }).then(({ body }) => body);
if (shouldPublish) {
request = request.then(query =>
cy.request("POST", `/api/queries/${query.id}`, { is_draft: false }).then(() => query)
post({ url: `/api/queries/${query.id}`, body: { is_draft: false } }).then(() => query)
);
}

Expand All @@ -33,7 +38,7 @@ export function createQuery(data, shouldPublish = true) {

export function createVisualization(queryId, type, name, options) {
const data = { query_id: queryId, type, name, options };
return cy.request("POST", "/api/visualizations", data).then(({ body }) => ({
return post({ url: "/api/visualizations", body: data }).then(({ body }) => ({
query_id: queryId,
...body,
}));
Expand All @@ -52,7 +57,7 @@ export function addTextbox(dashboardId, text = "text", options = {}) {
options: merge(defaultOptions, options),
};

return cy.request("POST", "api/widgets", data).then(({ body }) => {
return post({ url: "api/widgets", body: data }).then(({ body }) => {
const id = get(body, "id");
assert.isDefined(id, "Widget api call returns widget id");
return body;
Expand All @@ -71,7 +76,7 @@ export function addWidget(dashboardId, visualizationId, options = {}) {
options: merge(defaultOptions, options),
};

return cy.request("POST", "api/widgets", data).then(({ body }) => {
return post({ url: "api/widgets", body: data }).then(({ body }) => {
const id = get(body, "id");
assert.isDefined(id, "Widget api call returns widget id");
return body;
Expand All @@ -92,47 +97,42 @@ export function createAlert(queryId, options = {}, name) {
options: merge(defaultOptions, options),
};

return cy.request("POST", "api/alerts", data).then(({ body }) => {
return post({ url: "api/alerts", body: data }).then(({ body }) => {
const id = get(body, "id");
assert.isDefined(id, "Alert api call returns alert id");
assert.isDefined(id, "Alert api call retu ns alert id");
return body;
});
}

export function createUser({ name, email, password }) {
return cy
.request({
method: "POST",
url: "api/users?no_invite=yes",
body: { name, email },
failOnStatusCode: false,
})
.then(xhr => {
const { status, body } = xhr;
if (status < 200 || status > 400) {
throw new Error(xhr);
}
return post({
url: "api/users?no_invite=yes",
body: { name, email },
failOnStatusCode: false,
}).then(xhr => {
const { status, body } = xhr;
if (status < 200 || status > 400) {
throw new Error(xhr);
}

if (status === 400 && body.message === "Email already taken.") {
// all is good, do nothing
return;
}
if (status === 400 && body.message === "Email already taken.") {
// all is good, do nothing
return;
}

const id = get(body, "id");
assert.isDefined(id, "User api call returns user id");
const id = get(body, "id");
assert.isDefined(id, "User api call returns user id");

return cy.request({
url: body.invite_link,
method: "POST",
form: true,
body: { password },
});
return post({
url: body.invite_link,
form: true,
body: { password },
});
});
}

export function createDestination(name, type, options = {}) {
return cy.request({
method: "POST",
return post({
url: "api/destinations",
body: { name, type, options },
failOnStatusCode: false,
Expand All @@ -150,9 +150,12 @@ export function addDestinationSubscription(alertId, destinationName) {
if (!destination) {
throw new Error("Destination not found");
}
return cy.request("POST", `api/alerts/${alertId}/subscriptions`, {
alert_id: alertId,
destination_id: destination.id,
return post({
url: `api/alerts/${alertId}/subscriptions`,
body: {
alert_id: alertId,
destination_id: destination.id,
},
});
})
.then(({ body }) => {
Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ x-redash-environment: &redash-environment
REDASH_RATELIMIT_ENABLED: "false"
REDASH_MAIL_DEFAULT_SENDER: "redash@example.com"
REDASH_MAIL_SERVER: "email"
REDASH_ENFORCE_CSRF: "true"
Copy link
Member

Choose a reason for hiding this comment

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

Don't you need to do the same in Cypress' docker-compose?

services:
server:
<<: *redash-service
Expand Down
25 changes: 25 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"jest": "TZ=Africa/Khartoum jest",
"test": "run-s type-check jest",
"test:watch": "jest --watch",
"cypress:install": "npm install --no-save cypress@~4.5.0 @percy/agent@0.26.2 @percy/cypress@^2.2.0 atob@2.1.2",
"cypress:install": "npm install --no-save cypress@~4.5.0 @percy/agent@0.26.2 @percy/cypress@^2.2.0 atob@2.1.2 lodash@^4.17.10 request-cookies@^1.1.0",
"cypress": "node client/cypress/cypress.js",
"postinstall": "(cd viz-lib && npm ci && npm run build:babel)"
},
Expand Down Expand Up @@ -57,6 +57,7 @@
"font-awesome": "^4.7.0",
"history": "^4.10.1",
"hoist-non-react-statics": "^3.3.0",
"js-cookie": "^2.2.1",
"lodash": "^4.17.10",
"markdown": "0.5.0",
"material-design-iconic-font": "^2.2.0",
Expand Down Expand Up @@ -130,6 +131,7 @@
"raw-loader": "^0.5.1",
"react-test-renderer": "^16.5.2",
"request": "^2.88.0",
"request-cookies": "^1.1.0",
"typescript": "^3.9.6",
"url-loader": "^1.1.2",
"webpack": "^4.20.2",
Expand Down
19 changes: 19 additions & 0 deletions redash/security.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import functools
from flask import session
from flask_login import current_user
from flask_talisman import talisman
from flask_wtf.csrf import CSRFProtect, generate_csrf


from redash import settings


talisman = talisman.Talisman()
csrf = CSRFProtect()


def csp_allows_embeding(fn):
Expand All @@ -19,6 +24,20 @@ def decorated(*args, **kwargs):


def init_app(app):
csrf.init_app(app)
app.config["WTF_CSRF_CHECK_DEFAULT"] = False

@app.after_request
def inject_csrf_token(response):
response.set_cookie("csrf_token", generate_csrf())
return response

if settings.ENFORCE_CSRF:
@app.before_request
def check_csrf():
if not current_user.is_authenticated or 'user_id' in session:
csrf.protect()

talisman.init_app(
app,
feature_policy=settings.FEATURE_POLICY,
Expand Down
6 changes: 6 additions & 0 deletions redash/settings/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,3 +498,9 @@ def email_server_is_configured():
REQUESTS_ALLOW_REDIRECTS = parse_boolean(
os.environ.get("REDASH_REQUESTS_ALLOW_REDIRECTS", "false")
)

# Enforces CSRF token validation on API requests.
# This is turned off by default to avoid breaking any existing deployments but it is highly recommended to turn this toggle on to prevent CSRF attacks.
ENFORCE_CSRF = parse_boolean(
os.environ.get("REDASH_ENFORCE_CSRF", "false")
)
1 change: 1 addition & 0 deletions redash/templates/forgot.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
</div>
{% else %}
<form role="form" method="post">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
<div class="form-group">
<label for="email">Enter the email address you used for this account:</label>
<input type="email" class="form-control" name="email" value="{{email}}">
Expand Down
1 change: 1 addition & 0 deletions redash/templates/invite.html
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
{% endif %}

<form role="form" method="post" name="invite">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
<div class="form-group">
<label for="password">Password</label>
<input type="password" class="form-control" id="password" name="password">
Expand Down
Loading