brettle/meteor-accounts-add-service

Accounts.onCreateNewUser, Accounts.onValidateNewUser, and Accounts.onValidateLoginAttempt called when service is added

Opened this issue · 2 comments

Currently, the service is added very late in the login process, essentially in an Accounts.onLoginFailure handler. At that point, a new account has been created (using the Accounts.onCreateNewUser handler, if specified), the new account has been validated by any Accounts.onValidateNewUser handlers, and logging in to that account has been validated by any Accounts.onValidateLoginAttempt handlers. Since the goal is just to add the login service to the current user's account, it's not clear whether this is the best approach. In particular, the current approach has the following effects:

  1. Since it deletes the new account after adding the service to the logged in user's current account, it leaves dangling any references to the new account that are created by the aforementioned handlers. A potential workaround is to use Meteor.users.observe() to watch for removal of a user and cleanup anything that refers to the removed user.
  2. The attempt.user passed to Accounts.onValidateLoginAttempt handlers refers to the new account instead of the account that the user will actually be logged in to (i.e. the one he is already logged in to). This could cause the handler to incorrectly validate or invalidate the login attempt. A potential workaround is to change the handler to use both attempt.user and Meteor.user() when making validation decisions.

Potential fix #1: Move the service-adding code to an Accounts.validateNewUser handler that adds the service associated with the new user to Meteor.user() and then throws an error indicating that the service was added instead of a new account being created (like it currently does). This would mean that onValidateLoginAttempt handlers would not be run, but it's not clear that they should be since the user was already validly logged in using their existing login token and we wouldn't be issuing a new token.

Potential fix #2: Move the service-adding code into a monkey patched Accounts.insertUserDoc. The service-adding code would be triggered when a login attempt is in progress and Meteor.user() !== null and it would just add the service to the current user's account. It would not run the onCreateUser handler or the onValidateNewUser handlers (because a new user was not created). The onValidateLoginAttempt handlers would run, but they'd run with the correct attempt.user === Meteor.user() (which seems correct) and a new login token would be issued if the login was valid.

For both fixes, the app developer should probably have some way to control under what conditions a new login service can be added to an existing account. Maybe we provide onValidateAddService?

Fix #2 would not permit the flow where the client creates the new account using a non-login method and then logs the user into the account in a separate login call. This is what useraccounts does, for example.

The aforementioned commit message was wrong. It does not address this issue.