New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

accounts-base: enroll tokens deleted too soon #8218

Closed
zarvox opened this Issue Jan 4, 2017 · 2 comments

Comments

Projects
None yet
5 participants
@zarvox

zarvox commented Jan 4, 2017

(expanded from #7794 (comment) and filed as a new issue)

#7817 was intended to allow separate timeouts for enrollment tokens and password reset tokens, so enrollment tokens could last for (by default) 30 days, whereas password reset tokens could be made to expire more quickly (default 3 days).

That pull request appears to have a bug which causes enrollment tokens to be cleaned up under the reset token expiry rules. The net effect is that under the default settings, enrollment tokens are destroyed after 3 days, rather than after 30 days. Regardless of what a developer sets passwordEnrollTokenExpirationInDays to in Accounts.config(), enrollment tokens will be removed from mongo after passwordResetTokenExpirationInDays days.

In particular, when processing password reset token expiry at https://github.com/meteor/meteor/pull/7817/files#diff-2ae5b6c36ab4132c1a2d0e33b5fd8443R1158 a tokenFilter with an $or specifying the appropriate token reason is passed to expirePasswordToken, but in https://github.com/meteor/meteor/pull/7817/files#diff-2ae5b6c36ab4132c1a2d0e33b5fd8443R1081 that $or clause is overwritten by the one which only looks at the when field. Perhaps the two clauses should be added to an $and clause instead?

Observed in Meteor 1.4.2.3 (accounts-base@1.2.14), but the bug I've identified is still in master as of this writing.

Reproduction:

  • Enroll a new user with Accounts.sendEnrollmentEmail()
  • Wait N days, with N between 3 and 30, or frob that user's enrollment token's when sufficiently far into the past to be between passwordResetTokenExpirationInDays and passwordEnrollTokenExpirationInDays, e.g. in meteor mongo shell run (with your new userId):
db.users.update({ _id: "p9ZJoQFecMFstfDuv"}, { $set: { "services.password.reset.when": new Date(new Date().getTime() - (4 * 24 * 60  * 60 * 1000)) }})
  • Wait EXPIRE_TOKENS_INTERVAL_MS until the server runs the periodic cleanup function (or trigger it yourself from meteor shell; run Accounts._expirePasswordResetTokens())
  • Expected behavior: enrollment token is still present in user object, since enrollment tokens should be valid for 30 days
  • Observed behavior: enrollment token has been deleted from user object

We're working around this by bumping the validity duration of password reset tokens, but that may not be a palatable option for everyone.

@mutdmour

This comment has been minimized.

Contributor

mutdmour commented Mar 10, 2017

created a pull request solving this bug #8474

abernix added a commit that referenced this issue Mar 29, 2017

[fix #8218] cleaning up reset tokens also cleans up enroll tokens (#8474
)

* solved issue 8218 with expireresettoken removing enroll tokens

* style fix

* tests fix

* solved issue 8218 with expireresettoken removing enroll tokens

* style fix

abernix added a commit that referenced this issue Mar 29, 2017

[fix #8218] cleaning up reset tokens also cleans up enroll tokens (#8474
)

* solved issue 8218 with expireresettoken removing enroll tokens

* style fix

* tests fix

* solved issue 8218 with expireresettoken removing enroll tokens

* style fix
@vikas0121

This comment has been minimized.

vikas0121 commented May 30, 2018

How can I stop this Accounts._expirePasswordResetTokens() ?

@mitar mitar self-assigned this Oct 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment