-
Notifications
You must be signed in to change notification settings - Fork 69
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
Migrating to Google Auth Library #180
Closed
Closed
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
0236bf9
Update setup.py with new google-auth dependencies
junpeng-jp 0b67559
Update threadsafe code following google recommendations
junpeng-jp 799b005
Migration to google-auth library (#89)
junpeng-jp 2a0a767
Implement lockfile storage (#89)
junpeng-jp 9230969
Cleanup of oauth tests & remove service config read/write checks
junpeng-jp 2a688e2
Add oauth test 10: google.auth.default auth
junpeng-jp a3bc162
Deprecate command line auth (#173)
junpeng-jp c9a0f90
Remove dependencies on oauth2client
junpeng-jp 012f9ed
Changed mocker.spy to track calls to FileBackend's __init__
junpeng-jp 015a84f
Add simulated token expiry test
junpeng-jp 51af33c
Reverted DEFAULT_SETTINGS to a class static variable
junpeng-jp 0b1c17b
Fixed typo in GoogleAuth __init__ documentation
junpeng-jp 9ea5409
Removed non-essential improvements to settings.py
junpeng-jp d8c1571
Remove unnecessary edits to .yaml cred files
junpeng-jp 35e9ae3
Update incorrect documentation for oauth Test 10
junpeng-jp badbd4f
Add back LoadAuth & thread_local HTTP
junpeng-jp 08d067c
Remove Refresh method as it is now handled by google auth library
junpeng-jp 2477cbe
Add back "Authentication successful" message
junpeng-jp 91a4f63
Clean-up LocalWebserverAuth & added OSError documentation
junpeng-jp 9a9af28
Allow trying of new ports for all OSErrors
junpeng-jp 46ba0c1
Allow manual overwrite of self.http for API resources
junpeng-jp 710eee1
Deprecate CommandLineAuth
junpeng-jp 6b3e901
Added Thread Locking & updated credential write logic
junpeng-jp 82a8960
Add threading import
junpeng-jp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
_OLD_CLIENT_CONFIG_KEYS = frozenset( | ||
( | ||
"client_id", | ||
"client_secret", | ||
"auth_uri", | ||
"token_uri", | ||
"revoke_uri", | ||
"redirect_uri", | ||
) | ||
) | ||
|
||
_CLIENT_CONFIG_KEYS = frozenset( | ||
( | ||
"client_id", | ||
"client_secret", | ||
"auth_uri", | ||
"token_uri", | ||
"redirect_uris", | ||
) | ||
) | ||
|
||
|
||
def verify_client_config(client_config, with_oauth_type=True): | ||
"""Verifies that format of the client config | ||
loaded from a Google-format client secrets file. | ||
""" | ||
|
||
oauth_type = None | ||
config = client_config | ||
|
||
if with_oauth_type: | ||
if "web" in client_config: | ||
oauth_type = "web" | ||
config = config["web"] | ||
|
||
elif "installed" in client_config: | ||
oauth_type = "installed" | ||
config = config["installed"] | ||
else: | ||
raise ValueError( | ||
"Client secrets must be for a web or installed app" | ||
) | ||
|
||
# This is the older format of client config | ||
if _OLD_CLIENT_CONFIG_KEYS.issubset(config.keys()): | ||
config["redirect_uris"] = [config["redirect_uri"]] | ||
|
||
# by default, the redirect uri is the first in the list | ||
if "redirect_uri" not in config: | ||
config["redirect_uri"] = config["redirect_uris"][0] | ||
|
||
if "revoke_uri" not in config: | ||
config["revoke_uri"] = "https://oauth2.googleapis.com/revoke" | ||
|
||
if not _CLIENT_CONFIG_KEYS.issubset(config.keys()): | ||
raise ValueError("Client secrets is not in the correct format.") | ||
|
||
return oauth_type, config |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
import os | ||
import json | ||
import warnings | ||
import threading | ||
|
||
|
||
_SYM_LINK_MESSAGE = "File: {0}: Is a symbolic link." | ||
_IS_DIR_MESSAGE = "{0}: Is a directory" | ||
_MISSING_FILE_MESSAGE = "Cannot access {0}: No such file or directory" | ||
|
||
|
||
def validate_file(filename): | ||
if os.path.islink(filename): | ||
raise IOError(_SYM_LINK_MESSAGE.format(filename)) | ||
elif os.path.isdir(filename): | ||
raise IOError(_IS_DIR_MESSAGE.format(filename)) | ||
elif not os.path.isfile(filename): | ||
warnings.warn(_MISSING_FILE_MESSAGE.format(filename)) | ||
|
||
|
||
class CredentialBackend(object): | ||
"""Adapter that provides a consistent interface to read and write credential files""" | ||
|
||
def _read_credentials(self, rpath): | ||
"""Specific implementation of how the storage object should retrieve a file.""" | ||
return NotImplementedError | ||
|
||
def _store_credentials(self, credential, rpath): | ||
"""Specific implementation of how the storage object should write a file""" | ||
return NotImplementedError | ||
|
||
def _delete_credentials(self, rpath): | ||
"""Specific implementation of how the storage object should delete a file.""" | ||
return NotImplementedError | ||
|
||
def read_credentials(self, rpath): | ||
"""Reads a credential config file and returns the config as a dictionary | ||
:param fname: host name of the local web server. | ||
:type host_name: str.` | ||
:return: A credential file | ||
""" | ||
return self._read_credentials(rpath) | ||
|
||
def store_credentials(self, credential, rpath): | ||
"""Write a credential to | ||
The Storage lock must be held when this is called. | ||
Args: | ||
credentials: Credentials, the credentials to store. | ||
""" | ||
self._store_credentials(credential, rpath) | ||
|
||
def delete_credentials(self, rpath): | ||
"""Delete credential. | ||
Frees any resources associated with storing the credential. | ||
The Storage lock must *not* be held when this is called. | ||
|
||
Returns: | ||
None | ||
""" | ||
self._delete_credentials(rpath) | ||
|
||
|
||
class FileBackend(CredentialBackend): | ||
"""Read and write credentials to a file backend with Thread-locking""" | ||
|
||
def __init__(self): | ||
self._thread_lock = threading.Lock() | ||
|
||
def _create_file_if_needed(self, rpath): | ||
"""Create an empty file if necessary. | ||
This method will not initialize the file. Instead it implements a | ||
simple version of "touch" to ensure the file has been created. | ||
""" | ||
if not os.path.exists(rpath): | ||
old_umask = os.umask(0o177) | ||
try: | ||
open(rpath, "a+b").close() | ||
finally: | ||
os.umask(old_umask) | ||
|
||
def _read_credentials(self, rpath): | ||
"""Reads a local json file and parses the information into a info dictionary. | ||
Returns: | ||
Raises: | ||
""" | ||
with self._thread_lock: | ||
validate_file(rpath) | ||
with open(rpath, "r") as json_file: | ||
return json.load(json_file) | ||
|
||
def _store_credentials(self, credentials, rpath): | ||
"""Writes current credentials to a local json file. | ||
Args: | ||
Raises: | ||
""" | ||
with self._thread_lock: | ||
# write new credentials to the temp file | ||
dirname, filename = os.path.split(rpath) | ||
temp_path = os.path.join(dirname, "temp_{}".format(filename)) | ||
self._create_file_if_needed(temp_path) | ||
|
||
with open(temp_path, "w") as json_file: | ||
json_file.write(credentials.to_json()) | ||
|
||
# replace the existing credential file | ||
os.replace(temp_path, rpath) | ||
|
||
def _delete_credentials(self, rpath): | ||
"""Delete Credentials file. | ||
Args: | ||
credentials: Credentials, the credentials to store. | ||
""" | ||
with self._thread_lock: | ||
os.unlink(rpath) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
client_config_backend: file | ||
client_config_file: tmp/pydrive2/user.json | ||
|
||
save_credentials: True | ||
save_credentials_backend: file | ||
save_credentials_file: credentials/default_user.dat | ||
|
||
oauth_scope: | ||
- https://www.googleapis.com/auth/drive | ||
|
||
get_refresh_token: True |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
client_config_backend: file | ||
client_config_file: tmp/pydrive2/user.json | ||
|
||
save_credentials: True | ||
save_credentials_backend: file | ||
save_credentials_file: credentials/default_user_no_refresh.dat | ||
|
||
oauth_scope: | ||
- https://www.googleapis.com/auth/drive |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
client_config_backend: service | ||
service_config: | ||
use_default: True | ||
oauth_scope: | ||
- https://www.googleapis.com/auth/drive |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
import pytest | ||
from pydrive2.auth import GoogleAuth | ||
from pydrive2.drive import GoogleDrive | ||
from pydrive2.test.test_util import ( | ||
settings_file_path, | ||
setup_credentials, | ||
pydrive_retry, | ||
delete_file, | ||
) | ||
from google.auth.exceptions import RefreshError | ||
|
||
|
||
@pytest.fixture | ||
def googleauth_refresh(): | ||
setup_credentials() | ||
# Delete old credentials file | ||
delete_file("credentials/default_user.dat") | ||
ga = GoogleAuth(settings_file_path("default_user.yaml")) | ||
ga.LocalWebserverAuth() | ||
|
||
return ga | ||
|
||
|
||
@pytest.fixture | ||
def googleauth_no_refresh(): | ||
setup_credentials() | ||
# Delete old credentials file | ||
delete_file("credentials/default_user_no_refresh.dat") | ||
ga = GoogleAuth(settings_file_path("default_user_no_refresh.yaml")) | ||
ga.LocalWebserverAuth() | ||
|
||
return ga | ||
|
||
|
||
@pytest.mark.manual | ||
def test_01_TokenExpiryWithRefreshToken(googleauth_refresh): | ||
gdrive = GoogleDrive(googleauth_refresh) | ||
|
||
about_object = pydrive_retry(gdrive.GetAbout) | ||
assert about_object is not None | ||
|
||
# save the first access token for comparison | ||
token1 = gdrive.auth.credentials.token | ||
|
||
# simulate token expiry by deleting the underlying token | ||
gdrive.auth.credentials.token = None | ||
|
||
# credential object should still exist but access token expired | ||
assert gdrive.auth.credentials | ||
assert gdrive.auth.access_token_expired | ||
|
||
about_object = pydrive_retry(gdrive.GetAbout) | ||
assert about_object is not None | ||
|
||
# save the second access token for comparison | ||
token2 = gdrive.auth.credentials.token | ||
|
||
assert token1 != token2 | ||
|
||
|
||
@pytest.mark.manual | ||
def test_02_TokenExpiryWithoutRefreshToken(googleauth_no_refresh): | ||
gdrive = GoogleDrive(googleauth_no_refresh) | ||
|
||
about_object = pydrive_retry(gdrive.GetAbout) | ||
assert about_object is not None | ||
|
||
# simulate token expiry by deleting the underlying token | ||
gdrive.auth.credentials.token = None | ||
|
||
# credential object should still exist but access token expired | ||
assert gdrive.auth.credentials | ||
assert gdrive.auth.access_token_expired | ||
|
||
# as credentials have no refresh token, this would fail | ||
with pytest.raises(RefreshError) as e_info: | ||
about_object = pydrive_retry(gdrive.GetAbout) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we skip for the sake of this PR the default credentials auth? I think it should be done outside of the ServiceAuth + probably we should be using a different setting for this (don't know which one yet)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll shift this into a subsequent PR once we've migrated to new google auth library