-
Notifications
You must be signed in to change notification settings - Fork 556
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
[SDK-2306] Add login and signup hooks #1976
Conversation
889aa27
to
678112d
Compare
const publicHooks = l.hooks(m).toJS(); | ||
|
||
if (l.validPublicHooks.indexOf(str) !== -1) { | ||
// If the SDK has been configured with a hook handler, run it. | ||
if (typeof publicHooks[str] === 'function') { | ||
publicHooks[str](...args); | ||
return m; | ||
} | ||
|
||
// Ensure the hook callback function is executed in the absence of a hook handler, | ||
// so that execution may continue. | ||
if (typeof args[1] === 'function') { | ||
args[1](); | ||
} | ||
|
||
return m; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lock already had an internal "hooks" mechanism that wasn't available to be used outside of the library. This change piggy-backs onto that and defers to a publicly-registered hook if one is available. Otherwise, it runs the private internal hook if available.
if (typeof args[1] === 'function') { | ||
args[1](); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Runs args[1]
since the inputs to a public hook are the context and the callback function, in that order.
// Custom rule error (except blocked_user) | ||
if (error.code === 'rule_error') { | ||
// Custom rule or hook error (except blocked_user) | ||
if (error.code === 'rule_error' || error.code === 'hook_error') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hooks: {
loggingIn: function(context, done) {
throw { code: 'hook_error', description: 'can you put <i>HTML</i> in here?' };
},
I wonder if this is a risk, given that a hook_error
is more likely to have reflected user input than a rule_error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good point. Currently you could put HTML in there, but for that to be reflected from user input the developer would have to specifically get it from the HTML within their hook, we don't give you those values in the context.
However, in the future there may be a case for making those available where HTML could inadvertently be written out, so we could clamp that down now and encode HTML when the error is output. Let me add that to the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see a use case for developer's wanting to display links in here. Seeing as the developer has to go out of their way to reflect user input in this field, let's cover it with guidance and make people aware that this will display HTML if you give it HTML.
Covered in ba9950a
Changes
This PR adds new functionality to add two async hooks that the developer can use to hook into the form submission process on login and signUp. The developer can handle these events then call a callback function to signal that their own processing has been completed before continuing. Or, they can throw an error which can be displayed on the Lock form that then cancels the login or signup process.
Available hooks
loggingIn
- called when the user presses the login button; after validating the login form, but before calling the login endpointsigningUp
- called when the user presses the button on the sign-up page; after validating the signup form, but before calling the sign up endpointBoth hooks accept two arguments:
context
- this argument is currently alwaysnull
but serves as a future-proofing mechanism to support providing additional data without us requiring breaking changes to the librarycb
- a callback function to call when the hook is finished. Execution of the login or signup process is blocked until this is calledAPI
Error handling
The developer can throw an error to block the login or sign-up process. The developer can either specify a specific object and show the error on the page, or throw a generic error which causes Lock to show a fallback error:
Testing
Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.
Checklist