Skip to content

Commit

Permalink
Deprecate is_write_action and write_permission=True when login
Browse files Browse the repository at this point in the history
  • Loading branch information
Wauplin committed Oct 24, 2024
1 parent ab87526 commit b6222de
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 110 deletions.
92 changes: 38 additions & 54 deletions src/huggingface_hub/_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import os
import subprocess
from functools import partial
from getpass import getpass
from pathlib import Path
from typing import Optional
Expand All @@ -42,6 +41,7 @@
_save_token,
get_stored_tokens,
)
from .utils._deprecation import _deprecate_arguments, _deprecate_positional_args


logger = logging.get_logger(__name__)
Expand All @@ -55,8 +55,15 @@
"""


@_deprecate_arguments(
version="1.0",
deprecated_args="write_permission",
custom_message="Fine-grained tokens added complexity to the permissions, making it irrelevant to check if a token has 'write' access.",
)
@_deprecate_positional_args(version="1.0")
def login(
token: Optional[str] = None,
*,
add_to_git_credential: bool = False,
new_session: bool = True,
write_permission: bool = False,
Expand Down Expand Up @@ -97,8 +104,8 @@ def login(
to the end user.
new_session (`bool`, defaults to `True`):
If `True`, will request a token even if one is already saved on the machine.
write_permission (`bool`, defaults to `False`):
If `True`, requires a token with write permission.
write_permission (`bool`):
Ignored and deprecated argument.
Raises:
[`ValueError`](https://docs.python.org/3/library/exceptions.html#ValueError)
If an organization token is passed. Only personal account tokens are valid
Expand All @@ -116,11 +123,11 @@ def login(
"`--add-to-git-credential` if using via `huggingface-cli` if "
"you want to set the git credential as well."
)
_login(token, add_to_git_credential=add_to_git_credential, write_permission=write_permission)
_login(token, add_to_git_credential=add_to_git_credential)
elif is_notebook():
notebook_login(new_session=new_session, write_permission=write_permission)
notebook_login(new_session=new_session)
else:
interpreter_login(new_session=new_session, write_permission=write_permission)
interpreter_login(new_session=new_session)


def logout(token_name: Optional[str] = None) -> None:
Expand Down Expand Up @@ -235,7 +242,13 @@ def auth_list() -> None:
###


def interpreter_login(new_session: bool = True, write_permission: bool = False) -> None:
@_deprecate_arguments(
version="1.0",
deprecated_args="write_permission",
custom_message="Fine-grained tokens added complexity to the permissions, making it irrelevant to check if a token has 'write' access.",
)
@_deprecate_positional_args(version="1.0")
def interpreter_login(*, new_session: bool = True, write_permission: bool = False) -> None:
"""
Displays a prompt to log in to the HF website and store the token.
Expand All @@ -248,11 +261,10 @@ def interpreter_login(new_session: bool = True, write_permission: bool = False)
Args:
new_session (`bool`, defaults to `True`):
If `True`, will request a token even if one is already saved on the machine.
write_permission (`bool`, defaults to `False`):
If `True`, requires a token with write permission.
write_permission (`bool`):
Ignored and deprecated argument.
"""
if not new_session and _current_token_okay(write_permission=write_permission):
if not new_session and get_token() is not None:
logger.info("User is already logged in.")
return

Expand All @@ -275,11 +287,7 @@ def interpreter_login(new_session: bool = True, write_permission: bool = False)
token = getpass("Enter your token (input will not be visible): ")
add_to_git_credential = _ask_for_confirmation_no_tui("Add token as git credential?")

_login(
token=token,
add_to_git_credential=add_to_git_credential,
write_permission=write_permission,
)
_login(token=token, add_to_git_credential=add_to_git_credential)


###
Expand All @@ -306,7 +314,13 @@ def interpreter_login(new_session: bool = True, write_permission: bool = False)
notebooks. </center>"""


def notebook_login(new_session: bool = True, write_permission: bool = False) -> None:
@_deprecate_arguments(
version="1.0",
deprecated_args="write_permission",
custom_message="Fine-grained tokens added complexity to the permissions, making it irrelevant to check if a token has 'write' access.",
)
@_deprecate_positional_args(version="1.0")
def notebook_login(*, new_session: bool = True, write_permission: bool = False) -> None:
"""
Displays a widget to log in to the HF website and store the token.
Expand All @@ -319,8 +333,8 @@ def notebook_login(new_session: bool = True, write_permission: bool = False) ->
Args:
new_session (`bool`, defaults to `True`):
If `True`, will request a token even if one is already saved on the machine.
write_permission (`bool`, defaults to `False`):
If `True`, requires a token with write permission.
write_permission (`bool`):
Ignored and deprecated argument.
"""
try:
import ipywidgets.widgets as widgets # type: ignore
Expand All @@ -330,7 +344,7 @@ def notebook_login(new_session: bool = True, write_permission: bool = False) ->
"The `notebook_login` function can only be used in a notebook (Jupyter or"
" Colab) and you need the `ipywidgets` module: `pip install ipywidgets`."
)
if not new_session and _current_token_okay(write_permission=write_permission):
if not new_session and get_token() is not None:
logger.info("User is already logged in.")
return

