Skip to content

Commit

Permalink
finish remote user tests and fix issue where multiple remote users co…
Browse files Browse the repository at this point in the history
…uldn't be associated with an anonymous user
  • Loading branch information
mishaschwartz committed Apr 22, 2024
1 parent 249f4a1 commit df90ba2
Show file tree
Hide file tree
Showing 11 changed files with 599 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def upgrade():
op.create_table("network_remote_users",
sa.Column("id", sa.Integer, primary_key=True, autoincrement=True),
sa.Column("user_id", sa.Integer, sa.ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"),
nullable=False),
nullable=True),
sa.Column("network_node_id", sa.Integer,
sa.ForeignKey("network_nodes.id", onupdate="CASCADE", ondelete="CASCADE"),
nullable=False),
Expand Down
6 changes: 5 additions & 1 deletion magpie/api/login/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,12 @@ def network_login(request):
return ax.raise_http(http_error=HTTPUnauthorized,
detail=s.Signin_POST_UnauthorizedResponseSchema.description, nothrow=True)
authenticated_user = network_token.network_remote_user.user
if authenticated_user is None:
authenticated_user = network_token.network_remote_user.network_node.anonymous_user(request.db)
# We should never create a token for protected users but just in case
anonymous_regex = protected_user_name_regex(include_admin=False, settings_container=request)
# Note that we *should* create tokens for anonymous network users
anonymous_regex = protected_user_name_regex(include_admin=False, include_network=False,
settings_container=request)
ax.verify_param(authenticated_user.user_name, not_matches=True, param_compare=anonymous_regex,
http_error=HTTPForbidden, msg_on_fail=s.Signin_POST_ForbiddenResponseSchema.description)
return login_success_external(request, authenticated_user)
Expand Down
3 changes: 1 addition & 2 deletions magpie/api/management/network/network_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ def get_network_models_from_request_token(request, create_network_remote_user=Fa
.filter(models.NetworkRemoteUser.network_node_id == node.id)
.first())
if network_remote_user is None and create_network_remote_user:
anonymous_user = node.anonymous_user(request.db)
network_remote_user = models.NetworkRemoteUser(user=anonymous_user, network_node=node, name=user_name)
network_remote_user = models.NetworkRemoteUser(network_node=node, name=user_name)
request.db.add(network_remote_user)
return node, network_remote_user
5 changes: 2 additions & 3 deletions magpie/api/management/network/network_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ def delete_network_token_view(request):
node, network_remote_user = get_network_models_from_request_token(request)
if network_remote_user and network_remote_user.network_token:
request.db.delete(network_remote_user.network_token)
if (network_remote_user.user.id == node.anonymous_user(request.db).id and
sqlalchemy.inspect(network_remote_user).persistent):
if network_remote_user.user is None and sqlalchemy.inspect(network_remote_user).persistent:
request.db.delete(network_remote_user) # clean up unused record in the database
return ax.valid_http(http_success=HTTPOk, detail=s.NetworkToken_DELETE_OkResponseSchema.description)
ax.raise_http(http_error=HTTPNotFound, detail=s.NetworkNodeToken_DELETE_NotFoundResponseSchema.description)
Expand All @@ -57,7 +56,7 @@ def delete_network_tokens_view(request):
anonymous_network_user_ids = [n.anonymous_user(request.db).id for n in request.db.query(models.NetworkNode).all()]
# clean up unused records in the database (no need to keep records associated with anonymous network users)
(request.db.query(models.NetworkRemoteUser)
.filter(models.NetworkRemoteUser.user_id.in_(anonymous_network_user_ids))
.filter(models.NetworkRemoteUser.user_id == None) # noqa: E711 # pylint: disable=singleton-comparison
.filter(models.NetworkRemoteUser.network_token_id == None) # noqa: E711 # pylint: disable=singleton-comparison
.delete())
return ax.valid_http(http_success=HTTPOk,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@ def check_remote_user_access_permissions(request, remote_user=None):
remote_user = requested_remote_user(request)
admin_group = get_constant("MAGPIE_ADMIN_GROUP", settings_container=request)
is_admin = admin_group in [group.group_name for group in request.user.groups]
is_logged_user = request.user.user_name == remote_user.user.user_name
if remote_user.user is None:
associated_user = remote_user.network_node.anonymous_user(request.db)
else:
associated_user = remote_user.user
is_logged_user = request.user.user_name == associated_user.user_name
if not (is_admin or is_logged_user):
# admins can access any remote user, other users can only delete remote users associated with themselves
ax.raise_http(http_error=HTTPForbidden,
Expand Down
61 changes: 34 additions & 27 deletions magpie/api/management/network/remote_user/remote_user_views.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from pyramid.httpexceptions import HTTPBadRequest, HTTPCreated, HTTPForbidden, HTTPNotFound, HTTPOk
from pyramid.security import Authenticated
from pyramid.settings import asbool
from pyramid.view import view_config

from magpie import models
Expand All @@ -14,11 +15,12 @@
from magpie.constants import protected_user_name_regex


@s.NetworkRemoteUsersAPI.get(tags=[s.NetworkTag], response_schemas=s.NetworkRemoteUsers_GET_responses)
@s.NetworkRemoteUsersAPI.get(tags=[s.NetworkTag], schema=s.NetworkRemoteUsers_GET_RequestSchema,
response_schemas=s.NetworkRemoteUsers_GET_responses)
@view_config(route_name=s.NetworkRemoteUsersAPI.name, request_method="GET", decorator=check_network_mode_enabled)
def get_network_remote_users_view(request):
query = request.db.query(models.NetworkRemoteUser)
user_name = ar.get_multiformat_body(request, "user_name", default=None)
user_name = request.GET.get("user_name")
if user_name is not None:
query = query.join(models.User).filter(models.User.user_name == user_name)
nodes = [n.as_dict() for n in query.all()]
Expand All @@ -40,8 +42,8 @@ def get_network_remote_user_view(request):
response_schemas=s.NetworkRemoteUsers_POST_responses)
@view_config(route_name=s.NetworkRemoteUsersAPI.name, request_method="POST", decorator=check_network_mode_enabled)
def post_network_remote_users_view(request):
required_params = ("remote_user_name", "user_name", "node_name")
kwargs = {}
required_params = ("remote_user_name", "node_name")
kwargs = {"user_name": ar.get_multiformat_body(request, "user_name", default=None)}
for param in required_params:
value = ar.get_multiformat_body(request, param, default=None)
if value is None:
Expand All @@ -53,22 +55,25 @@ def post_network_remote_users_view(request):
http_error=HTTPNotFound,
msg_on_fail="No network node with name '{}' found".format(kwargs["node_name"])
)
forbidden_user_names_regex = protected_user_name_regex(include_admin=False, settings_container=request)
ax.verify_param(kwargs["user_name"], not_matches=True, param_compare=forbidden_user_names_regex,
param_name="user_name",
http_error=HTTPForbidden, content={"user_name": kwargs["user_name"]},
msg_on_fail=s.NetworkRemoteUsers_POST_ForbiddenResponseSchema.description)
user = ax.evaluate_call(
lambda: request.db.query(models.User).filter(models.User.user_name == kwargs["user_name"]).one(),
http_error=HTTPNotFound,
msg_on_fail="No user with user_name '{}' found".format(kwargs["user_name"])
)
anonymous_user = node.anonymous_user(request.db)
if kwargs["user_name"] is None:
user = anonymous_user
else:
user = ax.evaluate_call(
lambda: request.db.query(models.User).filter(models.User.user_name == kwargs["user_name"]).one(),
http_error=HTTPNotFound,
msg_on_fail="No user with user_name '{}' found".format(kwargs["user_name"])
)
remote_user_name = kwargs["remote_user_name"]
ax.verify_param(remote_user_name, not_empty=True,
param_name="remote_user_name",
http_error=HTTPForbidden,
msg_on_fail="remote_user_name is empty")
remote_user = models.NetworkRemoteUser(user_id=user.id, network_node_id=node.id, name=remote_user_name)
if user.id == anonymous_user.id:
user_id = None
else:
user_id = user.id
remote_user = models.NetworkRemoteUser(user_id=user_id, network_node_id=node.id, name=remote_user_name)
request.db.add(remote_user)
return ax.valid_http(http_success=HTTPCreated, detail=s.NetworkRemoteUsers_POST_CreatedResponseSchema.description)

