Skip to content

Commit

Permalink
Add support for CSRF tokens (#5055)
Browse files Browse the repository at this point in the history
* add flask-wtf

* add CSRF tokens to all static forms

* add CSRF tokens to all axios requests

* disable CSRF validation in unit tests

* support CSRF-protected requests in *most* cypress tests

* don't enfroce CSRF checks by default

* avoid CSRF enforcement in unit tests

* remove redundant spread

* some camel casing hiccups

* always yield the CSRF cookie, but avoid enforcing it if CSRF toggle is off

* Restyled by prettier (#5056)

Co-authored-by: Restyled.io <commits@restyled.io>

* set a CSRF header only if cookie is present

* enforce CSRF in CI

* install lodash directly for Cypress

* install request-cookies directly for Cypress. We should probably start loading package.json deps

* enable CSRF support when logout and login happen within the same spec

Co-authored-by: restyled-io[bot] <32688539+restyled-io[bot]@users.noreply.github.com>
Co-authored-by: Restyled.io <commits@restyled.io>
  • Loading branch information
3 people authored Aug 9, 2020
1 parent eb603f6 commit 5afd055
Show file tree
Hide file tree
Showing 17 changed files with 163 additions and 57 deletions.
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"
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 @@ -500,3 +500,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

0 comments on commit 5afd055

Please sign in to comment.