Skip to content

Commit

Permalink
[FEATURE] remove random password generation when creating a user and …
Browse files Browse the repository at this point in the history
…password is not provided (#4993)

# Description

This PR remove the generation of a random password when a user is
created a password is not provided.

With these changes when a user is created and no password is provided a
`UnprocessableEntityError` will be raised from the backend showing that
a `password` attribute must be provided.

Closes #4933 

**Type of change**

(Please delete options that are not relevant. Remember to title the PR
according to the type of change)

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Refactor (change restructuring the codebase without changing
functionality)
- [x] Improvement (change adding some improvement to an existing
functionality)
- [ ] Documentation update

**How Has This Been Tested**

(Please describe the tests that you ran to verify your changes. And
ideally, reference `tests`)

- [x] Adding new tests and checking that old ones are working.

**Checklist**

- [ ] I added relevant documentation
- [ ] follows the style guidelines of this project
- [ ] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK)
(see text above)
- [ ] I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)
  • Loading branch information
jfcalvo authored Jun 12, 2024
1 parent 560bd7a commit a667a90
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 23 deletions.
5 changes: 1 addition & 4 deletions argilla-sdk/docs/how_to_guides/user.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,6 @@ current_user = client.me

To create a new user in Argilla, you can define it in the `User` class and then call the `create` method. This method is inherited from the `Resource` base class and operates without modifications.

!!! info Password generation
If you don't provide a password, a random one will be generated for you. Ensure you store this password securely by accessing it through `user.password` immediately after user creation. For security reasons, the password will not be retrievable later.

```python
import argilla_sdk as rg

Expand Down Expand Up @@ -198,4 +195,4 @@ client = rg.Argilla(api_url="<api_url>", api_key="<api_key>")
user_to_delete = client.users('my_username')

deleted_user = user_to_delete.delete()
```
```
20 changes: 4 additions & 16 deletions argilla-sdk/src/argilla_sdk/users/_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@

from argilla_sdk import Workspace
from argilla_sdk._api import UsersAPI
from argilla_sdk._models import UserModel, Role
from argilla_sdk._models import Role, UserModel
from argilla_sdk._resource import Resource
from argilla_sdk.client import Argilla


class User(Resource):
"""Class for interacting with Argilla users in the Argilla server. User profiles \
are used to manage access to the Argilla server and track responses to records.
Attributes:
username (str): The username of the user.
first_name (str): The first name of the user.
Expand Down Expand Up @@ -56,7 +56,7 @@ def __init__(
first_name (str): The first name of the user
last_name (str): The last name of the user
role (str): The role of the user, either 'annotator', admin, or 'owner'
password (str): The password of the user. If not provided, a random password will be generated
password (str): The password of the user
client (Argilla): The client used to interact with Argilla
Returns:
Expand All @@ -69,7 +69,7 @@ def __init__(
if _model is None:
_model = UserModel(
username=username,
password=password or self._generate_random_password(),
password=password,
first_name=first_name or username,
last_name=last_name,
role=role or Role.annotator,
Expand Down Expand Up @@ -176,15 +176,3 @@ def role(self) -> Role:
@role.setter
def role(self, value: Role) -> None:
self._model.role = value

############################
# Private methods
############################

@staticmethod
def _generate_random_password(n: int = 12) -> str:
"""Generates a random password for the user"""
import random
import string

return "".join(random.choices(string.ascii_letters + string.digits, k=n))
22 changes: 20 additions & 2 deletions argilla-sdk/tests/integration/test_manage_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
# limitations under the License.

import pytest

from argilla_sdk import User, Argilla
from argilla_sdk import Argilla, User
from argilla_sdk._exceptions import UnprocessableEntityError


@pytest.fixture(scope="session", autouse=True)
Expand All @@ -35,8 +35,26 @@ def test_create_user(self, client: Argilla):
assert user.id is not None
assert client.users(username=user.username).id == user.id

def test_create_user_without_password(self, client: Argilla):
user = User(username="test_user")
with pytest.raises(expected_exception=UnprocessableEntityError):
client.users.add(user)

def test_delete_user(self, client: Argilla):
user = User(username="test_delete_user", password="test_password")
client.users.add(user)
user.delete()
assert not user.exists()

def test_add_user_to_workspace(self, client: Argilla):
user = User(username="test_user", password="test_password")
client.users.add(user)

workspace = client.workspaces(name="test_workspace")
workspace.create()

user = client.users(username="test_user")
assert user.password is None

user.add_to_workspace(workspace)
assert user in workspace.users
4 changes: 3 additions & 1 deletion argilla-sdk/tests/unit/test_resources/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def test_serialize(self):
)

assert user.serialize()["username"] == "test-user"
assert user.serialize()["password"] is None

def test_json_serialize(self):
mock_uuid = uuid.uuid4()
Expand Down Expand Up @@ -283,6 +284,7 @@ def test_create_user(self, httpx_mock: HTTPXMock):
"first_name": "Test",
"last_name": "User",
"role": "admin",
"password": "test-password",
"inserted_at": datetime.utcnow().isoformat(),
"updated_at": datetime.utcnow().isoformat(),
}
Expand All @@ -295,4 +297,4 @@ def test_create_user(self, httpx_mock: HTTPXMock):
user = client.api.users.create(user_create)
assert user.id == user_id
assert user.username == "test-user"
assert user.password is None
assert user.password == "test-password"

0 comments on commit a667a90

Please sign in to comment.