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

Allow native authentication to return the full response like the exchange_code_for_token #411

Merged

Conversation

tvongaza
Copy link
Contributor

@tvongaza tvongaza commented Mar 24, 2023

Description

Adds a return_full_response argument to the Api#authenticate method, along with it an implementation in NativeAuthentication which mirrors the code in the exchange_code_for_token method.

This let's us perform NativeAuthentication, and get back the full token response vs just the access token. On the implementers end, this lets us share some of our account creation code between the two different types of authentication by being able to rely on the full_response.

Why is this needed? Nylas webhooks come in as "skinny payloads", just enough information to make a request to get the objects data with a request back to Nylas's servers (see: https://developer.nylas.com/docs/developer-guide/webhooks/available-webhooks/#event-webhooks). A calendar event's may look something like:

{"date"=>1679710508, "object"=>"event", "type"=>"event.created", "object_data"=>{"namespace_id"=>"3q5iuy16vqjr....", "account_id"=>"3q5iuy16vqj....", "object"=>"event", "attributes"=>nil, "id"=>"7o6mk16jw....", "metadata"=>nil}}

To look up this object you'll need the access_token for the related account, plus the event id.

Now when using Hosted Authentication, you exchange your oauth code for an access token (https://developer.nylas.com/docs/api/#post/oauth/token). The Ruby API lets you include return_full_response in the call to exchange_code_for_token which gives back not just the access_token, but the full response from the /oauth/token call:

{
  "access_token": "string",
  "account_id": "string",
  "email_address": "string",
  "provider": "eas",
  "token_type": "bearer"
}

However if you're using Native Authentication, you don't have this option, the call just returns the access_token, so now we have to made a follow up call to /account to get the account_id for the access_token in question. Not the end of the world, but ideally we could treat these two the same, and not make extra requests if we don't have to.

License

I confirm that this contribution is made under the terms of the MIT license and that I have the authority necessary to make this contribution on behalf of its copyright owner.

@mrashed-dev mrashed-dev self-requested a review April 4, 2023 13:00
Copy link
Contributor

@mrashed-dev mrashed-dev left a comment

Choose a reason for hiding this comment

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

Thank you for this awesome contribution, @tvongaza!

@codecov
Copy link

codecov bot commented Apr 4, 2023

Codecov Report

Merging #411 (5ce3536) into main (27c3e37) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #411   +/-   ##
=======================================
  Coverage   97.84%   97.85%           
=======================================
  Files         128      128           
  Lines        4732     4753   +21     
=======================================
+ Hits         4630     4651   +21     
  Misses        102      102           
Impacted Files Coverage Δ
lib/nylas/api.rb 72.22% <100.00%> (ø)
lib/nylas/native_authentication.rb 100.00% <100.00%> (ø)
spec/nylas/native_authentication_spec.rb 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mrashed-dev mrashed-dev merged commit 6f56da0 into nylas:main Apr 4, 2023
@mrashed-dev mrashed-dev mentioned this pull request Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants