Skip to content

Commit

Permalink
Remove client_id/client_secret authentication
Browse files Browse the repository at this point in the history
Authenticating via client_id and client_secret is dangerous, since they
appended to the query string, and can be easily seen from the URL.

client_id/client_secret have been deprecated since May 2020, and ignored
by GitHub since late 2020, so it's high time they were removed.
  • Loading branch information
s-t-e-v-e-n-k committed Mar 23, 2021
1 parent ddd437a commit 2441a3c
Show file tree
Hide file tree
Showing 7 changed files with 0 additions and 147 deletions.
15 changes: 0 additions & 15 deletions github/MainClass.py
Expand Up @@ -100,8 +100,6 @@ def __init__(
jwt=None,
base_url=DEFAULT_BASE_URL,
timeout=DEFAULT_TIMEOUT,
client_id=None,
client_secret=None,
user_agent="PyGithub/Python",
per_page=DEFAULT_PER_PAGE,
verify=True,
Expand All @@ -113,8 +111,6 @@ def __init__(
:param password: string
:param base_url: string
:param timeout: integer
:param client_id: string
:param client_secret: string
:param user_agent: string
:param per_page: int
:param verify: boolean or string
Expand All @@ -127,30 +123,19 @@ def __init__(
assert jwt is None or isinstance(jwt, str), jwt
assert isinstance(base_url, str), base_url
assert isinstance(timeout, int), timeout
assert client_id is None or isinstance(client_id, str), client_id
assert client_secret is None or isinstance(client_secret, str), client_secret
assert user_agent is None or isinstance(user_agent, str), user_agent
assert (
retry is None
or isinstance(retry, (int))
or isinstance(retry, (urllib3.util.Retry))
)
assert pool_size is None or isinstance(pool_size, (int)), pool_size

if client_id is not None or client_secret is not None:
warnings.warn(
"client_id and client_secret are deprecated and will be removed in a future release, switch to token authentication",
FutureWarning,
stacklevel=2,
)
self.__requester = Requester(
login_or_token,
password,
jwt,
base_url,
timeout,
client_id,
client_secret,
user_agent,
per_page,
verify,
Expand Down
8 changes: 0 additions & 8 deletions github/Requester.py
Expand Up @@ -296,8 +296,6 @@ def __init__(
jwt,
base_url,
timeout,
client_id,
client_secret,
user_agent,
per_page,
verify,
Expand Down Expand Up @@ -344,9 +342,6 @@ def __init__(

self.oauth_scopes = None

self.__clientId = client_id
self.__clientSecret = client_secret

assert user_agent is not None, (
"github now requires a user-agent. "
"See http://developer.github.com/v3/#user-agent-required"
Expand Down Expand Up @@ -581,9 +576,6 @@ def __requestRaw(self, cnx, verb, url, requestHeaders, input):
return status, responseHeaders, output

def __authenticate(self, url, requestHeaders, parameters):
if self.__clientId and self.__clientSecret and "client_id=" not in url:
parameters["client_id"] = self.__clientId
parameters["client_secret"] = self.__clientSecret
if self.__authorizationHeader is not None:
requestHeaders["Authorization"] = self.__authorizationHeader

Expand Down
14 changes: 0 additions & 14 deletions tests/Authentication.py
Expand Up @@ -26,8 +26,6 @@
# #
################################################################################

import warnings

import github

from . import Framework
Expand All @@ -50,18 +48,6 @@ def testJWTAuthentication(self):
g = github.Github(jwt=self.jwt)
self.assertEqual(g.get_user("jacquev6").name, "Vincent Jacques")

# Warning: I don't have a secret key, so the requests for this test are forged
def testSecretKeyAuthentication(self):
# Ignore the warning since client_{id,secret} are deprecated
warnings.filterwarnings("ignore", category=FutureWarning)
g = github.Github(client_id=self.client_id, client_secret=self.client_secret)
self.assertListKeyEqual(
g.get_organization("BeaverSoftware").get_repos("public"),
lambda r: r.name,
["FatherBeaver", "PyGithub"],
)
warnings.resetwarnings()

def testUserAgent(self):
g = github.Github(user_agent="PyGithubTester")
self.assertEqual(g.get_user("jacquev6").name, "Vincent Jacques")
Expand Down
5 changes: 0 additions & 5 deletions tests/Framework.py
Expand Up @@ -268,9 +268,6 @@ def setUp(self):
self.password = GithubCredentials.password
self.oauth_token = GithubCredentials.oauth_token
self.jwt = GithubCredentials.jwt
# @todo Remove client_id and client_secret from ReplayData (as we already remove login, password and oauth_token)
# self.client_id = GithubCredentials.client_id
# self.client_secret = GithubCredentials.client_secret
else:
github.Requester.Requester.injectConnectionClasses(
lambda ignored, *args, **kwds: ReplayingHttpConnection(
Expand All @@ -283,8 +280,6 @@ def setUp(self):
self.login = "login"
self.password = "password"
self.oauth_token = "oauth_token"
self.client_id = "client_id"
self.client_secret = "client_secret"
self.jwt = "jwt"

httpretty.enable(allow_net_connect=False)
Expand Down
50 changes: 0 additions & 50 deletions tests/Issue158.py

This file was deleted.

22 changes: 0 additions & 22 deletions tests/ReplayData/Authentication.testSecretKeyAuthentication.txt

This file was deleted.

This file was deleted.

0 comments on commit 2441a3c

Please sign in to comment.