mORMot Open Source

testgary

Member

Registered: 2025-02-06

Posts: 45

Need help

hi,ab

I've written code for a DDD-like framework to handle user registration and login, but there are some issues. I'm still learning, and some parts aren't fully understood yet. I considered consulting “AB,” but I haven't adopted the TDD approach you mentioned—it feels too cumbersome.

Code link:
https://github.com/pit500081/mormot2test

    1. Is it appropriate to use TUserRec for data transfer between layers?
2. Is it reasonable to store the verification code in CookieData and send it to the frontend via Base64?
3. Would it be sound to design two separate login modules handling API services and the web frontend respectively?
4. Is my current implementation approach correct?

Please also help review for any other potential pitfalls I may have overlooked. Thank you very much!

Last edited by testgary (Yesterday 06:36:59)

zen010101

Member

Registered: 2024-06-15

Posts: 159

Re: Need help

● Hi testgary,

  I reviewed your code using Claude and created a feedback issue on your repository:
  https://github.com/pit500081/mormot2test/issues/1

  Summary of your 4 questions:

  Q1 - TUserRec for inter-layer communication:
  Good approach. Separating TOrmUser from DTO is correct. However, TUserRec has 23 fields serving too many purposes. Consider splitting into smaller DTOs like TUserProfileRec, TUserLoginResultRec, etc.

  Q2 - Captcha in CookieData:
  Security concern. Captcha text should NOT be stored client-side (even encrypted). Move to server-side storage using TSynDictionary (Token → CaptchaText mapping). Delete after verification to prevent replay attacks.

  Q3 - Separate modules for API and Web:
  Architecture is correct - both layers sharing IDomainUserManager is the right design. However:
 

• REST layer (TRestUserManager.Register) is empty - needs implementation

• MVC layer is too bloated - Register/Login methods handle captcha, session, validation, response formatting all in one. Extract into separate services (TCaptchaService, etc.) for better testability

  Q4 - Overall strategy:
  Solid foundation. Well done:
 

• Password hashing with mcfSCrypt

• Layered architecture (Repo → Domain → Presentation)

• Dependency injection via interfaces

• Extends TAuthUser properly

  Issues to fix:
 

• GetGroupEnum bug in user.repo.impl.pas:55-56 - SizeOf(TUserGroupTypeEnum) returns 1, not enum count

• LoginAt/LoginIp not updated on successful login

• No brute-force protection - add login attempt rate limiting

  The architecture is sound - these are refinements rather than fundamental changes.

testgary

Member

Registered: 2025-02-06

Posts: 45

Re: Need help

hi,zen010101

TOrmUser.ComputeFieldsBeforeWrite

Is the code in my method appropriate and correct?

Last edited by testgary (Yesterday 13:32:37)

zen010101

Member

Registered: 2024-06-15

Posts: 159

Re: Need help

I see your ComputeFieldsBeforeWrite implementation:

  NextID := aRest.TableMaxID(TOrmUser) + 1;
  if (aOccasion = oeAdd) and ((FInviteCode = '') or (NextID <= 3)) then
    FInviteCode := GenerateUniqueInviteCode(NextID);

  Issues:

  1. Race condition — TableMaxID + 1 is unreliable under concurrent inserts. Two users registering simultaneously may get the same NextID, causing duplicate invite codes.
  2. Database query in ComputeFieldsBeforeWrite — This method is called during the write operation. Querying TableMaxID here adds latency and potential deadlocks.

  Recommended fix:

  Generate the invite code after the record is inserted, when you have the actual ID:

  // In your registration service:
  function TDomainUserManager.RegisterUser(...): TID;
  begin
    Result := fOrm.Add(aUser, True);  // Insert first, get real ID
    if Result > 0 then
    begin
      aUser.IDValue := Result;
      aUser.InviteCode := GenerateUniqueInviteCode(Result);
      fOrm.Update(aUser, 'InviteCode');  // Update only this field
    end;
  end;

  Or use a UUID-based invite code that doesn't depend on ID:

  FInviteCode := Int64ToHex(Random64) + Int64ToHex(Random64);  // 32-char hex

  Also, what's the purpose of NextID <= 3? If it's for seeding admin accounts, consider handling that separately in InitializeTable.

zen010101

Member

Registered: 2024-06-15

Posts: 159

Re: Need help

These questions are more about your application's business logic and architecture design rather than mORMot framework usage itself.

  For general code review and architecture advice, you might consider:
  - Stack Overflow / Code Review Stack Exchange
  - Hiring a consultant
  - Reading DDD/Clean Architecture books

  The mORMot forum is best suited for questions about how to use the framework's APIs and features.

  That said, regarding ComputeFieldsBeforeWrite specifically: the mORMot documentation shows it's intended for lightweight field computation (timestamps, derived values). Querying the database inside it is not recommended.