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

Add support for get_app() with App authentication #2549

Merged
merged 5 commits into from Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions github/AccessToken.py
Expand Up @@ -20,7 +20,7 @@
# #
################################################################################

from datetime import datetime, timedelta
from datetime import datetime, timedelta, timezone

import github.GithubObject

Expand Down Expand Up @@ -128,7 +128,7 @@ def _initAttributes(self):
self._refresh_expires_in = github.GithubObject.NotSet

def _useAttributes(self, attributes):
self._created = datetime.utcnow()
self._created = datetime.now(timezone.utc)
if "access_token" in attributes: # pragma no branch
self._token = self._makeStringAttribute(attributes["access_token"])
if "token_type" in attributes: # pragma no branch
Expand Down
10 changes: 6 additions & 4 deletions github/Auth.py
Expand Up @@ -23,7 +23,7 @@
import abc
import base64
import time
from datetime import datetime, timedelta
from datetime import datetime, timedelta, timezone
from typing import Dict, Optional, Union

import jwt
Expand Down Expand Up @@ -307,7 +307,7 @@ def _is_expired(self) -> bool:
self.__installation_authorization.expires_at
- TOKEN_REFRESH_THRESHOLD_TIMEDELTA
)
return token_expires_at < datetime.utcnow()
return token_expires_at < datetime.now(timezone.utc)

def _get_installation_authorization(self) -> InstallationAuthorization:
assert (
Expand Down Expand Up @@ -413,7 +413,9 @@ def withRequester(self, requester: Requester) -> "AppUserAuth":

@property
def _is_expired(self) -> bool:
return self._expires_at is not None and self._expires_at < datetime.utcnow()
return self._expires_at is not None and self._expires_at < datetime.now(
timezone.utc
)

def _refresh(self):
if self._refresh_token is None:
Expand All @@ -422,7 +424,7 @@ def _refresh(self):
)
if (
self._refresh_expires_at is not None
and self._refresh_expires_at < datetime.utcnow()
and self._refresh_expires_at < datetime.now(timezone.utc)
):
raise RuntimeError(
"Cannot refresh expired token because refresh token also expired"
Expand Down
4 changes: 3 additions & 1 deletion github/AuthenticatedUser.py
Expand Up @@ -1229,7 +1229,9 @@ def has_in_watched(self, watched):
)
return status == 200

def mark_notifications_as_read(self, last_read_at=datetime.datetime.utcnow()):
def mark_notifications_as_read(
self, last_read_at=datetime.datetime.now(datetime.timezone.utc)
):
"""
:calls: `PUT /notifications <https://docs.github.com/en/rest/reference/activity#notifications>`_
:param last_read_at: datetime
Expand Down
19 changes: 18 additions & 1 deletion github/GithubIntegration.py
Expand Up @@ -4,6 +4,7 @@

from github import Consts
from github.Auth import AppAuth
from github.GithubApp import GithubApp
from github.GithubException import GithubException
from github.Installation import Installation
from github.InstallationAuthorization import InstallationAuthorization
Expand Down Expand Up @@ -71,7 +72,10 @@ def __init__(
jwt_algorithm=jwt_algorithm,
)

assert auth is not None
assert isinstance(
auth, AppAuth
), f"GithubIntegration requires github.Auth.AppAuth authentication, not {type(auth)}"

EnricoMi marked this conversation as resolved.
Show resolved Hide resolved
self.auth = auth

self.__requester = Requester(
Expand Down Expand Up @@ -213,3 +217,16 @@ def get_app_installation(self, installation_id):
:rtype: :class:`github.Installation.Installation`
"""
return self._get_installed_app(url=f"/app/installations/{installation_id}")

def get_app(self):
"""
:calls: `GET /app <https://docs.github.com/en/rest/reference/apps#get-the-authenticated-app>`_
:rtype: :class:`github.GithubApp.GithubApp`
"""

