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

Add Management API Anomaly endpoints #179

Merged
merged 5 commits into from
Jul 15, 2019

Conversation

makoto-matsumoto
Copy link
Contributor

@makoto-matsumoto makoto-matsumoto commented Jul 11, 2019

Changes

Add endpoint for Anomaly API

References

https://auth0.com/docs/api/management/v2#!/Anomaly/get_ips_by_id
https://auth0.com/docs/api/management/v2#!/Anomaly/delete_ips_by_id

Testing

  • This change adds unit test coverage
  • This change adds integration test coverage
  • This change has been tested on the latest version of Ruby

Checklist

@makoto-matsumoto makoto-matsumoto requested a review from a team July 11, 2019 01:23
# @see https://auth0.com/docs/api/management/v2#!/Anomaly/get_ips_by_id
# @param ip [string] The IP to check.
def check_if_ip_is_blocked(ip)
raise Auth0::InvalidParameter, 'Must specify an IP' if ip.to_s.empty?
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about IP address validation here?

https://stackoverflow.com/a/15157862/728480

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that it is better to return the original response of Auth0 as much as possible.
If the format is incorrect, Auth0 will return:

RESPONSE CODE : 400
RESPONSE BODY
{
  "statusCode": 400,
  "error": "Bad Request",
  "message": "Path validation error: 'Object didn't pass validation for format ipv4: 9919191' on property id (The ip to check).",
  "errorCode": "invalid_uri"
}

Copy link
Contributor

Choose a reason for hiding this comment

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

The main thing I was thinking here is that we could save an HTTP call if we can catch the error (like we do in other cases). I'll check with the team to see what they think.

Content-Type:
- application/json
Auth0-Client:
- eyJuYW1lIjoicnVieS1hdXRoMCIsInZlcnNpb24iOiI0LjUuMCJ9
Copy link
Contributor

Choose a reason for hiding this comment

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

Your attention to detail is commendable!

I'll make a note to anonymize this in our existing cassettes. Nothing secret here but probably best not to have specific application data in here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your advice!
However, this value has been updated to the same value as the test already included in the master.
Ex:
should_add_a_token_to_the_blacklist.yml:19

You might want to include client id in filter_sensitive_data as well.

spec/spec_helper.rb

config.filter_sensitive_data('CLIENT_SECRET') { ENV['CLIENT_SECRET'] }
config.filter_sensitive_data('API_TOKEN') { ENV['MASTER_JWT'] }

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's probably the best way to deal with it. My main reason for commenting was that I appreciate you updating to the same value as the rest 👍

@joshcanhelp joshcanhelp added this to the v4.8.0 milestone Jul 12, 2019
makoto-matsumoto and others added 4 commits July 13, 2019 09:32
Co-Authored-By: Josh Cunningham <josh@joshcanhelp.com>
Co-Authored-By: Josh Cunningham <josh@joshcanhelp.com>
@joshcanhelp joshcanhelp changed the title add anomaly endpoints Add Management API Anomaly endpoints Jul 15, 2019
@joshcanhelp joshcanhelp merged commit 4792aeb into auth0:master Jul 15, 2019
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.

2 participants