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 new userinfo method for auth endpoints #130

Merged
merged 1 commit into from
Oct 12, 2018

Conversation

joshcanhelp
Copy link
Contributor

  • Add new userinfo method for Auth0::Api::AuthenticationEndpoints
  • Add the ability to include additional headers in get method in Auth0::Mixins::HTTPProxy
  • Add note to deprecate previous user_info method

@joshcanhelp joshcanhelp added this to the v4-Next milestone Sep 26, 2018
safe_path = URI.escape(path)
body = body.delete_if { |_, v| v.nil? }
result = if [:get, :delete].include?(method)
call(method, url(safe_path), timeout, add_headers(params: body))
add_headers(extra_headers.nil? ? {params: body} : extra_headers )
Copy link

Choose a reason for hiding this comment

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

Is this usage of add_headers compatible with the existing one? Does add_headers return a value or does it mutate something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this usage of add_headers compatible with the existing one?

Yes. That's there to ... add headers but the current implementation uses it to add URL params by setting :params to be the body that's passed in for GET requests. It's a bit confusing in it's current state but I think this is a good compromise between necessary functionality and legacy support.

Does add_headers return a value or does it mutate something?

Both. It mutates the module's @headers property and returns it. I'm using it just to mutate, then pass the property to that call() method.

it { expect(@instance).to respond_to(:userinfo) }
it 'is expected to make a GET request to /userinfo' do
expect(@instance).to receive(:get).with('/userinfo', nil, {Authorization: 'Bearer access-token'})
@instance.userinfo 'access-token'
Copy link

Choose a reason for hiding this comment

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

What is this doing?

@@ -330,6 +330,14 @@
end
end

context '.userinfo' do
it { expect(@instance).to respond_to(:userinfo) }
Copy link

Choose a reason for hiding this comment

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

If you setup something like subject { @instance }, you could even shorten this to it { is_expected.to respond_to(:user_info) }, I believe. Not suggesting you need to do this, just showing you some fun RSpec stuff 👍

@joshcanhelp joshcanhelp force-pushed the add-auth-userinfo-method branch 2 times, most recently from 8a0fc2f to eee645c Compare October 10, 2018 23:33
@joshcanhelp
Copy link
Contributor Author

@machuga - Added integration tests and swapped out the new Client ID. Removed Client Secret and API Token out of the recordings 👍

@joshcanhelp joshcanhelp force-pushed the add-auth-userinfo-method branch 2 times, most recently from 7feb9c2 to c4be5da Compare October 11, 2018 01:31
Content-Type:
- application/json
Auth0-Client:
- eyJuYW1lIjoicnVieS1hdXRoMCIsInZlcnNpb24iOiI0LjUuMCJ9
Authorization:
- Bearer eyJ0eXAiOiJKV1QiLCJhbGc.eyJpc3MiOiJodHRwczovL2F1dGgwLXNkay10ZXN0cy5hdXRoMC5jb20vIiwic3ViIjoiQjRvUEhsMDA3VmExR0JkWlhpU0hhRUNVcVZXa2Q5WGtAY2xpZW50cyIsIm.PxfZjVd4wTb_bFbSTENdTmbj13CDQaPK352w3UNL3i3DnWcVJa6qSiA0hCr_tSU_uC34mHkK52O8tFVeZjxCYSeM9yOZUcKVya5N0I7G90X
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dummy API token, replacing with something more clearly fake

Content-Type:
- application/json
Auth0-Client:
- eyJuYW1lIjoicnVieS1hdXRoMCIsInZlcnNpb24iOiI0LjUuMCJ9
Authorization:
- Bearer eyJ0eXAiOiJKV1QiLCJhbGc.eyJpc3MiOiJodHRwczovL2F1dGgwLXNkay10ZXN0cy5hdXRoMC5jb20vIiwic3ViIjoiQjRvUEhsMDA3VmExR0JkWlhpU0hhRUNVcVZXa2Q5WGtAY2xpZW50cyIsIm.PxfZjVd4wTb_bFbSTENdTmbj13CDQaPK352w3UNL3i3DnWcVJa6qSiA0hCr_tSU_uC34mHkK52O8tFVeZjxCYSeM9yOZUcKVya5N0I7G90X
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dummy API token, replacing with something more clearly fake

