Add secure credential storage, sessions, and a launch screen
Login now persists properly rather than through the JSON file the test
harness was using.
dr-plat SecretStore trait plus a Secret Service backend.
Verified against the live GNOME Keyring: store,
retrieve, delete, confirm-gone all round-trip.
Session/SessionStore splits credentials from settings — the app
password goes to the keyring (FR-NC-2), while
server, login, chosen root and format selection are
ordinary config. A test asserts the credential never
appears in the config file.
LaunchModel the launch-screen state machine, testable without a
display server: sign in, approve in browser, choose
folder, tick formats, sign out.
launch.slint the screen itself, in its own file.
Absence of a secrets daemon is an explicit degraded mode, not a silent
fallback to plaintext — the screen says sign-in will not persist rather
than letting the user find out next launch. Android's Keystore backend
fails loudly for the same reason: a no-op store would look like it
worked and then lose the credential.
Two bugs caught by tests rather than by running it:
- fail() after busy() signed the user out, because busy() had already
discarded the session. A failed *scan* would have logged you out.
Busy now carries the session.
- normalise_server upgrades http:// to https:// rather than accepting
it. NFR-SEC-3 requires TLS, and silently sending a credential in the
clear is not a decision to make on the user's behalf.
launch.slint is not yet wired into app.slint. Calling slint_build::compile
twice replaces the generated module rather than adding to it, which broke
the other in-flight work on dr-ui; I reverted that immediately. Wiring it
needs an import inside app.slint, which is that work's file to change.
419 tests passing across ten crates.
This commit is contained in:
@@ -0,0 +1,17 @@
|
||||
[package]
|
||||
name = "dr-plat"
|
||||
version.workspace = true
|
||||
edition.workspace = true
|
||||
rust-version.workspace = true
|
||||
license.workspace = true
|
||||
|
||||
[dependencies]
|
||||
dr-types.workspace = true
|
||||
thiserror.workspace = true
|
||||
log.workspace = true
|
||||
|
||||
[target.'cfg(all(unix, not(target_os = "android")))'.dependencies]
|
||||
keyring.workspace = true
|
||||
|
||||
[dev-dependencies]
|
||||
env_logger.workspace = true
|
||||
@@ -0,0 +1,64 @@
|
||||
//! Verify credentials round-trip through the real platform secret store.
|
||||
//!
|
||||
//! cargo run -p dr-plat --example keyring_check
|
||||
//!
|
||||
//! Writes a test value, reads it back, deletes it. Touches nothing else.
|
||||
|
||||
use dr_plat::{PlatformSecretStore, SecretRef, SecretStore};
|
||||
|
||||
fn main() {
|
||||
env_logger::init();
|
||||
let store = PlatformSecretStore::new();
|
||||
|
||||
println!("store available: {}", store.is_available());
|
||||
if !store.is_available() {
|
||||
println!("no secrets daemon — the app would run in degraded mode");
|
||||
return;
|
||||
}
|
||||
|
||||
let r = SecretRef::app_password("https://test.invalid", "darkroom-selftest");
|
||||
let secret = "test-token-do-not-reuse";
|
||||
|
||||
print!("store … ");
|
||||
match store.store(&r, secret) {
|
||||
Ok(()) => println!("ok"),
|
||||
Err(e) => {
|
||||
println!("FAILED: {e}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
print!("retrieve … ");
|
||||
match store.retrieve(&r) {
|
||||
Ok(v) if v == secret => println!("ok (round-tripped)"),
|
||||
Ok(_) => {
|
||||
println!("FAILED: wrong value");
|
||||
std::process::exit(1);
|
||||
}
|
||||
Err(e) => {
|
||||
println!("FAILED: {e}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
print!("delete … ");
|
||||
match store.delete(&r) {
|
||||
Ok(()) => println!("ok"),
|
||||
Err(e) => {
|
||||
println!("FAILED: {e}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
print!("confirm gone … ");
|
||||
match store.retrieve(&r) {
|
||||
Err(dr_plat::SecretError::NotFound) => println!("ok"),
|
||||
Ok(_) => {
|
||||
println!("FAILED: still present after delete");
|
||||
std::process::exit(1);
|
||||
}
|
||||
Err(e) => println!("unexpected: {e}"),
|
||||
}
|
||||
|
||||
println!("\nplatform secret store works (FR-NC-2)");
|
||||
}
|
||||
@@ -0,0 +1,11 @@
|
||||
//! Platform abstraction for DarkRoom.
|
||||
//!
|
||||
//! Traits are defined here and implemented per platform, then injected at
|
||||
//! construction, so `core/` contains no `#[cfg(target_os)]` (NFR-PORT-1,
|
||||
//! ARCH §10).
|
||||
|
||||
pub mod secrets;
|
||||
|
||||
pub use secrets::{
|
||||
EphemeralSecretStore, PlatformSecretStore, SecretError, SecretKind, SecretRef, SecretStore,
|
||||
};
|
||||
@@ -0,0 +1,317 @@
|
||||
//! Platform secure storage for credentials (FR-NC-2, NFR-SEC-2).
|
||||
//!
|
||||
//! Credentials are **never** written to the catalog, to a plain file, or to
|
||||
//! logs. On Linux they go to the Secret Service (GNOME Keyring, or KWallet via
|
||||
//! `ksecretd`, which exposes the same D-Bus interface). On Android they belong
|
||||
//! in Keystore-backed storage.
|
||||
//!
|
||||
//! Absence of a secrets daemon is an explicit degraded mode, not a silent
|
||||
//! fallback to plaintext: a headless box or a minimal window manager may have
|
||||
//! none, and quietly writing a password to disk there would be worse than
|
||||
//! refusing.
|
||||
|
||||
use std::fmt;
|
||||
|
||||
/// Which secret is being stored, so one account can hold several.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum SecretKind {
|
||||
/// A Nextcloud app password from Login Flow v2. Device-scoped and
|
||||
/// individually revocable — never the user's actual password.
|
||||
AppPassword,
|
||||
}
|
||||
|
||||
impl SecretKind {
|
||||
fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
SecretKind::AppPassword => "app-password",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Where a credential lives: one account on one server.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct SecretRef {
|
||||
pub server: String,
|
||||
pub login: String,
|
||||
pub kind: SecretKind,
|
||||
}
|
||||
|
||||
impl SecretRef {
|
||||
pub fn app_password(server: impl Into<String>, login: impl Into<String>) -> Self {
|
||||
Self {
|
||||
server: server.into(),
|
||||
login: login.into(),
|
||||
kind: SecretKind::AppPassword,
|
||||
}
|
||||
}
|
||||
|
||||
/// The key under which the platform store holds this secret.
|
||||
///
|
||||
/// Includes the server so two accounts on different servers with the same
|
||||
/// login do not collide.
|
||||
fn entry_key(&self) -> String {
|
||||
format!("{}@{}#{}", self.login, self.server, self.kind.as_str())
|
||||
}
|
||||
}
|
||||
|
||||
/// Deliberately opaque: the whole point is that a credential never appears in
|
||||
/// a log line or an error message (NFR-SEC-2).
|
||||
impl fmt::Display for SecretRef {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
write!(f, "{} on {}", self.login, self.server)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum SecretError {
|
||||
/// No secrets daemon. The app runs in a degraded mode where the user
|
||||
/// re-authenticates each session, rather than storing anything in plain.
|
||||
#[error("no platform secret store available: {0}")]
|
||||
Unavailable(String),
|
||||
|
||||
#[error("secret not found")]
|
||||
NotFound,
|
||||
|
||||
#[error("secret store rejected the request: {0}")]
|
||||
Denied(String),
|
||||
|
||||
#[error("secret store error: {0}")]
|
||||
Other(String),
|
||||
}
|
||||
|
||||
/// TRACES: FR-NC-2 | NFR-SEC-2 | M-2
|
||||
/// Store, retrieve and delete credentials.
|
||||
///
|
||||
/// Implemented per platform and injected at construction, so `core/` contains
|
||||
/// no `#[cfg(target_os)]` (NFR-PORT-1).
|
||||
pub trait SecretStore: Send + Sync {
|
||||
fn store(&self, secret_ref: &SecretRef, secret: &str) -> Result<(), SecretError>;
|
||||
fn retrieve(&self, secret_ref: &SecretRef) -> Result<String, SecretError>;
|
||||
fn delete(&self, secret_ref: &SecretRef) -> Result<(), SecretError>;
|
||||
|
||||
/// Whether the store is usable right now.
|
||||
///
|
||||
/// Checked before offering to remember a login, so the UI can say
|
||||
/// "you will need to sign in each time" rather than failing later.
|
||||
fn is_available(&self) -> bool;
|
||||
}
|
||||
|
||||
/// The service name entries are filed under.
|
||||
const SERVICE: &str = "DarkRoom";
|
||||
|
||||
/// Secret Service implementation (GNOME Keyring, KWallet via ksecretd).
|
||||
#[cfg(all(unix, not(target_os = "android")))]
|
||||
pub struct PlatformSecretStore;
|
||||
|
||||
#[cfg(all(unix, not(target_os = "android")))]
|
||||
impl PlatformSecretStore {
|
||||
pub fn new() -> Self {
|
||||
Self
|
||||
}
|
||||
|
||||
fn entry(r: &SecretRef) -> Result<keyring::Entry, SecretError> {
|
||||
keyring::Entry::new(SERVICE, &r.entry_key()).map_err(map_err)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(unix, not(target_os = "android")))]
|
||||
impl Default for PlatformSecretStore {
|
||||
fn default() -> Self {
|
||||
Self::new()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(unix, not(target_os = "android")))]
|
||||
impl SecretStore for PlatformSecretStore {
|
||||
fn store(&self, secret_ref: &SecretRef, secret: &str) -> Result<(), SecretError> {
|
||||
Self::entry(secret_ref)?
|
||||
.set_password(secret)
|
||||
.map_err(map_err)
|
||||
}
|
||||
|
||||
fn retrieve(&self, secret_ref: &SecretRef) -> Result<String, SecretError> {
|
||||
Self::entry(secret_ref)?.get_password().map_err(map_err)
|
||||
}
|
||||
|
||||
fn delete(&self, secret_ref: &SecretRef) -> Result<(), SecretError> {
|
||||
match Self::entry(secret_ref)?.delete_credential() {
|
||||
Ok(()) => Ok(()),
|
||||
// Deleting an absent secret is the desired end state, not a
|
||||
// failure — logout must be idempotent.
|
||||
Err(keyring::Error::NoEntry) => Ok(()),
|
||||
Err(e) => Err(map_err(e)),
|
||||
}
|
||||
}
|
||||
|
||||
fn is_available(&self) -> bool {
|
||||
// Probing a name that will not exist distinguishes "daemon absent"
|
||||
// from "secret absent": the former errors, the latter reports NoEntry.
|
||||
match keyring::Entry::new(SERVICE, "__availability_probe__") {
|
||||
Ok(e) => !matches!(
|
||||
e.get_password(),
|
||||
Err(keyring::Error::PlatformFailure(_)) | Err(keyring::Error::NoStorageAccess(_))
|
||||
),
|
||||
Err(_) => false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(unix, not(target_os = "android")))]
|
||||
fn map_err(e: keyring::Error) -> SecretError {
|
||||
match e {
|
||||
keyring::Error::NoEntry => SecretError::NotFound,
|
||||
keyring::Error::NoStorageAccess(e) => SecretError::Unavailable(e.to_string()),
|
||||
keyring::Error::PlatformFailure(e) => SecretError::Unavailable(e.to_string()),
|
||||
other => SecretError::Other(other.to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Placeholder for platforms without an implementation yet.
|
||||
///
|
||||
/// Android needs Keystore-backed storage via JNI (FR-PLAT-AND-1). Failing
|
||||
/// loudly is deliberate: a silent no-op store would look like it worked and
|
||||
/// then lose the credential.
|
||||
#[cfg(not(all(unix, not(target_os = "android"))))]
|
||||
pub struct PlatformSecretStore;
|
||||
|
||||
#[cfg(not(all(unix, not(target_os = "android"))))]
|
||||
impl PlatformSecretStore {
|
||||
pub fn new() -> Self {
|
||||
Self
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(all(unix, not(target_os = "android"))))]
|
||||
impl Default for PlatformSecretStore {
|
||||
fn default() -> Self {
|
||||
Self::new()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(all(unix, not(target_os = "android"))))]
|
||||
impl SecretStore for PlatformSecretStore {
|
||||
fn store(&self, _r: &SecretRef, _s: &str) -> Result<(), SecretError> {
|
||||
Err(SecretError::Unavailable(
|
||||
"Keystore-backed storage is not implemented on this platform yet".into(),
|
||||
))
|
||||
}
|
||||
fn retrieve(&self, _r: &SecretRef) -> Result<String, SecretError> {
|
||||
Err(SecretError::Unavailable(
|
||||
"Keystore-backed storage is not implemented on this platform yet".into(),
|
||||
))
|
||||
}
|
||||
fn delete(&self, _r: &SecretRef) -> Result<(), SecretError> {
|
||||
Ok(())
|
||||
}
|
||||
fn is_available(&self) -> bool {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
/// An in-memory store for tests and for the degraded no-daemon mode.
|
||||
///
|
||||
/// Credentials live only as long as the process, so a user without a secrets
|
||||
/// daemon re-authenticates each session — which is the honest behaviour.
|
||||
#[derive(Default)]
|
||||
pub struct EphemeralSecretStore {
|
||||
entries: std::sync::Mutex<std::collections::HashMap<String, String>>,
|
||||
}
|
||||
|
||||
impl EphemeralSecretStore {
|
||||
pub fn new() -> Self {
|
||||
Self::default()
|
||||
}
|
||||
}
|
||||
|
||||
impl SecretStore for EphemeralSecretStore {
|
||||
fn store(&self, secret_ref: &SecretRef, secret: &str) -> Result<(), SecretError> {
|
||||
self.entries
|
||||
.lock()
|
||||
.map_err(|e| SecretError::Other(e.to_string()))?
|
||||
.insert(secret_ref.entry_key(), secret.to_string());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn retrieve(&self, secret_ref: &SecretRef) -> Result<String, SecretError> {
|
||||
self.entries
|
||||
.lock()
|
||||
.map_err(|e| SecretError::Other(e.to_string()))?
|
||||
.get(&secret_ref.entry_key())
|
||||
.cloned()
|
||||
.ok_or(SecretError::NotFound)
|
||||
}
|
||||
|
||||
fn delete(&self, secret_ref: &SecretRef) -> Result<(), SecretError> {
|
||||
self.entries
|
||||
.lock()
|
||||
.map_err(|e| SecretError::Other(e.to_string()))?
|
||||
.remove(&secret_ref.entry_key());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn is_available(&self) -> bool {
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn keys_separate_accounts_across_servers() {
|
||||
// Same login on two servers must not collide, or signing into the
|
||||
// second would overwrite the first.
|
||||
let a = SecretRef::app_password("https://a.example", "duncan");
|
||||
let b = SecretRef::app_password("https://b.example", "duncan");
|
||||
assert_ne!(a.entry_key(), b.entry_key());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keys_separate_logins_on_one_server() {
|
||||
let a = SecretRef::app_password("https://a.example", "duncan");
|
||||
let b = SecretRef::app_password("https://a.example", "someone");
|
||||
assert_ne!(a.entry_key(), b.entry_key());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn display_never_reveals_the_secret_or_the_key() {
|
||||
let r = SecretRef::app_password("https://cloud.example", "duncan");
|
||||
let shown = r.to_string();
|
||||
assert!(shown.contains("duncan"));
|
||||
assert!(
|
||||
!shown.contains("app-password"),
|
||||
"internal key must not leak"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ephemeral_round_trips() {
|
||||
let s = EphemeralSecretStore::new();
|
||||
let r = SecretRef::app_password("https://cloud.example", "duncan");
|
||||
|
||||
assert!(matches!(s.retrieve(&r), Err(SecretError::NotFound)));
|
||||
s.store(&r, "token-value").unwrap();
|
||||
assert_eq!(s.retrieve(&r).unwrap(), "token-value");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deleting_is_idempotent() {
|
||||
// Logout must succeed whether or not a credential is present.
|
||||
let s = EphemeralSecretStore::new();
|
||||
let r = SecretRef::app_password("https://cloud.example", "duncan");
|
||||
assert!(s.delete(&r).is_ok());
|
||||
s.store(&r, "x").unwrap();
|
||||
assert!(s.delete(&r).is_ok());
|
||||
assert!(matches!(s.retrieve(&r), Err(SecretError::NotFound)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn storing_twice_overwrites() {
|
||||
let s = EphemeralSecretStore::new();
|
||||
let r = SecretRef::app_password("https://cloud.example", "duncan");
|
||||
s.store(&r, "first").unwrap();
|
||||
s.store(&r, "second").unwrap();
|
||||
assert_eq!(s.retrieve(&r).unwrap(), "second");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user