refactor(oidc): migrate provider into issuer

this make OIDC compliant with the invariant binding (issuer and subject)
admin can now rename their provider without breaking

clarifing federation_kind: report the kind of federation wired not the allowed login method
hybryd login method are still allowed
This commit is contained in:
Edouard Vanbelle
2026-08-08 15:24:06 +02:00
parent 10a8dd7d8b
commit d8b3f2e026
15 changed files with 390 additions and 61 deletions
+61 -7
View File
@@ -25,7 +25,36 @@ pub struct UserDto {
pub updated_at: DateTime<Utc>,
pub last_login_at: Option<DateTime<Utc>>,
pub active: bool,
pub auth_provider: String,
/// Which trust chain minted this user's federation identity —
/// `"oidc" | "ocm" | "magic_link"` — or `None` for pure local
/// users. Load-bearing for "is this user OIDC?"-shape predicates:
/// use `federation_kind == "oidc"` rather than string-scraping
/// `federation_issuer`. Serialized only when populated.
///
/// Mirrors `auth.users.federation_kind` verbatim — same name at
/// DB, entity, and wire layers so there's no translation to reason
/// about. See docs/plan/ocm.md § Identity & auth model.
#[serde(skip_serializing_if = "Option::is_none")]
pub federation_kind: Option<String>,
/// The authority that mints this user's `federation_subject` —
/// issuer URL for OIDC (id_token `iss` claim), peer domain for
/// OCM, `null` for local users (password / OPAQUE only).
///
/// Renamed from `auth_provider` (which was a `String` with the
/// sentinel `"local"` for non-federated users, and a human-readable
/// label like `"MockSSO"` before Phase B). This shape mirrors the
/// `auth.users.federation_issuer` column directly: nullable when
/// there's no federation involved. FE predicates for "is this user
/// federated?" should read `federation_kind`, not
/// string-compare this value.
///
/// When populated, FE code that wants a friendly display label
/// looks this value up against `OidcProviderInfoDto.issuer →
/// provider_name` to render the deployment's configured display
/// name; falls back to the raw issuer for foreign IdPs / legacy
/// rows still holding a pre-Phase-B label.
#[serde(skip_serializing_if = "Option::is_none")]
pub federation_issuer: Option<String>,
pub image: Option<String>,
pub can_edit_image: bool,
/// `true` for grant-only external recipients (magic-link, OIDC-only,
@@ -95,8 +124,8 @@ pub struct UserDto {
#[serde(default)]
pub force_password_change: bool,
/// TRUE when the account has a local Argon2id `password_hash` on
/// file. Distinct from `auth_provider`: an SSO-linked account
/// (auth_provider != "local") can ALSO carry a local password if
/// file. Distinct from `federation_kind`: an OIDC-linked account
/// (`federation_kind == "oidc"`) can ALSO carry a local password if
/// it was set at signup or later — a hybrid posture. The SPA
/// gates the profile page's change-password card on this flag,
/// so hybrid users can rotate their local password even though
@@ -127,7 +156,12 @@ pub struct AdminUserSummaryDto {
pub storage_used_bytes: i64,
pub last_login_at: Option<DateTime<Utc>>,
pub active: bool,
pub auth_provider: String,
/// See `UserDto::federation_kind` — same semantics, same wire spelling.
#[serde(skip_serializing_if = "Option::is_none")]
pub federation_kind: Option<String>,
/// See `UserDto::federation_issuer` — same semantics, same wire spelling.
#[serde(skip_serializing_if = "Option::is_none")]
pub federation_issuer: Option<String>,
pub is_external: bool,
/// TRUE when the user has a server-verifiable password on file
/// (`password_hash IS NOT NULL`). The admin table uses this
@@ -171,7 +205,8 @@ impl From<UserListEntry> for AdminUserSummaryDto {
storage_used_bytes: entry.storage_used_bytes,
last_login_at: entry.last_login_at,
active: entry.active,
auth_provider: entry.federation_issuer.unwrap_or_else(|| "local".to_string()),
federation_kind: entry.federation_kind,
federation_issuer: entry.federation_issuer,
is_external: entry.is_external,
has_password: entry.has_password,
opaque_registered: entry.opaque_registered,
@@ -207,8 +242,11 @@ impl From<User> for UserDto {
updated_at: p.updated_at,
last_login_at: p.last_login_at,
active: p.active,
// Some(provider) moves the String; None still allocates "local".
auth_provider: p.federation_issuer.unwrap_or_else(|| "local".to_string()),
// NULL on both fields for local users (no federation wired).
// FE predicates use `!!federation_kind` for "is federated?" —
// no "local" sentinel string; the null tells the whole story.
federation_kind: p.federation_kind.map(|k| k.as_str().to_string()),
federation_issuer: p.federation_issuer,
image: p.image,
can_edit_image,
is_external: p.is_external,
@@ -467,6 +505,22 @@ pub struct OidcExchangeDto {
#[derive(Debug, Serialize, Deserialize, ToSchema)]
pub struct OidcProviderInfoDto {
pub enabled: bool,
/// The authoritative issuer URL for THIS deployment's OIDC config —
/// same value that lands on `auth.users.federation_issuer` for
/// users JIT-provisioned via this IdP.
///
/// Populated so the frontend can resolve display: when
/// `UserDto.federation_issuer` equals this `issuer`, render
/// `provider_name` as the human-friendly label (avoids showing raw
/// issuer URLs like `https://sso.example.com/realms/main` in the
/// admin badge / profile view). Falls back to the raw issuer when
/// there's no match — happens for legacy rows not yet lazy-rebound,
/// or (future) users linked to a different IdP than the currently
/// configured one.
///
/// Empty string when OIDC is disabled on this deployment.
#[serde(default)]
pub issuer: String,
pub provider_name: String,
pub authorize_endpoint: String,
pub password_login_enabled: bool,
+34
View File
@@ -179,6 +179,20 @@ pub trait UserStoragePort: Send + Sync + 'static {
image: Option<&str>,
) -> Result<(), DomainError>;
/// Federation-identity Phase B lazy rebind: overwrite `federation_issuer`
/// on a specific user row. Called when an OIDC login's id_token `iss`
/// claim proves the stored value (typically a legacy display label) is
/// out of sync with the true issuer URL. Guarded (`IS DISTINCT FROM`)
/// so calling with the current value is a zero-write no-op.
///
/// Audit signal for the rebind lives at the caller (auth service) —
/// this repo method just moves the column value.
async fn rebind_federation_issuer(
&self,
user_id: Uuid,
new_issuer: &str,
) -> Result<(), DomainError>;
/// Lists users by role (e.g., "admin" or "user")
async fn list_users_by_role(&self, role: &str) -> Result<Vec<User>, DomainError>;
@@ -238,6 +252,15 @@ pub struct OidcTokenSet {
#[derive(Debug, Clone)]
pub struct OidcIdClaims {
pub sub: String,
/// The validated `iss` claim from the id_token. Equal to
/// `discovery.issuer` (the validator enforces `iss == discovery.issuer`,
/// so this is a safe echo of the authoritative issuer URL).
///
/// Load-bearing for the federation-identity Phase B lazy-rebind: the
/// app service compares this against `user.federation_issuer` and
/// updates the row when the stored value is still a legacy display
/// label (see docs/plan/ocm.md § Rename PR — Phase B).
pub iss: String,
pub email: Option<String>,
pub email_verified: Option<bool>,
pub preferred_username: Option<String>,
@@ -275,6 +298,17 @@ pub struct OidcLogoutClaims {
/// JWT identifier — used by the app service to prevent replay of the
/// same logout_token within the token's freshness window.
pub jti: Option<String>,
/// The validated `iss` claim from the logout_token — echoed from
/// `discovery.issuer` (the validator enforces `iss == discovery.issuer`,
/// so this is a safe echo of the authoritative issuer URL).
///
/// Load-bearing for the sub-based revocation path (BCL without sid):
/// the app service passes this to
/// `revoke_user_sessions_by_federation_subject(issuer, sub)`, and the
/// pg impl matches on `auth.users.federation_issuer` — which post
/// Phase B stores the iss URL, NOT the display label. Passing the
/// display label (via `oidc.provider_name()`) misses every row.
pub iss: String,
}
/// Port for OIDC operations — implemented in infrastructure layer
@@ -1516,16 +1516,19 @@ impl AuthApplicationService {
self.backchannel_logout_jti_seen.insert(jti.clone(), ());
}
let provider_name = oidc.provider_name().to_string();
// Resolve which sessions to revoke.
let affected_user_ids: Vec<Uuid> = if let Some(sid) = claims.sid.as_ref() {
self.session_storage
.revoke_sessions_by_oidc_sid(sid)
.await?
} else if let Some(sub) = claims.sub.as_ref() {
// Pass claims.iss (the id_token's real issuer URL from the
// logout_token), NOT the OIDC service's provider_name
// display label. Post Phase B of the federation-identity
// rename, `auth.users.federation_issuer` stores the iss
// URL — matching on the display label misses every row.
self.session_storage
.revoke_user_sessions_by_federation_subject(&provider_name, sub)
.revoke_user_sessions_by_federation_subject(&claims.iss, sub)
.await?
.into_iter()
.collect()
@@ -3396,13 +3399,63 @@ impl AuthApplicationService {
.clone()
.unwrap_or_else(|| format!("{}@oidc.local", oidc_username));
// 5. Look up existing user by OIDC subject
let user = match self
// 5. Look up existing user by OIDC subject.
//
// Two-step lookup implements the Phase B lazy-rebind of the
// federation-identity rename (docs/plan/ocm.md § Phase B):
// 1. Canonical lookup keyed on the id_token's real `iss` claim.
// Post-migration this is what every fresh JIT row uses.
// 2. Legacy fallback keyed on the OXICLOUD_OIDC_PROVIDER_NAME
// display label. Fires for rows minted before the rename.
// If the fallback hits, the row is rebound to the real iss
// before this branch returns — first login after upgrade
// self-heals the user; no admin action needed.
// If both miss, JIT provisioning kicks in below and writes the
// canonical value from the start.
let canonical = self
.user_storage
.get_user_by_federation_subject(&provider_name, &claims.sub)
.await
{
.get_user_by_federation_subject(&claims.iss, &claims.sub)
.await;
let lookup_result = match canonical {
Ok(u) => Ok(u),
// Only fall through to the legacy lookup if the canonical one
// said "not found" — treat all OTHER errors as fatal to avoid
// masking DB failures with a lookup that would probably fail
// the same way. NotFound is the only benign case here.
Err(e) if e.kind == ErrorKind::NotFound => {
self.user_storage
.get_user_by_federation_subject(&provider_name, &claims.sub)
.await
}
Err(e) => Err(e),
};
let user = match lookup_result {
Ok(mut existing_user) => {
// Lazy-rebind: if the row's stored issuer doesn't match
// the real iss claim, update it now. Covers the legacy-
// label case (fallback hit) AND any drift accumulated
// during Phase A when JIT was still writing labels.
// rebind_federation_issuer is a guarded UPDATE — same-value
// no-op costs nothing.
if existing_user.federation_issuer() != Some(claims.iss.as_str()) {
let old = existing_user
.federation_issuer()
.map(str::to_string)
.unwrap_or_default();
self.user_storage
.rebind_federation_issuer(existing_user.id(), &claims.iss)
.await?;
tracing::info!(
target: "audit",
event = "federation.issuer_rebound",
reason = "lazy_backfill",
user_id = %existing_user.id(),
federation_kind = "oidc",
old_issuer = %old,
new_issuer = %claims.iss,
"🔗 federation_issuer rebound from legacy label to true iss URL",
);
}
// User exists — dispatch login BEFORE register_login() so
// hooks observe `last_login_at = None` on the very first
// login (see tip #1 in the trait docstring).
@@ -3516,14 +3569,12 @@ impl AuthApplicationService {
Some(username.clone()),
None,
Some(crate::domain::entities::user::FederationKind::Oidc),
// TODO Phase B: `provider_name` still carries the
// OXICLOUD_OIDC_PROVIDER_NAME display label instead
// of the true `iss` URL. Lazy-rebind on subsequent
// logins converts the row (see docs/plan/ocm.md
// § Rename PR — Phase B). First-login value is the
// label for backwards compatibility with existing
// rows.
Some(provider_name.clone()),
// Phase B canonical value: the id_token's real `iss`
// claim (validated to equal discovery.issuer in
// OidcService). No more display-label writes at JIT —
// legacy rows are fixed via lazy rebind in the
// existing-user branch above.
Some(claims.iss.clone()),
Some(claims.sub.clone()),
role,
quota,
@@ -45,6 +45,7 @@ pub struct UserListEntry {
pub storage_used_bytes: i64,
pub last_login_at: Option<DateTime<Utc>>,
pub active: bool,
pub federation_kind: Option<String>,
pub federation_issuer: Option<String>,
pub is_external: bool,
/// TRUE when `auth.users.password_hash IS NOT NULL` — user has a
@@ -783,6 +783,7 @@ impl UserRepository for UserPgRepository {
Option<chrono::DateTime<chrono::Utc>>,
bool,
Option<String>,
Option<String>,
bool,
bool,
bool,
@@ -797,13 +798,15 @@ impl UserRepository for UserPgRepository {
// pays. `has_password` on the password_hash column tells
// the admin table whether a server-verifiable password is
// on file; combined with the two OPAQUE flags and
// federation_issuer, the SPA derives the full "capability
// set" per user (password / OPAQUE / SSO / passwordless).
// federation_kind / federation_issuer, the SPA derives the
// full "capability set" per user (password / OPAQUE / SSO /
// passwordless).
r#"
SELECT
id, username, email, role::text,
storage_quota_bytes, storage_used_bytes,
last_login_at, active, federation_issuer, is_external,
last_login_at, active,
federation_kind, federation_issuer, is_external,
(password_hash IS NOT NULL) AS has_password,
(opaque_envelope IS NOT NULL) AS opaque_registered,
(opaque_migrated_at IS NOT NULL) AS opaque_migrated
@@ -832,6 +835,7 @@ impl UserRepository for UserPgRepository {
storage_used_bytes,
last_login_at,
active,
federation_kind,
federation_issuer,
is_external,
has_password,
@@ -850,6 +854,7 @@ impl UserRepository for UserPgRepository {
storage_used_bytes,
last_login_at,
active,
federation_kind,
federation_issuer,
is_external,
has_password,
@@ -1369,6 +1374,34 @@ impl UserStoragePort for UserPgRepository {
Ok(())
}
async fn rebind_federation_issuer(
&self,
user_id: Uuid,
new_issuer: &str,
) -> Result<(), DomainError> {
// Same `IS DISTINCT FROM` guard as sync_oidc_login_profile: this
// fires on every OIDC login, so the common already-migrated case
// must be a zero-write no-op. Only actually flips the column
// when the stored value is stale (legacy display label vs the
// real issuer URL from the id_token's `iss` claim).
sqlx::query(
r#"
UPDATE auth.users
SET federation_issuer = $2,
updated_at = NOW()
WHERE id = $1
AND federation_issuer IS DISTINCT FROM $2
"#,
)
.bind(user_id)
.bind(new_issuer)
.execute(&*self.pool)
.await
.map_err(Self::map_sqlx_error)
.map_err(DomainError::from)?;
Ok(())
}
async fn list_users_by_role(&self, role: &str) -> Result<Vec<User>, DomainError> {
UserRepository::list_users_by_role(self, role)
.await
@@ -180,6 +180,39 @@ impl OidcService {
}
}
/// Authoritative issuer URL from the IdP's discovery document —
/// **cache-only, non-async, non-blocking**. Returns `Some(issuer)`
/// when discovery has been fetched successfully before AND is not
/// expired; returns `None` otherwise (cold cache OR expired without
/// re-fetch).
///
/// Deliberately does NOT trigger a network fetch — this is the
/// accessor that public endpoints (`/api/auth/oidc/providers`)
/// use, and driving IdP HTTP off every unauthenticated request is
/// a DoS amplifier. The cache gets warmed as a side-effect of
/// every real OIDC flow (authorize / callback / login /
/// validate_id_token all call `get_discovery`), so within seconds
/// of the first legit login this returns `Some`.
///
/// Callers that need the definitive answer (validate_id_token, JIT
/// provisioning) should keep going through the async
/// discovery-fetching path. Callers that need a display hint
/// (providers endpoint) MUST use this non-async path and fall back
/// to a config value when it returns `None`.
pub fn cached_issuer(&self) -> Option<String> {
// try_read is non-blocking; if the cache write lock is held
// (extremely rare, only during a discovery refresh), we return
// None rather than block on public traffic.
let cache = self.discovery.try_read().ok()?;
cache.as_ref().and_then(|cached| {
if cached.is_expired() {
None
} else {
Some(cached.value.issuer.clone())
}
})
}
/// Fetch and cache the OIDC discovery document (TTL: 1 hour)
async fn get_discovery(&self) -> Result<OidcDiscovery, DomainError> {
// Check cache first (return cached value only if not expired)
@@ -495,6 +528,13 @@ impl OidcServicePort for OidcService {
Ok(OidcIdClaims {
sub: claims.sub,
// Safe echo: jsonwebtoken::decode with
// `validation.set_issuer(&[&discovery.issuer])` above already
// enforced iss == discovery.issuer, so the discovery value
// IS the validated iss claim. The caller (auth service uses
// it for Phase B lazy-rebind) can trust this without a
// second validation pass.
iss: discovery.issuer.clone(),
email: claims.email,
email_verified: claims.email_verified,
preferred_username: claims.preferred_username,
@@ -510,6 +550,11 @@ impl OidcServicePort for OidcService {
async fn fetch_user_info(&self, access_token: &str) -> Result<OidcIdClaims, DomainError> {
let discovery = self.get_discovery().await?;
// Capture issuer before we move `userinfo_endpoint` out below.
// Same rationale as validate_id_token: discovery.issuer IS the
// authoritative iss for this deployment; fetch_user_info is only
// called after a successful token exchange with this same issuer.
let iss = discovery.issuer.clone();
let userinfo_url = discovery.userinfo_endpoint.ok_or_else(|| {
DomainError::new(
@@ -551,6 +596,7 @@ impl OidcServicePort for OidcService {
Ok(OidcIdClaims {
sub: info.sub,
iss,
email: info.email,
email_verified: info.email_verified,
preferred_username: info.preferred_username,
@@ -741,6 +787,7 @@ impl OidcServicePort for OidcService {
sub: claims.sub,
sid: claims.sid,
jti: claims.jti,
iss: claims.iss,
})
}
}
@@ -1294,6 +1294,7 @@ pub async fn oidc_providers(
if !auth_app.oidc_enabled() {
return Ok(Json(OidcProviderInfoDto {
enabled: false,
issuer: String::new(),
provider_name: String::new(),
authorize_endpoint: String::new(),
password_login_enabled,
@@ -1305,8 +1306,31 @@ pub async fn oidc_providers(
let config = auth_app.oidc_config().unwrap();
// Prefer the DISCOVERY document's issuer — that's what
// OidcService uses to validate id_tokens AND what lands on
// `auth.users.federation_issuer` at JIT provisioning / lazy
// rebind. The config's `issuer_url` is only what the operator
// typed to point at discovery; the discovery document publishes
// the authoritative value (may differ by trailing slash, host
// casing, etc). **Cache-only lookup on purpose** — this
// endpoint is PUBLIC + UNAUTHENTICATED, so triggering an IdP
// HTTP fetch per request is a DoS amplifier (attacker at N req/s
// → we hit the IdP at N req/s, and the cache only stores on
// success so a degraded IdP means every call retries). On cold
// cache (before the first real OIDC flow warms it), fall back to
// the operator-typed `config.issuer_url`. In practice the cache
// is warm within seconds of the first login; the fallback only
// shows during that window and is only wrong if the IdP
// publishes an issuer that differs from the URL used to fetch
// discovery (rare in normal deployments).
let issuer = auth_app
.oidc_service()
.and_then(|svc| svc.cached_issuer())
.unwrap_or_else(|| config.issuer_url.clone());
Ok(Json(OidcProviderInfoDto {
enabled: true,
issuer,
provider_name: config.provider_name.clone(),
authorize_endpoint: "/api/auth/oidc/authorize".to_string(),
password_login_enabled,
+8 -2
View File
@@ -213,8 +213,14 @@ async fn user_provisioning_response(
vec!["users"]
};
// Determine backend based on auth provider
let backend = if user_dto.auth_provider.to_lowercase().contains("oidc") {
// Determine backend based on federation kind. Historically checked
// `auth_provider.to_lowercase().contains("oidc")` which happened to
// work when the DTO field held a display label containing "oidc"
// (e.g. "OIDC-Google") — but broke silently when the label was
// "MockSSO" or, post Phase B of the federation-identity rename, when
// the field became an issuer URL that doesn't contain "oidc". The
// kind field is the load-bearing signal.
let backend = if user_dto.federation_kind.as_deref() == Some("oidc") {
"OIDC"
} else {
"Database"