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

Fix/rest #17083

Merged
merged 5 commits into from
Oct 7, 2024
Merged

Fix/rest #17083

Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion conan/cli/commands/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ def remote_login(conan_api, parser, subparser, *args):
if args.username is not None and args.password is not None:
user, password = args.username, args.password
else:
user, password = creds.auth(r, args.username)
user, password, _ = creds.auth(r, args.username)
if args.username is not None and args.username != user:
raise ConanException(f"User '{args.username}' doesn't match user '{user}' in "
f"credentials.json or environment variables")
Expand Down
27 changes: 9 additions & 18 deletions conans/client/rest/auth_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@
get_conan with the new token.
"""

import hashlib
from uuid import getnode as get_mac

from conan.api.output import ConanOutput
from conans.client.rest.remote_credentials import RemoteCredentials
from conans.client.rest.rest_client import RestApiClient
Expand Down Expand Up @@ -51,7 +48,8 @@ def call_rest_api_method(self, remote, method_name, *args, **kwargs):
# Anonymous is not enough, ask for a user
ConanOutput().info('Please log in to "%s" to perform this action. '
'Execute "conan remote login" command.' % remote.name)
return self._retry_with_new_token(user, remote, method_name, *args, **kwargs)
if self._get_credentials_and_authenticate(user, remote):
return self.call_rest_api_method(remote, method_name, *args, **kwargs)
elif token and refresh_token:
# If we have a refresh token try to refresh the access token
try:
Expand All @@ -67,13 +65,13 @@ def call_rest_api_method(self, remote, method_name, *args, **kwargs):
self._clear_user_tokens_in_db(user, remote)
return self.call_rest_api_method(remote, method_name, *args, **kwargs)

def _retry_with_new_token(self, user, remote, method_name, *args, **kwargs):
def _get_credentials_and_authenticate(self, user, remote):
"""Try LOGIN_RETRIES to obtain a password from user input for which
we can get a valid token from api_client. If a token is returned,
credentials are stored in localdb and rest method is called"""
creds = RemoteCredentials(self._cache_folder, self._global_conf)
for _ in range(LOGIN_RETRIES):
creds = RemoteCredentials(self._cache_folder, self._global_conf)
input_user, input_password = creds.auth(remote)
input_user, input_password, interactive = creds.auth(remote)
try:
self._authenticate(remote, input_user, input_password)
except AuthenticationException:
Expand All @@ -82,16 +80,15 @@ def _retry_with_new_token(self, user, remote, method_name, *args, **kwargs):
out.error('Wrong user or password', error_type="exception")
else:
out.error(f'Wrong password for user "{user}"', error_type="exception")
if not interactive:
raise AuthenticationException(f"Authentication error in remote '{remote.name}'")
else:
return self.call_rest_api_method(remote, method_name, *args, **kwargs)

return True
raise AuthenticationException("Too many failed login attempts, bye!")

def _get_rest_client(self, remote):
username, token, refresh_token = self._localdb.get_login(remote.url)
custom_headers = {'X-Client-Anonymous-Id': self._get_mac_digest(),
'X-Client-Id': str(username or "")}
return RestApiClient(remote, token, refresh_token, custom_headers, self._requester,
return RestApiClient(remote, token, refresh_token, self._requester,
self._global_conf, self._cached_capabilities)

def _clear_user_tokens_in_db(self, user, remote):
Expand All @@ -102,12 +99,6 @@ def _clear_user_tokens_in_db(self, user, remote):
out.error('Your credentials could not be stored in local cache\n', error_type="exception")
out.debug(str(e) + '\n')

@staticmethod
def _get_mac_digest():
sha1 = hashlib.sha1()
sha1.update(str(get_mac()).encode())
return str(sha1.hexdigest())

def _authenticate(self, remote, user, password):
rest_client = self._get_rest_client(remote)
if user is None: # The user is already in DB, just need the password
Expand Down
8 changes: 4 additions & 4 deletions conans/client/rest/remote_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ def auth(self, remote, user=None):
msg = scoped_traceback(msg, e, scope="/extensions/plugins")
raise ConanException(msg)
if plugin_user and plugin_password:
return plugin_user, plugin_password
return plugin_user, plugin_password, False

# Then prioritize the cache "credentials.json" file
creds = self._urls.get(remote.name)
if creds is not None:
try:
return creds["user"], creds["password"]
return creds["user"], creds["password"], False
except KeyError as e:
raise ConanException(f"Authentication error, wrong credentials.json: {e}")

Expand All @@ -58,12 +58,12 @@ def auth(self, remote, user=None):
if env_passwd is not None:
if env_user is None:
raise ConanException("Found password in env-var, but not defined user")
return env_user, env_passwd
return env_user, env_passwd, False

# If not found, then interactive prompt
ui = UserInput(self._global_conf.get("core:non_interactive", check_type=bool))
input_user, input_password = ui.request_login(remote.name, user)
return input_user, input_password
return input_user, input_password, True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like UserInput throws if core:non_interactive is True, so at this point we're always interactive 👍


@staticmethod
def _get_env(remote, user):
Expand Down
11 changes: 4 additions & 7 deletions conans/client/rest/rest_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@ class RestApiClient:
Rest Api Client for handle remote.
"""

def __init__(self, remote, token, refresh_token, custom_headers, requester,
config, cached_capabilities):

