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

Added a custom connection resolver option #1052

Merged
merged 1 commit into from
Jul 6, 2017

Conversation

luisrudge
Copy link
Contributor

fix #937

@luisrudge luisrudge added this to the v10-Next milestone Jun 29, 2017
@luisrudge luisrudge force-pushed the feature-connection-resolver branch 3 times, most recently from a4d4dfb to 35762d9 Compare June 29, 2017 20:57
@auth0 auth0 locked and limited conversation to collaborators Jun 29, 2017
@luisrudge luisrudge force-pushed the feature-connection-resolver branch from 35762d9 to f06d3a0 Compare July 5, 2017 17:11
README.md Outdated
@@ -292,6 +292,23 @@ var options = {
- **configurationBaseUrl {String}**: Overrides client settings base url. By default it uses Auth0's CDN url when `domain` has the format `*.auth0.com`. Otherwise, it uses the provided `domain`.
- **languageBaseUrl {String}**: Overrides the language source url for Auth0's provided translations. By default it uses to Auth0's CDN url `https://cdn.auth0.com`.
- **hashCleanup {Boolean}**: When enabled, it will remove the hash part of the callback url after the user authentication. Defaults to `true`.
- **connectionResolver {Function}**: When in use, provides an extensibility point to make it possible to chose which connection to use based on the username information. Has `username`, `context` and `callback` as parameters. The callback expects an object like: `{type: 'database', name: 'connection name'}`. **This only works for database connections.**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chose -> choose

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name connectionResolver is ok as long we don't have a provider or similar concept elsewhere. If we do lets be consistent 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only gravatar provider, but it's not part of the public api.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

README.md Outdated
cb({ type: 'database', name: domain });
} else {
// Use the default approach to figure it out the connection
cb(undefined);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather use cb(null) or even cb() and we should handle both cases gracefully

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also in the implementation lets add a warning or timeout if this operation takes too long (could be in a diff PR as long as we add a note saying Keep It Simple)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't really matter what you set here.. null, empty, undefined, false.. everything that evaluates to false will make it behave the same.
I'll add a note in the readme because the timeout will come back to bite us for sure

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok but let's remove the cb(undefined) from he example, and that we have a good error when the object you pass is empty or has a wrong format

@luisrudge luisrudge force-pushed the feature-connection-resolver branch 2 times, most recently from 7da2733 to cde8a9a Compare July 6, 2017 21:43
@luisrudge luisrudge force-pushed the feature-connection-resolver branch from cde8a9a to 9c4c21f Compare July 6, 2017 22:25
@hzalaz hzalaz merged commit 146abae into master Jul 6, 2017
@hzalaz hzalaz deleted the feature-connection-resolver branch July 6, 2017 22:31
@luisrudge luisrudge changed the title Adding a custom connection resolver option Added a custom connection resolver option Jul 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Extensible Connection Name Resolution (HRD)
2 participants