Add scope to auth return

[prplmg]

Adds scope key to the ElixirAuthGithub.github_auth return object

[prplmg]

@nelsonic can I request a review? It's just a small addition of scope info to allow for conditional handling

[nelsonic]

@prplmg if you can revert the change made to set_user_details/2 we can get this merged and published to Hex.pm today. 👌

[SimonLab]

@prplmg thanks for opening this PR. However I'm trying to understand the use case for it. When you using this package you already define the scope "manually" as a parameter to the login_url\1 function: https://github.com/dwyl/elixir-auth-github/blob/a851e1e736fc84fbae5e32885c5236e38bc29810/lib/elixir_auth_github.ex#L46-L54

Is there an edge case where the scope is unkown to the person using this package? I'm not sure I understand why we need to add the scope information to the get_user_details Map.

Edit:

After reading the Github documentation, https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps#2-users-are-redirected-back-to-your-site-by-github We can see that the json response when exchanging the code for an access_token is the following:

So the reply contains access_token, scope and token_type values. Still not sure about the scope information use case.

[prplmg]

@prplmg if you can revert the change made to set_user_details/2 we can get this merged and published to Hex.pm today. 👌

Just pushed some changes that reverts the changes to [set_user_details/2] and updates the tests, thanks for the input!

[prplmg]

@prplmg thanks for opening this PR. However I'm trying to understand the use case for it. When you using this package you already define the scope "manually" as a parameter to the login_url\1 function:

https://github.com/dwyl/elixir-auth-github/blob/a851e1e736fc84fbae5e32885c5236e38bc29810/lib/elixir_auth_github.ex#L46-L54

Is there an edge case where the scope is unkown to the person using this package? I'm not sure I understand why we need to add the scope information to the get_user_details Map.

Great question! We're developing an app where we can login via github, but not necessarity grant repo access on first/simple login. SInce we still want to be able to add repo access later on, we would have two different urls to authenticate via github: one that grants user scope (on the login screen) and one that adds the repo scope (on the 'integrations' tab in our app). We need to check the returning scope map to know whether the user has granted us repo access (and thus store the new key) or if we're just on user access.

[codecov[bot]]

Codecov Report

Merging #81 (f809bbe) into main (a851e1e) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #81   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines           32        34    +2     
=========================================
+ Hits            32        34    +2     

Impacted Files	Coverage Δ
lib/elixir_auth_github.ex	100.00% <100.00%> (ø)
lib/httpoison_mock.ex	100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[prplmg]

@nelsonic reverted the breaking changes, updated the tests, and fixed the coverage issue, I think it's good to go!

[SimonLab]

Great question! We're developing an app where we can login via github, but not necessarity grant repo access on first/simple login. SInce we still want to be able to add repo access later on, we would have two different urls to authenticate via github: one that grants user scope (on the login screen) and one that adds the repo scope (on the 'integrations' tab in our app). We need to check the returning scope map to know whether the user has granted us repo access (and thus store the new key) or if we're just on user access.

Ok I think I can see the use case, you're adding more scopes at a later stage. Thanks for the explanation :+1:

[nelsonic]

@prplmg thanks for updating. 👌 @SimonLab is our resident GitHub Auth (and Elixir) wizard. 🧙‍♂️ As long as there is no downside to having the scope property I'm in favour. 👍 What would be even better would be if this was somewhere in the Docs/README ... 📝

[nelsonic]

Package published to https://hex.pm/packages/elixir_auth_github/1.6.4 📦 🚀

[prplmg]

Ok. Let's ship this "undocumented feature" so @prplmg can use it. :shipit: Ideally some docs would be added later. 🙏

How should I add it to the doc? I added it to the return object example, but I understand if it's a bit vague to not explain where it comes from. However, I could not find a more suitable place to mention it.