Expand All @@ -78,32 +83,34 @@ def post_network_remote_users_view(request):
@view_config(route_name=s.NetworkRemoteUserAPI.name, request_method="PATCH", decorator=check_network_mode_enabled)
def patch_network_remote_user_view(request):
kwargs = {p: ar.get_multiformat_body(request, p, default=None) for p in
("remote_user_name", "user_name", "node_name")}
("remote_user_name", "user_name", "node_name", "assign_anonymous")}
if not any(kwargs.values()):
ax.raise_http(http_error=HTTPBadRequest, detail=s.NetworkRemoteUser_PATCH_BadRequestResponseSchema.description)
remote_user = requested_remote_user(request)
if "remote_user_name" in kwargs:
if kwargs["remote_user_name"]:
remote_user_name = kwargs["remote_user_name"]
ax.verify_param(remote_user_name, not_empty=True,
param_name="remote_user_name",
http_error=HTTPForbidden,
http_error=HTTPBadRequest,
msg_on_fail="remote_user_name is empty")
remote_user.name = remote_user_name
if "user_name" in kwargs:
user = ax.evaluate_call(
lambda: request.db.query(models.User).filter(models.User.user_name == kwargs["user_name"]).one(),
http_error=HTTPNotFound,
msg_on_fail="No user with user_name '{}' found".format(kwargs["user_name"])
)
remote_user.user_id = user.id
if "node_name" in kwargs:
if kwargs["node_name"]:
node = ax.evaluate_call(
lambda: request.db.query(models.NetworkNode).filter(
models.NetworkNode.name == kwargs["node_name"]).one(),
http_error=HTTPNotFound,
msg_on_fail="No network node with name '{}' found".format(kwargs["node_name"])
)
remote_user.network_node_id = node.id
if kwargs["user_name"]:
user = ax.evaluate_call(
lambda: request.db.query(models.User).filter(models.User.user_name == kwargs["user_name"]).one(),
http_error=HTTPNotFound,
msg_on_fail="No user with user_name '{}' found".format(kwargs["user_name"])
)
remote_user.user_id = user.id
elif asbool(kwargs["assign_anonymous"]):
remote_user.user_id = None
return ax.valid_http(http_success=HTTPOk, detail=s.NetworkRemoteUsers_PATCH_OkResponseSchema.description)


