Skip to content

Conversation

@TaprootFreak
Copy link
Collaborator

Summary

  • Fix mail login users having kycLevel 0 despite verified email
  • Add initializeProgress() method to KycService
  • Call it in completeSignInByMail() to trigger KYC flow

Problem

Mail login users had kycLevel = 0 even though their email was verified via OTP. This caused:

  • EmailRequired error when trying to buy (instead of RecommendationRequired)
  • Inconsistent behavior compared to wallet login with email

Root Cause

The KYC flow was never triggered after mail login:

  1. User logs in via mail → createUserData() with mail set
  2. completeSignInByMail() generates token and redirects
  3. No KYC flow triggered → No CONTACT_DATA step created → kycLevel stays 0

Solution

Call kycService.initializeProgress(account) in completeSignInByMail():

  1. initializeProgress() triggers updateProgress()
  2. CONTACT_DATA step is initiated
  3. Since user.mail exists → step auto-completes via trySetMail()
  4. PERSONAL_DATA becomes next step → kycLevel set to 10

Test plan

  • Mail login with new account → verify kycLevel = 10
  • Mail login with existing account → verify kycLevel unchanged if already ≥ 10
  • Verify CONTACT_DATA step is created and completed
  • Verify buy flow shows "RecommendationRequired" instead of "EmailRequired"

Mail login users had kycLevel 0 even though their email was verified
via OTP. This happened because the KYC flow was never triggered after
mail login, leaving CONTACT_DATA step uncompleted.

Changes:
- Add initializeProgress() method to KycService that triggers
  updateProgress() for a given user
- Call initializeProgress() in completeSignInByMail() after successful
  authentication

Now when a user completes mail login:
1. initializeProgress() triggers updateProgress()
2. CONTACT_DATA step is auto-completed (user.mail exists)
3. PERSONAL_DATA becomes next step → kycLevel set to 10

This makes mail login behavior consistent with wallet login where
adding an email triggers the same KYC flow.
@github-actions
Copy link

github-actions bot commented Jan 9, 2026

⚠️ Security: 0 critical, 65 high vulnerabilities

- Set shouldContinue=false to only set kycLevel without initiating
  next KYC steps (PERSONAL_DATA)
- Add Util.retry() with duplicate key check for race conditions
  (e.g., user double-clicks OTP link)
- Make KYC initialization non-blocking in completeSignInByMail()
  so login succeeds even if KYC init fails
The previous fix with shouldContinue=false was incorrect - it prevented
any KYC progress from happening because CONTACT_DATA doesn't return a
nextLevel value.

The correct solution is shouldContinue=true, autoStep=false:
- shouldContinue=true: allows CONTACT_DATA to be initiated and auto-completed
- autoStep=false: prevents PERSONAL_DATA from being initiated (depth > 0)

Flow:
1. depth=0: (autoStep || depth===0) = true → CONTACT_DATA initiated/completed
2. depth=1: (autoStep || depth===0) = false → Level 10 set, no further steps
Prevents unintentionally initiating PERSONAL_DATA step for returning
users who already have CONTACT_DATA completed. The level should already
be set for these users.
4 active users have CONTACT_DATA completed but kycLevel = 0.
This SQL script updates their level to 10.

Affected user IDs: 257036, 229330, 1158, 1058
- Comment out UPDATE statement (must be uncommented manually)
- Add transaction wrapper (BEGIN/COMMIT/ROLLBACK)
- Use JOIN-based UPDATE syntax for SQL Server
- Add clear step-by-step instructions
- Add row count verification check
@TaprootFreak
Copy link
Collaborator Author

🐛 Pre-existing Bug entdeckt: autoTradeApproval Check funktioniert nicht für Mail-Login

Problem

In completeSignInByMail() wird die wallet Relation nicht geladen, wodurch der autoTradeApproval Check in checkPendingRecommendation() fehlschlägt:

// auth.service.ts:306
const account = await this.userDataService.getUserData(entry.userDataId, { users: true });
// ❌ wallet wird NICHT geladen!

// auth.service.ts:318
if (!account.tradeApprovalDate) await this.checkPendingRecommendation(account);
// ❌ Ohne zweiten Parameter (userWallet)!

Im Vergleich dazu laden andere Login-Methoden die wallet korrekt:

// Wallet-Login (signIn) - Zeile 179:
await this.checkPendingRecommendation(user.userData, wallet);  // ✅ wallet übergeben

// Wallet-Login (signInWithKey) - Zeile 216:
await this.checkPendingRecommendation(user.userData, user.wallet);  // ✅ wallet übergeben

// Mail-Login (completeSignInByMail) - Zeile 318:
await this.checkPendingRecommendation(account);  // ❌ wallet NICHT übergeben!

Auswirkung

checkPendingRecommendation() prüft:

if (userData.wallet?.autoTradeApproval || userWallet?.autoTradeApproval)
  await this.userDataService.updateUserDataInternal(userData, { tradeApprovalDate: new Date() });

Da userData.wallet nicht geladen und userWallet nicht übergeben wird, wird tradeApprovalDate nie automatisch gesetzt, selbst wenn das Wallet autoTradeApproval = true hat.

Betroffene User (Produktion)

SELECT COUNT(*) FROM user_data ud 
INNER JOIN wallet w ON ud.walletId = w.id 
WHERE ud.status = 'KycOnly' 
  AND w.autoTradeApproval = 1 
  AND ud.tradeApprovalDate IS NULL
