Skip to content

Commit 7d258f7

Browse files
committed
fs: simplify auth
Compared to how we were using it in dvc before: 1) doesn't require/accept `tmp_dir` at all, caches creds in `appdirs("pydrive2fs", "iterative")`. 2) caching is per client_id, so no risk of using mismatched client_id and cached creds. 3) doesn't accept `user_credentials_file` (was used as a manual caching destination), because no one really wants to use it anyway. This eliminates the risk of users using mismatched client_id and cached creds. 4) renamed `service_account_json_file_path` into `client_json_file_path` for consistency. 5) doesn't accept `use_service_account`, as this is easilly derived from `client_json_file_path` being used. Other than that, this is mostly old code. Fixes #193
1 parent 0f5eb74 commit 7d258f7

File tree

3 files changed

+139
-3
lines changed

3 files changed

+139
-3
lines changed

pydrive2/fs/spec.py

Lines changed: 115 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import appdirs
12
import errno
23
import io
34
import logging
@@ -13,12 +14,17 @@
1314

1415
from pydrive2.drive import GoogleDrive
1516
from pydrive2.fs.utils import IterStream
17+
from pydrive2.auth import GoogleAuth
1618

1719
logger = logging.getLogger(__name__)
1820

1921
FOLDER_MIME_TYPE = "application/vnd.google-apps.folder"
2022

2123

24+
class GDriveAuthError(Exception):
25+
pass
26+
27+
2228
def _gdrive_retry(func):
2329
def should_retry(exc):
2430
from pydrive2.files import ApiRequestError
@@ -49,13 +55,120 @@ def should_retry(exc):
4955
)(func)
5056

5157

58+
def _cache_file(client_id):
59+
cache_dir = appdirs.user_cache_dir("pydrive2fs", "iterative")
60+
os.makedirs(cache_dir, exist_ok=True)
61+
return os.path.join(cache_dir, client_id + ".json")
62+
63+
5264
class GDriveFileSystem(AbstractFileSystem):
53-
def __init__(self, path, google_auth, trash_only=True, **kwargs):
65+
GDRIVE_CREDENTIALS_DATA = "GDRIVE_CREDENTIALS_DATA"
66+
67+
def __init__(
68+
self,
69+
path,
70+
google_auth=None,
71+
trash_only=True,
72+
client_id=None,
73+
client_secret=None,
74+
client_user_email=None,
75+
client_json_file_path=None,
76+
**kwargs,
77+
):
78+
super().__init__(**kwargs)
5479
self.path = path
5580
self.root, self.base = self.split_path(self.path)
81+
82+
if not google_auth:
83+
google_auth = self._auth(
84+
client_id=client_id,
85+
client_secret=client_secret,
86+
client_user_email=client_user_email,
87+
client_json_file_path=client_json_file_path,
88+
)
89+
5690
self.client = GoogleDrive(google_auth)
5791
self._trash_only = trash_only
58-
super().__init__(**kwargs)
92+
93+
def _auth(
94+
self,
95+
client_id=None,
96+
client_secret=None,
97+
client_user_email=None,
98+
client_json_file_path=None,
99+
):
100+
settings = {
101+
"get_refresh_token": True,
102+
"oauth_scope": [
103+
"https://www.googleapis.com/auth/drive",
104+
"https://www.googleapis.com/auth/drive.appdata",
105+
],
106+
}
107+
108+
env_creds = os.getenv(self.GDRIVE_CREDENTIALS_DATA)
109+
110+
if (
111+
not env_creds
112+
and not client_json_file_path
113+
and not (client_id and client_secret)
114+
):
115+
raise ValueError(
116+
"Specify credentials using one of these methods: "
117+
"client_id/client_secret or "
118+
"client_json_file_path or "
119+
f"{self.GDRIVE_CREDENTIALS_DATA}"
120+
)
121+
122+
if env_creds:
123+
settings["save_credentials"] = True
124+
settings["save_credentials_backend"] = "dictionary"
125+
settings["save_credentials_dict"] = {"creds": env_creds}
126+
settings["save_credentials_key"] = "creds"
127+
elif not client_json_file_path:
128+
settings["save_credentials"] = True
129+
settings["save_credentials_backend"] = "file"
130+
settings["save_credentials_file"] = _cache_file(client_id)
131+
132+
if client_json_file_path:
133+
config = {}
134+
135+
if client_user_email:
136+
config["client_user_email"] = client_user_email
137+
138+
if env_creds:
139+
config["client_json"] = env_creds
140+
else:
141+
config["client_json_file_path"] = client_json_file_path
142+
143+
settings["client_config_backend"] = "service"
144+
settings["service_config"] = config
145+
else:
146+
settings["client_config_backend"] = "settings"
147+
settings["client_config"] = {
148+
"client_id": client_id,
149+
"client_secret": client_secret,
150+
"auth_uri": "https://accounts.google.com/o/oauth2/auth",
151+
"token_uri": "https://oauth2.googleapis.com/token",
152+
"revoke_uri": "https://oauth2.googleapis.com/revoke",
153+
"redirect_uri": "",
154+
}
155+
156+
google_auth = GoogleAuth(settings=settings)
157+
158+
try:
159+
logger.debug("GDrive auth with config '%s'.", settings)
160+
if client_json_file_path:
161+
google_auth.ServiceAuth()
162+
else:
163+
google_auth.LocalWebserverAuth()
164+
except Exception as exc:
165+
# Handle AuthenticationError, RefreshError and other auth failures
166+
# It's hard to come up with a narrow exception, since PyDrive throws
167+
# a lot of different errors - broken credentials file, refresh token
168+
# expired, flow failed, etc.
169+
raise GDriveAuthError("Failed to authenticate GDrive") from exc
170+
171+
return google_auth
59172

60173
def split_path(self, path):
61174
parts = path.replace("//", "/").rstrip("/").split("/", 1)

pydrive2/test/test_fs.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,24 @@ def fs(tmpdir, base_remote_dir):
3636
return fs
3737

3838

39+
@pytest.mark.manual
40+
def test_fs_oauth(base_remote_dir):
41+
GDriveFileSystem(
42+
base_remote_dir,
43+
client_id="47794215776-cd9ssb6a4vv5otkq6n0iadpgc4efgjb1.apps.googleusercontent.com", # noqa: E501
44+
client_secret="i2gerGA7uBjZbR08HqSOSt9Z",
45+
)
46+
47+
48+
def test_fs_service_from_settings(base_remote_dir):
49+
creds = "credentials/fs.dat"
50+
setup_credentials(creds)
51+
GDriveFileSystem(
52+
base_remote_dir,
53+
client_json_file_path=creds,
54+
)
55+
56+
3957
def test_info(fs, tmpdir, remote_dir):
4058
fs.touch(remote_dir + "/info/a.txt")
4159
fs.touch(remote_dir + "/info/b.txt")

setup.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,12 @@
3737
"pyOpenSSL >= 19.1.0",
3838
],
3939
extras_require={
40-
"fsspec": ["fsspec >= 2021.07.0", "tqdm >= 4.0.0", "funcy >= 1.14"],
40+
"fsspec": [
41+
"fsspec >= 2021.07.0",
42+
"tqdm >= 4.0.0",
43+
"funcy >= 1.14",
44+
"appdirs >= 1.4.3",
45+
],
4146
"tests": tests_requirements,
4247
},
4348
python_requires=">=3.7",

0 commit comments

Comments
 (0)