Revive startup parameter --server.session-timeout (#14118)

Co-authored-by: Tobias Gödderz <tobias@arangodb.com>
arangodb · Dronplane · Oct 5, 2021 · Jun 10, 2021 · Jun 11, 2021 · Jun 11, 2021
commit e9c6ee9dcca7b9b4fbcd02a0b323d205bee838d3
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,13 @@
 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.
+
 * Add prefix parameter to LEVENSHTEIN_MATCH function in ArangoSearch
 
 * Removed redirects from /_admin/cluster* to /_admin/cluster/*. Adjusted

diff --git 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(static_cast<double>(1 * std::chrono::hours(1) / std::chrono::seconds(1))) { // 1 hour
   setOptional(false);
   startsAfter<application_features::BasicFeaturePhaseServer>();
 
@@ -89,10 +90,23 @@ void AuthenticationFeature::collectOptions(std::shared_ptr<ProgramOptions> 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(30900);
 
   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<ProgramOptions> 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<ProgramOptions> 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
@@ -72,6 +72,8 @@ class AuthenticationFeature final : public application_features::ApplicationFeat
   /// verification only secrets
   std::pair<std::string, std::vector<std::string>> 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
@@ -27,12 +27,12 @@
 #include <velocypack/Builder.h>
 #include <velocypack/velocypack-aliases.h>
 
+#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
@@ -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
@@ -103,10 +103,6 @@ void ServerFeature::collectOptions(std::shared_ptr<ProgramOptions> options) {
   options->addObsoleteOption("--vst.maxsize", "maxim
179B
al 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",

diff --git a/tests/js/client/server_parameters/test-server-session-timeout.js 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();