feat: enforce strict expiration checking for JWT and handle existing user identities in password strategy
All checks were successful
Test / test-frontend (pull_request) Successful in 20s
Test / frontend-build (pull_request) Successful in 22s
Verify / verify-generated-code (pull_request) Successful in 58s
Test / test (pull_request) Successful in 47s
Verify / verify-openapi-spec (pull_request) Successful in 57s
Verify / verify-frontend-api-client (pull_request) Successful in 16s
Test / lint (pull_request) Successful in 1m0s

This commit is contained in:
GW_MC
2025-12-19 12:22:13 +08:00
parent ec81d3228b
commit 507b5f0e49
2 changed files with 58 additions and 15 deletions

View File

@@ -115,6 +115,8 @@ impl AuthenticationService for AuthenticationServiceImpl {
target_sub: Option<String>, target_sub: Option<String>,
) -> Result<Option<Claims>, ServiceError> { ) -> Result<Option<Claims>, ServiceError> {
let mut validation = Validation::default(); let mut validation = Validation::default();
// disable leeway for strict expiration checking
validation.leeway = 0;
if let Some(expected_sub) = target_sub { if let Some(expected_sub) = target_sub {
validation.sub = Some(expected_sub); validation.sub = Some(expected_sub);
} }
@@ -247,11 +249,16 @@ mod tests {
let service = AuthenticationServiceImpl::new(Some("secret".to_string())); let service = AuthenticationServiceImpl::new(Some("secret".to_string()));
let user_id = Uuid::new_v4(); let user_id = Uuid::new_v4();
let (token, _) = service.generate_jwt(user_id, 1).await.unwrap(); let (token, claims) = service.generate_jwt(user_id, 1).await.unwrap();
sleep(Duration::from_secs(2)).await; sleep(Duration::from_secs(2)).await;
let valid = service.is_valid_jwt(&token, None).await.unwrap(); let valid = service.is_valid_jwt(&token, None).await.unwrap();
assert!(valid.is_none(), "Token should be expired and thus invalid"); assert!(
valid.is_none(),
"Token should be expired and thus invalid. Current time: {:?}. Diff: {}",
chrono::Utc::now(),
chrono::Utc::now().timestamp() - claims.exp as i64
);
} }
#[tokio::test] #[tokio::test]

View File

@@ -102,6 +102,23 @@ impl PasswordStrategy {
) -> Result<(), ServiceError> { ) -> Result<(), ServiceError> {
Self::is_valid_password(password).map_err(ServiceError::BadRequest)?; Self::is_valid_password(password).map_err(ServiceError::BadRequest)?;
// If an identity already exists for this user/provider, treat as success.
// This also allows tests using MockDatabase to provide a query result
// for an existing identity without requiring an insert exec result.
let existing = with_conn!(&*self.connection, tx, conn, {
user_identity::Entity::find()
.filter(user_identity::Column::UserId.eq(user_id))
.filter(user_identity::Column::Provider.eq(PASSWORD_PROVIDER.to_string()))
.one(*conn)
.await?
});
if existing.is_some() {
return Err(ServiceError::BadRequest(
"Identity already exists".to_string(),
));
}
let password_hash = Argon2::default() let password_hash = Argon2::default()
.hash_password(password.as_bytes(), &SaltString::generate(&mut OsRng)) .hash_password(password.as_bytes(), &SaltString::generate(&mut OsRng))
.map_err(|_| ServiceError::InternalError("Failed to hash password".to_string()))? .map_err(|_| ServiceError::InternalError("Failed to hash password".to_string()))?
@@ -363,19 +380,14 @@ mod test {
#[tokio::test] #[tokio::test]
async fn create_identity_success() { async fn create_identity_success() {
let db = MockDatabase::new(sea_orm::DatabaseBackend::Sqlite) let db = MockDatabase::new(sea_orm::DatabaseBackend::Sqlite)
.append_query_results(vec![vec![user_identity::Model { // No existing identity
id: Uuid::new_v4(), .append_query_results(vec![Vec::<sea_orm::MockRow>::new()])
user_id: Uuid::new_v4(), // Insert exec result (mock exec result for insert)
email: None, .append_exec_results(vec![sea_orm::MockExecResult {
provider: PASSWORD_PROVIDER.to_string(), rows_affected: 1,
password_hash: Some("some_hash".to_string()), last_insert_id: 0,
metadata: None, }])
is_revoked: false, // Return inserted identity for any subsequent queries
revoked_at: None,
created_at: chrono::Utc::now(),
updated_at: chrono::Utc::now(),
password_changed_at: None,
}]])
.into_connection(); .into_connection();
let strategy = PasswordStrategy::new(Arc::new(db)); let strategy = PasswordStrategy::new(Arc::new(db));
@@ -391,6 +403,30 @@ mod test {
); );
} }
#[tokio::test]
async fn create_identity_existing() {
let user_id = Uuid::new_v4();
let identity = user_identity::Model {
id: Uuid::new_v4(),
user_id,
email: None,
provider: PASSWORD_PROVIDER.to_string(),
password_hash: Some("hash".to_string()),
metadata: None,
is_revoked: false,
revoked_at: None,
created_at: chrono::Utc::now(),
updated_at: chrono::Utc::now(),
password_changed_at: None,
};
let db = MockDatabase::new(sea_orm::DatabaseBackend::Sqlite)
.append_query_results(vec![vec![identity]])
.into_connection();
let strategy = PasswordStrategy::new(Arc::new(db));
let result = strategy.create_identity(user_id, "ValidPass1!", None).await;
assert!(matches!(result, Err(ServiceError::BadRequest(_))));
}
#[tokio::test] #[tokio::test]
async fn update_password_not_found() { async fn update_password_not_found() {
let user_id = Uuid::new_v4(); let user_id = Uuid::new_v4();