Skip to content

Commit 1066489

Browse files
Warn when we see passwords where they are not needed. (#10)
* Warn when we see passwords where they are not needed. * cleanup docs
1 parent 02d7ac0 commit 1066489

File tree

5 files changed

+52
-1
lines changed

5 files changed

+52
-1
lines changed

src/planet_auth/auth_client.py

-1
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,6 @@ def login(
260260
a browser window.
261261
allow_tty_prompt: specify whether login is permitted to request
262262
input from the terminal.
263-
264263
Returns:
265264
Upon successful login, a Credential will be returned. The returned
266265
value will be in memory only. It is the responsibility of the

src/planet_auth/oidc/auth_client.py

+42
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@
2828
from planet_auth.oidc.api_clients.userinfo_api_client import UserinfoApiClient
2929
from planet_auth.oidc.api_clients.token_api_client import TokenApiClient
3030
from planet_auth.oidc.oidc_credential import FileBackedOidcCredential
31+
import planet_auth.logging.auth_logger
32+
33+
34+
auth_logger = planet_auth.logging.auth_logger.getAuthLogger()
3135

3236

3337
class OidcAuthClientConfig(AuthClientConfig, ABC):
@@ -426,6 +430,44 @@ def login(
426430
**kwargs,
427431
)
428432

433+
def _warn_password_kwarg(self, **kwargs):
434+
"""
435+
Helper function for _oidc_flow_login implementations to offer guidance to users
436+
and developers when options are unnecessary and will be ignored.
437+
"""
438+
if "password" in kwargs:
439+
if kwargs["password"]:
440+
# Safety check. "password" is a legitimate kwarg for some OAuth flows
441+
# like Resource Owner Flow. But, it should never be provided to the client
442+
# of other flows such as Auth Code or Device Code flows.
443+
# We could simply ignore it in the kwargs, but it's a good opportunity
444+
# to improve user or developer security practices.
445+
warning_msg = (
446+
"Supplying your password is not a supported option for the current login process. "
447+
"Protect your password. Do not expose your password unnecessarily."
448+
)
449+
# If we decide we want to not just warn, but halt user interactive
450+
# clients, uncomment this:
451+
# if allow_open_browser or allow_tty_prompt:
452+
# raise AuthCodeAuthClientException(message=warning_msg)
453+
auth_logger.warning(msg=warning_msg)
454+
455+
def _warn_ignored_kwargs(self, ignore_kws: list, **kwargs):
456+
"""
457+
Helper function for _oidc_flow_login implementations to offer guidance to users
458+
and developers when options are unnecessary and will be ignored. This is mostly
459+
to steer users away from habitually passing unnecessary arguments. OAuth flows
460+
behave differently enough that extra data through the generic login() kwargs
461+
is a problem we can anticipate.
462+
"""
463+
for ignore_kw in ignore_kws:
464+
if ignore_kw in kwargs:
465+
if kwargs[ignore_kw]:
466+
auth_logger.debug(
467+
msg=f'Ignoring "{ignore_kw}" argument to login. '
468+
"It is not used for the current login process."
469+
)
470+
429471
@abstractmethod
430472
def _oidc_flow_login(
431473
self,

src/planet_auth/oidc/auth_clients/auth_code_flow.py

+3
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ def _oidc_flow_login(
132132
A FileBackedOidcCredential object
133133
"""
134134

135+
self._warn_password_kwarg(**kwargs)
136+
self._warn_ignored_kwargs(["username", "password", "client_id", "client_secret"], **kwargs)
137+
135138
pkce_code_verifier, pkce_code_challenge = create_pkce_challenge_verifier_pair()
136139
if allow_open_browser:
137140
redirect_uri = self._authcode_client_config.local_redirect_uri()

src/planet_auth/oidc/auth_clients/client_credentials_flow.py

+4
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ def _oidc_flow_login(
9797
extra: Optional[dict],
9898
**kwargs,
9999
) -> FileBackedOidcCredential:
100+
self._warn_password_kwarg(**kwargs)
101+
self._warn_ignored_kwargs(["username", "password"], **kwargs)
100102
return FileBackedOidcCredential(
101103
self.token_client().get_token_from_client_credentials(
102104
client_id=self._ccauth_client_config.client_id(),
@@ -144,6 +146,8 @@ def _oidc_flow_login(
144146
extra: Optional[dict],
145147
**kwargs,
146148
):
149+
self._warn_password_kwarg(**kwargs)
150+
self._warn_ignored_kwargs(["username", "password"], **kwargs)
147151
return FileBackedOidcCredential(
148152
self.token_client().get_token_from_client_credentials(
149153
client_id=self._pubkey_client_config.client_id(),

src/planet_auth/oidc/auth_clients/device_code_flow.py

+3
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ def _oidc_flow_login(
161161
Returns:
162162
A FileBackedOidcCredential object
163163
"""
164+
self._warn_password_kwarg(**kwargs)
165+
self._warn_ignored_kwargs(["username", "password", "client_id", "client_secret"], **kwargs)
166+
164167
if not allow_tty_prompt:
165168
# Without a TTY to confirm the authorization code with the user,
166169
# the client using this library really must use device_login_initiate and

0 commit comments

Comments
 (0)