Skip to content

Commit 7d697e3

Browse files
committed
Refined Cookie production
Before this commit, logging in over http didn't work anymore due to a new Set-Cookie. After this commit, it works again.
1 parent 9eacab4 commit 7d697e3

File tree

4 files changed

+36
-8
lines changed

4 files changed

+36
-8
lines changed

session/logic.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ bool Session_Logic::attempt_login (std::string user_or_email, const std::string&
166166
open ();
167167
set_username (user_or_email);
168168
m_logged_in = true;
169-
std::string cookie = m_webserver_request.session_identifier;
169+
const std::string cookie = m_webserver_request.session_identifier;
170170
Database_Login::setTokens (user_or_email, "", "", "", cookie, touch_enabled_in);
171171
get_level (true);
172172
return true;

session/login.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ std::string session_login (Webserver_Request& webserver_request)
6767
Assets_View view{};
6868

6969
// Form submission handler.
70-
if (webserver_request.post_get("submit") != "") {
70+
if (webserver_request.post_count("submit")) {
7171
bool form_is_valid = true;
7272
const std::string user = webserver_request.post_get("user");
7373
const std::string pass = webserver_request.post_get("pass");

unittests/http.cpp

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -319,10 +319,24 @@ TEST (http, parse_header)
319319
}
320320
// Test cookie session.
321321
{
322-
Webserver_Request request{};
323-
const bool parsed = http_parse_header ("Cookie: Session=session; foo=bar; extra=clutter", request);
324-
EXPECT_TRUE(parsed);
325-
EXPECT_EQ(request.session_identifier, "session");
322+
{
323+
Webserver_Request request{};
324+
const bool parsed = http_parse_header ("Cookie: Session=session; foo=bar; extra=clutter", request);
325+
EXPECT_TRUE(parsed);
326+
EXPECT_EQ(request.session_identifier, "session");
327+
}
328+
{
329+
Webserver_Request request{};
330+
const bool parsed = http_parse_header ("Cookie: Session=1d15e82d0cf17d8b2b675ee9a7b8bb2f", request);
331+
EXPECT_TRUE(parsed);
332+
EXPECT_EQ(request.session_identifier, "1d15e82d0cf17d8b2b675ee9a7b8bb2f");
333+
}
334+
{
335+
Webserver_Request request{};
336+
const bool parsed = http_parse_header ("Cookie: Session=a809d7bdf691e86a409db85c9cfe69f2; Path=/; Max-Age=2678400; HttpOnly; SameSite=None; Secure", request);
337+
EXPECT_TRUE(parsed);
338+
EXPECT_EQ(request.session_identifier, "a809d7bdf691e86a409db85c9cfe69f2");
339+
}
326340
}
327341
// Test empty line as end of header.
328342
{

webserver/http.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,19 +243,33 @@ void http_assemble_response (Webserver_Request& webserver_request)
243243
// The Secure attribute is set.
244244
// It is meant to keep cookie communication limited to encrypted transmission,
245245
// directing browsers to use cookies only via secure/encrypted connections.
246-
// For maximum security, cookies with the Secure attribute should only be set over a secure connection.
246+
// Cookies with the Secure attribute should only be set over a secure connection.
247+
// If the Secure attribute is set and the connection is over http,
248+
// then the browser indicates:
249+
// This attempt to set a cookie via a Set-Cookie header was blocked
250+
// because it had the "Secure" attribute but was not received over a secure connection.
251+
// The result of this is that a user cannot login to Bibledit Cloud over a plain connection.
252+
// The localhost binding works because most browsers have special-case code
253+
// to treat connections to that host name as "secure", even if they don't use HTTPS.
247254

248255
// The HttpOnly attribute means that the cookie can be accessed by the HTTP API only,
249256
// and not by for example Javascript running in the browser.
250257
// This provides extra security.
251258

252259
// The setting "SameSite" is set to "None" to enable cookies while embedded in e.g. NextCloud,
253260
// which works via an iframe on a different origin.
261+
// If set to "None", this only works on a secure connection.
262+
// On a plain connection, the browser says:
263+
// This attempt to set a cookie via a Set-Cookie header was blocked
264+
// because it had the "SameSite=None" attribute but did not have the "Secure" attribute,
265+
// which is required in order to use "SameSite=None".
254266

255267
std::string identifier = webserver_request.session_identifier;
256268
if (identifier.empty ())
257269
identifier = filter::strings::get_new_random_string ();
258-
const std::string cookie = "Session=" + identifier + "; Path=/; Max-Age=2678400; HttpOnly; SameSite=None; Secure";
270+
std::string cookie = "Session=" + identifier + "; Path=/; Max-Age=2678400; HttpOnly";
271+
if (webserver_request.secure)
272+
cookie.append("; SameSite=None; Secure");
259273
response.push_back ("Set-Cookie: " + cookie);
260274
}
261275
if (!webserver_request.header.empty ())

0 commit comments

Comments
 (0)