Skip to content

Commit

Permalink
When returning OAuth unauthorized failures, include more detail about…
Browse files Browse the repository at this point in the history
… why the request was rejected.
  • Loading branch information
zerebubuth committed Oct 24, 2016
1 parent 7ba233c commit 54c597a
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 22 deletions.
5 changes: 4 additions & 1 deletion include/cgimap/oauth.hpp
Expand Up @@ -72,7 +72,10 @@ struct bad_request {};

// something bad about the oauth request, and it looks like it's an invalid or
// replayed message, or one without the relevant permissions.
struct unauthorized {};
struct unauthorized {
explicit unauthorized(const std::string &r) : reason(r) {}
std::string reason;
};

typedef boost::variant<
copacetic,
Expand Down
15 changes: 10 additions & 5 deletions src/oauth.cpp
Expand Up @@ -479,14 +479,16 @@ validity::validity is_valid_signature(
// check the signatures are identical
if (*calculated_signature != *provided_signature) {
// 401 - unauthorized, according to section 3.2.
return validity::unauthorized();
return validity::unauthorized(
"Calculated signature differs from provided.");
}

boost::optional<std::string>
token = find_auth_header_value("oauth_token", req);
if (!token) {
// 401 - unauthorized, according to section 3.2.
return validity::unauthorized();
return validity::unauthorized(
"No oauth_token Authorization parameter found.");
}

if (signature_method(req) != std::string("PLAINTEXT")) {
Expand All @@ -497,7 +499,8 @@ validity::validity is_valid_signature(

if (!nonce || !timestamp_str) {
// 401 - unauthorized, according to section 3.2.
return validity::unauthorized();
return validity::unauthorized(
"Both nonce and timestamp must be provided.");
}

// the timestamp MUST be an integer (section 8).
Expand All @@ -512,7 +515,8 @@ validity::validity is_valid_signature(
// check that the nonce hasn't been used before.
if (!nonces.use_nonce(*nonce, timestamp)) {
// 401 - unauthorized, according to section 3.2.
return validity::unauthorized();
return validity::unauthorized(
"Nonce has been used before.");
}

// TODO: reject stale timestamps?
Expand All @@ -522,7 +526,8 @@ validity::validity is_valid_signature(
if (!tokens.allow_read_api(*token)) {
// the signature is okay, but the token isn't authorized for API access,
// so probably best to return a 401 - unauthorized.
return validity::unauthorized();
return validity::unauthorized(
"The user token does not allow read API access.");
}

boost::optional<std::string>
Expand Down
31 changes: 16 additions & 15 deletions src/process_request.cpp
Expand Up @@ -238,11 +238,18 @@ std::string get_oauth_token::operator()<oauth::validity::copacetic>(
return c.token;
}

struct oauth_status_code : public boost::static_visitor<int> {
int operator()(const oauth::validity::copacetic &) const { return 200; }
int operator()(const oauth::validity::not_signed &) const { return 200; }
int operator()(const oauth::validity::bad_request &) const { return 400; }
int operator()(const oauth::validity::unauthorized &) const { return 401; }
struct oauth_status_response : public boost::static_visitor<void> {
void operator()(const oauth::validity::copacetic &) const {}
void operator()(const oauth::validity::not_signed &) const {}
void operator()(const oauth::validity::bad_request &) const {
throw http::bad_request("Bad OAuth request.");
}
void operator()(const oauth::validity::unauthorized &u) const {
std::ostringstream message;
message << "Unauthorized OAuth request, because "
<< u.reason;
throw http::unauthorized(message.str());
}
};

} // anonymous namespace
Expand Down Expand Up @@ -282,17 +289,11 @@ void process_request(request &req, rate_limiter &limiter,
}

} else {
int status_code = boost::apply_visitor(oauth_status_code(), oauth_valid);

if (status_code == 400) {
throw http::bad_request("Bad OAuth request.");
boost::apply_visitor(oauth_status_response(), oauth_valid);

} else if (status_code == 401) {
throw http::unauthorized("Unauthorized OAuth request.");

} else {
client_key = addr_prefix + ip;
}
// if we got here then oauth_status_response didn't throw, which means
// the request must have been unsigned.
client_key = addr_prefix + ip;
}
} else {
client_key = addr_prefix + ip;
Expand Down
2 changes: 1 addition & 1 deletion test/test_oauth.cpp
Expand Up @@ -369,7 +369,7 @@ void oauth_check_invalid_signature_header() {

test_secret_store store("dpf43f3p2l4k3l03", "kd94hf93k423kf44",
"nnch734d00sl2jdk", "pfkkdhi9sl3r4s00");
assert_equal(oauth::validity::validity(oauth::validity::unauthorized()),
assert_equal(oauth::validity::validity(oauth::validity::unauthorized("")),
oauth::is_valid_signature(req, store, store, store));
}

Expand Down

0 comments on commit 54c597a

Please sign in to comment.