Skip to content

Commit e257ee9

Browse files
etrclaude
etr
and
claude
committed
TASK-062: address review feedback — HA1 resources + wire-format coverage
* test/integ/authentication.cpp: migrate digest_ha1_md5_resource and digest_ha1_sha256_resource to emit digest_challenge{} (with algorithm=MD5 / SHA256) so curl --digest can complete the handshake; HA1 round-trip tests now assert 200 SUCCESS instead of the static 401 FAIL. Refresh the file-top tracking note and per-test comments to reflect the new state; flag the still-uncovered NONCE_STALE re-challenge branch as a follow-up (needs real-time nonce expiry). * test/integ/digest_challenge_format_test.cpp: add wire-format tests for the SHA-256 algorithm token and for the opaque/domain fields. * test/unit/http_response_digest_factory_test.cpp: drop assertions that reached into detail::digest_challenge_body via the friend hook (algorithm/opaque/domain/body-size); pin only body_kind discrimination at the unit level. Wire-format coverage of those fields lives in the integ test above, per the test-quality-reviewer recommendation (iter1-3). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 86195d6 commit e257ee9

3 files changed

Lines changed: 273 additions & 93 deletions

File tree

‎test/integ/authentication.cpp‎

Lines changed: 94 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -41,23 +41,28 @@
4141

4242
#define MY_OPAQUE "11733b200778ce33060f31c9af70a870ba96ddd4"
4343

44-
// v2-digest tracking note (consolidated):
45-
// Under v2, libhttpserver only emits the static 401 Digest challenge; the
46-
// nonce/opaque state machine is not driven, so check_digest_auth_check[_digest]()
47-
// is never reached and get_digested_user() always returns an empty view.
48-
// As a result the following tests are all observationally indistinguishable
49-
// from the canonical `digest_auth` case (both correct- and wrong-pass arms
50-
// see the same 401 + "FAIL"):
44+
// v2-digest tracking note (updated after TASK-062):
45+
// After TASK-062, digest_resource and the HA1 resources (digest_ha1_md5_resource,
46+
// digest_ha1_sha256_resource) all emit full RFC-7616 challenges via
47+
// http_response::unauthorized(digest_challenge{...}). The nonce/opaque
48+
// state machine is driven by MHD_queue_auth_required_response3, and the
49+
// following tests now complete the full digest handshake and assert 200 SUCCESS:
50+
// - digest_auth, digest_auth_with_ha1_md5, digest_auth_with_ha1_sha256
51+
//
52+
// The following tests still assert the 401 FAIL contract (wrong credentials or
53+
// no auth provided -- the handshake completes but auth fails):
5154
// - digest_auth_wrong_pass
52-
// - digest_auth_with_ha1_md5 / *_wrong_pass
53-
// - digest_auth_with_ha1_sha256 / *_wrong_pass
54-
// - digest_user_cache_with_auth
55-
// They are retained as pins for the static-challenge contract and become
56-
// meaningful again only when full v2 Digest support (MHD nonce/opaque state
57-
// machine) lands. At that point the wrong-pass arms should assert a distinct
58-
// 403/401-with-stale, the HA1-MD5/SHA-256 arms should validate the chosen
59-
// algorithm, and digest_user_cache_with_auth should exercise the cache-hit
60-
// path. See PRD §digest-auth for the follow-up.
55+
// - digest_auth_with_ha1_md5_wrong_pass, *_sha256_wrong_pass
56+
//
57+
// Remaining gap (signal_stale re-challenge path):
58+
// The NONCE_STALE branch inside digest_resource/digest_ha1_*_resource
59+
// (signal_stale = true) cannot be reliably triggered in CI because MHD's
60+
// nonce expiry requires a real time delay and concurrent replay requests.
61+
// This gap is documented in TASK-062 test-quality-reviewer iter1-2; it will
62+
// be covered when full nonce-expiry testing infrastructure lands.
63+
//
64+
// Also pending: digest_user_cache_with_auth exercises the cache-hit path only
65+
// once the digest handshake completes (see resource definition below).
6166

6267
using std::shared_ptr;
6368
using httpserver::webserver;
@@ -213,51 +218,77 @@ static const unsigned char PRECOMPUTED_HA1_SHA256[32] = {
213218
0x20, 0x7e, 0x02, 0xd7, 0xc4, 0xbd, 0x8a, 0x05
214219
};
215220

