Skip to content

Commit 2e66950

Browse files
author
Erwin Jansen
committed
Increase code coverage
- Add a set of unit tests - Fix a set of small bugs
1 parent 9ac81db commit 2e66950

15 files changed

+253
-41
lines changed

‎README.md

+8
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,14 @@ cmake -DCMAKE_BUILD_TYPE=Debug ..
4747
make
4848
```
4949

50+
Running the tests with code coverage:
51+
```
52+
mkdir debug
53+
cd debug
54+
cmake -DCMAKE_BUILD_TYPE=Debug ..
55+
make cov_all_tests
56+
```
57+
5058

5159
### Dependencies in linux
5260

‎src/include/jwt/hmacvalidator.h

-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
*/
3737
class HMACValidator : public MessageSigner {
3838
public:
39-
explicit HMACValidator(const char *algorithm);
4039
explicit HMACValidator(const char *algorithm, const EVP_MD *md, const std::string &key);
4140
virtual ~HMACValidator();
4241

‎src/include/private/clock.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ class IClock {
3737
class UtcClock : public IClock {
3838
public:
3939
uint64_t Now() {
40-
time_t rawtime;
41-
struct tm ptm;
40+
time_t rawtime = 0;
41+
struct tm ptm = {0};
4242
time(&rawtime);
4343
gmtime_r(&rawtime, &ptm);
4444
return mktime(&ptm);

‎src/jwt/jwt.cpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ JWT *JWT::Decode(const char *jws_token, size_t num_jws_token, MessageValidator *
132132
str_ptr dec_header(new char[num_dec_header]);
133133

134134
if (Base64Encode::DecodeUrl(header, num_header, dec_header.get(), &num_dec_header) != 0) {
135+
// This cannot happen, as we have checked for valid characters already..
135136
throw std::logic_error("validated header block has invalid characters");
136137
}
137138

@@ -163,6 +164,7 @@ json_t *JWT::ExtractPayload(const char *payload, size_t num_payload) {
163164
str_ptr dec_payload(new char[num_dec_payload]);
164165

165166
if (Base64Encode::DecodeUrl(payload, num_payload, dec_payload.get(), &num_dec_payload) != 0) {
167+
// This cannot happen, as we have checked for valid characters already..
166168
throw std::logic_error("validated block has base64 error in payload");
167169
}
168170

@@ -171,7 +173,7 @@ json_t *JWT::ExtractPayload(const char *payload, size_t num_payload) {
171173
json_error_t error;
172174
json_t *json = json_loads(dec_payload.get(), JSON_REJECT_DUPLICATES, &error);
173175
if (!json) {
174-
throw TokenFormatError(std::string("header contains invalid json, ") += error.text);
176+
throw TokenFormatError(std::string("payload contains invalid json, ") += error.text);
175177
}
176178
return json;
177179
}
@@ -205,6 +207,7 @@ bool JWT::VerifySignature(json_t *header_claims_, const char *header,
205207
}
206208

207209
if (Base64Encode::DecodeUrl(signature, num_signature, dec_signature, &num_dec_signature)) {
210+
// Shouldn't happen. At this point the token contains valid base64 chars.
208211
throw std::logic_error("validated block has base64 error in signature");
209212
}
210213

‎src/validators/claims/claimvalidator.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ AllClaimValidator::AllClaimValidator(std::vector<ClaimValidator*> validators) :
3838

3939
bool AllClaimValidator::IsValid(const json_t *claimset) const {
4040
for (auto validator : validators_) {
41-
if (!validator->IsValid(claimset))
42-
return false;
41+
validator->IsValid(claimset);
4342
}
4443
return true;
4544
}

‎src/validators/claims/claimvalidatorfactory.cpp

-3
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,6 @@ ClaimValidator *ClaimValidatorFactory::Build(json_t *json) {
8282
} else if (json_object_get(json, "optional")) {
8383
json_t *val = json_object_get(json, "optional");
8484
ClaimValidator *inner = Build(val);
85-
if (inner->property() == NULL) {
86-
throw std::logic_error("optional property not defined for inner claim");
87-
}
8885
constructed = new OptionalClaimValidator(inner);
8986
}
9087
} catch (std::exception &le) {

‎src/validators/hmacvalidator.cpp

-3
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,6 @@
2626
#include <sstream>
2727
#include <string>
2828

29-
HMACValidator::HMACValidator(const char *algorithm) : algorithm_(algorithm), key_size_(0) {
30-
}
31-
3229
HMACValidator::HMACValidator(const char *algorithm,
3330
const EVP_MD *md, const std::string &key) :
3431
md_(md), algorithm_(algorithm), key_size_(md->md_size), key_(key) {

‎src/validators/messagevalidatorfactory.cpp

+14-13
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,13 @@ MessageSigner* MessageValidatorFactory::BuildSigner(std::string fromJson) {
7272
constructed.reset(new HS512Validator(ParseSecret("secret", json_object_get(json, "HS512"))));
7373
} else if (json_object_get(json, "RS256")) {
7474
constructed.reset(new RS256Validator(ParseSecret("public", json_object_get(json, "RS256")),
75-
ParseSecret("public", json_object_get(json, "RS256"))));
75+
ParseSecret("private", json_object_get(json, "RS256"))));
7676
} else if (json_object_get(json, "RS384")) {
77-
constructed.reset(new RS384Validator(ParseSecret("public", json_object_get(json, "RS256")),
78-
ParseSecret("public", json_object_get(json, "RS384"))));
77+
constructed.reset(new RS384Validator(ParseSecret("public", json_object_get(json, "RS384")),
78+
ParseSecret("private", json_object_get(json, "RS384"))));
7979
} else if (json_object_get(json, "RS512")) {
80-
constructed.reset(new RS512Validator(ParseSecret("public", json_object_get(json, "RS256")),
81-
ParseSecret("public", json_object_get(json, "RS512"))));
80+
constructed.reset(new RS512Validator(ParseSecret("public", json_object_get(json, "RS512")),
81+
ParseSecret("private", json_object_get(json, "RS512"))));
8282
}
8383

8484
if (constructed.get() == nullptr) {
@@ -141,13 +141,10 @@ MessageValidator *MessageValidatorFactory::Build(json_t *json) {
141141

142142
if (!constructed.get()) {
143143
char *fail = json_dumps(json, 0);
144-
if (fail) {
145-
std::ostringstream msg;
146-
msg << "Missing property at: " << fail;
147-
free(fail);
148-
throw std::logic_error(msg.str());
149-
}
150-
throw std::logic_error("Missing property");
144+
std::ostringstream msg;
145+
msg << "Missing validator property at: " << fail;
146+
free(fail);
147+
throw std::logic_error(msg.str());
151148
}
152149

153150
build_.push_back(constructed.get());
@@ -202,7 +199,11 @@ MessageValidator *MessageValidatorFactory::BuildKid(KidValidator *kid, json_t *k
202199
std::string MessageValidatorFactory::ParseSecret(const char *property, json_t *object) {
203200
json_t *secret = json_object_get(object, property);
204201
if (!secret) {
205-
throw std::logic_error("There is no secret!");
202+
std::ostringstream msg;
203+
char* fail = object ? json_dumps(object, 0) : NULL;
204+
msg << "parsing secret, property: " << property << " is missing from: " << fail;
205+
free(fail);
206+
throw std::logic_error(msg.str());
206207
}
207208

208209
if (json_is_string(secret)) {

‎src/validators/rsavalidator.cpp

+6-8
Original file line numberDiff line numberDiff line change
@@ -124,16 +124,14 @@ std::string RSAValidator::toJson() const {
124124

125125
if (private_key_) {
126126
msg << ", ";
127+
BIO *out = BIO_new(BIO_s_mem());
128+
PEM_write_bio_PrivateKey(out, private_key_, NULL, NULL, 0, 0, NULL);
129+
uint64_t len = BIO_get_mem_data(out, &key);
130+
std::string privkey = std::string(key, len);
131+
msg << "\"private\" : \"" << std::regex_replace(privkey, newline, "\\n") << "\"";
132+
BIO_free(out);
127133
}
128134
}
129-
if (private_key_) {
130-
BIO *out = BIO_new(BIO_s_mem());
131-
PEM_write_bio_PUBKEY(out, private_key_);
132-
uint64_t len = BIO_get_mem_data(out, &key);
133-
std::string privkey = std::string(key, len);
134-
msg << "\"private\" : \"" << std::regex_replace(privkey, newline, "\\n") << "\"";
135-
BIO_free(out);
136-
}
137135

138136
msg << "} }";
139137
return msg.str();

‎test/CMakeLists.txt

+1-4
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,4 @@ ENDIF(ENABLE_GPERF_TOOLS MATCHES "ON")
3434
ADD_EXECUTABLE (all_tests all.cpp)
3535
SET_PROPERTY(TARGET all_tests PROPERTY CXX_STANDARD 11)
3636
TARGET_LINK_LIBRARIES (all_tests jwt gtest_main)
37-
38-
IF(CMAKE_BUILD_TYPE MATCHES DEBUG)
39-
SETUP_TARGET_FOR_COVERAGE("cov_all_tests" all_tests coverage)
40-
ENDIF(CMAKE_BUILD_TYPE MATCHES DEBUG)
37+
SETUP_TARGET_FOR_COVERAGE("cov_all_tests" all_tests coverage)

‎test/token/token_test.cpp

+39
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,17 @@ TEST_F(TokenTest, bad_format_tokens) {
4343
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ");
4444
BadHeader("foo");
4545
BadHeader("......");
46+
47+
// Bad JSON header
48+
BadHeader("eyB7IGZvbyB9."
49+
"eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWV9."
50+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ");
51+
52+
// Bad JSON payload
53+
BadHeader("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9."
54+
"eyB7IGZvbyB9."
55+
"eyB7IGZvbyB9");
56+
4657
}
4758

4859
TEST_F(TokenTest, valid_hs256) {
@@ -73,13 +84,40 @@ TEST_F(TokenTest, encoded_token_has_duplicates) {
7384
ASSERT_THROW(JWT::Decode(token, &validator_, &lst_), InvalidTokenError);
7485
}
7586

87+
TEST_F(TokenTest, token_with_large_wrong_signature) {
88+
89+
std::string token =
90+
"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9."
91+
"eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWV9."
92+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQTJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ"
93+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQTJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ"
94+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQTJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ"
95+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQTJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ"
96+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQTJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ"
97+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQTJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ"
98+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQTJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ"
99+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQTJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ"
100+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQTJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ"
101+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQTJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ"
102+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQTJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ"
103+
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQTJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ";
104+
ASSERT_THROW(JWT::Decode(token, &validator_, &lst_), InvalidTokenError);
105+
}
106+
76107
TEST_F(TokenTest, encoded_token_missing_alg) {
77108
std::string noAlg = "eyJmb28iOiJIUzI1NiJ9."
78109
"eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWV9."
79110
"TJVA95OrM7E2cBab30RMHrHDcEfxjoYZgeFONFh7HgQ";
80111
ASSERT_THROW(JWT::Decode(noAlg, &validator_, &lst_), InvalidTokenError);
81112
}
82113

114+
TEST_F(TokenTest, bad_alg) {
115+
std::string badAlg = "eyJhbGciOiJCTEEiLCJ0eXAiOiJKV1QifQ."
116+
"eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWV9."
117+
"Pmjzsg4UPL6vKXK2pFGqH60qudcr4YHQ1e9Ddsl_ONo";
118+
ASSERT_THROW(JWT::Decode(badAlg, &validator_, &lst_), InvalidSignatureError);
119+
}
120+
83121
TEST_F(TokenTest, encoded_token_has_custom_header) {
84122
json_ptr json(json_pack("{ss, ss, sb}", "sub", "1234567890", "name", "John Doe", "admin", true));
85123
json_ptr header(json_pack("{ss}", "foo", "bar"));
@@ -107,6 +145,7 @@ TEST_F(TokenTest, just_parse) {
107145
EXPECT_TRUE(token.get() != NULL);
108146
}
109147

148+
110149
TEST_F(TokenTest, parse_and_validate_bad_signature) {
111150
HS256Validator hs256("Not the right secret");
112151
ASSERT_THROW(JWT::Decode(validToken_, &hs256), InvalidSignatureError);

‎test/validators/claim_validators_factory_test.cpp

+12
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,23 @@ TEST(parse_test, any_not_a_list) {
3333
ASSERT_THROW(ClaimValidatorFactory::Build(json), std::logic_error);
3434
}
3535

36+
TEST(parse_test, iss_not_a_stringlist) {
37+
std::string json = "{ \"iss\" : [ { \"foo\" : \"bar\" } , \"zz\" ]}";
38+
ASSERT_THROW(ClaimValidatorFactory::Build(json), std::logic_error);
39+
}
40+
3641
TEST(parse_test, optional_exp) {
3742
std::string json = "{ \"optional\" : { \"exp\" : { \"leeway\" : 32} } }";
3843
claim_ptr valid(ClaimValidatorFactory::Build(json));
3944
EXPECT_NE(nullptr, valid.get());
4045
EXPECT_STREQ("exp", valid->property());
46+
json_ptr iat(json_pack("{si}", "iat", 9));
47+
EXPECT_TRUE(valid->IsValid(iat.get()));
48+
}
49+
50+
TEST(parse_test, optional_bad) {
51+
std::string json = "{ \"optional\" : { \"foo\" : \"bar\" } }";
52+
ASSERT_THROW(ClaimValidatorFactory::Build(json), std::logic_error);
4153
}
4254

4355
TEST(parse_test, optional_empty) {

‎test/validators/claim_validators_test.cpp

+25
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,29 @@
55
#include "jwt/claimvalidator.h"
66
#include "jwt/listclaimvalidator.h"
77
#include "jwt/timevalidator.h"
8+
#include <unistd.h>
89

910
// Test for the various validators.
11+
TEST(clock, clock_test) {
12+
UtcClock clk;
13+
uint64_t now = clk.Now();
14+
sleep(1);
15+
uint64_t later = clk.Now();
16+
EXPECT_LT(now, later);
17+
}
1018

1119
TEST(iat_test, before) {
1220
json_ptr json(json_pack("{si}", "iat", 9));
1321
IatValidator iat(0, &fakeClock);
1422
EXPECT_TRUE(iat.IsValid(json.get()));
1523
}
1624

25+
TEST(iat_test, negative) {
26+
json_ptr json(json_pack("{si}", "iat", -9));
27+
IatValidator iat(0, &fakeClock);
28+
ASSERT_THROW(iat.IsValid(json.get()), InvalidClaimError);
29+
}
30+
1731
TEST(iat_test, wrong_type) {
1832
json_ptr json(json_pack("{si}", "iat", "foo"));
1933
IatValidator iat(0, &fakeClock);
@@ -207,6 +221,17 @@ TEST(any_test, no_aud_no_sub) {
207221
ASSERT_THROW(any.IsValid(json.get()), InvalidClaimError);
208222
}
209223

224+
TEST(all_test, has_sub_no_aud) {
225+
json_ptr json(json_pack("{ss}", "sub", "foo"));
226+
AudValidator aud(accepted, 2);
227+
SubValidator sub(accepted, 2);
228+
229+
ClaimValidator *claims[] = {&aud, &sub};
230+
AllClaimValidator all(claims, 2);
231+
232+
ASSERT_THROW(all.IsValid(json.get()), InvalidClaimError);
233+
}
234+
210235
TEST(all_test, no_aud) {
211236
json_ptr json(json_pack("{ss}", "sub", "foo"));
212237
AudValidator aud(accepted, 2);

0 commit comments

Comments
 (0)