Skip to content

test: add proptest suites for issuerd-protocol parsers - #18

Merged
dmandreev merged 1 commit into
issuerd:mainfrom
Furox-Art:test/gfi-issuerd-proptest
Oct 10, 2026
Merged

dmandreev merged 1 commit into
issuerd:mainfrom
Furox-Art:test/gfi-issuerd-proptest

Conversation

@Furox-Art

Copy link
Copy Markdown
Contributor

Summary

Adds property-based (proptest) suites to the issuerd-protocol parsers, closing
#3. Both directions are asserted where meaningful: well-formed input is accepted
and roundtrips, malformed input is rejected with the exact IssuerdError /
OAuth2 error code, and arbitrary input never panics. No new workspace
dependencies — proptest was already a dev-dependency of the crate.

What was added

crates/issuerd-protocol/src/pkce.rs — PKCE (RFC 7636) property tests:

  • s256_challenge_roundtrip_within_length_window — verifier → challenge →
    verify, for arbitrary unreserved-character verifiers; also pins the challenge
    shape (43 base64url chars, unpadded SHA-256).
  • length_guard_rejects_outside_window — outside 43–128 bytes, always
    InvalidRequest("code_verifier must be 43-128 characters"), whatever the
    challenge.
  • wrong_challenge_never_verifies — a mismatched challenge always fails with
    pkce verification failed.
  • non_s256_methods_always_rejected — plain and a missing method are always
    rejected with code_challenge_method must be S256 (S256-only policy, RFC
    7636 §4.3).
  • arbitrary_unicode_verifiers_are_rejected_without_panic — arbitrary Unicode
    (empty, over-long, control chars, multi-byte) is rejected cleanly.

crates/issuerd-protocol/src/token.rs — token-endpoint property tests:

  • grant_type_parsing_roundtrip — accepted ⇔ known spelling; roundtrips
    through as_str, otherwise unsupported grant_type.
  • parse_rejects_missing_or_unsupported_grant_type — arbitrary bodies never
    panic; success implies a known grant_type.
  • malformed_redirect_uri_rejected / valid_redirect_uri_roundtrips —
    rejection with invalid_request naming redirect_uri, and URL roundtrip.
  • empty_params_are_dropped — empty form values are treated as absent for all
    nine documented fields.
  • authorization_details_structural_validation — RFC 9396 §2 structural rule,
    including the invalid_authorization_details OAuth2 error code.
  • token_exchange_validation — RFC 8693: required subject pair, access-token
    URNs for subject/requested type, delegation rejected.
  • grant_required_fields_enforced — per-grant required fields yield
    invalid_grant when omitted.

crates/issuerd-protocol/src/authorization.rs — authorization-endpoint
property tests (the existing no-panic test is kept and generalized to
adversarial values):

  • parse_never_panics_on_adversarial_values, response_type_parsing_is_exhaustive,
    response_mode_parsing_is_exhaustive (incl. the JARM dotted forms),
    prompt_parsing_rejects_unknown_and_mixed_none, claims_must_be_valid_json,
    request_params_are_always_rejected (JAR → request_not_supported),
    authorization_details_structural_validation,
    request_object_claims_conversion_drops_envelope.

Generators are adversarial (empty, over-long, non-ASCII, control characters,
near-boundary lengths) and explicitly size-capped, so the harnesses stay
deterministic.

Verification

  • cargo test -p issuerd-protocol — 219 passed, 0 failed (2 doctests also green).
  • cargo fmt --all -- --check — clean.
  • cargo clippy -p issuerd-protocol --all-targets --all-features -- -D warnings — clean.

The workspace-wide cargo clippy --workspace --all-targets --all-features -- -D warnings
was not run (it builds every crate including the OpenSSL/WebAuthn paths and would
have exceeded the run budget); the crate-scoped run covers every file this PR
touches. CHANGELOG.md has an ## [Unreleased] → ### Added entry, so the
changelog PR gate passes without the no-changelog label.

@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@dmandreev

Copy link
Copy Markdown
Contributor

Thanks for the thorough write-up, @Furox-Art — the suite reads well, and closing the loop on #3 this fast is impressive. Two process items before this can move forward:

  1. CLA: the bot above asked you to sign the Contributor License Agreement. Just post a comment here saying I have read the CLA Document and I hereby sign the CLA and the CLAAssistant check flips green.
  2. Rebase needed: main moved after you branched — the [Unreleased] section of CHANGELOG.md gained an entry (the dotenv provider, feat(admin-api): serve dotenv installation config as text/plain #17), which is why GitHub reports a conflict. Rebase onto current main and keep both entries (yours and the dotenv one). Note that GitHub does not schedule pull_request workflows while a PR has a merge conflict, which is why no CI has run here yet; once you push the rebased branch I'll approve the CI run right away (first-time-contributor gate).

For when CI runs, the gates are: cargo fmt --all -- --check, cargo clippy --workspace --all-targets --all-features -- -D warnings, the full workspace test suite under coverage, and the CodeQL analysis.

@Furox-Art
Furox-Art force-pushed the test/gfi-issuerd-proptest branch from 008794d to 3fe3051 Compare October 10, 2026 07:02
@Furox-Art

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

dmandreev added a commit that referenced this pull request Oct 10, 2026
The contributor signed with the exact phrase, but the CLA Assistant
step was skipped: the comment body carries a trailing newline and the
workflow condition requires exact equality. Recording the existing
public signature comment faithfully.
@dmandreev

Copy link
Copy Markdown
Contributor

recheck

dmandreev added a commit that referenced this pull request Oct 10, 2026
…uality

Comment bodies posted from some clients carry a trailing newline, which
made the exact-equality condition false and silently skipped the CLA
step (job green, signature never recorded). Furox-Art's signature on
PR #18 hit this; it was recorded manually from the public comment.
@dmandreev
dmandreev merged commit 5342dff into issuerd:main Oct 10, 2026
20 of 21 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 10, 2026
@dmandreev

Copy link
Copy Markdown
Contributor

Merged — thank you, @Furox-Art! The suites are a solid addition to the parser hardening story, and the roundtrip assertions in both directions are exactly what #3 was asking for. Welcome aboard, and congratulations on landing it with the full CI battery green (fmt/clippy, proptest run, coverage, CodeQL, the works).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants