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

Fixed Github deprecation warning issue #76

Closed
wants to merge 3 commits into from

Conversation

rodikh
Copy link

@rodikh rodikh commented Feb 6, 2020

Proposed fix to the deprecation email being sent out by Github. #75

#75 (comment)

Tested with my own Github App oAuth.

- Passing access token as an Authorization header instead of a query param to the '/user/emails' APi request inside the userProfile method
- still passing access token in case it is needed elsewhere.
@rodikh rodikh changed the title Fixed Github deprecation warning issue #75 Fixed Github deprecation warning issue Feb 6, 2020
- code styling to match the rest of file
@coveralls
Copy link

coveralls commented Feb 6, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 162b330 on rodikh:master into 46b96e8 on jaredhanson:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 0fefb57 on rodikh:master into 46b96e8 on jaredhanson:master.

@ashvin777
Copy link

If this fixes the issue, please get this in.

@Martii
Copy link

Martii commented May 6, 2021

@rodikh

From what I understand token prefixing is purely for PAT (Personal Access Token). basic prefix is what we use as an organization... so not sure this PR as it is now will work in that respect for orgs. Needs verification.


Verification (at least) at https://docs.github.com/en/rest/overview/other-authentication-methods#authenticating-for-saml-sso

If you're using the API to access an organization that enforces SAML SSO for authentication, you'll need to create a personal access token (PAT) and authorize the token for that organization. Visit the URL specified in X-GitHub-SSO to authorize the token for the organization.
$ curl -v -H "Authorization: token TOKEN" https://api.github.com/repos/octodocs-test/test

So that's what would need to be verified is being used by this dep at this point in the code.

I can tell you for sure that orgs can use ...Authorization: basic TOKEN"... at this time. That's what this little test section does/returns as a property for Authorization under headers for the GH API v3 at https://github.com/OpenUserJS/OpenUserJS.org/blob/a9247b8/libs/githubClient.js#L40-L49 (when dumped to the console of course).


If I were to take a guess for maximum compatibility the PR should possibly retain accessToken as is... and somehow in your tests send token with whatever the caller is i.e. , 'Authorization': accessToken where accessToken would be sent token LKSJDLFJKlk;ljSAMPLE66lkjskdjfalFFF or basic OImDLKJF1SAMPLEaksldjflkajsdj. Perhaps this could be helpful to track down?.

@rodikh rodikh closed this Aug 9, 2021
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.

4 participants