Content-Type:
- application/json
Auth0-Client:
- eyJuYW1lIjoicnVieS1hdXRoMCIsInZlcnNpb24iOiI0LjUuMCJ9
Authorization:
- Bearer eyJ0eXAiOiJKV1QiLCJhbGc.eyJpc3MiOiJodHRwczovL2F1dGgwLXNkay10ZXN0cy5hdXRoMC5jb20vIiwic3ViIjoiQjRvUEhsMDA3VmExR0JkWlhpU0hhRUNVcVZXa2Q5WGtAY2xpZW50cyIsIm.PxfZjVd4wTb_bFbSTENdTmbj13CDQaPK352w3UNL3i3DnWcVJa6qSiA0hCr_tSU_uC34mHkK52O8tFVeZjxCYSeM9yOZUcKVya5N0I7G90X
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dummy API token, replacing with something more clearly fake

Content-Type:
- application/json
Auth0-Client:
- eyJuYW1lIjoicnVieS1hdXRoMCIsInZlcnNpb24iOiI0LjUuMCJ9
Authorization:
- Bearer eyJ0eXAiOiJKV1QiLCJhbGc.eyJpc3MiOiJodHRwczovL2F1dGgwLXNkay10ZXN0cy5hdXRoMC5jb20vIiwic3ViIjoiQjRvUEhsMDA3VmExR0JkWlhpU0hhRUNVcVZXa2Q5WGtAY2xpZW50cyIsIm.PxfZjVd4wTb_bFbSTENdTmbj13CDQaPK352w3UNL3i3DnWcVJa6qSiA0hCr_tSU_uC34mHkK52O8tFVeZjxCYSeM9yOZUcKVya5N0I7G90X
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dummy API token, replacing with something more clearly fake

Content-Type:
- application/json
Auth0-Client:
- eyJuYW1lIjoicnVieS1hdXRoMCIsInZlcnNpb24iOiI0LjUuMCJ9
Authorization:
- Bearer eyJ0eXAiOiJKV1QiLCJhbGc.eyJpc3MiOiJodHRwczovL2F1dGgwLXNkay10ZXN0cy5hdXRoMC5jb20vIiwic3ViIjoiQjRvUEhsMDA3VmExR0JkWlhpU0hhRUNVcVZXa2Q5WGtAY2xpZW50cyIsIm.PxfZjVd4wTb_bFbSTENdTmbj13CDQaPK352w3UNL3i3DnWcVJa6qSiA0hCr_tSU_uC34mHkK52O8tFVeZjxCYSeM9yOZUcKVya5N0I7G90X
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dummy API token, replacing with something more clearly fake

Content-Type:
- application/json
Auth0-Client:
- eyJuYW1lIjoicnVieS1hdXRoMCIsInZlcnNpb24iOiI0LjUuMCJ9
Authorization:
- Bearer eyJ0eXAiOiJKV1QiLCJhbGc.eyJpc3MiOiJodHRwczovL2F1dGgwLXNkay10ZXN0cy5hdXRoMC5jb20vIiwic3ViIjoiQjRvUEhsMDA3VmExR0JkWlhpU0hhRUNVcVZXa2Q5WGtAY2xpZW50cyIsIm.PxfZjVd4wTb_bFbSTENdTmbj13CDQaPK352w3UNL3i3DnWcVJa6qSiA0hCr_tSU_uC34mHkK52O8tFVeZjxCYSeM9yOZUcKVya5N0I7G90X
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dummy API token, replacing with something more clearly fake

Content-Type:
- application/json
Auth0-Client:
- eyJuYW1lIjoicnVieS1hdXRoMCIsInZlcnNpb24iOiI0LjUuMCJ9
Authorization:
- Bearer eyJ0eXAiOiJKV1QiLCJhbGc.eyJpc3MiOiJodHRwczovL2F1dGgwLXNkay10ZXN0cy5hdXRoMC5jb20vIiwic3ViIjoiQjRvUEhsMDA3VmExR0JkWlhpU0hhRUNVcVZXa2Q5WGtAY2xpZW50cyIsIm.PxfZjVd4wTb_bFbSTENdTmbj13CDQaPK352w3UNL3i3DnWcVJa6qSiA0hCr_tSU_uC34mHkK52O8tFVeZjxCYSeM9yOZUcKVya5N0I7G90X
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dummy API token, replacing with something more clearly fake

config.cassette_library_dir = 'spec/fixtures/vcr_cassettes'
config.configure_rspec_metadata!
config.hook_into :webmock
config.filter_sensitive_data('CLIENT_SECRET') { ENV['CLIENT_SECRET'] }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🎉

Copy link

@machuga machuga left a comment

Choose a reason for hiding this comment

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

LGTM!

@joshcanhelp joshcanhelp merged commit 085ee53 into master Oct 12, 2018
@joshcanhelp joshcanhelp deleted the add-auth-userinfo-method branch October 13, 2018 19:57
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.

3 participants