Devolutions / Devolutions/sspi-rs
Remove `Username::domain_name()` in favor of `Username::parts()`
- Dominant language
- Rust
- Stars
- 83
- Forks
- 50
- Avg merge
- 3d 12h
- Merged PRs (30d)
- 9
Description
### Background
`Username::domain_name()` returns `Option<&str>` but the `&str` means two different things depending on the internal format:
- **UPN** (`user@corp.example.com`) → returns the UPN **suffix** (`corp.example.com`)
- **Down-level logon name** (`CORP\user`) → returns the **NetBIOS domain** (`CORP`)
A UPN suffix and a NetBIOS domain are not interchangeable, but the accessor hands them back through the same type with no signal of which one you got. A caller that wants "the NetBIOS domain for a down-level logon" and calls `domain_name()` on a UPN silently receives a DNS-style suffix instead, the wrong value, shaped plausibly enough to flow downstream undetected.
#709 adds a format-tagged replacement, `Username::parts() -> UsernameParts<'_>`, whose UPN arm exposes only `suffix` and whose down-level arm exposes only `netbios_domain: Option<&str>`. Because the enum is matched exhaustively, treating a UPN suffix as a NetBIOS domain no longer compiles. `domain_name()` is deprecated in that PR; this issue tracks its removal, which is a breaking change and must ship in its own `fix!`/`feat!` release.
### Why this is a real footgun, not a theoretical one
`impl From for AuthIdentityBuffers` writes `domain_name().unwrap_or_default()` straight into the `domain` field of `AuthIdentityBuffers`, a slot consumed as a NetBIOS domain by the NTLM/Kerberos paths. For a UPN identity this places the DNS suffix where a NetBIOS domain is expected. This is the exact confusion the type-level split is meant to make impossible.
### External impact
`Username` is part of the published `sspi` crate's public API and is used by downstream consumers (IronRDP, Devolutions Gateway, and third parties). Removal is a breaking change:
### Checklist
- [ ] Land the deprecation (companion PR) in a minor release first, giving downstreams a compiler-visible migration window.
- [ ] Remove in a later dedicated breaking release, commit marked `fix!`/`feat!` with a `BREAKING CHANGE:` footer.
### Migration example for downstreams
```rust
// Before
let domain = username.domain_name().unwrap_or_default();
// After
let domain = match username.parts() {
UsernameParts::DownLevelLogonName { netbios_domain, .. } => netbios_domain.unwrap_or_default(),
UsernameParts::UserPrincipalName { .. } => "", // a UPN has no NetBIOS domain
};
```
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by searching the repository for Username::domain_name(), especially the AuthIdentity to AuthIdentityBuffers conversion, and read the Username::parts() API introduced by #709. Check all in-repository callers and relevant tests for migration to the format-specific match. Done means the deprecated accessor is removed, callers are migrated, and the breaking release commit includes the required fix!/feat! marker and BREAKING CHANGE: footer.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- authentication
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100