Use resolvedConnection where available

[lukevmorris]

Looks like the connectionResolver option was added in 10.19 to allow control over which connection to authenticate against during logIn. We've been using this feature since its release, and it's worked well so far.

We've noticed that resetPassword does not respect this function, and this can result in some really confusing behavior. For users whose default connection is overridden by the connectionResolver, they could be logging into one connection but resetting their password in another. And so their interaction with the Lock might go as follows:

• Log in for the first time in a month and don't really remember their password. They try their default. Incorrect.
• They click "Forgot your password?" and trigger a password reset email. They click on the link in the email and submit a new password.
• They try to log in again with their new password. Incorrect.
• Typically we get a support email after this.

I looked at the original connectionResolver PR (#1052) to get my bearings in the project. I was hoping to mirror some test cases for resetPassword, but I only saw new pane tests, which I believe should remain unchanged in this PR. Let me know if there are any changes you'd like to see here, and I'll try to update ASAP.

Thanks!

[luisrudge]

Thanks for the PR. Do you think it makes sense to add this to the signup as well?

[lukevmorris]

@luisrudge Hmm -- probably. If I think back to when I implemented my connectionResolver, I guess I expected it to work throughout the Lock. So it would probably help to clear up some developer confusion in the future if signUp used it as well. I'll add that in.

[luisrudge]

Great! What did you mean about the tests? Why can't you add tests for this?

[lukevmorris]

I'm happy to add tests, I am just not sure where in the project I should add them. Would you be able to direct me?

[luisrudge]

actually, now that I'm taking a look at this, this PR will fix the same issue. we're only missing two tests (here) to merge it. Can you help with those tests? I'm focusing on the other PRs right now.

The PR changes from resolving the connection onBlur to onSubmit and will make all screens resolve the connection before submitting (will fix forgot password, signup etc). Let's concentrate our efforts in that PR instead, what do you think?

[lukevmorris]

I might be wrong, but my current understanding is that this PR is orthogonal to #1087.

It looks like the onBlur / onSubmit callback only sets the resolved connection. But that resolved connection gets ignored unless explicitly asked for in the functions in database/actions.js, with l.resolvedConnection(lock). Before this PR, logIn calls that function, but signUp and resetPassword do not.

[luisrudge]

You're totally correct. I was taking a look at the PR and thought it was fixing the same issue. Sorry about that.

[luisrudge]

You can't directly access .name here because it's an Immutable object.
it should be someting like

const connection = customResolvedConnection || standardConnection || new Map();
return connection.get('name');

[luisrudge]

.

[lukevmorris]

Gotcha

wrong review

[luisrudge]

.

[lukevmorris]

It looks like I need to push this logic further to the core. There are other functions like databaseConnectionRequiresUsername that need access to the resolved connection as well.

[luisrudge]

you can rollback this changes entirely

[lukevmorris]

okiedoke

[lukevmorris]

Ok -- I believe comments are addressed.

[luisrudge]

Thanks for the PR and patience 🎉

[lukevmorris]

Thanks very much @luisrudge!