refactor(dgw): move the session store out of the credential module - #1893
Open
irvingouj@Devolutions (irvingoujAtDevolution) wants to merge 1 commit into
Open
Conversation
irvingouj@Devolutions (irvingoujAtDevolution)
force-pushed
the
feat/provisioning-store
branch
from
July 29, 2026 17:26
b57acdd to
c7d93d5
Compare
| State(DgwState { | ||
| conf_handle, | ||
| sessions, | ||
| credentials, |
Contributor
Author
There was a problem hiding this comment.
Reasoning:
Provisioning starting to exhibit sementics exceeding more than just credential.
For this PR stack, we will starting to have KDC server as provisioned value, which is by discussion, not part of credential. Hence we repalced it and use more generic term, provisioning as store.
irvingouj@Devolutions (irvingoujAtDevolution)
force-pushed
the
feat/provisioning-store
branch
2 times, most recently
from
July 29, 2026 19:36
fc64469 to
0cdff84
Compare
irvingouj@Devolutions (irvingoujAtDevolution)
changed the title
The store keyed by association-token JTI is not credential-specific: a session can need other things provisioned ahead of the connection, and the next commit adds the first one. Move it to a `provisioning` module and name it for what it holds, leaving `credential` with just the credential types. Pure move and rename — same types, same API, same behavior. Callers follow.
irvingouj@Devolutions (irvingoujAtDevolution)
force-pushed
the
feat/provisioning-store
branch
from
July 30, 2026 02:49
0cdff84 to
197e15a
Compare
irvingouj@Devolutions (irvingoujAtDevolution)
marked this pull request as ready for review
July 30, 2026 15:15
Copilot started reviewing on behalf of
irvingouj@Devolutions (irvingoujAtDevolution)
July 30, 2026 15:16
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Moves credential/token provisioning storage into a dedicated module while preserving existing service behavior.
Changes:
- Adds the
provisioningmodule and relocates store types, errors, and cleanup. - Updates credential injection, preflight, and task wiring.
- Adjusts related documentation and tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
devolutions-gateway/src/service.rs |
Registers the relocated cleanup task. |
devolutions-gateway/src/provisioning.rs |
Defines the provisioning store and cleanup logic. |
devolutions-gateway/src/lib.rs |
Exports the new module. |
devolutions-gateway/src/credential/mod.rs |
Removes storage logic and exposes encryption internally. |
devolutions-gateway/src/credential_injection_kdc.rs |
Migrates consumers and tests to the new store. |
devolutions-gateway/src/api/preflight.rs |
Imports the relocated insertion error. |
Comments suppressed due to low confidence (2)
devolutions-gateway/src/credential/mod.rs:70
- This intra-doc link is unresolved because
ProvisioningStoreis in a sibling module and is not in this module's scope. Qualify the path so the generated API documentation links to the moved method.
/// Passwords are encrypted on write. Hand this directly to [`ProvisioningStore::insert`].
devolutions-gateway/src/credential_injection_kdc.rs:859
- This test comment still points to
credential::cleanup_task, which no longer exists after the move. Update it to the provisioning module so it accurately describes the deterministic substitute used here.
credentials: ProvisioningStore::new(),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Passwords are encrypted and stored as [`AppCredential`] inside the credential store. | ||
| /// This type is never stored directly — hand it to [`CredentialStoreHandle::insert`]. | ||
| /// Passwords are encrypted and stored as [`AppCredential`] inside the provisioning store. | ||
| /// This type is never stored directly — hand it to [`ProvisioningStore::insert`]. |
Comment on lines
489
to
492
| // Snapshot the JTI from the new token so we can invalidate the matching session entry | ||
| // regardless of whether the credential store reports a replacement. `CredentialStore::insert` | ||
| // re-extracts internally; both calls go through the same code path, so an invalid token | ||
| // here will surface as the same `InvalidToken` error downstream. |
| // `credential::cleanup_task`) and `sweep_orphans` has not run yet. A fresh provisioning | ||
| // under the same JTI must drop the stale session regardless of whether | ||
| // `CredentialStoreHandle::insert` reports a replacement, otherwise the next `kdc_for` | ||
| // `ProvisioningStore::insert` reports a replacement, otherwise the next `kdc_for` |
| pub mod middleware; | ||
| pub mod ngrok; | ||
| pub mod plugin_manager; | ||
| pub mod provisioning; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is mechanical, migrate the credentials name to provision, with a some relocation of code to provisioning.rs