def __init__(self, remote, token, refresh_token, requester, config, cached_capabilities):
# Set to instance
self._token = token
self._refresh_token = refresh_token
self._remote_url = remote.url
self._custom_headers = custom_headers
self._requester = requester

self._verify_ssl = remote.verify_ssl
Expand All @@ -27,7 +24,7 @@ def __init__(self, remote, token, refresh_token, custom_headers, requester,
def _capable(self, capability, user=None, password=None):
capabilities = self._cached_capabilities.get(self._remote_url)
if capabilities is None:
tmp = RestV2Methods(self._remote_url, self._token, self._custom_headers,
tmp = RestV2Methods(self._remote_url, self._token,
self._requester, self._config, self._verify_ssl)
capabilities = tmp.server_capabilities(user, password)
self._cached_capabilities[self._remote_url] = capabilities
Expand All @@ -41,7 +38,7 @@ def _get_api(self):
"Conan 2.0 is no longer compatible with "
"remotes that don't accept revisions.")
checksum_deploy = self._capable(CHECKSUM_DEPLOY)
return RestV2Methods(self._remote_url, self._token, self._custom_headers,
return RestV2Methods(self._remote_url, self._token,
self._requester, self._config, self._verify_ssl,
checksum_deploy)

Expand All @@ -61,7 +58,7 @@ def upload_package(self, pref, files_to_upload):
return self._get_api().upload_package(pref, files_to_upload)

def authenticate(self, user, password):
api_v2 = RestV2Methods(self._remote_url, self._token, self._custom_headers,
api_v2 = RestV2Methods(self._remote_url, self._token,
self._requester, self._config, self._verify_ssl)

if self._refresh_token and self._token:
Expand Down
35 changes: 20 additions & 15 deletions conans/client/rest/rest_client_v2.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import copy
import fnmatch
import hashlib
import json
import os

from requests.auth import AuthBase, HTTPBasicAuth
from uuid import getnode as get_mac

from conan.api.output import ConanOutput

Expand All @@ -24,11 +26,11 @@ class JWTAuth(AuthBase):
"""Attaches JWT Authentication to the given Request object."""

def __init__(self, token):
self.token = token
self.bearer = "Bearer %s" % str(token) if token else None

def __call__(self, request):
if self.token:
request.headers['Authorization'] = "Bearer %s" % str(self.token)
if self.bearer:
request.headers['Authorization'] = self.bearer
return request


Expand All @@ -47,21 +49,28 @@ def get_exception_from_error(error_code):
return None


def _get_mac_digest(): # To avoid re-hashing all the time the same mac
cached = getattr(_get_mac_digest, "_cached_value", None)
if cached is not None:
return cached
sha1 = hashlib.sha1()
sha1.update(str(get_mac()).encode())
cached = str(sha1.hexdigest())
_get_mac_digest._cached_value = cached
return cached


class RestV2Methods:

def __init__(self, remote_url, token, custom_headers, requester, config, verify_ssl,
checksum_deploy=False):
self.token = token
def __init__(self, remote_url, token, requester, config, verify_ssl, checksum_deploy=False):
self.remote_url = remote_url
self.custom_headers = custom_headers
self.custom_headers = {'X-Client-Anonymous-Id': _get_mac_digest()}
self.requester = requester
self._config = config
self.verify_ssl = verify_ssl
self._checksum_deploy = checksum_deploy

@property
def auth(self):
return JWTAuth(self.token)
self.router = ClientV2Router(self.remote_url.rstrip("/"))
self.auth = JWTAuth(token)

@staticmethod
def _check_error_response(ret):
Expand Down Expand Up @@ -240,10 +249,6 @@ def search_packages(self, ref):
package_infos = self._get_json(url)
return package_infos

@property
def router(self):
return ClientV2Router(self.remote_url.rstrip("/"))

def _get_file_list_json(self, url):
data = self._get_json(url)
# Discarding (.keys()) still empty metadata for files
Expand Down
6 changes: 3 additions & 3 deletions test/integration/remote/auth_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,11 @@ def get(url, **kwargs):
elif "ping" in url:
resp_basic_auth.headers = {"Content-Type": "application/json",
"X-Conan-Server-Capabilities": "revisions"}
token = getattr(kwargs["auth"], "token", None)
bearer = getattr(kwargs["auth"], "bearer", None)
password = getattr(kwargs["auth"], "password", None)
if token and token != "TOKEN":
if bearer and bearer != "Bearer TOKEN":
raise Exception("Bad JWT Token")
if not token and not password:
if not bearer and not password:
raise AuthenticationException(
"I'm an Artifactory without anonymous access that "
"requires authentication for the ping endpoint and "
Expand Down
2 changes: 1 addition & 1 deletion test/integration/remote/rest_api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def setUpClass(cls):
cls.auth_manager._authenticate(cls.remote, user="private_user",
password="private_pass")
cls.api = RestApiClient(cls.remote, localdb.access_token, localdb.refresh_token,
{}, requester, config, {})
requester, config, {})

@classmethod
def tearDownClass(cls):
Expand Down
2 changes: 1 addition & 1 deletion test/integration/remote/token_refresh_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def __init__(self):
class RequesterWithTokenMock(object):

def get(self, url, **kwargs):
if not kwargs["auth"].token or kwargs["auth"].token == "expired":
if not kwargs["auth"].bearer or "expired" in kwargs["auth"].bearer:
return ResponseAuthenticationRequired()
if url.endswith("files"):
return ResponseDownloadURLs()
Expand Down