-- Ergebnis: 1 User (ID: 242230, CakeWallet)

Warum kein Impact für die meisten Mail-Login User?

Das Default-Wallet (ID 1, "DFX Wallet") hat autoTradeApproval = false. Daher sind nur User betroffen, deren userData mit einem Wallet verknüpft ist, das autoTradeApproval = true hat (z.B. CakeWallet, OnRamper, etc.).

Empfohlener Fix (separater PR)

// Option A: wallet Relation laden
const account = await this.userDataService.getUserData(entry.userDataId, { users: true, wallet: true });
if (!account.tradeApprovalDate) await this.checkPendingRecommendation(account);

// Option B: wallet explizit übergeben (wie bei anderen Login-Methoden)
const account = await this.userDataService.getUserData(entry.userDataId, { users: true, wallet: true });
if (!account.tradeApprovalDate) await this.checkPendingRecommendation(account, account.wallet);

Fazit

Dieser Bug ist pre-existierend und nicht Teil dieses PRs. Der Impact ist minimal (1 User), aber der Bug sollte in einem separaten PR gefixt werden.

@TaprootFreak TaprootFreak merged commit d31c779 into develop Jan 12, 2026
7 checks passed
@TaprootFreak TaprootFreak deleted the fix/mail-login-kyc-level-initialization branch January 12, 2026 12:01
TaprootFreak added a commit that referenced this pull request Jan 12, 2026
* chore: rename master branch references to main (#2900)

* chore: rename master branch references to main

Update all workflow files to use 'main' instead of 'master':
- api-prd.yaml: trigger on main branch
- api-pr.yaml: run PR checks for main branch
- codeql.yml: scan main branch
- auto-release-pr.yaml: create release PRs to main

* docs: fix CitreaScan branch reference (master -> main)

* fix: load wallet relation for autoTradeApproval check in mail login (#2904)

In completeSignInByMail(), the wallet relation was not loaded when
fetching userData, causing the autoTradeApproval check in
checkPendingRecommendation() to always fail.

Changes:
- Add wallet to relations in getUserData() call
- Pass account.wallet to checkPendingRecommendation()

This aligns mail-login with wallet-login behavior where the wallet
is properly passed to checkPendingRecommendation().

* feat: improve local development experience for mail handling (#2905)

- Skip mail sending in local environment and log mail details instead
- Log mail login URL in local environment for easy testing
- Add SERVICES_URL to .env.local.example for complete login URLs

* fix: initialize KYC progress on mail login to set kycLevel 10 (#2903)

* fix: initialize KYC progress on mail login to set kycLevel 10

Mail login users had kycLevel 0 even though their email was verified
via OTP. This happened because the KYC flow was never triggered after
mail login, leaving CONTACT_DATA step uncompleted.

Changes:
- Add initializeProgress() method to KycService that triggers
  updateProgress() for a given user
- Call initializeProgress() in completeSignInByMail() after successful
  authentication

Now when a user completes mail login:
1. initializeProgress() triggers updateProgress()
2. CONTACT_DATA step is auto-completed (user.mail exists)
3. PERSONAL_DATA becomes next step → kycLevel set to 10

This makes mail login behavior consistent with wallet login where
adding an email triggers the same KYC flow.

* fix: improve initializeProgress with retry logic and error handling

- Set shouldContinue=false to only set kycLevel without initiating
  next KYC steps (PERSONAL_DATA)
- Add Util.retry() with duplicate key check for race conditions
  (e.g., user double-clicks OTP link)
- Make KYC initialization non-blocking in completeSignInByMail()
  so login succeeds even if KYC init fails

* fix: correct initializeProgress to use autoStep=false

The previous fix with shouldContinue=false was incorrect - it prevented
any KYC progress from happening because CONTACT_DATA doesn't return a
nextLevel value.

The correct solution is shouldContinue=true, autoStep=false:
- shouldContinue=true: allows CONTACT_DATA to be initiated and auto-completed
- autoStep=false: prevents PERSONAL_DATA from being initiated (depth > 0)

Flow:
1. depth=0: (autoStep || depth===0) = true → CONTACT_DATA initiated/completed
2. depth=1: (autoStep || depth===0) = false → Level 10 set, no further steps

* fix: skip initializeProgress for users with CONTACT_DATA completed

Prevents unintentionally initiating PERSONAL_DATA step for returning
users who already have CONTACT_DATA completed. The level should already
be set for these users.

* chore: add migration script to fix kycLevel for edge case users

4 active users have CONTACT_DATA completed but kycLevel = 0.
This SQL script updates their level to 10.

Affected user IDs: 257036, 229330, 1158, 1058

* fix: make migration script safer

- Comment out UPDATE statement (must be uncommented manually)
- Add transaction wrapper (BEGIN/COMMIT/ROLLBACK)
- Use JOIN-based UPDATE syntax for SQL Server
- Add clear step-by-step instructions
- Add row count verification check

* fix: Start KYC process on mail add

* fix: script executed

---------

Co-authored-by: David May <david.leo.may@gmail.com>

---------

Co-authored-by: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com>
Co-authored-by: bernd2022 <104787072+bernd2022@users.noreply.github.com>
Co-authored-by: David May <david.leo.may@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants