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

feat(guardian): support policies, selected-provider, message-types methods [MFA-310] #501

Merged
merged 3 commits into from Jun 30, 2020

Conversation

pmalouin
Copy link
Contributor

@pmalouin pmalouin commented Jun 19, 2020

Changes

Adding support for 6 Management API endpoints, related to Guardian/MFA features. Here is the list of API endpoints and their related method added to this SDK:

  • GET /api/v2/guardian/policies
    • management.guardian.getPolicies()
    • management.getGuardianPolicies() (alias)
  • PUT /api/v2/guardian/policies
    • management.guardian.updatePolicies()
    • management.updateGuardianPolicies() (alias)
  • GET /api/v2/guardian/factors/sms/selected-provider
    • management.guardian.getPhoneFactorSelectedProvider()
    • management.getGuardianPhoneFactorSelectedProvider() (alias)
  • PUT /api/v2/guardian/factors/sms/selected-provider
    • management.guardian.updatePhoneFactorSelectedProvider()
    • management.updateGuardianPhoneFactorSelectedProvider() (alias)
  • GET /api/v2/guardian/factors/phone/message-types
    • management.guardian.getPhoneFactorMessageTypes()
    • management.getGuardianPhoneFactorMessageTypes() (alias)
  • PUT /api/v2/guardian/factors/phone/message-types
    • management.guardian.updatePhoneFactorMessageTypes()
    • management.updateGuardianPhoneFactorMessageTypes() (alias)

These endpoints are going to be publicly documented in our API explorer once we kick off our Voice MFA beta program.

References

https://auth0team.atlassian.net/browse/MFA-310

Testing

  • Do a manual round of testing by calling the methods.

  • Generate the jsdoc (npm run jsdoc:generate) and browse the resulting documentation website to ensure the methods are clearly documented.

  • This change adds unit test coverage

  • This change adds integration test coverage

Checklist

* @memberOf module:management.GuardianManager.prototype
*
* @example
* management.guardian.updatePolicies({}, [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first argument here is the generic "params", that we have no use for. But since this is a convention for all GET endpoints, it seems that it is preferable to keep it here.

See also other methods that have the same approach:

  • /**
    * Update the prompts settings.
    *
    * @method updateSettings
    * @memberOf module:management.PromptsManager.prototype
    *
    * @example
    * management.prompts.updateSettings(data, function (err, prompts) {
    * if (err) {
    * // Handle error.
    * }
    *
    * // Updated prompts
    * console.log(prompts);
    * });
    *
    * @param {Object} params Prompts parameters.
    * @param {Object} data Updated prompts data.
    * @param {Function} [cb] Callback function.
    *
    * @return {Promise|undefined}
    */
    utils.wrapPropertyMethod(PromptsManager, 'updateSettings', 'resource.patch');
  • /**
    * Update the branding settings.
    *
    * @method updateSettings
    * @memberOf module:management.BrandingManager.prototype
    *
    * @example
    * management.branding.updateSettings(data, function (err, branding) {
    * if (err) {
    * // Handle error.
    * }
    *
    * // Updated branding
    * console.log(branding);
    * });
    *
    * @param {Object} params Branding parameters.
    * @param {Object} data Updated branding data.
    * @param {Function} [cb] Callback function.
    *
    * @return {Promise|undefined}
    */
    utils.wrapPropertyMethod(BrandingManager, 'updateSettings', 'resource.patch');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One argument to keep the param argument is to allow extending the method with additional parameters/options in the future (ex. if the API endpoint starts accepting optional query params).

@pmalouin pmalouin changed the title feat(guardian): support for selected-provider and message-type methods [MFA-310] feat(guardian): support for selected-provider and message-type methods Jun 23, 2020
@pmalouin pmalouin changed the title [MFA-310] feat(guardian): support for selected-provider and message-type methods feat(guardian): support for selected-provider and message-type methods [MFA-310] Jun 23, 2020
@pmalouin pmalouin marked this pull request as ready for review June 23, 2020 17:54
@pmalouin pmalouin requested a review from a team June 23, 2020 17:54
@pmalouin pmalouin changed the title feat(guardian): support for selected-provider and message-type methods [MFA-310] feat(guardian): support policies, selected-provider, message-types methods [MFA-310] Jun 23, 2020
* @type {external:RestClient}
*/
var guardianFactorsPhoneSelectedProviderAuth0RestClient = new Auth0RestClient(
options.baseUrl + '/guardian/factors/sms/selected-provider',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We discussed whether this should be /sms/selected-provider (the one that is already in production) or the new alias /phone/selected-provider (almost ready, but not available in PSaaS until July's release). To make the release smooth and ship this early, I opted to use the /sms/selected-provider. cc @joseluisdiaz

Choose a reason for hiding this comment

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

👍

Choose a reason for hiding this comment

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

ok, shall we create a task to change it in the future ?

Copy link

@TSLarson TSLarson left a comment

Choose a reason for hiding this comment

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

LGTM!

@julienwoll
Copy link

What about /phone/twilio ?

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

Successfully merging this pull request may close these issues.

None yet

5 participants