parse-server icon indicating copy to clipboard operation
parse-server copied to clipboard

feat: Add `autoSignupLogin` to signup when user doesn't already exist

Open coratgerl opened this issue 2 weeks ago • 9 comments

Pull Request

Issue

Fixes: https://github.com/parse-community/parse-server/issues/9560

  • [x] Add tests
  • [ ] Add changes to documentation (guides, repository pages, code comments)
  • [ ] Add security check
  • [ ] Add new Parse Error codes to Parse JS SDK

Summary by CodeRabbit

  • New Features

    • Added autoSignupOnLogin option (default: false) to auto-create accounts during login when no match exists; supports username/password and email/password, respects email verification rules, and returns a session token while avoiding duplicates.
  • Tests

    • Added comprehensive tests for auto-signup and verification behaviors (note: the test suite appears duplicated).

✏️ Tip: You can customize this high-level summary in your review settings.

coratgerl avatar Dec 05 '25 19:12 coratgerl

🚀 Thanks for opening this pull request!

[!WARNING]

Rate limit exceeded

@coratgerl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 36 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between f16762421cbf0bdfcdbd4b4bdd53ce86568a8df1 and 1894c600e09c16dfdf0658ce70c36a8593a6f7d1.

📒 Files selected for processing (2)
  • spec/ParseUser.spec.js (1 hunks)
  • src/Routers/UsersRouter.js (5 hunks)
📝 Walkthrough

Walkthrough

Adds a new Parse Server option autoSignupOnLogin, centralizes login payload extraction, and implements an auto-signup path in the login flow that creates a _User when login returns OBJECT_NOT_FOUND. Adds tests validating auto-signup behavior and email verification constraints.

Changes

Cohort / File(s) Summary
Configuration & Types
src/Options/Definitions.js, src/Options/docs.js, src/Options/index.js
Introduce new boolean option autoSignupOnLogin (env: PARSE_SERVER_AUTO_SIGNUP_ON_LOGIN, default: false) in option definitions, docs, and interface/type declarations.
Login Router & Flow
src/Routers/UsersRouter.js
Add _getLoginPayload(req) to extract/validate login credentials; add async _resolveEmailVerificationFlags(req, userObj) to centralize verification flag resolution; add async _autoSignupOnLogin(req) to create a _User when no user is found and enforce verification rules; refactor _authenticateUserFromRequest(req) to use the payload extractor; update handleLogIn(req) to catch OBJECT_NOT_FOUND and delegate to auto-signup when enabled; conditionally skip standard session creation when _autoSignupOnLogin returns a session token.
Tests
spec/ParseUser.spec.js
Add "autoSignupOnLogin option" test suite covering disabled behavior (OBJECT_NOT_FOUND, no user created), enabled behavior (creates user and returns session for username/password and email/password, email used as username), no duplicate creation when user exists, and enforcement of email verification constraints. (Patch contains duplicate copy of the same test block.)

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant LoginEndpoint as Login Endpoint
    participant Auth as _authenticateUserFromRequest
    participant AutoSignup as _autoSignupOnLogin
    participant RestWrite as RestWrite (Create _User)
    participant DB as User Database

    Client->>LoginEndpoint: POST /login (username/email + password)
    LoginEndpoint->>LoginEndpoint: _getLoginPayload(req)
    LoginEndpoint->>Auth: _authenticateUserFromRequest(req)
    Auth->>DB: Query user by username/email
    alt User Found
        DB-->>Auth: User record
        Auth-->>LoginEndpoint: Auth success
        LoginEndpoint-->>Client: 200 OK (user + session)
    else User Not Found (OBJECT_NOT_FOUND)
        DB-->>Auth: No user
        Auth-->>LoginEndpoint: Error: OBJECT_NOT_FOUND
        alt autoSignupOnLogin = true
            LoginEndpoint->>AutoSignup: _autoSignupOnLogin(req)
            AutoSignup->>AutoSignup: Build new _User payload (username/email, password)
            AutoSignup->>RestWrite: Create _User
            RestWrite->>DB: Insert user
            DB-->>RestWrite: Insert result
            AutoSignup->>DB: Fetch created user (fresh)
            DB-->>AutoSignup: User record
            AutoSignup->>AutoSignup: Enforce email verification rules
            AutoSignup-->>LoginEndpoint: { user, sessionToken, authDataResponse }
            LoginEndpoint-->>Client: 200 OK (new user + session)
        else autoSignupOnLogin = false
            LoginEndpoint-->>Client: 400 / OBJECT_NOT_FOUND
        end
    end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pay attention to:
    • src/Routers/UsersRouter.js — try/catch flow around login, correctness of session creation/skipping, and interplay with authData handling.
    • Email verification logic in _autoSignupOnLogin and _resolveEmailVerificationFlags, including async option handling and behavior when preventLoginWithUnverifiedEmail / verifyUserEmails are configured.
    • Tests in spec/ParseUser.spec.js — duplication of the added test block and correctness of assertions against the new flows.

