feat: Automatic delegation sessions#527
Draft
adamspofford-dfinity wants to merge 7 commits intomainfrom
Draft
Conversation
There was a problem hiding this comment.
Pull request overview
Adds automatic, cached “session delegation” support for password-protected PEM identities so users don’t need to re-enter passwords on every identity-based command, with an explicit icp identity login flow and a configurable session duration.
Changes:
- Introduces PEM session delegation caching (key in keyring + delegation chain on disk) and reuses it on subsequent loads.
- Adds
icp settings session-lengthto configure/disable automatic session caching, and makesicp identity loginpublic with PEM support. - Updates CLI docs, changelog, and adds integration tests covering automatic + explicit session creation.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/reference/cli.md | Documents icp identity login and icp settings session-length. |
| crates/icp/src/settings.rs | Adds session_length setting (minutes) with default and serde handling. |
| crates/icp/src/identity/mod.rs | Passes session duration into identity loader; makes password func clonable via Arc. |
| crates/icp/src/identity/key.rs | Implements session delegation load/create/store + best-effort migration/cleanup on rename/delete. |
| crates/icp/src/context/mod.rs | Stores password_func on Context for reuse by commands. |
| crates/icp/src/context/init.rs | Wires password func + session duration into Context/identity loader. |
| crates/icp-cli/tests/identity_tests.rs | Adds end-to-end tests for session reuse and explicit login behavior. |
| crates/icp-cli/src/main.rs | Loads settings to compute default session duration and passes it into context init. |
| crates/icp-cli/src/commands/settings.rs | Adds settings session-length subcommand and value parsing/printing. |
| crates/icp-cli/src/commands/identity/mod.rs | Exposes identity login (removes hidden flag). |
| crates/icp-cli/src/commands/identity/login.rs | Extends identity login to support PEM sessions + duration behavior. |
| CHANGELOG.md | Notes the new password-prompt reduction behavior for password identities. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
adamspofford-dfinity
force-pushed
the
spofford/sudo-mode
branch
from
1dd69d7 to
befd24b
Compare
viviveevee
approved these changes
Apr 24, 2026
|
|
||
| #[derive(Debug, Args)] | ||
| struct SessionLengthArgs { | ||
| /// Set to `<N>m` (e.g. `5m`) or `disabled`. If omitted, prints the current value. |
Contributor
There was a problem hiding this comment.
Inconsistent suffix support with icp identity login --duration. Would be nice to support identical syntax
Comment on lines
+215
to
+216
| && let Ok(delegated) = | ||
| create_pem_session_and_build_identity(dirs, name, &*identity, duration) |
Contributor
There was a problem hiding this comment.
Do you think it could be appropriate to log errors in --verbose mode? I imagine without some form of logging errors it will be very hard to debug any issues with sessions
| /// | ||
| /// Returns `None` on any failure (missing, expired, or keyring unavailable) so the | ||
| /// caller falls back to normal PEM loading. | ||
| fn try_load_pem_session(dirs: LRead<&IdentityPaths>, name: &str) -> Option<Arc<dyn Identity>> { |
Contributor
There was a problem hiding this comment.
Same here w.r.t. logging
| let contents = fs::read(&old_path).context(CopyKeyFileSnafu)?; | ||
| fs::write(&new_path, &contents).context(CopyKeyFileSnafu)?; | ||
|
|
||
| // Best-effort: migrate any cached session delegation. |
Contributor
There was a problem hiding this comment.
I would add a test to show that it can work, even if it's ok if it doesn't
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.
Password-protected identities will generate 'session' delegations by default so you don't have to keep entering your password.
icp identity loginA user setting configures the session duration, with five more minutes tacked on to account for clock drift, or can disable the automatic form of this feature.
Only keyring storage is used, since plaintext would not be secure enough for automation. Keyring errors are not an error in the automatic path; it just uses the identity normally.