Expand All @@ -124,4 +131,4 @@ def get_network_remote_users_current_view(request):
nodes = [n.as_dict() for n in
request.db.query(models.NetworkRemoteUser).filter(models.NetworkRemoteUser.user_id == request.user.id)]
return ax.valid_http(http_success=HTTPOk, detail=s.NetworkRemoteUsers_GET_OkResponseSchema.description,
content={"nodes": nodes})
content={"remote_users": nodes})
21 changes: 20 additions & 1 deletion magpie/api/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -3724,7 +3724,7 @@ class NetworkNodeLink_GET_OkResponseSchema(BaseResponseSchemaAPI):
class NetworkRemoteUser_PATCH_RequestBodySchema(colander.MappingSchema):
remote_user_name = colander.SchemaNode(
colander.String(),
description="Name of the associated user a remote Magpie node (instance) in the network.",
description="Name of the associated user from another remote Magpie node (instance) in the network.",
example="userAremote",
missing=colander.drop
)
Expand All @@ -3740,6 +3740,13 @@ class NetworkRemoteUser_PATCH_RequestBodySchema(colander.MappingSchema):
example="NodeA",
missing=colander.drop
)
assign_anonymous = colander.SchemaNode(
colander.Boolean(),
description="Associate this network user with the anonymous user for the associated Magpie node (instance). "
"Only has an effect if the user_name parameter is not specified.",
example=True,
default=False
)


