fix(dimentorin): verify-email validates OTP before activating user
- new app_otp_cache table + OtpCache entity (ResourceEnum::OtpCache) - PostgresOtpRepository upsert/find/delete keyed by email - register/resend persist otp_hash+expiry after email sent (no orphan OTP) - verify_email validates via OtpManager::validate_otp_hash, single-use delete - 8 unit tests pass, e2e verified: wrong OTP 400, correct OTP 200
This commit is contained in:
@@ -10,23 +10,18 @@ Semua diuji lokal (Postgres `dimentorin`, backend :4099).
|
|||||||
- **Dampak**: mentee/mentor baru tak bisa menerima OTP lewat email → tak bisa aktivasi → tak bisa login, kecuali via verify-email langsung.
|
- **Dampak**: mentee/mentor baru tak bisa menerima OTP lewat email → tak bisa aktivasi → tak bisa login, kecuali via verify-email langsung.
|
||||||
- **Diperlukan**: SMTP credential institution yang valid (Gmail App Password atau SMTP relay), sebaiknya dari BWS secret management, bukan hardcode.
|
- **Diperlukan**: SMTP credential institution yang valid (Gmail App Password atau SMTP relay), sebaiknya dari BWS secret management, bukan hardcode.
|
||||||
|
|
||||||
## 2. verify-email TIDAK memverifikasi OTP (security issue)
|
## 2. ✅ FIXED — verify-email TIDAK memverifikasi OTP (security issue)
|
||||||
|
|
||||||
|
**Status: FIXED di branch feat/dimentorin-postgres (2026-08-04).**
|
||||||
|
|
||||||
`imphnen-iam/src/auth/application/mod.rs` → `verify_email()`:
|
`imphnen-iam/src/auth/application/mod.rs` → `verify_email()`:
|
||||||
|
|
||||||
```rust
|
- OTP sekarang dipersist ke tabel **`app_otp_cache`** (entity baru `imphnen-entities/src/seaorm/common/otp_cache.rs`, resource `app_otp_cache` sudah direncanakan di `ResourceEnum::OtpCache`).
|
||||||
async fn verify_email(&self, payload: VerifyEmailInput) -> ... {
|
- `register()` & `resend_otp()` menyimpan `otp_hash` + `expires_at` setelah email terkirim (kalau email gagal, tidak ada OTP yatim / OTP lama tidak di-overwrite).
|
||||||
let user = ...find_by_email...;
|
- `verify_email()` memanggil `OtpManager::validate_otp_hash(stored_hash, expires_at, payload.otp)` sebelum set `is_active`. `validate_otp_hash` ditambahkan ke `OtpManager` (pure hash+expiry tanpa perlu plaintext code).
|
||||||
if user.is_active { return Err(...); }
|
- OTP **single-use**: di-delete setelah verifikasi sukses. Reuse / OTP tanpa cache / OTP expired semua ditolak (400).
|
||||||
self.user_repo.update(UserEntity { is_active: true, ..user }).await?;
|
- Uji lokal (Postgres, :4099): OTP salah → 400 "Invalid or expired OTP", user tetap inactive; OTP benar → 200 "Email verified successfully", user aktif, OTP dihapus; verify ulang → 400 "User already active"; email tanpa OTP → 400 "No OTP issued".
|
||||||
Ok(())
|
- Tabel dibuat via SQL manual (`create_schema.rs` ditambah `otp_cache` untuk bootstrap penuh).
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
- OTP di-generate (`OtpManager::generate_otp`) + dikirim via email, TAPI **`payload.otp` tidak pernah divalidasi**.
|
|
||||||
- `OtpManager::validate_otp` ada tapi tak dipanggil di handler ini.
|
|
||||||
- **Dampak**: siapa pun yang tahu email bisa mengaktifkan akun dengan otp sembarang. Verifikasi email berbasis kepemilikan = tidak ada.
|
|
||||||
- **Fix yang disarankan**: `verify_email` harus menyimpan `OtpData` (hash+expiry) saat kirim, lalu memanggil `OtpManager::validate_otp(stored, payload.otp)` sebelum set `is_active`.
|
|
||||||
|
|
||||||
## 3. (OK, sudah benar) Register mentor + booking
|
## 3. (OK, sudah benar) Register mentor + booking
|
||||||
|
|
||||||
|
|||||||
@@ -39,6 +39,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
|
|||||||
.await?;
|
.await?;
|
||||||
drop_and_create_table(&db, builder, "rate_limits", common::rate_limit::Entity)
|
drop_and_create_table(&db, builder, "rate_limits", common::rate_limit::Entity)
|
||||||
.await?;
|
.await?;
|
||||||
|
drop_and_create_table(&db, builder, "otp_cache", common::otp_cache::Entity).await?;
|
||||||
|
|
||||||
drop_and_create_table(&db, builder, "gacha_credits", gacha::gacha_credits::Entity)
|
drop_and_create_table(&db, builder, "gacha_credits", gacha::gacha_credits::Entity)
|
||||||
.await?;
|
.await?;
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ pub mod audit_log;
|
|||||||
pub mod enum_impls;
|
pub mod enum_impls;
|
||||||
pub mod enums;
|
pub mod enums;
|
||||||
pub mod events;
|
pub mod events;
|
||||||
|
pub mod otp_cache;
|
||||||
pub mod rate_limit;
|
pub mod rate_limit;
|
||||||
pub mod roadmap_items;
|
pub mod roadmap_items;
|
||||||
pub mod testimonials;
|
pub mod testimonials;
|
||||||
|
|||||||
@@ -0,0 +1,31 @@
|
|||||||
|
use chrono::{DateTime, Utc};
|
||||||
|
use sea_orm::entity::prelude::*;
|
||||||
|
use serde::{Deserialize, Serialize};
|
||||||
|
use uuid::Uuid;
|
||||||
|
|
||||||
|
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, DeriveEntityModel)]
|
||||||
|
#[sea_orm(table_name = "app_otp_cache")]
|
||||||
|
pub struct Model {
|
||||||
|
#[sea_orm(primary_key, default = "gen_random_uuid()", auto_increment = false)]
|
||||||
|
pub id: Uuid,
|
||||||
|
|
||||||
|
#[sea_orm(unique, not_null)]
|
||||||
|
pub email: String,
|
||||||
|
|
||||||
|
#[sea_orm(not_null)]
|
||||||
|
pub otp_hash: String,
|
||||||
|
|
||||||
|
#[sea_orm(not_null)]
|
||||||
|
pub expires_at: DateTime<Utc>,
|
||||||
|
|
||||||
|
#[sea_orm(not_null, default = "now()")]
|
||||||
|
pub created_at: DateTime<Utc>,
|
||||||
|
|
||||||
|
#[sea_orm(not_null, default = "now()")]
|
||||||
|
pub updated_at: DateTime<Utc>,
|
||||||
|
}
|
||||||
|
|
||||||
|
#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)]
|
||||||
|
pub enum Relation {}
|
||||||
|
|
||||||
|
impl ActiveModelBehavior for ActiveModel {}
|
||||||
@@ -1,3 +1,4 @@
|
|||||||
|
use crate::auth::domain::otp::{OtpCacheRecord, OtpRepository};
|
||||||
use crate::auth::domain::AuthService;
|
use crate::auth::domain::AuthService;
|
||||||
use crate::auth::domain::types::{
|
use crate::auth::domain::types::{
|
||||||
AuthTokens, AuthUserDetail, LoginInput, LoginOutput, NewPasswordInput,
|
AuthTokens, AuthUserDetail, LoginInput, LoginOutput, NewPasswordInput,
|
||||||
@@ -22,16 +23,19 @@ use uuid::Uuid;
|
|||||||
pub struct AuthServiceImpl {
|
pub struct AuthServiceImpl {
|
||||||
user_repo: Arc<dyn UserRepository>,
|
user_repo: Arc<dyn UserRepository>,
|
||||||
role_repo: Arc<dyn RoleRepository>,
|
role_repo: Arc<dyn RoleRepository>,
|
||||||
|
otp_repo: Arc<dyn OtpRepository>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl AuthServiceImpl {
|
impl AuthServiceImpl {
|
||||||
pub fn new(
|
pub fn new(
|
||||||
user_repo: Arc<dyn UserRepository>,
|
user_repo: Arc<dyn UserRepository>,
|
||||||
role_repo: Arc<dyn RoleRepository>,
|
role_repo: Arc<dyn RoleRepository>,
|
||||||
|
otp_repo: Arc<dyn OtpRepository>,
|
||||||
) -> Self {
|
) -> Self {
|
||||||
Self {
|
Self {
|
||||||
user_repo,
|
user_repo,
|
||||||
role_repo,
|
role_repo,
|
||||||
|
otp_repo,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -187,6 +191,16 @@ impl AuthService for AuthServiceImpl {
|
|||||||
..Default::default()
|
..Default::default()
|
||||||
})
|
})
|
||||||
.await?;
|
.await?;
|
||||||
|
// Persist OTP only after user creation succeeded, so a failed
|
||||||
|
// registration leaves no orphan OTP record behind.
|
||||||
|
self
|
||||||
|
.otp_repo
|
||||||
|
.save(OtpCacheRecord {
|
||||||
|
email: payload.email.clone(),
|
||||||
|
otp_hash: otp.hash.clone(),
|
||||||
|
expires_at: otp.expires_at,
|
||||||
|
})
|
||||||
|
.await?;
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -203,6 +217,16 @@ impl AuthService for AuthServiceImpl {
|
|||||||
&format!("Your OTP code is {}", otp.code),
|
&format!("Your OTP code is {}", otp.code),
|
||||||
)
|
)
|
||||||
.map_err(|e| AppError::BadRequestError(e.to_string()))?;
|
.map_err(|e| AppError::BadRequestError(e.to_string()))?;
|
||||||
|
// Overwrite stored OTP only after the email was actually sent, so a
|
||||||
|
// failed resend never invalidates the previous (still valid) code.
|
||||||
|
self
|
||||||
|
.otp_repo
|
||||||
|
.save(OtpCacheRecord {
|
||||||
|
email: payload.email.clone(),
|
||||||
|
otp_hash: otp.hash.clone(),
|
||||||
|
expires_at: otp.expires_at,
|
||||||
|
})
|
||||||
|
.await?;
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -265,6 +289,24 @@ impl AuthService for AuthServiceImpl {
|
|||||||
if user.is_active {
|
if user.is_active {
|
||||||
return Err(AppError::BadRequestError("User already active".into()));
|
return Err(AppError::BadRequestError("User already active".into()));
|
||||||
}
|
}
|
||||||
|
let stored = self
|
||||||
|
.otp_repo
|
||||||
|
.find_by_email(&payload.email)
|
||||||
|
.await
|
||||||
|
.map_err(|_| {
|
||||||
|
AppError::BadRequestError(
|
||||||
|
"No OTP issued for this email, request a new code".into(),
|
||||||
|
)
|
||||||
|
})?;
|
||||||
|
if !OtpManager::validate_otp_hash(
|
||||||
|
&stored.otp_hash,
|
||||||
|
&stored.expires_at,
|
||||||
|
payload.otp,
|
||||||
|
) {
|
||||||
|
return Err(AppError::BadRequestError(
|
||||||
|
"Invalid or expired OTP".into(),
|
||||||
|
));
|
||||||
|
}
|
||||||
self
|
self
|
||||||
.user_repo
|
.user_repo
|
||||||
.update(UserEntity {
|
.update(UserEntity {
|
||||||
@@ -272,6 +314,8 @@ impl AuthService for AuthServiceImpl {
|
|||||||
..user
|
..user
|
||||||
})
|
})
|
||||||
.await?;
|
.await?;
|
||||||
|
// OTP is single-use — consume it on successful verification.
|
||||||
|
self.otp_repo.delete_by_email(&payload.email).await?;
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
pub mod otp;
|
||||||
pub mod types;
|
pub mod types;
|
||||||
|
|
||||||
use async_trait::async_trait;
|
use async_trait::async_trait;
|
||||||
|
|||||||
@@ -0,0 +1,18 @@
|
|||||||
|
use async_trait::async_trait;
|
||||||
|
use chrono::{DateTime, Utc};
|
||||||
|
use imphnen_utils::AppError;
|
||||||
|
|
||||||
|
#[derive(Clone, Debug)]
|
||||||
|
pub struct OtpCacheRecord {
|
||||||
|
pub email: String,
|
||||||
|
pub otp_hash: String,
|
||||||
|
pub expires_at: DateTime<Utc>,
|
||||||
|
}
|
||||||
|
|
||||||
|
#[async_trait]
|
||||||
|
pub trait OtpRepository: Send + Sync {
|
||||||
|
/// Upsert OTP record keyed by email (one active OTP per email).
|
||||||
|
async fn save(&self, record: OtpCacheRecord) -> Result<(), AppError>;
|
||||||
|
async fn find_by_email(&self, email: &str) -> Result<OtpCacheRecord, AppError>;
|
||||||
|
async fn delete_by_email(&self, email: &str) -> Result<(), AppError>;
|
||||||
|
}
|
||||||
@@ -4,6 +4,7 @@ use super::handlers::{
|
|||||||
};
|
};
|
||||||
use crate::auth::application::AuthServiceImpl;
|
use crate::auth::application::AuthServiceImpl;
|
||||||
use crate::auth::domain::AuthService;
|
use crate::auth::domain::AuthService;
|
||||||
|
use crate::auth::infrastructure::PostgresOtpRepository;
|
||||||
use crate::roles::infrastructure::persistence::PostgresRoleRepository;
|
use crate::roles::infrastructure::persistence::PostgresRoleRepository;
|
||||||
use crate::users::infrastructure::persistence::PostgresUserRepository;
|
use crate::users::infrastructure::persistence::PostgresUserRepository;
|
||||||
use axum::{Extension, Router, routing::post};
|
use axum::{Extension, Router, routing::post};
|
||||||
@@ -18,8 +19,11 @@ pub fn auth_public_routes(_db: DatabaseConnection, state: Arc<AppState>) -> Rout
|
|||||||
let role_repo = Arc::new(PostgresRoleRepository::new(
|
let role_repo = Arc::new(PostgresRoleRepository::new(
|
||||||
state.postgres_connection.conn.clone(),
|
state.postgres_connection.conn.clone(),
|
||||||
));
|
));
|
||||||
|
let otp_repo = Arc::new(PostgresOtpRepository::new(
|
||||||
|
state.postgres_connection.conn.clone(),
|
||||||
|
));
|
||||||
let auth_service: Arc<dyn AuthService> =
|
let auth_service: Arc<dyn AuthService> =
|
||||||
Arc::new(AuthServiceImpl::new(user_repo, role_repo));
|
Arc::new(AuthServiceImpl::new(user_repo, role_repo, otp_repo));
|
||||||
Router::new()
|
Router::new()
|
||||||
.route("/auth/login", post(post_login))
|
.route("/auth/login", post(post_login))
|
||||||
.route("/auth/login-mentor", post(post_login_mentor))
|
.route("/auth/login-mentor", post(post_login_mentor))
|
||||||
|
|||||||
@@ -1,2 +1,4 @@
|
|||||||
pub mod http;
|
pub mod http;
|
||||||
pub mod persistence;
|
pub mod persistence;
|
||||||
|
|
||||||
|
pub use persistence::postgres_otp_repository::PostgresOtpRepository;
|
||||||
|
|||||||
@@ -1 +1 @@
|
|||||||
|
pub mod postgres_otp_repository;
|
||||||
|
|||||||
@@ -0,0 +1,84 @@
|
|||||||
|
use crate::auth::domain::otp::{OtpCacheRecord, OtpRepository};
|
||||||
|
use async_trait::async_trait;
|
||||||
|
use chrono::Utc;
|
||||||
|
use imphnen_entities::seaorm::common::otp_cache::{
|
||||||
|
ActiveModel as OtpCacheActiveModel, Column as OtpCacheColumn, Entity as OtpCacheEntity,
|
||||||
|
};
|
||||||
|
use imphnen_utils::AppError;
|
||||||
|
use sea_orm::ActiveValue::Set;
|
||||||
|
use sea_orm::prelude::*;
|
||||||
|
use std::sync::Arc;
|
||||||
|
|
||||||
|
pub struct PostgresOtpRepository {
|
||||||
|
db: Arc<DatabaseConnection>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl PostgresOtpRepository {
|
||||||
|
pub fn new(db: DatabaseConnection) -> Self {
|
||||||
|
Self { db: Arc::new(db) }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[async_trait]
|
||||||
|
impl OtpRepository for PostgresOtpRepository {
|
||||||
|
async fn save(&self, record: OtpCacheRecord) -> Result<(), AppError> {
|
||||||
|
let now = Utc::now();
|
||||||
|
let otp_hash = record.otp_hash.clone();
|
||||||
|
let expires_at = record.expires_at;
|
||||||
|
let active_model = OtpCacheActiveModel {
|
||||||
|
id: Set(uuid::Uuid::new_v4()),
|
||||||
|
email: Set(record.email.clone()),
|
||||||
|
otp_hash: Set(otp_hash.clone()),
|
||||||
|
expires_at: Set(expires_at),
|
||||||
|
created_at: Set(now),
|
||||||
|
updated_at: Set(now),
|
||||||
|
};
|
||||||
|
|
||||||
|
// Upsert: replace any existing (possibly expired) OTP for the same email.
|
||||||
|
let existing = OtpCacheEntity::find()
|
||||||
|
.filter(OtpCacheColumn::Email.eq(record.email.clone()))
|
||||||
|
.one(self.db.as_ref())
|
||||||
|
.await
|
||||||
|
.map_err(|e| AppError::InternalServerError(e.to_string()))?;
|
||||||
|
|
||||||
|
if let Some(existing) = existing {
|
||||||
|
let mut update: OtpCacheActiveModel = existing.into();
|
||||||
|
update.otp_hash = Set(otp_hash);
|
||||||
|
update.expires_at = Set(expires_at);
|
||||||
|
update.updated_at = Set(now);
|
||||||
|
update
|
||||||
|
.update(self.db.as_ref())
|
||||||
|
.await
|
||||||
|
.map_err(|e| AppError::InternalServerError(e.to_string()))?;
|
||||||
|
} else {
|
||||||
|
OtpCacheEntity::insert(active_model)
|
||||||
|
.exec(self.db.as_ref())
|
||||||
|
.await
|
||||||
|
.map_err(|e| AppError::InternalServerError(e.to_string()))?;
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn find_by_email(&self, email: &str) -> Result<OtpCacheRecord, AppError> {
|
||||||
|
let row = OtpCacheEntity::find()
|
||||||
|
.filter(OtpCacheColumn::Email.eq(email))
|
||||||
|
.one(self.db.as_ref())
|
||||||
|
.await
|
||||||
|
.map_err(|e| AppError::InternalServerError(e.to_string()))?
|
||||||
|
.ok_or_else(|| AppError::NotFoundError("No OTP issued for this email".into()))?;
|
||||||
|
Ok(OtpCacheRecord {
|
||||||
|
email: row.email,
|
||||||
|
otp_hash: row.otp_hash,
|
||||||
|
expires_at: row.expires_at,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn delete_by_email(&self, email: &str) -> Result<(), AppError> {
|
||||||
|
OtpCacheEntity::delete_many()
|
||||||
|
.filter(OtpCacheColumn::Email.eq(email))
|
||||||
|
.exec(self.db.as_ref())
|
||||||
|
.await
|
||||||
|
.map_err(|e| AppError::InternalServerError(e.to_string()))?;
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -27,14 +27,24 @@ impl OtpManager {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub fn validate_otp(stored: &OtpData, user_otp: u32) -> bool {
|
pub fn validate_otp(stored: &OtpData, user_otp: u32) -> bool {
|
||||||
if Utc::now() > stored.expires_at {
|
Self::validate_otp_hash(&stored.hash, &stored.expires_at, user_otp)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Validate a user-supplied OTP against a stored hash + expiry (e.g. from a
|
||||||
|
/// cache table where the plaintext code is not persisted).
|
||||||
|
pub fn validate_otp_hash(
|
||||||
|
stored_hash: &str,
|
||||||
|
expires_at: &DateTime<Utc>,
|
||||||
|
user_otp: u32,
|
||||||
|
) -> bool {
|
||||||
|
if Utc::now() > *expires_at {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
let user_otp_str = user_otp.to_string();
|
let user_otp_str = user_otp.to_string();
|
||||||
let mut hasher = Sha256::new();
|
let mut hasher = Sha256::new();
|
||||||
hasher.update(user_otp_str.as_bytes());
|
hasher.update(user_otp_str.as_bytes());
|
||||||
let user_hash = format!("{:x}", hasher.finalize());
|
let user_hash = format!("{:x}", hasher.finalize());
|
||||||
user_hash == stored.hash
|
user_hash == stored_hash
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -63,6 +73,37 @@ mod tests {
|
|||||||
assert!(!OtpManager::validate_otp(&otp, 123456));
|
assert!(!OtpManager::validate_otp(&otp, 123456));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_validate_otp_hash_valid() {
|
||||||
|
let otp = OtpManager::generate_otp();
|
||||||
|
assert!(OtpManager::validate_otp_hash(
|
||||||
|
&otp.hash,
|
||||||
|
&otp.expires_at,
|
||||||
|
otp.code
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_validate_otp_hash_invalid() {
|
||||||
|
let otp = OtpManager::generate_otp();
|
||||||
|
assert!(!OtpManager::validate_otp_hash(
|
||||||
|
&otp.hash,
|
||||||
|
&otp.expires_at,
|
||||||
|
123456
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_validate_otp_hash_expired() {
|
||||||
|
let mut otp = OtpManager::generate_otp();
|
||||||
|
otp.expires_at = Utc::now() - chrono::Duration::seconds(1);
|
||||||
|
assert!(!OtpManager::validate_otp_hash(
|
||||||
|
&otp.hash,
|
||||||
|
&otp.expires_at,
|
||||||
|
otp.code
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_validate_otp_expired() {
|
fn test_validate_otp_expired() {
|
||||||
let mut otp = OtpManager::generate_otp();
|
let mut otp = OtpManager::generate_otp();
|
||||||
|
|||||||
Reference in New Issue
Block a user