From 66d9f0eb7103268d125078425214402dc6e06791 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Fri, 30 Apr 2021 19:26:16 +0200 Subject: [PATCH 1/8] Revive startup parameter `--server.session-timeout` Revive startup parameter `--server.session-timeout` to control the timeout for web interface sessions and other sessions that are based on JWTs created by the `/_open/auth` API. The default timeout value is 30 days, which is identical to the currently hard-coded timeout used. --- CHANGELOG | 4 ++ .../GeneralServer/AuthenticationFeature.cpp | 31 ++++++++-- arangod/GeneralServer/AuthenticationFeature.h | 3 + arangod/RestHandler/RestAuthHandler.cpp | 56 ++++++++++--------- arangod/RestHandler/RestAuthHandler.h | 9 +-- arangod/RestServer/ServerFeature.cpp | 4 -- 6 files changed, 64 insertions(+), 43 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index ab86f9cbc263..37cc6c74a960 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,10 @@ devel ----- +* Revive startup parameter `--server.session-timeout` to control the timeout + for web interface sessions and other sessions that are based on JWTs created + by the `/_open/auth` API. + * Fixed bug in error reporting when a database create did not work, which lead to a busy loop reporting this error to the agency. diff --git a/arangod/GeneralServer/AuthenticationFeature.cpp b/arangod/GeneralServer/AuthenticationFeature.cpp index 0caba2962bbc..267d0c14c16b 100644 --- a/arangod/GeneralServer/AuthenticationFeature.cpp +++ b/arangod/GeneralServer/AuthenticationFeature.cpp @@ -57,7 +57,8 @@ AuthenticationFeature::AuthenticationFeature(application_features::ApplicationSe _authenticationSystemOnly(true), _localAuthentication(true), _active(true), - _authenticationTimeout(0.0) { + _authenticationTimeout(0.0), + _sessionTimeout(60.0 * 60.9 * 24.0 * 30.0 /*30 days*/) { setOptional(false); startsAfter(); @@ -89,10 +90,23 @@ void AuthenticationFeature::collectOptions(std::shared_ptr optio "--server.authentication-timeout", "timeout for the authentication cache in seconds (0 = indefinitely)", new DoubleParameter(&_authenticationTimeout)); + + options->addOption("--server.session-timeout", + "timeout in seconds for web interface JWT sessions", + new DoubleParameter(&_sessionTimeout), + arangodb::options::makeFlags( + arangodb::options::Flags::DefaultNoComponents, + arangodb::options::Flags::OnCoordinator, + arangodb::options::Flags::OnSingle)) + .setIntroducedIn(30712); options->addOption("--server.local-authentication", "enable authentication using the local user database", - new BooleanParameter(&_localAuthentication)); + new BooleanParameter(&_localAuthentication), + arangodb::options::makeFlags( + arangodb::options::Flags::DefaultNoComponents, + arangodb::options::Flags::OnCoordinator, + arangodb::options::Flags::OnSingle)); options->addOption( "--server.authentication-system-only", @@ -102,10 +116,13 @@ void AuthenticationFeature::collectOptions(std::shared_ptr optio #ifdef ARANGODB_HAVE_DOMAIN_SOCKETS options->addOption("--server.authentication-unix-sockets", "authentication for requests via UNIX domain sockets", - new BooleanParameter(&_authenticationUnixSockets)); + new BooleanParameter(&_authenticationUnixSockets), + arangodb::options::makeFlags( + arangodb::options::Flags::DefaultNoOs, + arangodb::options::Flags::OsLinux, + arangodb::options::Flags::OsMac)); #endif - // Maybe deprecate this option in devel options ->addOption("--server.jwt-secret", "secret to use when doing jwt authentication", @@ -151,6 +168,12 @@ void AuthenticationFeature::validateOptions(std::shared_ptr opti FATAL_ERROR_EXIT(); } } + + if (_sessionTimeout <= 1.0) { + LOG_TOPIC("85046", FATAL, arangodb::Logger::AUTHENTICATION) + << "--server.session-timeout has an invalid value: " << _sessionTimeout; + FATAL_ERROR_EXIT(); + } if (options->processingResult().touched("server.jwt-secret")) { LOG_TOPIC("1aaae", WARN, arangodb::Logger::AUTHENTICATION) diff --git a/arangod/GeneralServer/AuthenticationFeature.h b/arangod/GeneralServer/AuthenticationFeature.h index 52708808afa0..f947d342c57f 100644 --- a/arangod/GeneralServer/AuthenticationFeature.h +++ b/arangod/GeneralServer/AuthenticationFeature.h @@ -72,6 +72,8 @@ class AuthenticationFeature final : public application_features::ApplicationFeat /// verification only secrets std::pair> jwtSecrets() const; #endif + + double sessionTimeout() const { return _sessionTimeout; } // load secrets from file(s) [[nodiscard]] Result loadJwtSecretsFromFile(); @@ -91,6 +93,7 @@ class AuthenticationFeature final : public application_features::ApplicationFeat bool _localAuthentication; bool _active; double _authenticationTimeout; + double _sessionTimeout; mutable std::mutex _jwtSecretsLock; diff --git a/arangod/RestHandler/RestAuthHandler.cpp b/arangod/RestHandler/RestAuthHandler.cpp index b40227244758..97d0ca184c69 100644 --- a/arangod/RestHandler/RestAuthHandler.cpp +++ b/arangod/RestHandler/RestAuthHandler.cpp @@ -27,12 +27,12 @@ #include #include +#include "Basics/ScopeGuard.h" #include "Basics/StringUtils.h" #include "GeneralServer/AuthenticationFeature.h" #include "Logger/LogMacros.h" #include "Logger/Logger.h" #include "Logger/LoggerStream.h" -#include "Ssl/SslInterface.h" #include "Utils/Events.h" using namespace arangodb; @@ -41,15 +41,7 @@ using namespace arangodb::rest; RestAuthHandler::RestAuthHandler(application_features::ApplicationServer& server, GeneralRequest* request, GeneralResponse* response) - : RestVocbaseBaseHandler(server, request, response), - _validFor(60 * 60 * 24 * 30) {} - -std::string RestAuthHandler::generateJwt(std::string const& username, - std::string const& password) { - AuthenticationFeature* af = AuthenticationFeature::instance(); - TRI_ASSERT(af != nullptr); - return fuerte::jwt::generateUserToken(af->tokenCache().jwtSecret(), username, _validFor); -} + : RestVocbaseBaseHandler(server, request, response) {} RestStatus RestAuthHandler::execute() { auto const type = _request->requestType(); @@ -75,23 +67,36 @@ RestStatus RestAuthHandler::execute() { return badRequest(); } - _username = usernameSlice.copyString(); + std::string const username = usernameSlice.copyString(); std::string const password = passwordSlice.copyString(); + bool isValid = false; + + auto guard = scopeGuard([&]() { + try { + if (isValid) { + events::LoggedIn(*_request, username); + } else { + events::CredentialsBad(*_request, username); + } + } catch (...) { + // nothing we can do + } + }); + auth::UserManager* um = AuthenticationFeature::instance()->userManager(); if (um == nullptr) { std::string msg = "This server does not support users"; LOG_TOPIC("2e7d4", ERR, Logger::AUTHENTICATION) << msg; generateError(rest::ResponseCode::UNAUTHORIZED, TRI_ERROR_HTTP_UNAUTHORIZED, msg); - } else if (um->checkPassword(_username, password)) { + } else if (um->checkPassword(username, password)) { VPackBuilder resultBuilder; { VPackObjectBuilder b(&resultBuilder); - std::string jwt = generateJwt(_username, password); - resultBuilder.add("jwt", VPackValue(jwt)); + resultBuilder.add("jwt", VPackValue(generateJwt(username))); } - _isValid = true; + isValid = true; generateDocument(resultBuilder.slice(), true, &VPackOptions::Defaults); } else { // mop: rfc 2616 10.4.2 (if credentials wrong 401) @@ -101,20 +106,17 @@ RestStatus RestAuthHandler::execute() { return RestStatus::DONE; } +std::string RestAuthHandler::generateJwt(std::string const& username) const { + AuthenticationFeature* af = AuthenticationFeature::instance(); + TRI_ASSERT(af != nullptr); + return fuerte::jwt::generateUserToken( + af->tokenCache().jwtSecret(), + username, + std::chrono::seconds(uint64_t(af->sessionTimeout()))); +} + RestStatus RestAuthHandler::badRequest() { generateError(rest::ResponseCode::BAD, TRI_ERROR_HTTP_BAD_PARAMETER, "invalid JSON"); return RestStatus::DONE; } - -void RestAuthHandler::shutdownExecute(bool isFinalized) noexcept { - try { - if (_isValid) { - events::LoggedIn(*_request, _username); - } else { - events::CredentialsBad(*_request, _username); - } - } catch (...) { - } - RestVocbaseBaseHandler::shutdownExecute(isFinalized); -} diff --git a/arangod/RestHandler/RestAuthHandler.h b/arangod/RestHandler/RestAuthHandler.h index 34c4578fd743..b500cfeb215a 100644 --- a/arangod/RestHandler/RestAuthHandler.h +++ b/arangod/RestHandler/RestAuthHandler.h @@ -33,21 +33,14 @@ class RestAuthHandler : public RestVocbaseBaseHandler { public: RestAuthHandler(application_features::ApplicationServer&, GeneralRequest*, GeneralResponse*); - std::string generateJwt(std::string const&, std::string const&); - public: char const* name() const override final { return "RestAuthHandler"; } RequestLane lane() const override final { return RequestLane::CLIENT_SLOW; } RestStatus execute() override; - void shutdownExecute(bool isFinalized) noexcept override; private: + std::string generateJwt(std::string const& username) const; RestStatus badRequest(); - - private: - std::string _username; - bool _isValid = false; - std::chrono::seconds _validFor; }; } // namespace arangodb diff --git a/arangod/RestServer/ServerFeature.cpp b/arangod/RestServer/ServerFeature.cpp index 709a28d75240..93c5c875c733 100644 --- a/arangod/RestServer/ServerFeature.cpp +++ b/arangod/RestServer/ServerFeature.cpp @@ -103,10 +103,6 @@ void ServerFeature::collectOptions(std::shared_ptr options) { options->addObsoleteOption("--vst.maxsize", "maximal size (in bytes) " "for a VelocyPack chunk", true); - options->addObsoleteOption( - "--server.session-timeout", - "timeout of web interface server sessions (in seconds)", true); - // add obsolete MMFiles WAL options (obsoleted in 3.7) options->addSection("wal", "WAL of the MMFiles engine", "", true, true); options->addObsoleteOption("--wal.allow-oversize-entries", From 22ef28a3dbcbf21632ebb3e170537867dd47653a Mon Sep 17 00:00:00 2001 From: Jan Date: Thu, 10 Jun 2021 14:53:41 +0200 Subject: [PATCH 2/8] Update arangod/GeneralServer/AuthenticationFeature.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tobias Gödderz --- arangod/GeneralServer/AuthenticationFeature.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/GeneralServer/AuthenticationFeature.cpp b/arangod/GeneralServer/AuthenticationFeature.cpp index 267d0c14c16b..5f5341fb5883 100644 --- a/arangod/GeneralServer/AuthenticationFeature.cpp +++ b/arangod/GeneralServer/AuthenticationFeature.cpp @@ -58,7 +58,7 @@ AuthenticationFeature::AuthenticationFeature(application_features::ApplicationSe _localAuthentication(true), _active(true), _authenticationTimeout(0.0), - _sessionTimeout(60.0 * 60.9 * 24.0 * 30.0 /*30 days*/) { + _sessionTimeout(30 * std::chrono::hours(24) / std::chrono::seconds(1)) { setOptional(false); startsAfter(); From 7c2b445549f3c23165d88730cb9364e8a6c7794e Mon Sep 17 00:00:00 2001 From: Jan Date: Thu, 10 Jun 2021 17:34:10 +0200 Subject: [PATCH 3/8] Update arangod/GeneralServer/AuthenticationFeature.cpp --- arangod/GeneralServer/AuthenticationFeature.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/GeneralServer/AuthenticationFeature.cpp b/arangod/GeneralServer/AuthenticationFeature.cpp index 5f5341fb5883..fbdb4fe35566 100644 --- a/arangod/GeneralServer/AuthenticationFeature.cpp +++ b/arangod/GeneralServer/AuthenticationFeature.cpp @@ -58,7 +58,7 @@ AuthenticationFeature::AuthenticationFeature(application_features::ApplicationSe _localAuthentication(true), _active(true), _authenticationTimeout(0.0), - _sessionTimeout(30 * std::chrono::hours(24) / std::chrono::seconds(1)) { + _sessionTimeout(static_cast(30 * std::chrono::hours(24) / std::chrono::seconds(1))) { setOptional(false); startsAfter(); From bbc7822e2133b5bbb52a6c452ed85d64e299ef2e Mon Sep 17 00:00:00 2001 From: Jan Date: Tue, 29 Jun 2021 16:20:27 +0200 Subject: [PATCH 4/8] Update arangod/GeneralServer/AuthenticationFeature.cpp --- arangod/GeneralServer/AuthenticationFeature.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/GeneralServer/AuthenticationFeature.cpp b/arangod/GeneralServer/AuthenticationFeature.cpp index fbdb4fe35566..7768d129e472 100644 --- a/arangod/GeneralServer/AuthenticationFeature.cpp +++ b/arangod/GeneralServer/AuthenticationFeature.cpp @@ -98,7 +98,7 @@ void AuthenticationFeature::collectOptions(std::shared_ptr optio arangodb::options::Flags::DefaultNoComponents, arangodb::options::Flags::OnCoordinator, arangodb::options::Flags::OnSingle)) - .setIntroducedIn(30712); + .setIntroducedIn(30900); options->addOption("--server.local-authentication", "enable authentication using the local user database", From 7a7f5aee57a30d2f648725257242bba92815ffb7 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 29 Jun 2021 16:31:47 +0200 Subject: [PATCH 5/8] adjust timeout to one hour --- arangod/GeneralServer/AuthenticationFeature.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/GeneralServer/AuthenticationFeature.cpp b/arangod/GeneralServer/AuthenticationFeature.cpp index 7768d129e472..49dc785aca73 100644 --- a/arangod/GeneralServer/AuthenticationFeature.cpp +++ b/arangod/GeneralServer/AuthenticationFeature.cpp @@ -58,7 +58,7 @@ AuthenticationFeature::AuthenticationFeature(application_features::ApplicationSe _localAuthentication(true), _active(true), _authenticationTimeout(0.0), - _sessionTimeout(static_cast(30 * std::chrono::hours(24) / std::chrono::seconds(1))) { + _sessionTimeout(static_cast(1 * std::chrono::hours(24) / std::chrono::seconds(1))) { // 1 hour setOptional(false); startsAfter(); From 19158bbecd62a544e770c6eb9200998e27af1e7a Mon Sep 17 00:00:00 2001 From: jsteemann Date: Wed, 30 Jun 2021 12:11:48 +0200 Subject: [PATCH 6/8] added test --- .../test-server-session-timeout.js | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 tests/js/client/server_parameters/test-server-session-timeout.js diff --git a/tests/js/client/server_parameters/test-server-session-timeout.js b/tests/js/client/server_parameters/test-server-session-timeout.js new file mode 100644 index 000000000000..1721c6833d0c --- /dev/null +++ b/tests/js/client/server_parameters/test-server-session-timeout.js @@ -0,0 +1,88 @@ +/*jshint globalstrict:false, strict:false */ +/* global getOptions, assertEqual, arango */ + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test for server parameters +/// +/// DISCLAIMER +/// +/// Copyright 2010-2012 triagens GmbH, Cologne, Germany +/// +/// Licensed under the Apache License, Version 2.0 (the "License"); +/// you may not use this file except in compliance with the License. +/// You may obtain a copy of the License at +/// +/// http://www.apache.org/licenses/LICENSE-2.0 +/// +/// Unless required by applicable law or agreed to in writing, software +/// distributed under the License is distributed on an "AS IS" BASIS, +/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +/// See the License for the specific language governing permissions and +/// limitations under the License. +/// +/// Copyright holder is ArangoDB Inc, Cologne, Germany +/// +/// @author Jan Steemann +/// @author Copyright 2019, ArangoDB Inc, Cologne, Germany +//////////////////////////////////////////////////////////////////////////////// + +if (getOptions === true) { + return { + 'server.session-timeout': '5', + 'server.authentication': 'true', + 'server.jwt-secret': 'haxxmann', + }; +} +const jsunity = require('jsunity'); +const request = require('@arangodb/request'); + +function testSuite() { + let baseUrl = function () { + return arango.getEndpoint().replace(/^tcp:/, 'http:').replace(/^ssl:/, 'https:'); + }; + + return { + testSessionTimeout: function() { + let result = request.get(baseUrl() + "/_api/version"); + // no access + assertEqual(401, result.statusCode); + + result = request.post({ + url: baseUrl() + "/_open/auth", + body: { + username: "root", + password: "" + }, + json: true + }); + + assertEqual(200, result.statusCode); + const jwt = result.json.jwt; + + result = request.get({ + url: baseUrl() + "/_api/version", + auth: { + bearer: jwt, + } + }); + + // access granted + assertEqual(200, result.statusCode); + + require("internal").sleep(7); + + result = request.get({ + url: baseUrl() + "/_api/version", + auth: { + bearer: jwt, + } + }); + + // JWT expired + assertEqual(401, result.statusCode); + }, + }; +} + +jsunity.run(testSuite); +return jsunity.done(); From 43602756d7c93e153fbc906d47bd4f9d7a6bc415 Mon Sep 17 00:00:00 2001 From: Jan Date: Thu, 1 Jul 2021 10:06:18 +0200 Subject: [PATCH 7/8] Update CHANGELOG --- CHANGELOG | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 25c23052a73b..ea126bce58d6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,9 @@ devel * Revive startup parameter `--server.session-timeout` to control the timeout for web interface sessions and other sessions that are based on JWTs created by the `/_open/auth` API. + + This PR also changes the default session timeout for web interface sessions + to one hour. Older versions of ArangoDB had longer session timeouts. * Fix potential stack overflow when executing large queries. This is achieved by splitting the callstack and moving part of the execution From 98e335157b64b1a13f026e92889a4aa8eb5c024e Mon Sep 17 00:00:00 2001 From: Jan Date: Thu, 1 Jul 2021 10:06:45 +0200 Subject: [PATCH 8/8] Update arangod/GeneralServer/AuthenticationFeature.cpp --- arangod/GeneralServer/AuthenticationFeature.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/GeneralServer/AuthenticationFeature.cpp b/arangod/GeneralServer/AuthenticationFeature.cpp index 49dc785aca73..f1b1860695e6 100644 --- a/arangod/GeneralServer/AuthenticationFeature.cpp +++ b/arangod/GeneralServer/AuthenticationFeature.cpp @@ -58,7 +58,7 @@ AuthenticationFeature::AuthenticationFeature(application_features::ApplicationSe _localAuthentication(true), _active(true), _authenticationTimeout(0.0), - _sessionTimeout(static_cast(1 * std::chrono::hours(24) / std::chrono::seconds(1))) { // 1 hour + _sessionTimeout(static_cast(1 * std::chrono::hours(1) / std::chrono::seconds(1))) { // 1 hour setOptional(false); startsAfter();