Skip to content

feat(pkg): configurable email attribute type for API-key signing identities#244

Merged
rubenhensen merged 2 commits into
mainfrom
feat/email-attribute
Jul 16, 2026
Merged

feat(pkg): configurable email attribute type for API-key signing identities#244
rubenhensen merged 2 commits into
mainfrom
feat/email-attribute

Conversation

@rubenhensen

Copy link
Copy Markdown
Contributor

Part 1 of #236 (Phase 2 of EPIC #201) — the PKG leg.

Problem: the API-key signing identity hardcodes pbdf.sidn-pbdf.email.email (middleware/auth.rs). No test environment can issue pbdf.* credentials (issuer keys are production secrets), which is the single blocker to driving the real SDKs through the e2e harness — and it makes any future scheme migration a multi-repo coordinated break.

Change: --email-attribute / PKG_EMAIL_ATTRIBUTE, default pbdf.sidn-pbdf.email.emailproduction deployments are unaffected unless they set it. Threaded as Auth::with_email_attribute(...) into the API-key auth path; the other identity attributes (kvk/phone/orgName) stay as-is, out of scope.

Why this unblocks the harness: the public irma-demo scheme mirrors sidn-pbdf.email with published issuer private keys (verified: privacybydesign/irma-demo-schememanager/sidn-pbdf/{Issues/email,PrivateKeys/0.xml}), so the e2e stack can set PKG_EMAIL_ATTRIBUTE=irma-demo.sidn-pbdf.email.email, issue itself real email credentials, and run the identical code path as production.

Regression test: test_api_key_signing_uses_configured_email_attribute — signing identity carries the configured type + email value. Suite: 59 + 2 green, clippy clean.

Companion PRs (same issue): cryptify (email_attribute config for the finalize sender check) and pg-js (emailAttribute option for recipient/policy builders) — coming next; the issue closes when all three land.

…tities

Part 1 of #236 (the PKG leg). The attribute type carrying the email in
API-key signing identities was hardcoded to pbdf.sidn-pbdf.email.email; test
environments cannot issue pbdf credentials, so no test setup could exercise
the real signing/decrypt paths end to end.

--email-attribute (env PKG_EMAIL_ATTRIBUTE) now configures it, defaulting to
the production value — production deployments are unaffected. The e2e stack
will set irma-demo.sidn-pbdf.email.email (the public demo scheme mirrors the
email credential with published issuer keys), giving the harness real email
semantics. Companion changes land in cryptify (finalize sender check) and
pg-js (recipient/policy builders).
@dobby-coder

dobby-coder Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Thanks for the detailed writeup, @rubenhensen! I'm on it — kicking off a review of this PR now, checking the --email-attribute/Auth::with_email_attribute threading and the regression test, and I'll follow up with a consolidated review (approve / request changes / comment).

@dobby-coder dobby-coder Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-scoped PKG leg for issue #236. The change is additive — a new optional --email-attribute / PKG_EMAIL_ATTRIBUTE flag defaulting to the current production value, plus a with_email_attribute builder — so production is unaffected and no breaking-change footer is needed. The Rc<str> is threaded through AuthMethods::Key and both API-key auth wrap sites, and the regression test asserts the configured attribute type flows into the signing identity.

Checks: conventional-commit title ✓, regression test present ✓, correctly uses "Part 1 of #236" with no Closes/Fixes keyword so the epic isn't prematurely closed ✓ (verified via closingIssuesReferences), no rule violations found.

One non-blocking nit inline. Approving.

Comment thread pg-pkg/src/opts.rs Outdated
#[clap(
long,
env = "PKG_EMAIL_ATTRIBUTE",
default_value = "pbdf.sidn-pbdf.email.email"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the default email attribute is now duplicated — this clap literal "pbdf.sidn-pbdf.email.email" and the DEFAULT_EMAIL_ATTRIBUTE const in middleware/auth.rs:12. Since clap always supplies this value in production, editing only the const would silently diverge behaviour for direct Auth::new API callers while production stays on the literal. Consider default_value_t = DEFAULT_EMAIL_ATTRIBUTE.to_string() (clap accepts any Display type) to keep a single source of truth. Non-blocking.

@dobby-coder

dobby-coder Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Reviewed and approved — PR #244 is a clean, correctly-scoped PKG leg for #236. It's purely additive (defaults to the current production value), the regression test proves the configured attribute reaches the signing identity, and the "Part 1 of #236" wording correctly avoids closing the epic early.

Left one non-blocking nit inline: the default pbdf.sidn-pbdf.email.email is duplicated between opts.rs:71 and the DEFAULT_EMAIL_ATTRIBUTE const in auth.rs:12 — worth wiring default_value_t to the const so there's a single source of truth. Nothing blocking merge. Looking forward to the cryptify and pg-js companions.

@rubenhensen

Copy link
Copy Markdown
Contributor Author

Nit fixed, /dobby — clap now uses default_value_t = DEFAULT_EMAIL_ATTRIBUTE.to_string(), so the const in middleware/auth.rs is the single source of truth for both direct Auth::new callers and the CLI/env path. Tests still green.

@dobby-coder

dobby-coder Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

On it! I'm reviewing PR #244 now, including this nit fix consolidating on DEFAULT_EMAIL_ATTRIBUTE.to_string() in middleware/auth.rs. I'll follow up with a single consolidated review shortly.

@rubenhensen
rubenhensen merged commit 4920e7b into main Jul 16, 2026
23 checks passed
@dobby-coder

dobby-coder Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Thanks for the fix! Since #244 is already merged there's nothing left to gate, but I took a look anyway for the record: DEFAULT_EMAIL_ATTRIBUTE is now the single source of truth for both the direct Auth::new path and the clap/env path, the regression test covers the configurable attribute, and everything's green (59 unit + 2 contract). No findings from my end — nicely closed out.

rubenhensen added a commit to encryption4all/cryptify that referenced this pull request Jul 17, 2026
…#193)

Part 2 of encryption4all/postguard#236 (the cryptify leg). Finalize required
exactly pbdf.sidn-pbdf.email.email in the sender's signing identity; test
environments cannot issue pbdf credentials, blocking real end-to-end flows.
The `email_attribute` config key (default: the production value, so deployed
configs are unaffected) now names the attribute type carrying the sender's
email. Companion changes: pg-pkg (encryption4all/postguard#244) and pg-js.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant