fix(auth): tighten refresh session revocation
This commit is contained in:
@@ -100,6 +100,12 @@ pub struct ListActiveRefreshSessionsResult {
|
||||
pub sessions: Vec<RefreshSessionRecord>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct RevokeRefreshSessionResult {
|
||||
pub session_id: String,
|
||||
pub revoked: bool,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct LogoutCurrentSessionResult {
|
||||
pub user: AuthUser,
|
||||
|
||||
@@ -87,10 +87,17 @@ pub struct RotateRefreshSessionInput {
|
||||
pub next_refresh_token_hash: String,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct RevokeRefreshSessionByUserInput {
|
||||
pub user_id: String,
|
||||
pub session_id: String,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct LogoutCurrentSessionInput {
|
||||
pub user_id: String,
|
||||
pub refresh_token_hash: Option<String>,
|
||||
pub session_id: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
|
||||
@@ -230,6 +230,22 @@ impl PasswordEntryService {
|
||||
pub async fn change_password(
|
||||
&self,
|
||||
input: ChangePasswordInput,
|
||||
) -> Result<ChangePasswordResult, PasswordEntryError> {
|
||||
self.change_password_internal(input, None).await
|
||||
}
|
||||
|
||||
pub async fn change_password_and_revoke_all_sessions(
|
||||
&self,
|
||||
input: ChangePasswordInput,
|
||||
now: OffsetDateTime,
|
||||
) -> Result<ChangePasswordResult, PasswordEntryError> {
|
||||
self.change_password_internal(input, Some(now)).await
|
||||
}
|
||||
|
||||
async fn change_password_internal(
|
||||
&self,
|
||||
input: ChangePasswordInput,
|
||||
revoke_all_sessions_at: Option<OffsetDateTime>,
|
||||
) -> Result<ChangePasswordResult, PasswordEntryError> {
|
||||
validate_password(&input.new_password)?;
|
||||
let stored_user = self
|
||||
@@ -257,7 +273,7 @@ impl PasswordEntryService {
|
||||
.map_err(|error| PasswordEntryError::PasswordHash(error.to_string()))?;
|
||||
let user = self
|
||||
.store
|
||||
.set_user_password_hash(&input.user_id, password_hash)?
|
||||
.set_user_password_hash(&input.user_id, password_hash, revoke_all_sessions_at)?
|
||||
.ok_or(PasswordEntryError::UserNotFound)?;
|
||||
|
||||
Ok(ChangePasswordResult { user })
|
||||
@@ -375,6 +391,39 @@ impl RefreshSessionService {
|
||||
let sessions = self.store.list_active_sessions_by_user(user_id, now)?;
|
||||
Ok(ListActiveRefreshSessionsResult { sessions })
|
||||
}
|
||||
|
||||
pub fn revoke_session_by_user_and_session(
|
||||
&self,
|
||||
input: RevokeRefreshSessionByUserInput,
|
||||
now: OffsetDateTime,
|
||||
) -> Result<RevokeRefreshSessionResult, RefreshSessionError> {
|
||||
self.store
|
||||
.find_by_user_id(&input.user_id)
|
||||
.map_err(map_password_store_error)?
|
||||
.ok_or(RefreshSessionError::UserNotFound)?;
|
||||
|
||||
let Some(session_id) = normalize_required_string(&input.session_id) else {
|
||||
return Err(RefreshSessionError::SessionNotFound);
|
||||
};
|
||||
let revoked =
|
||||
self.store
|
||||
.revoke_session_by_user_and_session_id(&input.user_id, &session_id, now)?;
|
||||
|
||||
Ok(RevokeRefreshSessionResult {
|
||||
session_id,
|
||||
revoked,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn is_session_active_for_user(
|
||||
&self,
|
||||
user_id: &str,
|
||||
session_id: &str,
|
||||
now: OffsetDateTime,
|
||||
) -> Result<bool, RefreshSessionError> {
|
||||
self.store
|
||||
.is_session_active_for_user(user_id, session_id.trim(), now)
|
||||
}
|
||||
}
|
||||
|
||||
impl PhoneAuthService {
|
||||
@@ -779,7 +828,7 @@ impl AuthUserService {
|
||||
input: LogoutCurrentSessionInput,
|
||||
now: OffsetDateTime,
|
||||
) -> Result<LogoutCurrentSessionResult, LogoutError> {
|
||||
if let Some(refresh_token_hash) = input
|
||||
let revoked_by_hash = if let Some(refresh_token_hash) = input
|
||||
.refresh_token_hash
|
||||
.as_ref()
|
||||
.map(|value| value.trim())
|
||||
@@ -788,6 +837,21 @@ impl AuthUserService {
|
||||
self.store
|
||||
.revoke_session_by_refresh_token_hash(refresh_token_hash, now)
|
||||
.map_err(map_refresh_error_to_logout_error)?;
|
||||
true
|
||||
} else {
|
||||
false
|
||||
};
|
||||
|
||||
if !revoked_by_hash
|
||||
&& let Some(session_id) = input
|
||||
.session_id
|
||||
.as_ref()
|
||||
.map(|value| value.trim())
|
||||
.filter(|value| !value.is_empty())
|
||||
{
|
||||
self.store
|
||||
.revoke_session_by_user_and_session_id(&input.user_id, session_id, now)
|
||||
.map_err(map_refresh_error_to_logout_error)?;
|
||||
}
|
||||
|
||||
let user = self
|
||||
@@ -1685,6 +1749,36 @@ impl InMemoryAuthStore {
|
||||
Ok(sessions)
|
||||
}
|
||||
|
||||
fn is_session_active_for_user(
|
||||
&self,
|
||||
user_id: &str,
|
||||
session_id: &str,
|
||||
now: OffsetDateTime,
|
||||
) -> Result<bool, RefreshSessionError> {
|
||||
if session_id.trim().is_empty() {
|
||||
return Ok(false);
|
||||
}
|
||||
|
||||
let state = self
|
||||
.inner
|
||||
.lock()
|
||||
.map_err(|_| RefreshSessionError::Store("会话仓储锁已中毒".to_string()))?;
|
||||
let Some(stored) = state.sessions_by_id.get(session_id) else {
|
||||
return Ok(false);
|
||||
};
|
||||
if stored.session.user_id != user_id || stored.session.revoked_at.is_some() {
|
||||
return Ok(false);
|
||||
}
|
||||
|
||||
let expires_at = OffsetDateTime::parse(
|
||||
&stored.session.expires_at,
|
||||
&time::format_description::well_known::Rfc3339,
|
||||
)
|
||||
.map_err(|error| RefreshSessionError::Store(format!("会话过期时间解析失败:{error}")))?;
|
||||
|
||||
Ok(expires_at > now)
|
||||
}
|
||||
|
||||
fn rotate_session(
|
||||
&self,
|
||||
session_id: &str,
|
||||
@@ -1774,6 +1868,37 @@ impl InMemoryAuthStore {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn revoke_session_by_user_and_session_id(
|
||||
&self,
|
||||
user_id: &str,
|
||||
session_id: &str,
|
||||
now: OffsetDateTime,
|
||||
) -> Result<bool, RefreshSessionError> {
|
||||
let mut state = self
|
||||
.inner
|
||||
.lock()
|
||||
.map_err(|_| RefreshSessionError::Store("会话仓储锁已中毒".to_string()))?;
|
||||
let Some(stored) = state.sessions_by_id.get_mut(session_id) else {
|
||||
return Ok(false);
|
||||
};
|
||||
if stored.session.user_id != user_id {
|
||||
return Ok(false);
|
||||
}
|
||||
if stored.session.revoked_at.is_some() {
|
||||
return Ok(false);
|
||||
}
|
||||
let now_iso = now
|
||||
.format(&time::format_description::well_known::Rfc3339)
|
||||
.map_err(|error| {
|
||||
RefreshSessionError::Store(format!("会话吊销时间格式化失败:{error}"))
|
||||
})?;
|
||||
stored.session.revoked_at = Some(now_iso.clone());
|
||||
stored.session.updated_at = now_iso;
|
||||
self.persist_refresh_state(&state)?;
|
||||
|
||||
Ok(true)
|
||||
}
|
||||
|
||||
fn revoke_all_sessions_by_user_id(
|
||||
&self,
|
||||
user_id: &str,
|
||||
@@ -1832,11 +1957,21 @@ impl InMemoryAuthStore {
|
||||
&self,
|
||||
user_id: &str,
|
||||
password_hash: String,
|
||||
revoke_all_sessions_at: Option<OffsetDateTime>,
|
||||
) -> Result<Option<AuthUser>, PasswordEntryError> {
|
||||
let mut state = self
|
||||
.inner
|
||||
.lock()
|
||||
.map_err(|_| PasswordEntryError::Store("用户仓储锁已中毒".to_string()))?;
|
||||
let revoke_all_sessions_at = match revoke_all_sessions_at {
|
||||
Some(now) => Some(
|
||||
now.format(&time::format_description::well_known::Rfc3339)
|
||||
.map_err(|error| {
|
||||
PasswordEntryError::Store(format!("会话吊销时间格式化失败:{error}"))
|
||||
})?,
|
||||
),
|
||||
None => None,
|
||||
};
|
||||
|
||||
for stored_user in state.users_by_username.values_mut() {
|
||||
if stored_user.user.id != user_id {
|
||||
@@ -1847,6 +1982,18 @@ impl InMemoryAuthStore {
|
||||
stored_user.password_login_enabled = true;
|
||||
stored_user.user.token_version += 1;
|
||||
let next_user = stored_user.user.clone();
|
||||
if let Some(now_iso) = revoke_all_sessions_at.as_ref() {
|
||||
for stored_session in state.sessions_by_id.values_mut() {
|
||||
if stored_session.session.user_id != user_id
|
||||
|| stored_session.session.revoked_at.is_some()
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
stored_session.session.revoked_at = Some(now_iso.clone());
|
||||
stored_session.session.updated_at = now_iso.clone();
|
||||
}
|
||||
}
|
||||
self.persist_password_state(&state)?;
|
||||
return Ok(Some(next_user));
|
||||
}
|
||||
@@ -2177,6 +2324,118 @@ mod tests {
|
||||
assert_eq!(result.user.login_method, AuthLoginMethod::Password);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn change_password_and_revoke_all_sessions_revokes_every_refresh_session() {
|
||||
let store = build_store();
|
||||
let user = create_phone_login_user(store.clone(), "13800138030").await;
|
||||
let password_service = build_password_service(store.clone());
|
||||
let refresh_service = build_refresh_service(store.clone());
|
||||
let now = OffsetDateTime::now_utc();
|
||||
|
||||
let first_password_user = password_service
|
||||
.change_password(ChangePasswordInput {
|
||||
user_id: user.id.clone(),
|
||||
current_password: None,
|
||||
new_password: "secret123".to_string(),
|
||||
})
|
||||
.await
|
||||
.expect("first password should set")
|
||||
.user;
|
||||
let first_token_hash = hash_refresh_session_token("change-password-token-01");
|
||||
let second_token_hash = hash_refresh_session_token("change-password-token-02");
|
||||
refresh_service
|
||||
.create_session(
|
||||
CreateRefreshSessionInput {
|
||||
user_id: user.id.clone(),
|
||||
refresh_token_hash: first_token_hash.clone(),
|
||||
issued_by_provider: AuthLoginMethod::Password,
|
||||
client_info: build_client_info(),
|
||||
},
|
||||
now,
|
||||
)
|
||||
.expect("first session should create");
|
||||
refresh_service
|
||||
.create_session(
|
||||
CreateRefreshSessionInput {
|
||||
user_id: user.id.clone(),
|
||||
refresh_token_hash: second_token_hash.clone(),
|
||||
issued_by_provider: AuthLoginMethod::Password,
|
||||
client_info: RefreshSessionClientInfo {
|
||||
client_runtime: "safari".to_string(),
|
||||
device_display_name: "iOS / Safari".to_string(),
|
||||
..build_client_info()
|
||||
},
|
||||
},
|
||||
now + Duration::seconds(1),
|
||||
)
|
||||
.expect("second session should create");
|
||||
|
||||
let changed_user = password_service
|
||||
.change_password_and_revoke_all_sessions(
|
||||
ChangePasswordInput {
|
||||
user_id: user.id.clone(),
|
||||
current_password: Some("secret123".to_string()),
|
||||
new_password: "secret456".to_string(),
|
||||
},
|
||||
now + Duration::minutes(1),
|
||||
)
|
||||
.await
|
||||
.expect("password change should revoke all sessions")
|
||||
.user;
|
||||
|
||||
assert_eq!(
|
||||
changed_user.token_version,
|
||||
first_password_user.token_version + 1
|
||||
);
|
||||
assert!(
|
||||
refresh_service
|
||||
.list_active_sessions_by_user(&user.id, now + Duration::minutes(2))
|
||||
.expect("active sessions should list")
|
||||
.sessions
|
||||
.is_empty()
|
||||
);
|
||||
for (token_hash, next_hash) in [
|
||||
(
|
||||
first_token_hash,
|
||||
hash_refresh_session_token("change-password-token-01-next"),
|
||||
),
|
||||
(
|
||||
second_token_hash,
|
||||
hash_refresh_session_token("change-password-token-02-next"),
|
||||
),
|
||||
] {
|
||||
let refresh_error = refresh_service
|
||||
.rotate_session(
|
||||
RotateRefreshSessionInput {
|
||||
refresh_token_hash: token_hash,
|
||||
next_refresh_token_hash: next_hash,
|
||||
},
|
||||
now + Duration::minutes(2),
|
||||
)
|
||||
.expect_err("revoked session should not rotate");
|
||||
assert_eq!(refresh_error, RefreshSessionError::SessionNotFound);
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
password_service
|
||||
.execute(PasswordEntryInput {
|
||||
phone_number: "13800138030".to_string(),
|
||||
password: "secret123".to_string(),
|
||||
})
|
||||
.await
|
||||
.expect_err("old password should fail"),
|
||||
PasswordEntryError::InvalidCredentials
|
||||
);
|
||||
let login = password_service
|
||||
.execute(PasswordEntryInput {
|
||||
phone_number: "13800138030".to_string(),
|
||||
password: "secret456".to_string(),
|
||||
})
|
||||
.await
|
||||
.expect("new password should login");
|
||||
assert_eq!(login.user.id, user.id);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn password_entry_rejects_wrong_password_after_set() {
|
||||
let store = build_store();
|
||||
@@ -2524,6 +2783,7 @@ mod tests {
|
||||
LogoutCurrentSessionInput {
|
||||
user_id: user.id.clone(),
|
||||
refresh_token_hash: Some(refresh_token_hash.clone()),
|
||||
session_id: None,
|
||||
},
|
||||
OffsetDateTime::now_utc(),
|
||||
)
|
||||
@@ -2543,6 +2803,148 @@ mod tests {
|
||||
assert_eq!(refresh_error, RefreshSessionError::SessionNotFound);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn revoke_session_by_user_and_session_revokes_only_target_without_token_bump() {
|
||||
let store = build_store();
|
||||
let user = create_phone_login_user(store.clone(), "13800138028").await;
|
||||
let refresh_service = build_refresh_service(store.clone());
|
||||
let now = OffsetDateTime::now_utc();
|
||||
let first_token_hash = hash_refresh_session_token("revoke-target-token");
|
||||
let second_token_hash = hash_refresh_session_token("revoke-current-token");
|
||||
|
||||
let target = refresh_service
|
||||
.create_session(
|
||||
CreateRefreshSessionInput {
|
||||
user_id: user.id.clone(),
|
||||
refresh_token_hash: first_token_hash.clone(),
|
||||
issued_by_provider: AuthLoginMethod::Password,
|
||||
client_info: build_client_info(),
|
||||
},
|
||||
now,
|
||||
)
|
||||
.expect("target session should create");
|
||||
let current = refresh_service
|
||||
.create_session(
|
||||
CreateRefreshSessionInput {
|
||||
user_id: user.id.clone(),
|
||||
refresh_token_hash: second_token_hash,
|
||||
issued_by_provider: AuthLoginMethod::Password,
|
||||
client_info: RefreshSessionClientInfo {
|
||||
client_runtime: "firefox".to_string(),
|
||||
device_display_name: "Windows / Firefox".to_string(),
|
||||
..build_client_info()
|
||||
},
|
||||
},
|
||||
now + Duration::seconds(1),
|
||||
)
|
||||
.expect("current session should create");
|
||||
|
||||
let revoke = refresh_service
|
||||
.revoke_session_by_user_and_session(
|
||||
RevokeRefreshSessionByUserInput {
|
||||
user_id: user.id.clone(),
|
||||
session_id: target.session.session_id.clone(),
|
||||
},
|
||||
now + Duration::minutes(1),
|
||||
)
|
||||
.expect("target session should revoke");
|
||||
|
||||
assert!(revoke.revoked);
|
||||
assert_eq!(revoke.session_id, target.session.session_id);
|
||||
assert!(
|
||||
!refresh_service
|
||||
.is_session_active_for_user(
|
||||
&user.id,
|
||||
&target.session.session_id,
|
||||
now + Duration::minutes(2)
|
||||
)
|
||||
.expect("target active check should succeed")
|
||||
);
|
||||
assert!(
|
||||
refresh_service
|
||||
.is_session_active_for_user(
|
||||
&user.id,
|
||||
¤t.session.session_id,
|
||||
now + Duration::minutes(2)
|
||||
)
|
||||
.expect("current active check should succeed")
|
||||
);
|
||||
assert_eq!(
|
||||
store
|
||||
.find_by_user_id(&user.id)
|
||||
.expect("user lookup should succeed")
|
||||
.expect("user should exist")
|
||||
.user
|
||||
.token_version,
|
||||
user.token_version
|
||||
);
|
||||
|
||||
let refresh_error = refresh_service
|
||||
.rotate_session(
|
||||
RotateRefreshSessionInput {
|
||||
refresh_token_hash: first_token_hash,
|
||||
next_refresh_token_hash: hash_refresh_session_token("revoke-target-next"),
|
||||
},
|
||||
now + Duration::minutes(2),
|
||||
)
|
||||
.expect_err("revoked target should not rotate");
|
||||
assert_eq!(refresh_error, RefreshSessionError::SessionNotFound);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn logout_current_session_uses_session_id_when_refresh_cookie_missing() {
|
||||
let store = build_store();
|
||||
let user = create_phone_login_user(store.clone(), "13800138029").await;
|
||||
let refresh_service = build_refresh_service(store.clone());
|
||||
let user_service = build_user_service(store);
|
||||
let now = OffsetDateTime::now_utc();
|
||||
let refresh_token_hash = hash_refresh_session_token("logout-sid-token");
|
||||
let session = refresh_service
|
||||
.create_session(
|
||||
CreateRefreshSessionInput {
|
||||
user_id: user.id.clone(),
|
||||
refresh_token_hash: refresh_token_hash.clone(),
|
||||
issued_by_provider: AuthLoginMethod::Password,
|
||||
client_info: build_client_info(),
|
||||
},
|
||||
now,
|
||||
)
|
||||
.expect("session should create");
|
||||
|
||||
let result = user_service
|
||||
.logout_current_session(
|
||||
LogoutCurrentSessionInput {
|
||||
user_id: user.id.clone(),
|
||||
refresh_token_hash: None,
|
||||
session_id: Some(session.session.session_id.clone()),
|
||||
},
|
||||
now + Duration::minutes(1),
|
||||
)
|
||||
.expect("logout should succeed");
|
||||
|
||||
assert_eq!(result.user.token_version, user.token_version + 1);
|
||||
assert!(
|
||||
!refresh_service
|
||||
.is_session_active_for_user(
|
||||
&user.id,
|
||||
&session.session.session_id,
|
||||
now + Duration::minutes(2)
|
||||
)
|
||||
.expect("session active check should succeed")
|
||||
);
|
||||
|
||||
let refresh_error = refresh_service
|
||||
.rotate_session(
|
||||
RotateRefreshSessionInput {
|
||||
refresh_token_hash,
|
||||
next_refresh_token_hash: hash_refresh_session_token("logout-sid-next"),
|
||||
},
|
||||
now + Duration::minutes(2),
|
||||
)
|
||||
.expect_err("sid-revoked session should fail");
|
||||
assert_eq!(refresh_error, RefreshSessionError::SessionNotFound);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn logout_all_sessions_revokes_all_sessions_and_increments_token_version_once() {
|
||||
let store = build_store();
|
||||
|
||||
Reference in New Issue
Block a user