Skip to content
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

Pass login type to client side Accounts.onLogin callbacks #9512

Merged
merged 2 commits into from Jan 10, 2018

Conversation

@hwillson
Copy link
Member

@hwillson hwillson commented Jan 3, 2018

Client side Accounts.onLogin callbacks are triggered after a successful login, but they're also triggered after a successful DDP reconnect (if already logged in). As discussed in #5127, some people think this is the correct behaviour, while others feel that onLogin callbacks should really only fire after a user has actually logged in (e.g. after using something like Meteor.loginWithPassword). Since people are split on the proper approach here, an alternative solution would be to provide client side Accounts.onLogin callbacks with a way to tell if they're being called after a login or after a reconnect, then let them decide what to do.

Server side Accounts.onLogin callbacks receive an object that contains various login details. One of those items is a login type, that can hold values like password, resume, etc. When a user completes a true password based login, the returned login type is password. When the user is already logged in but undergoes a DDP disconnect + reconnect, the returned login type is resume. This means server side Accounts.onLogin callbacks have a way to tell which type of login is happening, allowing them to respond accordingly.

This PR adjusts client side Accounts.onLogin callbacks such that they also receive a single login details object, that contains login type information. Unlike the server side login details object, the client side login details object only contains the login type (to help reduce network transfer overhead, make sure we're not exposing server details on the client we shouldn't be, etc.).

This approach should give developers a better way to respond to different client side login types, and act accordingly.

Fixes #5127.

Client side `Accounts.onLogin` callbacks are triggered after a
successful login, but they're also triggered after a successful
DDP reconnect (if already logged in). As discussed in #5127,
some people think this is the correct behaviour, while others
feel that `onLogin` callbacks should really only fire after a
user has actually logged in (e.g. after using something like
`Meteor.loginWithPassword`). Since people are split on the
proper approach here, an alternative solution would be to
provide client side `Accounts.onLogin` callbacks with a way
to tell if they're being called after a login or after a
reconnect, then let them decide what to do.

Server side `Accounts.onLogin` callbacks receive an object that
contains various login details. One of those items is a login
`type`, that can hold values like `password`, `resume`, etc.
When a user completes a true password based login, the returned
login `type` is `password`. When the user is already logged in
but undergoes a DDP disconnect + reconnect, the returned login
`type` is `resume`. This means server side `Accounts.onLogin`
callbacks have a way to tell which type of login is happening,
allowing them to respond accordingly.

This PR adjusts client side `Accounts.onLogin` callbacks such
that they also receive a single login details object, that
contains login type information. Unlike the server side login
details object, the client side login details object only
contains the login type (to help reduce network transfer
overhead, make sure we're not exposing server details on the
client we shouldn't be, etc.).

This approach should give developers a better way to respond to
different client side login types, and act accordingly.

Fixes #5127.
Copy link
Member

@abernix abernix left a comment

LGTM!

@abernix abernix added this to the Package Patches milestone Jan 10, 2018
@abernix abernix merged commit f551a56 into meteor:devel Jan 10, 2018
12 checks passed
12 checks passed
CLA Author has signed the Meteor CLA.
Details
ci/circleci: Get Ready Your tests passed on CircleCI!
Details
ci/circleci: Group 0 Your tests passed on CircleCI!
Details
ci/circleci: Group 1 Your tests passed on CircleCI!
Details
ci/circleci: Group 2 Your tests passed on CircleCI!
Details
ci/circleci: Group 3 Your tests passed on CircleCI!
Details
ci/circleci: Group 4 Your tests passed on CircleCI!
Details
ci/circleci: Group 5 Your tests passed on CircleCI!
Details
ci/circleci: Group 6 Your tests passed on CircleCI!
Details
ci/circleci: Group 7 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abernix
Copy link
Member

@abernix abernix commented Jan 10, 2018

Published as accounts-base@1.4.2!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants