Add tests and implement missing apidb oauth methods.

* Extracted interface for rate limiter so that it can be mocked in tests.
* Extracted test request to be shared between `test_core` and `test_apidb_backend`.
* Added end-to-end test case for OAuth, using the `process_request` method.

include/cgimap/rate_limiter.hpp

@@ -5,13 +5,32 @@
 #include <libmemcached/memcached.h>
 #include <boost/program_options.hpp>
 
-class rate_limiter {
+struct rate_limiter {
+  virtual ~rate_limiter();
+
+  // check if the key is below the rate limit. return true to indicate that it
+  // is.
+  virtual bool check(const std::string &key) = 0;
+
+  // update the limit for the key to say it has consumed this number of bytes.
+  virtual void update(const std::string &key, int bytes) = 0;
+};
+
+struct null_rate_limiter
+  : public rate_limiter {
+  ~null_rate_limiter();
+  bool check(const std::string &key);
+  void update(const std::string &key, int bytes);
+};
+
+class memcached_rate_limiter
+  : public rate_limiter {
 public:
   /**
    * Methods.
    */
-  rate_limiter(const boost::program_options::variables_map &options);
-  ~rate_limiter(void);
+  memcached_rate_limiter(const boost::program_options::variables_map &options);
+  ~memcached_rate_limiter(void);
   bool check(const std::string &key);
   void update(const std::string &key, int bytes);
 

src/Makefile.am

@@ -51,7 +51,8 @@ ___openstreetmap_cgimap_SOURCES=\
 	main.cpp
 
 ___test_test_core_SOURCES=\
-	../test/test_core.cpp
+	../test/test_core.cpp \
+	../test/test_request.cpp
 
 ___test_test_parse_id_list_SOURCES=\
 	../test/test_parse_id_list.cpp
@@ -69,7 +70,8 @@ if ENABLE_APIDB
 ___test_test_apidb_backend_SOURCES=\
 	../test/test_apidb_backend.cpp \
 	../test/test_formatter.cpp \
-	../test/test_database.cpp
+	../test/test_database.cpp \
+	../test/test_request.cpp
 endif
 
 libcgimap_fcgi_la_SOURCES=\

src/backend/apidb/oauth_store.cpp

@@ -90,21 +90,47 @@ oauth_store::oauth_store(const po::variables_map &opts)
     "WHERE token=$1")
     PREPARE_ARGS(("character varying"));
 
+  // return a row with the consumer secret for a given consumer key.
+  m_connection.prepare("consumer_secret_for_key",
+    "SELECT secret FROM client_applications "
+    "WHERE key=$1")
+    PREPARE_ARGS(("character varying"));
+
+  // return a row with the token secret given the token ID.
+  m_connection.prepare("token_secret_for_id",
+    "SELECT secret FROM oauth_tokens "
+    "WHERE token=$1")
+    PREPARE_ARGS(("character varying"));
+
   // clang-format on
 }
 
 oauth_store::~oauth_store() {}
 
 boost::optional<std::string>
 oauth_store::consumer_secret(const std::string &consumer_key) {
-  // TODO: implement me!
-  return boost::none;
+  pqxx::work w(m_connection, "oauth_get_consumer_secret_for_key");
+  pqxx::result res = w.prepared("consumer_secret_for_key")(consumer_key).exec();
+
+  if (res.affected_rows() > 0) {
+    return res[0][0].as<std::string>();
+
+  } else {
+    return boost::none;
+  }
 }
 
 boost::optional<std::string>
 oauth_store::token_secret(const std::string &token_id) {
-  // TODO: implement me!
-  return boost::none;
+  pqxx::work w(m_connection, "oauth_get_token_secret_for_id");
+  pqxx::result res = w.prepared("token_secret_for_id")(token_id).exec();
+
+  if (res.affected_rows() > 0) {
+    return res[0][0].as<std::string>();
+
+  } else {
+    return boost::none;
+  }
 }
 
 bool

src/main.cpp

@@ -151,7 +151,7 @@ static void process_requests(int socket, const po::variables_map &options) {
   }
 
   // create the rate limiter
