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

Users RPC transition #4026

Merged
merged 50 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
a7878c9
add users namespace in settings
Anish9901 Nov 12, 2024
f2da5f6
add utility functions for getting, listing, adding, updating and dele…
Anish9901 Nov 13, 2024
cf013c1
add RPC functions for getting, listing, adding, updating and deleting…
Anish9901 Nov 13, 2024
9e0416d
reorganise endpoints
Anish9901 Nov 13, 2024
ca3325c
add endpoint tests
Anish9901 Nov 13, 2024
e7e449e
manage imports in users.py
Anish9901 Nov 13, 2024
78a8e8c
create a users const for rpc methods
Anish9901 Nov 13, 2024
5197017
add users import in buildRpcApi
Anish9901 Nov 13, 2024
243cbab
transition users delete endpoint to RPC in the frontend
Anish9901 Nov 13, 2024
171ac96
use users.add RPC endpoint on the frontend
Anish9901 Nov 13, 2024
c5f6423
fix a bug in patch logic
Anish9901 Nov 14, 2024
b36ca38
define patch rpc in frontend
Anish9901 Nov 14, 2024
3cc4f5c
implement users.list for users stores
Anish9901 Nov 14, 2024
c8c1cf9
add users.list rpc endpoint for db settings route
Anish9901 Nov 14, 2024
c24c2f4
update imports and reorder them
Anish9901 Nov 14, 2024
9f5e3eb
run prettier
Anish9901 Nov 14, 2024
6af243b
reorder import
Anish9901 Nov 14, 2024
a1afc8c
make typecheck happy
Anish9901 Nov 14, 2024
ae5c437
add revoke_password util function
Anish9901 Nov 14, 2024
05d7b65
add users.password.revoke rpc function
Anish9901 Nov 14, 2024
659744c
update the logic in revoke password and change function names
Anish9901 Nov 14, 2024
6dcd983
add util function for changing own password
Anish9901 Nov 14, 2024
2a46837
add users.password.replace_own rpc function
Anish9901 Nov 14, 2024
b331deb
password_change should not be needed once it is changed by the user
Anish9901 Nov 14, 2024
1b81d47
make creating users atomic
Anish9901 Nov 14, 2024
d52de05
add replace_own and revoke rpc definition on the frontend
Anish9901 Nov 14, 2024
7c362f2
use users.replace_own and users.revoke RPC functions on the frontend
Anish9901 Nov 14, 2024
f1f733d
add endpoint tests for users.replace_own and users.revoke
Anish9901 Nov 14, 2024
f57d02f
use correct password revoke and replace_own endpoints from the frontend
Anish9901 Nov 14, 2024
8ddce75
rm user access policy
Anish9901 Nov 14, 2024
efb2573
rm user related viewsets and serializers
Anish9901 Nov 14, 2024
7f98158
rm rest/users.ts
Anish9901 Nov 14, 2024
ab44b2e
rm api/permission_conditions.py
Anish9901 Nov 14, 2024
06dcd2f
rm user api tests
Anish9901 Nov 14, 2024
5815fdd
rm test_custom_exceptions
Anish9901 Nov 14, 2024
c0f017d
Merge branch 'develop' into transition_user_endpoints_to_RPC
Anish9901 Nov 14, 2024
1a2b1fa
for some reason vulture is getting mad about pk in data_files
Anish9901 Nov 14, 2024
c33846b
simplify get_user_data
Anish9901 Nov 14, 2024
edb4ef9
remove user.password_change_needed from change_password as it happens…
Anish9901 Nov 14, 2024
b893977
add user.password_change_needed = False in change_password anyway
Anish9901 Nov 14, 2024
2213b43
i am already setting password_change_needed during user creation
Anish9901 Nov 14, 2024
b34300e
Fix request type to stay as CancellablePromise
seancolsen Nov 14, 2024
1718ccf
add auth decorators
Anish9901 Nov 15, 2024
a65d47f
add validation like logic in rpc definitions instead of underlying fu…
Anish9901 Nov 15, 2024
5fef10f
fix tests
Anish9901 Nov 15, 2024
7b31e81
implement patch_self & patch_other on the UI
Anish9901 Nov 18, 2024
98dad47
backend implementation for patch_self and patch_other
Anish9901 Nov 18, 2024
9730085
add endpoint tests
Anish9901 Nov 18, 2024
34742a2
remove short_name from UserDef
Anish9901 Nov 18, 2024
57857cf
Clean up User types
seancolsen Nov 19, 2024
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
1 change: 1 addition & 0 deletions config/settings/common_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def pipe_delim(pipe_string):
'mathesar.rpc.tables',
'mathesar.rpc.tables.metadata',
'mathesar.rpc.tables.privileges',
'mathesar.rpc.users'
]