221+
// TASK-062: migrated to digest_challenge so the nonce/opaque handshake can
222+
// complete and check_digest_auth_digest() is exercised. Previously used the
223+
// legacy string overload which emitted a static challenge with no nonce,
224+
// making the handshake impossible and the algorithm parameter meaningless.
216225
class digest_ha1_md5_resource : public http_resource {
217226
public:
218227
http_response render_get(const http_request& req) {
219228
using httpserver::http::http_utils;
229+
using httpserver::digest_challenge;
220230
if (req.get_digested_user() == "") {
221-
return http_response::unauthorized("Digest", "examplerealm", "FAIL");
231+
return http_response::unauthorized(
232+
digest_challenge{.realm = "examplerealm",
233+
.algorithm = http_utils::digest_algorithm::MD5,
234+
.body = "FAIL"});
222235
}
223236
auto result = req.check_digest_auth_digest("examplerealm", PRECOMPUTED_HA1_MD5,
224237
http_utils::md5_digest_size, 300, 0,
225238
http_utils::digest_algorithm::MD5);
226239
if (result == http_utils::digest_auth_result::NONCE_STALE) {
227-
return http_response::unauthorized("Digest", "examplerealm", "FAIL");
240+
return http_response::unauthorized(
241+
digest_challenge{.realm = "examplerealm",
242+
.algorithm = http_utils::digest_algorithm::MD5,
243+
.signal_stale = true,
244+
.body = "FAIL"});
228245
} else if (result != http_utils::digest_auth_result::OK) {
229-
return http_response::unauthorized("Digest", "examplerealm", "FAIL");
246+
return http_response::unauthorized(
247+
digest_challenge{.realm = "examplerealm",
248+
.algorithm = http_utils::digest_algorithm::MD5,
249+
.body = "FAIL"});
230250
}
231251
return http_response::string("SUCCESS");
232252
}
233253
};
234254

255+
// TASK-062: migrated to digest_challenge with algorithm=SHA256 so the
256+
// nonce/opaque handshake can complete and check_digest_auth_digest() is
257+
// exercised with the SHA-256 algorithm. Previously used the legacy string
258+
// overload which made the algorithm parameter meaningless.
235259
class digest_ha1_sha256_resource : public http_resource {
236260
public:
237261
http_response render_get(const http_request& req) {
238262
using httpserver::http::http_utils;
263+
using httpserver::digest_challenge;
239264
if (req.get_digested_user() == "") {
240-
return http_response::unauthorized("Digest", "examplerealm", "FAIL");
265+
return http_response::unauthorized(
266+
digest_challenge{.realm = "examplerealm",
267+
.algorithm = http_utils::digest_algorithm::SHA256,
268+
.body = "FAIL"});
241269
}
242270
auto result = req.check_digest_auth_digest("examplerealm", PRECOMPUTED_HA1_SHA256,
243271
http_utils::sha256_digest_size, 300, 0,
244272
http_utils::digest_algorithm::SHA256);
245273
if (result == http_utils::digest_auth_result::NONCE_STALE) {
246-
return http_response::unauthorized("Digest", "examplerealm", "FAIL");
274+
return http_response::unauthorized(
275+
digest_challenge{.realm = "examplerealm",
276+
.algorithm = http_utils::digest_algorithm::SHA256,
277+
.signal_stale = true,
278+
.body = "FAIL"});
247279
} else if (result != http_utils::digest_auth_result::OK) {
248-
return http_response::unauthorized("Digest", "examplerealm", "FAIL");
280+
return http_response::unauthorized(
281+
digest_challenge{.realm = "examplerealm",
282+
.algorithm = http_utils::digest_algorithm::SHA256,
283+
.body = "FAIL"});
249284
}
250285
return http_response::string("SUCCESS");
251286
}
252287
};
253288

254-
// TASK-013 §2 / §10: full digest-auth round-trip is a v1-only behaviour.
255-
// The v1 `digest_auth_fail_response::enqueue_response` path called
256-
// MHD_queue_auth_required_response3 to drive libmicrohttpd's nonce/opaque
257-
// state machine; v2's `unauthorized("Digest", ...)` only emits a static
258-
// WWW-Authenticate challenge (see http_response.hpp:175-180 doxygen).
259-
// These tests now assert the v2 contract: the resource emits FAIL on the
260-
// initial request because curl's nonce roundtrip cannot complete.
289+
// TASK-062: digest_resource uses http_response::unauthorized(digest_challenge{...})
290+
// which routes through MHD_queue_auth_required_response3 so curl --digest can
291+
// complete the full RFC-7616 nonce/opaque handshake and receive 200 SUCCESS.
261292
LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth)
262293
webserver ws{create_webserver(PORT)
263294
.digest_auth_random("myrandom")
@@ -305,7 +336,18 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth)
305336
ws.stop();
306337
LT_END_AUTO_TEST(digest_auth)
307338