Expand All @@ -353,14 +367,8 @@ def notebook_login(new_session: bool = True, write_permission: bool = False) ->
display(login_token_widget)

# On click events
def login_token_event(t, write_permission: bool = False):
"""
Event handler for the login button.
Args:
write_permission (`bool`, defaults to `False`):
If `True`, requires a token with write permission.
"""
def login_token_event(t):
"""Event handler for the login button."""
token = token_widget.value
add_to_git_credential = git_checkbox_widget.value
# Erase token and clear value to make sure it's not saved in the notebook.
Expand All @@ -369,14 +377,14 @@ def login_token_event(t, write_permission: bool = False):
login_token_widget.children = [widgets.Label("Connecting...")]
try:
with capture_output() as captured:
_login(token, add_to_git_credential=add_to_git_credential, write_permission=write_permission)
_login(token, add_to_git_credential=add_to_git_credential)
message = captured.getvalue()
except Exception as error:
message = str(error)
# Print result (success message or error)
login_token_widget.children = [widgets.Label(line) for line in message.split("\n") if line.strip()]

token_finish_button.on_click(partial(login_token_event, write_permission=write_permission))
token_finish_button.on_click(login_token_event)


###
Expand All @@ -387,7 +395,6 @@ def login_token_event(t, write_permission: bool = False):
def _login(
token: str,
add_to_git_credential: bool,
write_permission: bool = False,
) -> None:
from .hf_api import whoami # avoid circular import

