Fix bugs:

1. Use template specialisation in the correct way to avoid always returning as if it were a 500 error. 2. If path already starts with / then don't add a second one when normalising the request URL.
zerebubuth · Oct 6, 2016 · c5b062b · c5b062b
1 parent 7be2d3f
commit c5b062b
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 13 deletions.
diff --git a/src/oauth.cpp b/src/oauth.cpp
@@ -385,8 +385,12 @@ std::string normalise_request_url(request &req) {
   std::ostringstream out;
 
   out << downcase(scheme(req)) << "://"
-      << downcase(authority(req)) << "/"
-      << path(req);
+      << downcase(authority(req));
+  std::string p = path(req);
+  if (p.size() > 0 && p[0] != '/') {
+    out << "/";
+  }
+  out << p;
 
   return out.str();
 }

diff --git a/src/process_request.cpp b/src/process_request.cpp
@@ -217,27 +217,28 @@ const std::string user_prefix("user:");
 struct is_copacetic : public boost::static_visitor<bool> {
   template <typename T>
   bool operator()(const T &) const { return false; }
-
-  bool operator()(const oauth::validity::copacetic &) const {
-    return true;
-  }
 };
 
+template <>
+bool is_copacetic::operator()<oauth::validity::copacetic>(
+  const oauth::validity::copacetic &) const {
+  return true;
+}
+
 struct get_oauth_token : public boost::static_visitor<std::string> {
   template <typename T>
   std::string operator()(const T &) const {
     throw std::runtime_error("Type does not contain an OAuth token.");
   }
-
-  std::string operator()(const oauth::validity::copacetic &c) const {
-    return c.token;
-  }
 };
 
-struct oauth_status_code : public boost::static_visitor<int> {
-  template <typename T>
-  bool operator()(const T &) const { return 500; }
+template <>
+std::string get_oauth_token::operator()<oauth::validity::copacetic>(
+  const oauth::validity::copacetic &c) const {
+  return c.token;
+}
 
+struct oauth_status_code : public boost::static_visitor<int> {
   bool operator()(const oauth::validity::copacetic &) const { return 200; }
   bool operator()(const oauth::validity::not_signed &) const { return 200; }
   bool operator()(const oauth::validity::bad_request &) const { return 400; }

diff --git a/test/test_oauth.cpp b/test/test_oauth.cpp
@@ -398,6 +398,33 @@ void oauth_check_missing_signature() {
                oauth::is_valid_signature(req, store, store, store));
 }
 
+void oauth_check_valid_signature_header_2() {
+  boost::optional<std::string> auth_header = std::string("OAuth oauth_consumer_key=\"x3tHSMbotPe5fBlItMbg\", oauth_nonce=\"ZGsGj6qzGYUhSLHJWUC8tyW6RbxOQuX4mv6PKj0mU\", oauth_signature=\"H%2Fxl6jdk4dC0WaONfohWfZhcHYA%3D\", oauth_signature_method=\"HMAC-SHA1\", oauth_timestamp=\"1475754589\", oauth_token=\"15zpwgGjdjBu1DD65X7kcHzaWqfQpvqmMtqa3ZIO\", oauth_version=\"1.0\"");
+  test_request req(
+    "GET",
+    "http", "www.openstreetmap.org", "80", "/api/0.6/relation/165475/full", "",
+    auth_header);
+
+  assert_equal<boost::optional<std::string> >(
+    oauth::detail::signature_base_string(req),
+    std::string("GET&http%3A%2F%2Fwww.openstreetmap.org%2Fapi%2F0.6%2Frelation%2F165475%2Ffull&oauth_consumer_key%3Dx3tHSMbotPe5fBlItMbg%26oauth_nonce%3DZGsGj6qzGYUhSLHJWUC8tyW6RbxOQuX4mv6PKj0mU%26oauth_signature_method%3DHMAC-SHA1%26oauth_timestamp%3D1475754589%26oauth_token%3D15zpwgGjdjBu1DD65X7kcHzaWqfQpvqmMtqa3ZIO%26oauth_version%3D1.0"));
+
+  std::string consumer_key("x3tHSMbotPe5fBlItMbg");
+  std::string consumer_secret("1NZRJ0u2o7OilPDe60nfZsKJTC7RUZPrNfYwGBjATw");
+  std::string token_id("15zpwgGjdjBu1DD65X7kcHzaWqfQpvqmMtqa3ZIO");
+  std::string token_secret("H3Vb9Kgf4LpTyVlft5xsI9MwzknQsTu6CkHE0qK3");
+
+  test_secret_store store(consumer_key, consumer_secret, token_id, token_secret);
+
+  assert_equal<boost::optional<std::string> >(
+    oauth::detail::hashed_signature(req, store),
+    std::string("H/xl6jdk4dC0WaONfohWfZhcHYA="));
+
+  oauth::validity::copacetic copacetic(token_id);
+  oauth::validity::validity expected(copacetic);
+  assert_equal(oauth::is_valid_signature(req, store, store, store), expected);
+}
+
 int main() {
   try {
     ANNOTATE_EXCEPTION(oauth_check_signature_base_string());
@@ -412,6 +439,7 @@ int main() {
     ANNOTATE_EXCEPTION(oauth_check_invalid_signature_header());
     ANNOTATE_EXCEPTION(oauth_check_valid_signature_params());
     ANNOTATE_EXCEPTION(oauth_check_missing_signature());
+    ANNOTATE_EXCEPTION(oauth_check_valid_signature_header_2());
 
   } catch (const std::exception &e) {
     std::cerr << "EXCEPTION: " << e.what() << std::endl;