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

Issue-394: Add disable_provider_selection kwarg to Api.authentication_url #395

Merged
merged 2 commits into from
Dec 16, 2022
Merged

Issue-394: Add disable_provider_selection kwarg to Api.authentication_url #395

merged 2 commits into from
Dec 16, 2022

Conversation

PickertJoe
Copy link
Contributor

@PickertJoe PickertJoe commented Dec 15, 2022

Closes #394

Description

We need the option to disable the "Select different provider" button in Nylas' hosted auth when we are trying to authenticate against a specific provider. This is necessary to prevent our users from escaping the intended auth flow and selecting a provider that we do not support.

This commit addresses this need by adding an optional keyword argument to the Api.authentication_url method and, if it is provided, including it in the params payload appended on the call to the /oauth/authorize endpoint. Tests have been added to account for this change and I tested it locally to confirm that the "Select different provider" button was no longer visible when I included disable_provider_selection: true in my call to the authentication_url method.

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.

…_url

We need the option to disable the "Select different provider" button
in Nylas' hosted auth when we are trying to authenticate against a
specific provider. This is necessary to prevent our users from escaping
the intended auth flow and selecting a provider that we do not support.

This commit addresses this need by adding and optional keyword argument
to the Api.authentication_url method and, if it is provided, including it
in the params payload appended on the call to the /oauth/authorize endpoint.
Tests have been added to account for this change and I tested it locally to
confirm that the "Select different provider" button was no longer visible
when I included disable_provider_selection: true in my call to
the authentication_url method.
@PickertJoe
Copy link
Contributor Author

PickertJoe commented Dec 15, 2022

Hey @mrashed-dev @andreccosta! I'm not able to tag reviewers for this PR, but I saw you'd contributed recently and was curious if you'd mind taking a look at this for me. I'd be happy to share more about these changes and our need for them if needed. Thanks so much!

Copy link

@kellyredding kellyredding left a comment

Choose a reason for hiding this comment

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

@PickertJoe Can I approve this? We'll see! 🚀

I think this change looks great! Nice that it was so easy to make.

@mrashed-dev mrashed-dev self-requested a review December 16, 2022 14:15
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.

Thanks for your contribution @PickertJoe, looks good to me!

@codecov
Copy link

codecov bot commented Dec 16, 2022

Codecov Report

Merging #395 (3226339) into main (b22c7d1) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #395   +/-   ##
=======================================
  Coverage   97.73%   97.73%           
=======================================
  Files         126      126           
  Lines        4555     4555           
=======================================
  Hits         4452     4452           
  Misses        103      103           
Impacted Files Coverage Δ
spec/nylas/api_spec.rb 100.00% <ø> (ø)
lib/nylas/api.rb 72.22% <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 ff97a17 into nylas:main Dec 16, 2022
@mrashed-dev mrashed-dev mentioned this pull request Dec 16, 2022
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.

Add disable_provider_selection kwarg to Api.authenticate_url
3 participants