308-
// See v2-digest tracking note at top of file.
339+
// Wrong-password case: curl completes the nonce handshake but check_digest_auth()
340+
// returns NOT_AUTHORIZED, so the resource emits a new 401 challenge (body "FAIL").
341+
//
342+
// TODO(TASK-062 test-quality-reviewer iter1-2): The signal_stale=true re-challenge
343+
// branch in digest_resource::render_get (NONCE_STALE path) is not covered here.
344+
// Triggering NONCE_STALE requires sending a replayed/expired nonce. MHD's nonce
345+
// expiry (nonce_nc_size window) cannot be reliably exhausted in a single-threaded
346+
// CI test without real-time delays or replay injection. Once nonce-expiry
347+
// infrastructure is available, add a test that:
348+
// (a) sends the correct credentials with a replayed/expired nonce,
349+
// (b) asserts status 401 with WWW-Authenticate containing stale=true,
350+
// (c) asserts body is "FAIL".
309351
LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_wrong_pass)
310352
webserver ws{create_webserver(PORT)
311353
.digest_auth_random("myrandom")
@@ -350,7 +392,10 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_wrong_pass)
350392
ws.stop();
351393
LT_END_AUTO_TEST(digest_auth_wrong_pass)
352394

353-
// See v2-digest tracking note at top of file.
395+
// TASK-062: digest_ha1_md5_resource now emits a full RFC-7616 digest_challenge
396+
// with algorithm=MD5 so the nonce/opaque handshake can complete. curl --digest
397+
// authenticates using the precomputed HA1 and the server verifies via
398+
// check_digest_auth_digest(). On success the resource returns 200 SUCCESS.
354399
LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_md5)
355400
webserver ws{create_webserver(PORT)
356401
.digest_auth_random("myrandom")
@@ -387,17 +432,17 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_md5)
387432
res = curl_easy_perform(curl);
388433
LT_ASSERT_EQ(res, 0);
389434
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
390-
// v2 contract: static 401 Digest challenge, no handshake completes.
391-
LT_CHECK_EQ(http_code, 401);
392-
// TASK-013 §2 / §10: v2 digest auth only emits a static challenge — see
393-
// digest_auth test above. Handshake cannot complete; body remains FAIL.
394-
LT_CHECK_EQ(s, "FAIL");
435+
// TASK-062 contract: digest_challenge emits a full RFC-7616 challenge with
436+
// algorithm=MD5; curl --digest completes the handshake. Final response: 200.
437+
LT_CHECK_EQ(http_code, 200);
438+
LT_CHECK_EQ(s, "SUCCESS");
395439
curl_easy_cleanup(curl);
396440

397441
ws.stop();
398442
LT_END_AUTO_TEST(digest_auth_with_ha1_md5)
399443

400-
// See v2-digest tracking note at top of file.
444+
// TASK-062: digest_ha1_md5_resource now uses digest_challenge; curl completes
445+
// the handshake but check_digest_auth_digest() rejects the wrong HA1. 401 FAIL.
401446
LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_md5_wrong_pass)
402447
webserver ws{create_webserver(PORT)
403448
.digest_auth_random("myrandom")
@@ -434,15 +479,18 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_md5_wrong_pass)
434479
res = curl_easy_perform(curl);
435480
LT_ASSERT_EQ(res, 0);
436481
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
437-
// v2 contract: static 401 Digest challenge, no handshake completes.
482+
// TASK-062 contract: handshake completes, but wrong HA1 fails check_digest_auth_digest.
438483
LT_CHECK_EQ(http_code, 401);
439484
LT_CHECK_EQ(s, "FAIL");
440485
curl_easy_cleanup(curl);
441486

442487
ws.stop();
443488
LT_END_AUTO_TEST(digest_auth_with_ha1_md5_wrong_pass)
444489

445-
// See v2-digest tracking note at top of file.
490+
// TASK-062: digest_ha1_sha256_resource now emits a full RFC-7616 digest_challenge
491+
// with algorithm=SHA256 so the nonce/opaque handshake can complete. curl --digest
492+
// authenticates using the precomputed HA1 and the server verifies via
493+
// check_digest_auth_digest() with SHA-256. On success the resource returns 200 SUCCESS.
446494
LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_sha256)
447495
webserver ws{create_webserver(PORT)
448496
.digest_auth_random("myrandom")
@@ -479,17 +527,17 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_sha256)
479527
res = curl_easy_perform(curl);
480528
LT_ASSERT_EQ(res, 0);
481529
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
482-
// v2 contract: static 401 Digest challenge, no handshake completes.
483-
LT_CHECK_EQ(http_code, 401);
484-
// TASK-013 §2 / §10: v2 digest auth only emits a static challenge — see
485-
// digest_auth test above. Handshake cannot complete; body remains FAIL.
486-
LT_CHECK_EQ(s, "FAIL");
530+
// TASK-062 contract: digest_challenge emits a full RFC-7616 challenge with
531+
// algorithm=SHA256; curl --digest completes the handshake. Final response: 200.
532+
LT_CHECK_EQ(http_code, 200);
533+
LT_CHECK_EQ(s, "SUCCESS");
487534
curl_easy_cleanup(curl);
488535

489536
ws.stop();
490537
LT_END_AUTO_TEST(digest_auth_with_ha1_sha256)
491538

492-
// See v2-digest tracking note at top of file.
539+
// TASK-062: digest_ha1_sha256_resource now uses digest_challenge; curl completes
540+
// the handshake but check_digest_auth_digest() rejects the wrong HA1. 401 FAIL.
493541
LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_sha256_wrong_pass)
494542
webserver ws{create_webserver(PORT)
495543
.digest_auth_random("myrandom")
@@ -526,7 +574,7 @@ LT_BEGIN_AUTO_TEST(authentication_suite, digest_auth_with_ha1_sha256_wrong_pass)
526574
res = curl_easy_perform(curl);
527575
LT_ASSERT_EQ(res, 0);
528576
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
529-
// v2 contract: static 401 Digest challenge, no handshake completes.
577+
// TASK-062 contract: handshake completes, but wrong HA1 fails check_digest_auth_digest.
530578
LT_CHECK_EQ(http_code, 401);
531579
LT_CHECK_EQ(s, "FAIL");
532580
curl_easy_cleanup(curl);

‎test/integ/digest_challenge_format_test.cpp‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,31 @@ class challenge_resource : public http_resource {
111111
.body = "access denied"});
112112
}
113113
};
114+
115+
// Resource that emits a SHA-256 Digest challenge (RFC 7616 §3.3 algorithm
116+
// coverage — wire-observable, unlike the unit-test friend-hook assertions).
117+
class sha256_challenge_resource : public http_resource {
118+
public:
119+
http_response render_get(const http_request& /*req*/) override {
120+
return http_response::unauthorized(
121+
digest_challenge{.realm = "sha256realm@host.com",
122+
.algorithm = http_utils::digest_algorithm::SHA256,
123+
.body = "denied"});
124+
}
125+
};
126+
127+
// Resource that emits a challenge with an explicit opaque and domain so we
128+
// can verify those fields appear in the wire challenge (RFC 7616 §3.3).
129+
class opaque_domain_challenge_resource : public http_resource {
130+
public:
131+
http_response render_get(const http_request& /*req*/) override {
132+
return http_response::unauthorized(
133+
digest_challenge{.realm = "odrealm@host.com",
134+
.opaque = "deadbeef",
135+
.domain = "/protected",
136+
.body = "denied"});
137+
}
138+
};
114139
#endif // HAVE_DAUTH
115140

