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

Update OAuth Token operations to new APIs #2116

Merged
merged 2 commits into from
Mar 2, 2020

Conversation

MGudgin
Copy link
Contributor

@MGudgin MGudgin commented Mar 2, 2020

Per 'Deprecating OAuth Application API'
the HTTP API endpoints called by CheckApplicationAuthentication,
ResetApplicationAuthentication and RevokeApplicationAuthentication are
being deprecated.

This PR updates those APIs to call the new HTTP API endpoints as
documented at the above link.

Details

Amend CheckApplicationAuthentication, ResetApplicationAuthentication and
RevokeApplicationAuthentication to create an object containing the OAuth
access token and to call the single arg version of
ApiUrls.ApplicationAuthorization. The object is used as the request
body.

Amend CheckApplicationAuthentication to use POST.

Amend ResetApplicationAuthentication to use PATCH.

Remove the two arg version of ApiUrls.ApplicationAuthorization as it is
no longer called. Amend the single arg version to use the new API path.

Amend unit tests to account for the above changes.

Per ['Deprecating OAuth Application API'](https://developer.github.com/changes/2020-02-14-deprecating-oauth-app-endpoint/)
the HTTP API endpoints called by CheckApplicationAuthentication,
ResetApplicationAuthentication and RevokeApplicationAuthentication are
being deprecated.

This PR updates those APIs to call the new HTTP API endpoints as
documented at the above link.

* Details

Amend CheckApplicationAuthentication, ResetApplicationAuthentication and
RevokeApplicationAuthentication to create an object containing the OAuth
access token and to call the single arg version of
ApiUrls.ApplicationAuthorization. The object is used as the request
body.

Amend CheckApplicationAuthentication to use POST.

Amend ResetApplicationAuthentication to use PATCH.

Remove the two arg version of ApiUrls.ApplicationAuthorization as it is
no longer called. Amend the single arg version to use the new API path.

Amend unit tests to account for the above changes.
@codecov
Copy link

codecov bot commented Mar 2, 2020

Codecov Report

Merging #2116 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2116      +/-   ##
==========================================
+ Coverage   66.96%   66.98%   +0.01%     
==========================================
  Files         542      542              
  Lines       14228    14239      +11     
==========================================
+ Hits         9528     9538      +10     
- Misses       4700     4701       +1
Impacted Files Coverage Δ
Octokit/Helpers/ApiUrls.Authorizations.cs 100% <100%> (+16.66%) ⬆️
Octokit/Clients/AuthorizationsClient.cs 56.03% <100%> (+5.07%) ⬆️
Octokit/SimpleJson.cs 70.15% <0%> (-0.27%) ⬇️

Copy link
Member

@shiftkey shiftkey 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 the quick PR @MGudgin!

Just one thing about asserting the change works as expected...

Octokit.Tests/Clients/AuthorizationsClientTests.cs Outdated Show resolved Hide resolved
Add a check to the unit tests to verify that the request payload
contains an access_token field with the expected value.
@shiftkey
Copy link
Member

shiftkey commented Mar 2, 2020

Thanks for this @MGudgin!

Feel free to open other issues or PRs if you wanted to improve this area.

@shiftkey shiftkey merged commit c1c6366 into octokit:master Mar 2, 2020
@shiftkey
Copy link
Member

shiftkey commented Mar 2, 2020

release_notes: In preparation for the Deprecating OAuth Application API brownout and eventual removal on July 1 2020 we have updated the client internals to ensure we are calling the supported endpoint well in advance

@nickfloyd nickfloyd added Type: Bug Something isn't working as documented and removed category: bug labels Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants