fix(dgw): address reviewer feedback on in-memory credential encryption#1690
fix(dgw): address reviewer feedback on in-memory credential encryption#1690Benoît Cortier (CBenoit) merged 3 commits intoDGW-326from
Conversation
…docs Co-authored-by: CBenoit <3809077+CBenoit@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. |
| // Validate the token JTI up front so that a bad/missing JTI is reported as | ||
| // InvalidParams (client error) rather than InternalServerError. | ||
| crate::token::extract_jti(&token) | ||
| .map_err(|e| PreflightError::new(PreflightAlertStatus::InvalidParams, format!("{e:#}")))?; | ||
|
|
There was a problem hiding this comment.
Copilot modify the insert function such that it returns an error type that allows us to distinguish between invalid parameters and internal errors (eg; using an enum). This avoids doing the need for knowing the internals and doing the job twice as precheck (that's an anti pattern)
There was a problem hiding this comment.
Done in commit 1da4403. Added a InsertError enum with InvalidToken and Internal variants to credential/mod.rs. Both CredentialStore::insert and CredentialStoreHandle::insert now return Result<_, InsertError>, with JTI extraction failures mapped to InvalidToken and encryption failures to Internal. The pre-check in preflight.rs is replaced by a match on the error variant.
Co-authored-by: CBenoit <3809077+CBenoit@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. |
Fixes three issues flagged during review of the in-memory credential encryption PR.
Changes
rdp_proxy.rs,rd_clean_path.rs): "an the FQDN format" → "in the FQDN format"credential/mod.rs,preflight.rs): Introduced a typedInsertErrorenum onCredentialStoreHandle::insertwithInvalidToken(client-side bad input) andInternal(server-side failure) variants, so callers can distinguish error kinds without needing to inspect error messages or duplicate logicREADME.md): Documentlibsodiumas a required native build dependency under "From sources", with platform-specific install instructions (Linux/macOS/Windows)Issue: DGW-326
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.