116141
} // namespace
@@ -176,6 +201,104 @@ LT_BEGIN_AUTO_TEST(digest_challenge_format_suite,
176201
ws.stop();
177202
LT_END_AUTO_TEST(rfc7616_challenge_carries_required_fields)
178203

204+
// RFC 7616 §3.3 algorithm coverage: when digest_challenge.algorithm is set
205+
// to SHA256 the WWW-Authenticate header on the wire must contain
206+
// "SHA-256" (the canonical token per RFC 7616 §3.3, distinct from MD5).
207+
// This is a wire-format assertion -- the only reliable place to verify it,
208+
// since the dispatch path (MHD_queue_auth_required_response3) owns the
209+
// algorithm token serialisation.
210+
LT_BEGIN_AUTO_TEST(digest_challenge_format_suite,
211+
rfc7616_challenge_sha256_algorithm_in_header)
212+
webserver ws{create_webserver(PORT)
213+
.digest_auth_random("myrandom")
214+
.nonce_nc_size(300)};
215+
216+
auto resource = std::make_shared<sha256_challenge_resource>();
217+
ws.register_path("sha256", resource);
218+
ws.start(false);
219+
220+
#if defined(_WINDOWS)
221+
curl_global_init(CURL_GLOBAL_WIN32);
222+
#else
223+
curl_global_init(CURL_GLOBAL_ALL);
224+
#endif
225+
226+
std::string body;
227+
std::string headers;
228+
CURL* curl = curl_easy_init();
229+
CURLcode res;
230+
long http_code = 0; // NOLINT(runtime/int)
231+
curl_easy_setopt(curl, CURLOPT_URL,
232+
"localhost:" PORT_STRING "/sha256");
233+
curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L);
234+
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc);
235+
curl_easy_setopt(curl, CURLOPT_WRITEDATA, &body);
236+
curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, headerfunc);
237+
curl_easy_setopt(curl, CURLOPT_HEADERDATA, &headers);
238+
curl_easy_setopt(curl, CURLOPT_TIMEOUT, 150L);
239+
curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 150L);
240+
curl_easy_setopt(curl, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1);
241+
curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1);
242+
res = curl_easy_perform(curl);
243+
LT_ASSERT_EQ(res, 0);
244+
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
245+
LT_CHECK_EQ(http_code, 401);
246+
LT_CHECK_EQ(body, std::string("denied"));
247+
// RFC 7616 §3.3: the algorithm token for SHA-256 is "SHA-256".
248+
LT_CHECK_EQ(ci_contains(headers, "SHA-256"), true);
249+
250+
curl_easy_cleanup(curl);
251+
ws.stop();
252+
LT_END_AUTO_TEST(rfc7616_challenge_sha256_algorithm_in_header)
253+
254+
// Wire-format round-trip for the opaque and domain fields. The unit tests
255+
// formerly asserted these through the internal get_params() friend hook;
256+
// wire-format assertions here are the appropriate place per the
257+
// test-quality-reviewer recommendation.
258+
LT_BEGIN_AUTO_TEST(digest_challenge_format_suite,
259+
rfc7616_challenge_opaque_and_domain_in_header)
260+
webserver ws{create_webserver(PORT)
261+
.digest_auth_random("myrandom")
262+
.nonce_nc_size(300)};
263+
264+
auto resource = std::make_shared<opaque_domain_challenge_resource>();
265+
ws.register_path("od", resource);
266+
ws.start(false);
267+
268+
#if defined(_WINDOWS)
269+
curl_global_init(CURL_GLOBAL_WIN32);
270+
#else
271+
curl_global_init(CURL_GLOBAL_ALL);
272+
#endif
273+
274+
std::string body;
275+
std::string headers;
276+
CURL* curl = curl_easy_init();
277+
CURLcode res;
278+
long http_code = 0; // NOLINT(runtime/int)
279+
curl_easy_setopt(curl, CURLOPT_URL, "localhost:" PORT_STRING "/od");
280+
curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L);
281+
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc);
282+
curl_easy_setopt(curl, CURLOPT_WRITEDATA, &body);
283+
curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, headerfunc);
284+
curl_easy_setopt(curl, CURLOPT_HEADERDATA, &headers);
285+
curl_easy_setopt(curl, CURLOPT_TIMEOUT, 150L);
286+
curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 150L);
287+
curl_easy_setopt(curl, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1);
288+
curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1);
289+
res = curl_easy_perform(curl);
290+
LT_ASSERT_EQ(res, 0);
291+
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
292+
LT_CHECK_EQ(http_code, 401);
293+
// The opaque value we set must appear verbatim in the challenge header.
294+
LT_CHECK_EQ(ci_contains(headers, "deadbeef"), true);
295+
// The domain URI must appear in the challenge header.
296+
LT_CHECK_EQ(ci_contains(headers, "/protected"), true);
297+
298+
curl_easy_cleanup(curl);
299+
ws.stop();
300+
LT_END_AUTO_TEST(rfc7616_challenge_opaque_and_domain_in_header)
301+
179302
#else // !HAVE_DAUTH
180303

181304
LT_BEGIN_AUTO_TEST(digest_challenge_format_suite, dauth_disabled_noop)

0 commit comments

Comments
 (0)