Skip to content

Commit

Permalink
keep CSRF token cookie as long as sign-in cookie; fixes #1538
Browse files Browse the repository at this point in the history
Fixes an issue in which opening a new browser window can cause you to
lose your CSRF token cookie (and therefore lose access to CRSF-guarded
pages such as signout), since this cookie had no expiration and was
therefore treated as a session cookie.
  • Loading branch information
jmcphers committed Oct 11, 2017
1 parent 9b1c9a0 commit bf6cd6d
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/cpp/server/ServerPAMAuth.cpp
Expand Up @@ -306,7 +306,7 @@ void setSignInCookies(const core::http::Request& request,
options().getOverlayOption("ssl-enabled") == "1");

// add cross site request forgery detection cookie
auth::csrf::setCSRFTokenCookie(request, pResponse);
auth::csrf::setCSRFTokenCookie(request, expiry, "", pResponse);
}

void doSignIn(const http::Request& request,
Expand Down
16 changes: 12 additions & 4 deletions src/cpp/server/auth/ServerCSRFToken.cpp
Expand Up @@ -32,22 +32,30 @@ namespace auth {
namespace csrf {

void setCSRFTokenCookie(const http::Request& request,
http::Response* pResponse,
const std::string& token)
boost::optional<boost::gregorian::days> expiry,
const std::string& token,
http::Response* pResponse)
{
// generate UUID for token if unspecified
std::string csrfToken(token);
if (csrfToken.empty())
csrfToken = core::system::generateUuid();

pResponse->addCookie(http::Cookie(
// generate CSRF token cookie
http::Cookie cookie(
request,
kCSRFTokenName,
csrfToken,
"/", // cookie for root path
false, // can't be HTTP only since it's read by client script
// secure if delivered via SSL
options().getOverlayOption("ssl-enabled") == "1"));
options().getOverlayOption("ssl-enabled") == "1");

// set expiration for cookie
if (expiry.is_initialized())
cookie.setExpires(*expiry);

pResponse->addCookie(cookie);
}

bool validateCSRFForm(const http::Request& request,
Expand Down
8 changes: 6 additions & 2 deletions src/cpp/server/include/server/auth/ServerCSRFToken.hpp
Expand Up @@ -17,6 +17,8 @@
#define SERVER_CSRF_TOKEN_HPP

#include <string>
#include <boost/optional.hpp>
#include <boost/date_time/posix_time/posix_time.hpp>

namespace rstudio {
namespace core {
Expand All @@ -34,8 +36,10 @@ namespace csrf {

// Adds a CSRF (cross site request forgery) cookie. This is simply a cookie with
// a random value (token).
void setCSRFTokenCookie(const core::http::Request&, core::http::Response*,
const std::string& token = std::string());
void setCSRFTokenCookie(const core::http::Request& request,
boost::optional<boost::gregorian::days> expiry,
const std::string& token,
core::http::Response* pResponse);

// Validates an HTTP POST request by ensuring that the submitted fields include
// a valid CSRF token.
Expand Down

0 comments on commit bf6cd6d

Please sign in to comment.