class NetworkRemoteUser_PATCH_RequestSchema(BaseRequestSchemaAPI):
Expand All @@ -3761,6 +3768,14 @@ class NetworkRemoteUsers_GET_NotFoundResponseSchema(BaseResponseSchemaAPI):
body = ErrorResponseBodySchema(code=HTTPNotFound.code, description=description)


class NetworkRemoteUser_GET_BodySchema(colander.MappingSchema):
user_name = colander.SchemaNode(
colander.String(),
description="Name of the associated user on this Magpie node (instance).",
example="userAlocal"
)


class NetworkRemoteUser_BodySchema(colander.MappingSchema):
remote_user_name = colander.SchemaNode(
colander.String(),
Expand Down Expand Up @@ -3805,6 +3820,10 @@ class NetworkRemoteUsers_GET_OkResponseSchema(BaseResponseSchemaAPI):
body = NetworkRemoteUsers_GET_OkResponseBodySchema(code=HTTPOk.code, description=description)


class NetworkRemoteUsers_GET_RequestSchema(BaseRequestSchemaAPI):
body = NetworkRemoteUser_GET_BodySchema()


class NetworkRemoteUsers_POST_RequestSchema(BaseRequestSchemaAPI):
body = NetworkRemoteUser_BodySchema()

Expand Down
4 changes: 1 addition & 3 deletions magpie/cli/purge_expired_network_tokens.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,9 @@ def main(args=None, parser=None, namespace=None):
else:
db_session = get_db_session_from_config_ini(args.ini_config)
deleted = models.NetworkToken.delete_expired(db_session)
anonymous_network_user_ids = [n.anonymous_user(db_session).id for n in
db_session.query(models.NetworkNode).all()]
# clean up unused records in the database (no need to keep records associated with anonymous network users)
(db_session.query(models.NetworkRemoteUser)
.filter(models.NetworkRemoteUser.user_id.in_(anonymous_network_user_ids))
.filter(models.NetworkRemoteUser.user_id == None) # noqa: E711 # pylint: disable=singleton-comparison
.filter(models.NetworkRemoteUser.network_token_id == None) # noqa: E711 # pylint: disable=singleton-comparison
.delete())
try:
Expand Down
7 changes: 5 additions & 2 deletions magpie/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1007,8 +1007,11 @@ class NetworkRemoteUser(BaseModel, Base):
__tablename__ = "network_remote_users"

id = sa.Column(sa.Integer(), primary_key=True, nullable=False, autoincrement=True)
# Note: a null user_id indicates that this NetworkRemoteUser is associated with the anonymous user for the
# associated network node. This is to ensure that the unique constraint defined below allows for multiple
# NetworkRemoteUsers to be associated with the anonymous users.
user_id = sa.Column(sa.Integer,
sa.ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False)
sa.ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=True)
user = relationship("User", foreign_keys=[user_id])

network_node_id = sa.Column(sa.Integer,
Expand All @@ -1027,7 +1030,7 @@ class NetworkRemoteUser(BaseModel, Base):
def as_dict(self):
# type: () -> Dict[Str, Str]
return {
"user_name": self.user.user_name,
"user_name": getattr(self.user, "user_name", self.network_node.anonymous_user_name()),
"remote_user_name": self.name,
"node_name": self.network_node.name
}
Expand Down
Loading

0 comments on commit df90ba2

Please sign in to comment.