TEMPLATES = [
Expand Down
11 changes: 0 additions & 11 deletions mathesar/api/permission_conditions.py

This file was deleted.

Empty file.
34 changes: 0 additions & 34 deletions mathesar/api/permissions/users.py

This file was deleted.

77 changes: 0 additions & 77 deletions mathesar/api/serializers/users.py

This file was deleted.

2 changes: 1 addition & 1 deletion mathesar/api/viewsets/data_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class DataFileViewSet(viewsets.GenericViewSet, ListModelMixin, RetrieveModelMixi
filterset_class = DataFileFilter
parser_classes = [MultiPartParser, JSONParser]

def partial_update(self, request, pk=None):
def partial_update(self, request, **kwargs):
serializer = DataFileSerializer(
data=request.data, context={'request': request}, partial=True
)
Expand Down
42 changes: 0 additions & 42 deletions mathesar/api/viewsets/users.py

This file was deleted.

140 changes: 140 additions & 0 deletions mathesar/rpc/users.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
"""
Classes and functions exposed to the RPC endpoint for managing mathesar users.
"""
from typing import Optional, TypedDict
from modernrpc.core import rpc_method, REQUEST_KEY
from modernrpc.auth.basic import (
http_basic_auth_login_required,
http_basic_auth_superuser_required
)

from mathesar.rpc.exceptions.handlers import handle_rpc_exceptions
from modernrpc.exceptions import AuthenticationFailed
from mathesar.utils.users import (
get_user,
list_users,
add_user,
update_user_info,
delete_user,
change_password,
revoke_password
)


class UserInfo(TypedDict):
id: int
username: str
is_superuser: bool
email: str
full_name: str
short_name: str
display_language: str

@classmethod
def from_model(cls, model):
return cls(
id=model.id,
username=model.username,
is_superuser=model.is_superuser,
email=model.email,
full_name=model.full_name,
short_name=model.short_name,
display_language=model.display_language
)


class UserDef(TypedDict):
username: str
password: str
is_superuser: bool
email: Optional[str]
full_name: Optional[str]
short_name: Optional[str]
display_language: Optional[str]


class SettableUserInfo(TypedDict):
username: Optional[str]
is_superuser: Optional[bool]
email: Optional[str]
full_name: Optional[str]
short_name: Optional[str]
display_language: Optional[str]


@rpc_method(name='users.add')
@http_basic_auth_superuser_required
@handle_rpc_exceptions
def add(*, user_def: UserDef) -> UserInfo:
user = add_user(user_def)
return UserInfo.from_model(user)


@rpc_method(name='users.delete')
@http_basic_auth_superuser_required
@handle_rpc_exceptions
def delete(*, user_id: int) -> None:
delete_user(user_id)
mathemancer marked this conversation as resolved.
Show resolved Hide resolved


@rpc_method(name="users.get")
@http_basic_auth_login_required
@handle_rpc_exceptions
def get(*, user_id: int) -> UserInfo:
user = get_user(user_id)
return UserInfo.from_model(user)


@rpc_method(name='users.list')
@http_basic_auth_login_required
@handle_rpc_exceptions
def list_() -> list[UserInfo]:
users = list_users()
return [UserInfo.from_model(user) for user in users]


@rpc_method(name='users.patch')
@http_basic_auth_login_required
@handle_rpc_exceptions
def patch(
*,
user_id: int,
user_info: SettableUserInfo,
**kwargs
) -> UserInfo:
user = kwargs.get(REQUEST_KEY).user
if not (user.id == user_id or user.is_superuser):
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you try to use the set_authentication_predicate decorator for this? I think it would be cleaner and more reusable to try to use that, or set up a custom auth decorator for the "is self or superuser" case.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would also ensure that if the upstream package changes the response for failure of these decorators, that this function doesn't behave differently.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is next to impossible to use set_authentication_predicate for testing is_self because, for is_self, we would require user_id which is a parameter that we pass through the request. The way django_modern_rpc is set up, the permission checks set up through set_authentication_predicate happen before the RPC function's args and kwargs are populated for RPC function execution.

So, even something like this doesn't work:

@wraps(func)
    def wrap(*args, **kwargs):
        user_id = kwargs.get('user_id')
        wrapper = auth.set_authentication_predicate(http_basic_auth_check_user, [user_is_self_or_superuser, user_id])
        return wrapper(func)(*args, **kwargs)

I also tried getting the user_id using the request, but request.POST results in None when printed inside http_basic_auth_check_user().

I finally settled on writing a custom decorator but without using set_authentication_predicate.

raise AuthenticationFailed('users.patch')
updated_user_info = update_user_info(
user_id,
user_info,
requesting_user=user
)
return UserInfo.from_model(updated_user_info)


@rpc_method(name='users.password.replace_own')
@http_basic_auth_login_required
@handle_rpc_exceptions
def replace_own(
*,
user_id: int,
old_password: str,
new_password: str,
**kwargs
) -> None:
user = kwargs.get(REQUEST_KEY).user
if not user.id == user_id:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. Try to use a custom auth decorator. You can just add one that guarantees that there's a user.id in the request that equals the user_id. Then, we have an added benefit of having a failure if a developer for some reason names the parameter something unexpected (i.e., something other than user_id)

raise AuthenticationFailed('users.password.replace_own')
change_password(user_id, old_password, new_password)


@rpc_method(name='users.password.revoke')
@http_basic_auth_superuser_required
@handle_rpc_exceptions
def revoke(
*,
user_id: int,
new_password: str,
) -> None:
revoke_password(user_id, new_password)
13 changes: 0 additions & 13 deletions mathesar/tests/api/test_custom_exceptions.py

This file was deleted.

Loading
Loading