Expand All @@ -396,11 +403,6 @@ def _login(

token_info = whoami(token)
permission = token_info["auth"]["accessToken"]["role"]
if write_permission and permission != "write":
raise ValueError(
"Token is valid but is 'read-only' and a 'write' token is required.\nPlease provide a new token with"
" correct permission."
)
logger.info(f"Token is valid (permission: {permission}).")

token_name = token_info["auth"]["accessToken"]["displayName"]
Expand Down Expand Up @@ -469,24 +471,6 @@ def _set_active_token(
logger.info(f"Your token has been saved to {constants.HF_TOKEN_PATH}")


def _current_token_okay(write_permission: bool = False):
"""Check if the current token is valid.
Args:
write_permission (`bool`, defaults to `False`):
If `True`, requires a token with write permission.
Returns:
`bool`: `True` if the current token is valid, `False` otherwise.
"""
from .hf_api import get_token_permission # avoid circular import

permission = get_token_permission()
if permission is None or (write_permission and permission != "write"):
return False
return True


def _is_git_credential_helper_configured() -> bool:
"""Check if a git credential helper is configured.
Expand Down
2 changes: 0 additions & 2 deletions src/huggingface_hub/hf_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -9082,7 +9082,6 @@ def delete_webhook(self, webhook_id: str, *, token: Union[bool, str, None] = Non
def _build_hf_headers(
self,
token: Union[bool, str, None] = None,
is_write_action: bool = False,
library_name: Optional[str] = None,
library_version: Optional[str] = None,
user_agent: Union[Dict, str, None] = None,
Expand All @@ -9096,7 +9095,6 @@ def _build_hf_headers(
token = self.token
return build_hf_headers(
token=token,
is_write_action=is_write_action,
library_name=library_name or self.library_name,
library_version=library_version or self.library_version,
user_agent=user_agent or self.user_agent,
Expand Down
34 changes: 9 additions & 25 deletions src/huggingface_hub/utils/_headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

from .. import constants
from ._auth import get_token
from ._deprecation import _deprecate_arguments
from ._runtime import (
get_fastai_version,
get_fastcore_version,
Expand All @@ -35,15 +36,20 @@
from ._validators import validate_hf_hub_args


@_deprecate_arguments(
version="1.0",
deprecated_args="is_write_action",
custom_message="This argument is ignored and we let the server handle the permission error instead (if any).",
)
@validate_hf_hub_args
def build_hf_headers(
*,
token: Optional[Union[bool, str]] = None,
is_write_action: bool = False,
library_name: Optional[str] = None,
library_version: Optional[str] = None,
user_agent: Union[Dict, str, None] = None,
headers: Optional[Dict[str, str]] = None,
is_write_action: bool = False,
) -> Dict[str, str]:
"""
Build headers dictionary to send in a HF Hub call.
Expand All @@ -68,9 +74,6 @@ def build_hf_headers(
- if `False`, authorization header is not set
- if `None`, the token is read from the machine only except if
`HF_HUB_DISABLE_IMPLICIT_TOKEN` env variable is set.
is_write_action (`bool`, default to `False`):
Set to True if the API call requires a write access. If `True`, the token
will be validated (cannot be `None`, cannot start by `"api_org***"`).
library_name (`str`, *optional*):
The name of the library that is making the HTTP request. Will be added to
the user-agent header.
Expand All @@ -83,6 +86,8 @@ def build_hf_headers(
headers (`dict`, *optional*):
Additional headers to include in the request. Those headers take precedence
over the ones generated by this function.
is_write_action (`bool`):
Ignored and deprecated argument.
Returns:
A `Dict` of headers to pass in your API call.
Expand All @@ -105,9 +110,6 @@ def build_hf_headers(
>>> build_hf_headers() # token is not sent
{"user-agent": ...}
>>> build_hf_headers(token="api_org_***", is_write_action=True)
ValueError: You must use your personal account token for write-access methods.
>>> build_hf_headers(library_name="transformers", library_version="1.2.3")
{"authorization": ..., "user-agent": "transformers/1.2.3; hf_hub/0.10.2; python/3.10.4; tensorflow/1.55"}
```
Expand All @@ -122,7 +124,6 @@ def build_hf_headers(
"""
# Get auth token to send
token_to_send = get_token_to_send(token)
_validate_token_to_send(token_to_send, is_write_action=is_write_action)

# Combine headers
hf_headers = {
Expand Down Expand Up @@ -171,23 +172,6 @@ def get_token_to_send(token: Optional[Union[bool, str]]) -> Optional[str]:
return cached_token


def _validate_token_to_send(token: Optional[str], is_write_action: bool) -> None:
if is_write_action:
if token is None:
raise ValueError(
"Token is required (write-access action) but no token found. You need"
" to provide a token or be logged in to Hugging Face with"
" `huggingface-cli login` or `huggingface_hub.login`. See"
" https://huggingface.co/settings/tokens."
)
if token.startswith("api_org"):
raise ValueError(
"You must use your personal account token for write-access methods. To"
" generate a write-access token, go to"
" https://huggingface.co/settings/tokens"
)


def _http_user_agent(
*,
library_name: Optional[str] = None,
Expand Down
16 changes: 0 additions & 16 deletions tests/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,22 +88,6 @@ def test_login_success(self, mock_whoami):
assert _get_token_by_name("test_token") == TOKEN
assert _get_token_from_file() == TOKEN

@patch(
"huggingface_hub.hf_api.whoami",
return_value={
"auth": {
"accessToken": {
"displayName": "test_token",
"role": "read",
"createdAt": "2024-01-01T00:00:00.000Z",
}
}
},
)
def test_login_errors(self, mock_whoami):
with pytest.raises(ValueError, match=r"Token is valid but is 'read-only' and a 'write' token is required.*"):
_login(TOKEN, add_to_git_credential=False, write_permission=True)


class TestLogout:
def test_logout_deletes_files(self):
Expand Down
13 changes: 0 additions & 13 deletions tests/test_utils_headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,6 @@ def test_use_auth_token_none_no_cached_token(self, mock_get_token: Mock) -> None
def test_use_auth_token_none_has_cached_token(self, mock_get_token: Mock) -> None:
self.assertEqual(build_hf_headers(), FAKE_TOKEN_HEADER)

def test_write_action_org_token(self) -> None:
with self.assertRaises(ValueError):
build_hf_headers(use_auth_token=FAKE_TOKEN_ORG, is_write_action=True)

@patch("huggingface_hub.utils._headers.get_token", return_value=None)
def test_write_action_none_token(self, mock_get_token: Mock) -> None:
with self.assertRaises(ValueError):
build_hf_headers(is_write_action=True)

def test_write_action_use_auth_token_false(self) -> None:
with self.assertRaises(ValueError):
build_hf_headers(use_auth_token=False, is_write_action=True)

@patch("huggingface_hub.utils._headers.get_token", return_value=FAKE_TOKEN)
def test_implicit_use_disabled(self, mock_get_token: Mock) -> None:
with patch( # not as decorator to avoid friction with @handle_injection
Expand Down

0 comments on commit b6222de

Please sign in to comment.