chantra marked this conversation as resolved.
Show resolved Hide resolved
headers, data = self.__requester.requestJsonAndCheck(
"GET", "/app", headers=self._get_headers()
)
return GithubApp(
requester=self.__requester, headers=headers, attributes=data, completed=True
)
8 changes: 6 additions & 2 deletions github/MainClass.py
Expand Up @@ -808,8 +808,12 @@ def get_app(self, slug=github.GithubObject.NotSet):
if slug is github.GithubObject.NotSet:
EnricoMi marked this conversation as resolved.
Show resolved Hide resolved
# with no slug given, calling /app returns the authenticated app,
# including the actual /apps/{slug}
headers, data = self.__requester.requestJsonAndCheck("GET", "/app")
return GithubApp.GithubApp(self.__requester, headers, data, completed=True)
warnings.warn(
"Argument slug is mandatory, calling this method without the slug argument is deprecated, please use "
"github.GithubIntegration(auth=github.Auth.AppAuth(...)).get_app() instead",
category=DeprecationWarning,
)
return GithubIntegration(auth=self.__requester.auth).get_app()
else:
# with a slug given, we can lazily load the GithubApp
return GithubApp.GithubApp(
Expand Down
4 changes: 3 additions & 1 deletion github/Repository.py
Expand Up @@ -3702,7 +3702,9 @@ def get_notifications(
params,
)

def mark_notifications_as_read(self, last_read_at=datetime.datetime.utcnow()):
def mark_notifications_as_read(
self, last_read_at=datetime.datetime.now(datetime.timezone.utc)
):
"""
:calls: `PUT /repos/{owner}/{repo}/notifications <https://docs.github.com/en/rest/reference/activity#notifications>`_
:param last_read_at: datetime
Expand Down
25 changes: 16 additions & 9 deletions tests/ApplicationOAuth.py
Expand Up @@ -83,8 +83,10 @@ def testGetAccessToken(self):

def testGetAccessTokenWithExpiry(self):
with mock.patch("github.AccessToken.datetime") as dt:
dt.utcnow = mock.Mock(
return_value=datetime.datetime(2023, 6, 7, 12, 0, 0, 123)
dt.now = mock.Mock(
return_value=datetime.datetime(
2023, 6, 7, 12, 0, 0, 123, tzinfo=datetime.timezone.utc
)
)
access_token = self.app.get_access_token(
"oauth_code_removed", state="state_removed"
Expand All @@ -100,13 +102,14 @@ def testGetAccessTokenWithExpiry(self):
self.assertEqual(access_token.scope, "")
self.assertEqual(access_token.expires_in, 28800)
self.assertEqual(
access_token.expires_at, datetime.datetime(2023, 6, 7, 20, 0, 0, 123)
access_token.expires_at,
datetime.datetime(2023, 6, 7, 20, 0, 0, 123, tzinfo=datetime.timezone.utc),
)
self.assertEqual(access_token.refresh_token, "refresh_token_removed")
self.assertEqual(access_token.refresh_expires_in, 15811200)
self.assertEqual(
access_token.refresh_expires_at,
datetime.datetime(2023, 12, 7, 12, 0, 0, 123),
datetime.datetime(2023, 12, 7, 12, 0, 0, 123, tzinfo=datetime.timezone.utc),
)

def testRefreshAccessToken(self):
Expand All @@ -115,8 +118,10 @@ def testRefreshAccessToken(self):
)

with mock.patch("github.AccessToken.datetime") as dt:
dt.utcnow = mock.Mock(
return_value=datetime.datetime(2023, 6, 7, 12, 0, 0, 123)
dt.now = mock.Mock(
return_value=datetime.datetime(
2023, 6, 7, 12, 0, 0, 123, tzinfo=datetime.timezone.utc
)
)
refreshed = self.app.refresh_access_token(access_token.refresh_token)

Expand All @@ -133,17 +138,19 @@ def testRefreshAccessToken(self):
self.assertEqual(refreshed.type, "bearer")
self.assertEqual(refreshed.scope, "")
self.assertEqual(
refreshed.created, datetime.datetime(2023, 6, 7, 12, 0, 0, 123)
refreshed.created,
datetime.datetime(2023, 6, 7, 12, 0, 0, 123, tzinfo=datetime.timezone.utc),
)
self.assertEqual(refreshed.expires_in, 28800)
self.assertEqual(
refreshed.expires_at, datetime.datetime(2023, 6, 7, 20, 0, 0, 123)
refreshed.expires_at,
datetime.datetime(2023, 6, 7, 20, 0, 0, 123, tzinfo=datetime.timezone.utc),
)
self.assertEqual(refreshed.refresh_token, "another_refresh_token_removed")
self.assertEqual(refreshed.refresh_expires_in, 15811200)
self.assertEqual(
refreshed.refresh_expires_at,
datetime.datetime(2023, 12, 7, 12, 0, 0, 123),
datetime.datetime(2023, 12, 7, 12, 0, 0, 123, tzinfo=datetime.timezone.utc),
)

def testGetAccessTokenBadCode(self):
Expand Down
37 changes: 18 additions & 19 deletions tests/Authentication.py
Expand Up @@ -26,7 +26,6 @@
# #
################################################################################
import datetime
import warnings
from unittest import mock

import jwt
Expand All @@ -42,16 +41,6 @@ def testNoAuthentication(self):
g = github.Github()
self.assertEqual(g.get_user("jacquev6").name, "Vincent Jacques")

def assertWarning(self, warning, expected):
self.assertWarnings(warning, expected)

def assertWarnings(self, warning, *expecteds):
self.assertEqual(len(warning.warnings), len(expecteds))
for message, expected in zip(warning.warnings, expecteds):
self.assertIsInstance(message, warnings.WarningMessage)
self.assertIsInstance(message.message, DeprecationWarning)
self.assertEqual(message.message.args, (expected,))

def testBasicAuthentication(self):
with self.assertWarns(DeprecationWarning) as warning:
g = github.Github(self.login.login, self.login.password)
Expand Down Expand Up @@ -124,25 +113,33 @@ def testAppUserAuthentication(self):
g = github.Github()
app = g.get_oauth_application(client_id, client_secret)
with mock.patch("github.AccessToken.datetime") as dt:
dt.utcnow = mock.Mock(
return_value=datetime.datetime(2023, 6, 7, 12, 0, 0, 123)
dt.now = mock.Mock(
return_value=datetime.datetime(
2023, 6, 7, 12, 0, 0, 123, tzinfo=datetime.timezone.utc
)
)
token = app.refresh_access_token(refresh_token)
self.assertEqual(token.token, "fresh access token")
self.assertEqual(token.type, "bearer")
self.assertEqual(token.scope, "")
self.assertEqual(token.expires_in, 28800)
self.assertEqual(token.expires_at, datetime.datetime(2023, 6, 7, 20, 0, 0, 123))
self.assertEqual(
token.expires_at,
datetime.datetime(2023, 6, 7, 20, 0, 0, 123, tzinfo=datetime.timezone.utc),
)
self.assertEqual(token.refresh_token, "fresh refresh token")
self.assertEqual(token.refresh_expires_in, 15811200)
self.assertEqual(
token.refresh_expires_at, datetime.datetime(2023, 12, 7, 12, 0, 0, 123)
token.refresh_expires_at,
datetime.datetime(2023, 12, 7, 12, 0, 0, 123, tzinfo=datetime.timezone.utc),
)

auth = app.get_app_user_auth(token)
with mock.patch("github.Auth.datetime") as dt:
dt.utcnow = mock.Mock(
return_value=datetime.datetime(2023, 6, 7, 20, 0, 0, 123)
dt.now = mock.Mock(
return_value=datetime.datetime(
2023, 6, 7, 20, 0, 0, 123, tzinfo=datetime.timezone.utc
)
)
self.assertEqual(auth._is_expired, False)
self.assertEqual(auth.token, "fresh access token")
Expand All @@ -151,8 +148,10 @@ def testAppUserAuthentication(self):

# expire auth token
with mock.patch("github.Auth.datetime") as dt:
dt.utcnow = mock.Mock(
return_value=datetime.datetime(2023, 6, 7, 20, 0, 1, 123)
dt.now = mock.Mock(
return_value=datetime.datetime(
2023, 6, 7, 20, 0, 1, 123, tzinfo=datetime.timezone.utc
)
)
self.assertEqual(auth._is_expired, True)
self.assertEqual(auth.token, "another access token")
Expand Down
16 changes: 16 additions & 0 deletions tests/Framework.py
Expand Up @@ -39,6 +39,7 @@
import os
import traceback
import unittest
import warnings

import httpretty # type: ignore
from requests.structures import CaseInsensitiveDict
Expand Down Expand Up @@ -328,6 +329,21 @@ def tearDown(self):
self.__closeReplayFileIfNeeded()
github.Requester.Requester.resetConnectionClasses()

def assertWarning(self, warning, expected):
EnricoMi marked this conversation as resolved.
Show resolved Hide resolved
self.assertWarnings(warning, expected)

def assertWarnings(self, warning, *expecteds):
self.assertEqual(len(warning.warnings), len(expecteds))
actual = [
(type(message), type(message.message), message.message.args)
for message in warning.warnings
]
expected = [
(warnings.WarningMessage, DeprecationWarning, (expected,))
for expected in expecteds
]
self.assertSequenceEqual(actual, expected)

def __openFile(self, mode):
for (_, _, functionName, _) in traceback.extract_stack():
if (
Expand Down
22 changes: 20 additions & 2 deletions tests/GithubApp.py
Expand Up @@ -22,7 +22,10 @@

from datetime import datetime

import github

from . import Framework
from .GithubIntegration import APP_ID, PRIVATE_KEY


class GithubApp(Framework.TestCase):
Expand Down Expand Up @@ -99,8 +102,23 @@ def testGetPublicApp(self):
self.assertEqual(app.url, "/apps/github-actions")

def testGetAuthenticatedApp(self):
# For this to work correctly in record mode, this test must be run with --auth_with_jwt
app = self.g.get_app()
auth = github.Auth.AppAuth(APP_ID, PRIVATE_KEY)
g = github.Github(auth=auth)

with self.assertWarns(DeprecationWarning) as warning:
# we ignore warnings from httpretty dependency
import warnings

warnings.filterwarnings("ignore", module="httpretty")

app = g.get_app()

self.assertWarning(
warning,
"Argument slug is mandatory, calling this method without the slug argument is deprecated, "
"please use github.GithubIntegration(auth=github.Auth.AppAuth(...)).get_app() instead",
)

self.assertEqual(app.created_at, datetime(2020, 8, 1, 17, 23, 46))
self.assertEqual(app.description, "Sample App to test PyGithub")
self.assertListEqual(
Expand Down
29 changes: 18 additions & 11 deletions tests/GithubIntegration.py
@@ -1,5 +1,4 @@
import time # NOQA
import warnings

import requests # NOQA

Expand Down Expand Up @@ -42,16 +41,6 @@ def setUp(self):
self.repo_installation_id = 30614431
self.user_installation_id = 30614431

def assertWarning(self, warning, expected):
self.assertWarnings(warning, expected)

def assertWarnings(self, warning, *expecteds):
self.assertEqual(len(warning.warnings), len(expecteds))
for message, expected in zip(warning.warnings, expecteds):
self.assertIsInstance(message, warnings.WarningMessage)
self.assertIsInstance(message.message, DeprecationWarning)
self.assertEqual(message.message.args, (expected,))

def testDeprecatedAppAuth(self):
# Replay data copied from testGetInstallations to test authentication only
with self.assertWarns(DeprecationWarning) as warning:
Expand All @@ -67,6 +56,16 @@ def testDeprecatedAppAuth(self):
"instead",
)

def testRequiredAppAuth(self):
# GithubIntegration requires AppAuth authentication.
for auth in [self.oauth_token, self.jwt, self.login]:
with self.assertRaises(AssertionError) as r:
github.GithubIntegration(auth=auth)
self.assertEqual(
str(r.exception),
f"GithubIntegration requires github.Auth.AppAuth authentication, not {type(auth)}",
)

def testAppAuth(self):
# Replay data copied from testDeprecatedAppAuth to test parity
auth = github.Auth.AppAuth(APP_ID, PRIVATE_KEY)
Expand Down Expand Up @@ -227,3 +226,11 @@ def testGetAccessTokenWithInvalidData(self):
)

self.assertEqual(raisedexp.exception.status, 400)

def testGetApp(self):
auth = github.Auth.AppAuth(APP_ID, PRIVATE_KEY)
github_integration = github.GithubIntegration(auth=auth)
app = github_integration.get_app()

self.assertEqual(app.name, "PyGithubTest")
self.assertEqual(app.url, "/apps/pygithubtest")