Suggested reviewers

  • mtrezza

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive Title references autoSignupLogin but the feature is named autoSignupOnLogin throughout the codebase and issue; the title's terminology is slightly inconsistent. Clarify whether the feature should be called autoSignupLogin or autoSignupOnLogin to match issue #9560 and codebase naming conventions.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed PR description includes the required issue link (Fixes: #9560) and checked 'Add tests', but 'Add changes to documentation', 'Add security check', and error code tasks are unchecked.
Linked Issues check ✅ Passed Changes implement autoSignupOnLogin option, create users on login miss, enforce email verification rules, and add comprehensive tests covering all specified requirements from issue #9560.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the autoSignupOnLogin feature; no unrelated or out-of-scope modifications detected in files changed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Dec 05 '25 19:12 coderabbitai[bot]

:white_check_mark: Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
:white_check_mark: Open Source Security 0 0 0 0 0 issues

:computer: Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

parseplatformorg avatar Dec 05 '25 19:12 parseplatformorg

Codecov Report

:x: Patch coverage is 92.85714% with 4 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 92.57%. Comparing base (e78e58d) to head (1894c60).

Files with missing lines Patch % Lines
src/Routers/UsersRouter.js 92.85% 4 Missing :warning:
Additional details and impacted files
@@           Coverage Diff           @@
##            alpha    #9961   +/-   ##
=======================================
  Coverage   92.56%   92.57%           
=======================================
  Files         191      191           
  Lines       15544    15582   +38     
  Branches      177      177           
=======================================
+ Hits        14389    14425   +36     
- Misses       1143     1145    +2     
  Partials       12       12           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Dec 05 '25 19:12 codecov[bot]

@coratgerl mind that there is an existing PR for this feature with a discussion about the approach. I'm also unsure about the state of the other PR, it may even be finished.

mtrezza avatar Dec 05 '25 20:12 mtrezza

@coratgerl mind that there is an existing PR for this feature with a discussion about the approach. I'm also unsure about the state of the other PR, it may even be finished.

Hum, i checking the other PR and seems inactive since 2 months so I propose an approach here with edges cases tests like asked by @Moumouls. Like tests based on preventLoginWithUnverifiedEmail, on the clean of session and user creation if error etc.

coratgerl avatar Dec 05 '25 20:12 coratgerl

The other PR author has asked if any more changes were needed, but there wasn't any specific change request. It would be good to ask the other PR author if they're interested in continuing the PR, provided that we give a concrete feedback on what needs to change.

mtrezza avatar Dec 05 '25 20:12 mtrezza

The other PR author has asked if any more changes were needed, but there wasn't any specific change request. It would be good to ask the other PR author if they're interested in continuing the PR, provided that we give a concrete feedback on what needs to change.

Ok ! We can wait any response of other contributor and otherwise we can move forward here 🙂

coratgerl avatar Dec 05 '25 20:12 coratgerl

Yes, but we'll have the same open questions. The reason the other PR got stale was because the questions were not resolved. Let's continue the discussion there and if the PR author abandoned the PR let's continue here.

mtrezza avatar Dec 05 '25 20:12 mtrezza