-  rate_limiter limiter(options);
+  memcached_rate_limiter limiter(options);
 
   // create the routes map (from URIs to handlers)
   routes route;

src/rate_limiter.cpp

@@ -3,12 +3,25 @@
 
 #include "cgimap/rate_limiter.hpp"
 
-struct rate_limiter::state {
+rate_limiter::~rate_limiter() {
+}
+
+null_rate_limiter::~null_rate_limiter() {
+}
+
+bool null_rate_limiter::check(const std::string &key) {
+  return true;
+}
+
+void null_rate_limiter::update(const std::string &key, int bytes) {
+}
+
+struct memcached_rate_limiter::state {
   time_t last_update;
   int bytes_served;
 };
 
-rate_limiter::rate_limiter(
+memcached_rate_limiter::memcached_rate_limiter(
     const boost::program_options::variables_map &options) {
   if (options.count("memcache") && (ptr = memcached_create(NULL)) != NULL) {
     memcached_server_st *server_list;
@@ -40,12 +53,12 @@ rate_limiter::rate_limiter(
   }
 }
 
-rate_limiter::~rate_limiter(void) {
+memcached_rate_limiter::~memcached_rate_limiter(void) {
   if (ptr)
     memcached_free(ptr);
 }
 
-bool rate_limiter::check(const std::string &key) {
+bool memcached_rate_limiter::check(const std::string &key) {
   int bytes_served = 0;
   std::string mc_key;
   state *sp;
@@ -72,7 +85,7 @@ bool rate_limiter::check(const std::string &key) {
   return bytes_served < max_bytes;
 }
 
-void rate_limiter::update(const std::string &key, int bytes) {
+void memcached_rate_limiter::update(const std::string &key, int bytes) {
   if (ptr) {
     time_t now = time(NULL);
     std::string mc_key;

test/test_apidb_backend.cpp

@@ -4,15 +4,22 @@
 #include <boost/format.hpp>
 #include <boost/foreach.hpp>
 #include <boost/optional/optional_io.hpp>
+#include <boost/make_shared.hpp>
+#include <boost/program_options.hpp>
 
 #include <sys/time.h>
 #include <stdio.h>
 
 #include "cgimap/config.hpp"
 #include "cgimap/time.hpp"
+#include "cgimap/oauth.hpp"
+#include "cgimap/rate_limiter.hpp"
+#include "cgimap/routes.hpp"
+#include "cgimap/process_request.hpp"
+
 #include "test_formatter.hpp"
 #include "test_database.hpp"
-#include "cgimap/oauth.hpp"
+#include "test_request.hpp"
 
 namespace {
 
@@ -395,6 +402,127 @@ void test_changeset_with_comments_including_discussions(boost::shared_ptr<data_s
   }
 }
 
+class empty_data_selection
+  : public data_selection {
+public:
+
+  virtual ~empty_data_selection() {}
+
+  void write_nodes(output_formatter &formatter) {}
+  void write_ways(output_formatter &formatter) {}
+  void write_relations(output_formatter &formatter) {}
+  void write_changesets(output_formatter &formatter,
+                        const boost::posix_time::ptime &now) {}
+
+  visibility_t check_node_visibility(osm_nwr_id_t id) {}
+  visibility_t check_way_visibility(osm_nwr_id_t id) {}
+  visibility_t check_relation_visibility(osm_nwr_id_t id) {}
+
+  int select_nodes(const std::vector<osm_nwr_id_t> &) { return 0; }
+  int select_ways(const std::vector<osm_nwr_id_t> &) { return 0; }
+  int select_relations(const std::vector<osm_nwr_id_t> &) { return 0; }
+  int select_nodes_from_bbox(const bbox &bounds, int max_nodes) { return 0; }
+  void select_nodes_from_relations() {}
+  void select_ways_from_nodes() {}
+  void select_ways_from_relations() {}
+  void select_relations_from_ways() {}
+  void select_nodes_from_way_nodes() {}
+  void select_relations_from_nodes() {}
+  void select_relations_from_relations() {}
+  void select_relations_members_of_relations() {}
+  bool supports_changesets() { return false; }
+  int select_changesets(const std::vector<osm_changeset_id_t> &) { return 0; }
+  void select_changeset_discussions() {}
+
+  struct factory
+    : public data_selection::factory {
+    virtual ~factory() {}
+    virtual boost::shared_ptr<data_selection> make_selection() {
+      return boost::make_shared<empty_data_selection>();
+    }
+  };
+};
+
+struct recording_rate_limiter
+  : public rate_limiter {
+  ~recording_rate_limiter() {}
+
+  bool check(const std::string &key) {
+    m_keys_seen.insert(key);
+    return true;
+  }
+
+  void update(const std::string &key, int bytes) {
+    m_keys_seen.insert(key);
+  }
+
+  bool saw_key(const std::string &key) {
+    return m_keys_seen.count(key) > 0;
+  }
+
+private:
+  std::set<std::string> m_keys_seen;
+};
+
+
+void test_oauth_end_to_end(boost::shared_ptr<oauth::store> store) {
+  recording_rate_limiter limiter;
+  std::string generator("test_apidb_backend.cpp");
+  routes route;
+  boost::shared_ptr<data_selection::factory> factory =
+    boost::make_shared<empty_data_selection::factory>();
+
+  test_request req;
+  req.set_header("SCRIPT_URL", "/api/0.6/relation/165475/full");
+  req.set_header("SCRIPT_URI",
+                 "http://www.openstreetmap.org/api/0.6/relation/165475/full");
+  req.set_header("HTTP_HOST", "www.openstreetmap.org");
+  req.set_header("HTTP_ACCEPT_ENCODING",
+                 "gzip;q=1.0,deflate;q=0.6,identity;q=0.3");
+  req.set_header("HTTP_ACCEPT", "*/*");
+  req.set_header("HTTP_USER_AGENT", "OAuth gem v0.4.7");
+  req.set_header("HTTP_AUTHORIZATION",
+                 "OAuth oauth_consumer_key=\"x3tHSMbotPe5fBlItMbg\", "
+                 "oauth_nonce=\"dvu3eTk8i1uvj8zQ8Wef91UF6ngQdlTA3xQ2vEf7xU\", "
+                 "oauth_signature=\"ewKFprItE5uaDHKFu3IVzuEHbno%3D\", "
+                 "oauth_signature_method=\"HMAC-SHA1\", "
+                 "oauth_timestamp=\"1475844649\", "
+                 "oauth_token=\"15zpwgGjdjBu1DD65X7kcHzaWqfQpvqmMtqa3ZIO\", "
+                 "oauth_version=\"1.0\"");
+  req.set_header("HTTP_X_REQUEST_ID", "V-eaKX8AAQEAAF4UzHwAAAHt");
+  req.set_header("HTTP_X_FORWARDED_HOST", "www.openstreetmap.org");
+  req.set_header("HTTP_X_FORWARDED_SERVER", "www.openstreetmap.org");
+  req.set_header("HTTP_CONNECTION", "Keep-Alive");
+  req.set_header("PATH", "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin");
+  req.set_header("SERVER_SIGNATURE", "<address>Apache/2.4.18 (Ubuntu) Server "
+                 "at www.openstreetmap.org Port 80</address>");
+  req.set_header("SERVER_SOFTWARE", "Apache/2.4.18 (Ubuntu)");
+  req.set_header("SERVER_NAME", "www.openstreetmap.org");
+  req.set_header("SERVER_ADDR", "127.0.0.1");
+  req.set_header("SERVER_PORT", "80");
+  req.set_header("REMOTE_ADDR", "127.0.0.1");
+  req.set_header("DOCUMENT_ROOT", "/srv/www.openstreetmap.org/rails/public");
+  req.set_header("REQUEST_SCHEME", "http");
+  req.set_header("SERVER_PROTOCOL", "HTTP/1.1");
+  req.set_header("REQUEST_METHOD", "GET");
+  req.set_header("QUERY_STRING", "");
+  req.set_header("REQUEST_URI", "/api/0.6/relation/165475/full");
+  req.set_header("SCRIPT_NAME", "/api/0.6/relation/165475/full");
+
+  assert_equal<boost::optional<std::string> >(
+    std::string("ewKFprItE5uaDHKFu3IVzuEHbno="),
+    oauth::detail::hashed_signature(req, *store),
+    "hashed signatures");
+
+  process_request(req, limiter, generator, route, factory, store);
+
+  assert_equal<int>(404, req.response_status(), "response status");
+  assert_equal<bool>(false, limiter.saw_key("addr:127.0.0.1"),
+                     "saw addr:127.0.0.1 as a rate limit key");
+  assert_equal<bool>(true, limiter.saw_key("user:1"),
+                     "saw user:1 as a rate limit key");
+}
+
 } // anonymous namespace
 
 int main(int, char **) {
@@ -417,8 +545,6 @@ int main(int, char **) {
     tdb.run(boost::function<void(boost::shared_ptr<oauth::store>)>(
         &test_get_user_id_for_token));
 
-    // TODO: add tests for consumer_secret and token_secret
-
     tdb.run(boost::function<void(boost::shared_ptr<data_selection>)>(
         &test_negative_changeset_ids));
 
@@ -437,7 +563,8 @@ int main(int, char **) {
     tdb.run(boost::function<void(boost::shared_ptr<data_selection>)>(
               &test_changeset_with_comments_including_discussions));
 
-    // TODO: add end-to-end test for OAuth request.
+    tdb.run(boost::function<void(boost::shared_ptr<oauth::store>)>(
+              &test_oauth_end_to_end));
 
   } catch (const test_database::setup_error &e) {
     std::cout << "Unable to set up test database: " << e.what() << std::endl;

test/test_apidb_backend.sql

@@ -42,24 +42,35 @@ VALUES (1,       0,       0, 1, true,  '2013-11-14T02:10:00Z', 3221225472, 1),
 -- the API is publicly readable, so _all_ tokens can read the API. there is no
 -- "allow_read_api" setting on the token itself. but some tokens might have been
 -- revoked or expired.
+INSERT INTO client_applications
+  (id, name, user_id, allow_read_prefs, key, secret)
+VALUES
+  (1, 'test_client_application', 1, true,
+   'x3tHSMbotPe5fBlItMbg', '1NZRJ0u2o7OilPDe60nfZsKJTC7RUZPrNfYwGBjATw');
+
 INSERT INTO oauth_tokens
-  (id, user_id, token, secret, created_at, authorized_at, invalidated_at)
+  (id, user_id, client_application_id, allow_read_prefs, token, secret,
+   created_at, authorized_at, invalidated_at)
 VALUES
   -- valid key
-  (1, 1,
+  (1, 1, 1, true,
    'OfkxM4sSeyXjzgDTIOaJxcutsnqBoalr842NHOrA',
    'fFCKdXsLxeHPlgrIPr5fZSpXKnDuLw0GvJTzeE99',
    '2016-07-11T19:12:00Z', '2016-07-11T19:12:00Z', NULL),
   -- not authorized
-  (2, 1,
+  (2, 1, 1, true,
    'wpNsXPhrgWl4ELPjPbhfwjjSbNk9npsKoNrMGFlC',
    'NZskvUUYlOuCsPKuMbSTz5eMpVJVI3LsyW11Z2Uq',
    '2016-07-11T19:12:00Z', NULL, NULL),
   -- invalidated
-  (3, 1,
+  (3, 1, 1, true,
    'Rzcm5aDiDgqgub8j96MfDaYyAc4cRwI9CmZB7HBf',
    '2UxsEFziZGv64hdWN3Qa90Vb6v1aovVxaTTQIn1D',
-   '2016-07-11T19:12:00Z', '2016-07-11T19:12:00Z', '2016-07-11T19:12:00Z');
+   '2016-07-11T19:12:00Z', '2016-07-11T19:12:00Z', '2016-07-11T19:12:00Z'),
+  (4, 1, 1, true,
+   '15zpwgGjdjBu1DD65X7kcHzaWqfQpvqmMtqa3ZIO',
+   'H3Vb9Kgf4LpTyVlft5xsI9MwzknQsTu6CkHE0qK3',
+   '2016-10-07T00:00:00Z', '2016-10-07T00:00:00Z', NULL);
 
 -- update the number of edits
 UPDATE changesets