engineering/unrhodecode
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

security: limit API impersonate feature to only super admins

Browse source
This commit is contained in:
RhodeCode Admin 2025-03-24 05:00:28 +01:00
parent c2914495c0
commit aef3cca3ea
7 changed files with 94 additions and 97 deletions
Download patch file Download diff file Expand all files Collapse all files

3
docs/release-notes/release-notes-5.5.1.rst
View file

@ -18,7 +18,8 @@ General
Security
^^^^^^^^
- API: changed impersonate features to only be allowed by super-admins instead of repo admins.
This applies better practices to this feature of API.
Performance
^^^^^^^^^^^

2
rhodecode/api/tests/test_close_pull_request.py
View file

@ -91,7 +91,7 @@ class TestClosePullRequest(object):
)
response = api_call(self.app, params)
expected = "userid is not the same as your user"
expected = "Provided userid is not the same the user calling this function"
assert_error(id_, expected, given=response.body)
@pytest.mark.backends("git", "hg")

4
rhodecode/api/tests/test_comment_pull_request.py
View file

@ -234,7 +234,7 @@ class TestCommentPullRequest(object):
)
response = api_call(self.app, params)
expected = "userid is not the same as your user"
expected = "Provided userid is not the same the user calling this function"
assert_error(id_, expected, given=response.body)
@pytest.mark.backends("git", "hg")
@ -360,7 +360,7 @@ class TestCommentPullRequest(object):
userid=TEST_USER_ADMIN_LOGIN,
)
response = api_call(self.app, params)
expected = "userid is not the same as your user"
expected = "Provided userid is not the same the user calling this function"
assert_error(id_, expected, given=response.body)
@pytest.mark.backends("git", "hg")

2
rhodecode/api/tests/test_merge_pull_request.py
View file

@ -263,5 +263,5 @@ class TestMergePullRequest(object):
)
response = api_call(self.app, params)
expected = "userid is not the same as your user"
expected = "Provided userid is not the same the user calling this function"
assert_error(id_, expected, given=response.body)

51
rhodecode/api/views/pull_request_api.py
View file

@ -147,7 +147,7 @@ def get_pull_request(request, apiuser, pullrequestid, repoid=Optional(None), mer
repo = pull_request.target_repo
if not PullRequestModel().check_user_read(pull_request, apiuser, api=True):
raise JSONRPCError("repository `%s` or pull request `%s` does not exist" % (repoid, pullrequestid))
raise JSONRPCError(f"repository `{repoid}` or pull request `{pullrequestid}` does not exist")
# NOTE(marcink): only calculate and return merge state if the pr state is 'created'
# otherwise we can lock the repo on calculation of merge state while update/merge
@ -302,21 +302,19 @@ def merge_pull_request(request, apiuser, pullrequestid, repoid=Optional(None), u
repo = get_repo_or_error(repoid)
else:
repo = pull_request.target_repo
auth_user = apiuser
auth_user = apiuser
if not isinstance(userid, Optional):
is_repo_admin = HasRepoPermissionAnyApi("repository.admin")(user=apiuser, repo_name=repo.repo_name)
if has_superadmin_permission(apiuser) or is_repo_admin:
# super-admin can impersonate other users
if has_superadmin_permission(apiuser):
apiuser = get_user_or_error(userid)
auth_user = apiuser.AuthUser()
else:
raise JSONRPCError("userid is not the same as your user")
raise JSONRPCError("Provided userid is not the same the user calling this function")
if pull_request.pull_request_state != PullRequest.STATE_CREATED:
raise JSONRPCError(
"Operation forbidden because pull request is in state {}, only state {} is allowed.".format(
pull_request.pull_request_state, PullRequest.STATE_CREATED
)
f"Operation forbidden because pull request is in state {pull_request.pull_request_state}, only state {PullRequest.STATE_CREATED} is allowed."
)
with pull_request.set_state(PullRequest.STATE_UPDATING):
@ -533,20 +531,26 @@ def comment_pull_request(
repo = pull_request.target_repo
db_repo_name = repo.repo_name
auth_user = apiuser
if not isinstance(userid, Optional):
is_repo_admin = HasRepoPermissionAnyApi("repository.admin")(user=apiuser, repo_name=db_repo_name)
if has_superadmin_permission(apiuser) or is_repo_admin:
if isinstance(userid, Optional):
userid = apiuser.user_id
else:
# super-admin can impersonate other users
if has_superadmin_permission(apiuser):
apiuser = get_user_or_error(userid)
userid = apiuser.user_id
auth_user = apiuser.AuthUser()
else:
raise JSONRPCError("userid is not the same as your user")
raise JSONRPCError("Provided userid is not the same the user calling this function")
if pull_request.is_closed():
raise JSONRPCError(f"pull request `{pullrequestid}` comment failed, pull request is closed")
if not PullRequestModel().check_user_read(pull_request, apiuser, api=True):
raise JSONRPCError(f"repository `{repoid}` does not exist")
user = get_user_or_error(userid)
message = Optional.extract(message)
status = Optional.extract(status)
commit_id = Optional.extract(commit_id)
@ -594,7 +598,7 @@ def comment_pull_request(
comment = CommentsModel().create(
text=text,
repo=pull_request.target_repo.repo_id,
user=apiuser.user_id,
user=user,
pull_request=pull_request.pull_request_id,
f_path=None,
line_no=None,
@ -928,9 +932,9 @@ def update_pull_request(
repo = pull_request.target_repo
if not PullRequestModel().check_user_update(pull_request, apiuser, api=True):
raise JSONRPCError("pull request `{}` update failed, no permission to update.".format(pullrequestid))
raise JSONRPCError(f"pull request `{pullrequestid}` update failed, no permission to update.")
if pull_request.is_closed():
raise JSONRPCError("pull request `{}` update failed, pull request is closed".format(pullrequestid))
raise JSONRPCError(f"pull request `{pullrequestid}` update failed, pull request is closed")
reviewer_objects = Optional.extract(reviewers) or []
observer_objects = Optional.extract(observers) or []
@ -959,9 +963,7 @@ def update_pull_request(
if str2bool(Optional.extract(update_commits)):
if pull_request.pull_request_state != PullRequest.STATE_CREATED:
raise JSONRPCError(
"Operation forbidden because pull request is in state {}, only state {} is allowed.".format(
pull_request.pull_request_state, PullRequest.STATE_CREATED
)
f"Operation forbidden because pull request is in state {pull_request.pull_request_state}, only state {PullRequest.STATE_CREATED} is allowed."
)
with pull_request.set_state(PullRequest.STATE_UPDATING):
@ -1092,12 +1094,13 @@ def close_pull_request(
else:
repo = pull_request.target_repo
is_repo_admin = HasRepoPermissionAnyApi("repository.admin")(user=apiuser, repo_name=repo.repo_name)
auth_user = apiuser
if not isinstance(userid, Optional):
if has_superadmin_permission(apiuser) or is_repo_admin:
apiuser = get_user_or_error(userid)
if has_superadmin_permission(apiuser):
# super-admin can impersonate other users
auth_user = get_user_or_error(userid)
else:
raise JSONRPCError("userid is not the same as your user")
raise JSONRPCError("Provided userid is not the same the user calling this function")
if pull_request.is_closed():
raise JSONRPCError(f"pull request `{pullrequestid}` is already closed")
@ -1106,14 +1109,14 @@ def close_pull_request(
allowed_to_close = PullRequestModel().check_user_update(pull_request, apiuser, api=True)
if not allowed_to_close:
raise JSONRPCError("pull request `{}` close failed, no permission to close.".format(pullrequestid))
raise JSONRPCError(f"pull request `{pullrequestid}` close failed, no permission to close.")
# message we're using to close the PR, else it's automatically generated
message = Optional.extract(message)
# finally close the PR, with proper message comment
comment, status = PullRequestModel().close_pull_request_with_comment(
pull_request, apiuser, repo, message=message, auth_user=apiuser
pull_request, apiuser, repo, message=message, auth_user=auth_user
)
status_lbl = ChangesetStatus.get_status_lbl(status)

101
rhodecode/api/views/repo_api.py
View file

@ -600,7 +600,7 @@ def get_repo_file(
_extended_types = ["minimal", "minimal+search", "basic", "full"]
if details not in _extended_types:
ret_types = ",".join(_extended_types)
raise JSONRPCError(f"ret_type must be one of %s, got {ret_types}", details)
raise JSONRPCError(f"ret_type must be one of {_extended_types}, got {ret_types}", details)
extended_info = False
content = False
@ -695,7 +695,7 @@ def get_repo_fts_tree(request, apiuser, repoid, commit_id, root_path):
except Exception:
log.exception("Exception occurred while trying to get repo nodes")
raise JSONRPCError("failed to get repo: `%s` nodes" % repo.repo_name)
raise JSONRPCError(f"failed to get repo: `{repo.repo_name}` nodes")
@jsonrpc_method()
@ -755,7 +755,7 @@ def get_repo_refs(request, apiuser, repoid):
return refs
except Exception:
log.exception("Exception occurred while trying to get repo refs")
raise JSONRPCError("failed to get repo: `%s` references" % repo.repo_name)
raise JSONRPCError(f"failed to get repo: `{repo.repo_name}` references")
@jsonrpc_method()
@ -918,14 +918,14 @@ def create_repo(
task_id = get_task_id(task)
# no commit, it's done in RepoModel, or async via celery
return {
"msg": "Created new repository `{}`".format(schema_data["repo_name"]),
"msg": f"Created new repository `{schema_data['repo_name']}`",
"success": True, # cannot return the repo data here since fork
# can be done async
"task": task_id,
}
except Exception:
log.exception("Exception while trying to create the repository %s", schema_data["repo_name"])
raise JSONRPCError("failed to create repository `{}`".format(schema_data["repo_name"]))
raise JSONRPCError(f"failed to create repository `{schema_data['repo_name']}`")
@jsonrpc_method()
@ -994,7 +994,7 @@ def remove_field_from_repo(request, apiuser, repoid, key):
field = RepositoryField.get_by_key_name(key, repo)
if not field:
raise JSONRPCError("Field with key `%s` does not exists for repo `%s`" % (key, repoid))
raise JSONRPCError(f"Field with key `{key}` does not exists for repo `{repoid}`")
try:
RepoModel().delete_repo_field(repo, field_key=key)
@ -1165,7 +1165,7 @@ def update_repo(
}
except Exception:
log.exception("Exception while trying to update the repository %s", repoid)
raise JSONRPCError("failed to update repo `%s`" % repoid)
raise JSONRPCError(f"failed to update repo `{repoid}`")
@jsonrpc_method()
@ -1310,14 +1310,14 @@ def fork_repo(
task_id = get_task_id(task)
return {
"msg": "Created fork of `{}` as `{}`".format(repo.repo_name, schema_data["repo_name"]),
"msg": f"Created fork of `{repo.repo_name}` as `{schema_data['repo_name']}`",
"success": True, # cannot return the repo data here since fork
# can be done async
"task": task_id,
}
except Exception:
log.exception("Exception while trying to create fork %s", schema_data["repo_name"])
raise JSONRPCError("failed to fork repository `{}` as `{}`".format(repo_name, schema_data["repo_name"]))
raise JSONRPCError(f"failed to fork repository `{repo_name}` as `{schema_data['repo_name']}`")
@jsonrpc_method()
@ -1361,11 +1361,11 @@ def delete_repo(request, apiuser, repoid, forks=Optional("")):
_forks_msg = ""
_forks = [f for f in repo.forks]
if handle_forks == "detach":
_forks_msg = " " + "Detached %s forks" % len(_forks)
_forks_msg = " " + f"Detached {len(_forks)} forks"
elif handle_forks == "delete":
_forks_msg = " " + "Deleted %s forks" % len(_forks)
_forks_msg = " " + f"Deleted {len(_forks)} forks"
elif _forks:
raise JSONRPCError("Cannot delete `%s` it still contains attached forks" % (repo.repo_name,))
raise JSONRPCError(f"Cannot delete `{repo.repo_name}` it still contains attached forks")
old_data = repo.get_api_data()
RepoModel().delete(repo, forks=forks)
@ -1509,7 +1509,7 @@ def lock(request, apiuser, repoid, locked=Optional(None), userid=Optional(OAttr(
# make sure normal user does not pass someone else userid,
# he is not allowed to do that
if not isinstance(userid, Optional) and userid != apiuser.user_id:
raise JSONRPCError("userid is not the same as your user")
raise JSONRPCError("Provided userid is not the same the user calling this function")
if isinstance(userid, Optional):
userid = apiuser.user_id
@ -1527,7 +1527,7 @@ def lock(request, apiuser, repoid, locked=Optional(None), userid=Optional(OAttr(
"locked_by": None,
"lock_reason": None,
"lock_state_changed": False,
"msg": "Repo `%s` not locked." % repo.repo_name,
"msg": f"Repo `{repo.repo_name}` not locked.",
}
return _d
else:
@ -1541,8 +1541,7 @@ def lock(request, apiuser, repoid, locked=Optional(None), userid=Optional(OAttr(
"lock_reason": _reason,
"lock_state_changed": False,
"msg": (
"Repo `%s` locked by `%s` on `%s`."
% (repo.repo_name, lock_user.username, json.dumps(time_to_datetime(_time)))
f"Repo `{repo.repo_name}` locked by `{lock_user.username}` on `{json.dumps(time_to_datetime(_time))}`."
),
}
return _d
@ -1565,12 +1564,12 @@ def lock(request, apiuser, repoid, locked=Optional(None), userid=Optional(OAttr(
"locked_by": user.username,
"lock_reason": lock_reason,
"lock_state_changed": True,
"msg": ("User `%s` set lock state for repo `%s` to `%s`" % (user.username, repo.repo_name, locked)),
"msg": f"User `{user.username}` set lock state for repo `{repo.repo_name}` to `{locked}`",
}
return _d
except Exception:
log.exception("Exception occurred while trying to lock repository")
raise JSONRPCError("Error occurred locking repository `%s`" % repo.repo_name)
raise JSONRPCError(f"Error occurred locking repository `{repo.repo_name}`")
@jsonrpc_method()
@ -1643,8 +1642,17 @@ def comment_commit(
log.exception("Failed to fetch commit")
raise JSONRPCError(safe_str(e))
auth_user = apiuser
if isinstance(userid, Optional):
userid = apiuser.user_id
else:
# super-admin can impersonate other users
if has_superadmin_permission(apiuser):
apiuser = get_user_or_error(userid)
userid = apiuser.user_id
auth_user = apiuser.AuthUser()
else:
raise JSONRPCError("Provided userid is not the same the user calling this function")
user = get_user_or_error(userid)
status = Optional.extract(status)
@ -1655,21 +1663,14 @@ def comment_commit(
allowed_statuses = [x[0] for x in ChangesetStatus.STATUSES]
if status and status not in allowed_statuses:
raise JSONRPCError(
"Bad status, must be on "
"of %s got %s"
% (
allowed_statuses,
status,
)
)
raise JSONRPCError(f"Bad status, must be on of {allowed_statuses} got {status}")
if resolves_comment_id:
comment = ChangesetComment.get(resolves_comment_id)
if not comment:
raise JSONRPCError("Invalid resolves_comment_id `%s` for this commit." % resolves_comment_id)
raise JSONRPCError(f"Invalid resolves_comment_id `{resolves_comment_id}` for this commit.")
if comment.comment_type != ChangesetComment.COMMENT_TYPE_TODO:
raise JSONRPCError("Comment `%s` is wrong type for setting status to resolved." % resolves_comment_id)
raise JSONRPCError(f"Comment `{resolves_comment_id}` is wrong type for setting status to resolved.")
try:
rc_config = SettingsModel().get_all_settings()
@ -1685,7 +1686,7 @@ def comment_commit(
renderer=renderer,
comment_type=comment_type,
resolves_comment_id=resolves_comment_id,
auth_user=apiuser,
auth_user=auth_user,
extra_recipients=extra_recipients,
send_email=send_email,
)
@ -1721,7 +1722,7 @@ def comment_commit(
)
return {
"msg": ("Commented on commit `{}` for repository `{}`".format(comment.revision, repo.repo_name)),
"msg": f"Commented on commit `{comment.revision}` for repository `{repo.repo_name}`",
"status_change": status,
"success": True,
}
@ -1792,9 +1793,7 @@ def get_repo_comments(
comment_type = Optional.extract(comment_type)
if comment_type and comment_type not in ChangesetComment.COMMENT_TYPES:
raise JSONRPCError(
"comment_type must be one of `{}` got {}".format(ChangesetComment.COMMENT_TYPES, comment_type)
)
raise JSONRPCError(f"comment_type must be one of `{ChangesetComment.COMMENT_TYPES}` got {comment_type}")
comments = CommentsModel().get_repository_comments(
repo=repo, comment_type=comment_type, user=user, commit_id=commit_id
@ -1886,11 +1885,12 @@ def edit_comment(request, apiuser, message, comment_id, version, userid=Optional
is_repo_admin = HasRepoPermissionAnyApi("repository.admin")(user=apiuser, repo_name=comment.repo.repo_name)
if not isinstance(userid, Optional):
if is_super_admin or is_repo_admin:
if is_super_admin:
# super-admin can impersonate other users
apiuser = get_user_or_error(userid)
auth_user = apiuser.AuthUser()
else:
raise JSONRPCError("userid is not the same as your user")
raise JSONRPCError("Provided userid is not the same the user calling this function")
comment_author = comment.author.user_id == auth_user.user_id
@ -2001,14 +2001,12 @@ def grant_user_permission(request, apiuser, repoid, userid, perm):
PermissionModel().flush_user_permission_caches(changes)
return {
"msg": "Granted perm: `{}` for user: `{}` in repo: `{}`".format(
perm.permission_name, user.username, repo.repo_name
),
"msg": f"Granted perm: `{perm.permission_name}` for user: `{user.username}` in repo: `{repo.repo_name}`",
"success": True,
}
except Exception:
log.exception("Exception occurred while trying edit permissions for repo")
raise JSONRPCError("failed to edit permission for user: `{}` in repo: `{}`".format(userid, repoid))
raise JSONRPCError(f"failed to edit permission for user: `{userid}` in repo: `{repoid}`")
@jsonrpc_method()
@ -2058,12 +2056,12 @@ def revoke_user_permission(request, apiuser, repoid, userid):
PermissionModel().flush_user_permission_caches(changes)
return {
"msg": "Revoked perm for user: `{}` in repo: `{}`".format(user.username, repo.repo_name),
"msg": f"Revoked perm for user: `{user.username}` in repo: `{repo.repo_name}`",
"success": True,
}
except Exception:
log.exception("Exception occurred while trying revoke permissions to repo")
raise JSONRPCError("failed to edit permission for user: `{}` in repo: `{}`".format(userid, repoid))
raise JSONRPCError(f"failed to edit permission for user: `{userid}` in repo: `{repoid}`")
@jsonrpc_method()
@ -2145,9 +2143,7 @@ def grant_user_group_permission(request, apiuser, repoid, usergroupid, perm):
}
except Exception:
log.exception("Exception occurred while trying change permission on repo")
raise JSONRPCError(
"failed to edit permission for user group: `%s` in repo: `%s`" % (usergroupid, repo.repo_name)
)
raise JSONRPCError(f"failed to edit permission for user group: `{usergroupid}` in repo: `{repo.repo_name}`")
@jsonrpc_method()
@ -2206,16 +2202,13 @@ def revoke_user_group_permission(request, apiuser, repoid, usergroupid):
PermissionModel().flush_user_permission_caches(changes)
return {
"msg": "Revoked perm for user group: `{}` in repo: `{}`".format(
user_group.users_group_name, repo.repo_name
),
"msg": f"Revoked perm for user group: `{user_group.users_group_name}` in repo: `{repo.repo_name}`",
"success": True,
}
except Exception:
log.exception("Exception occurred while trying revoke user group permission on repo")
raise JSONRPCError(
"failed to edit permission for user group: `%s` in "
"repo: `%s`" % (user_group.users_group_name, repo.repo_name)
f"failed to edit permission for user group: `{user_group.users_group_name}` in repo: `{repo.repo_name}`"
)
@ -2276,12 +2269,12 @@ def pull(request, apiuser, repoid, remote_uri=Optional(None), sync_large_objects
repo.repo_name, apiuser.username, remote_uri=remote_uri, sync_large_objects=sync_large_objects
)
return {
"msg": "Pulled from url `{}` on repo `{}`".format(remote_uri_display, repo.repo_name),
"msg": f"Pulled from url `{remote_uri_display}` on repo `{repo.repo_name}`",
"repository": repo.repo_name,
}
except Exception:
log.exception("Exception occurred while trying to pull changes from remote location")
raise JSONRPCError("Unable to pull changes from `%s`" % remote_uri_display)
raise JSONRPCError(f"Unable to pull changes from `{remote_uri_display}`")
@jsonrpc_method()
@ -2340,12 +2333,12 @@ def strip(request, apiuser, repoid, revision, branch):
)
return {
"msg": "Stripped commit {} from repo `{}`".format(revision, repo.repo_name),
"msg": f"Stripped commit {revision} from repo `{repo.repo_name}`",
"repository": repo.repo_name,
}
except Exception:
log.exception("Exception while trying to strip")
raise JSONRPCError("Unable to strip commit {} from repo `{}`".format(revision, repo.repo_name))
raise JSONRPCError(f"Unable to strip commit {revision} from repo `{repo.repo_name}`")
@jsonrpc_method()
@ -2515,4 +2508,4 @@ def maintenance(request, apiuser, repoid):
}
except Exception:
log.exception("Exception occurred while trying to run maintenance")
raise JSONRPCError("Unable to execute maintenance on `%s`" % repo.repo_name)
raise JSONRPCError(f"Unable to execute maintenance on `{repo.repo_name}`")

28
rhodecode/subscribers.py
View file

@ -60,30 +60,30 @@ def add_renderer_globals(event):
def auto_merge_pr_if_needed(event):
from rhodecode.model.db import PullRequest
from rhodecode.model.pull_request import (
PullRequestModel, ChangesetStatus, MergeCheck
)
from rhodecode.model.pull_request import PullRequestModel, ChangesetStatus, MergeCheck
pr_event_data = event.as_dict()["pullrequest"]
pull_request = PullRequest.get(pr_event_data["pull_request_id"])
calculated_status = pr_event_data["status"]
if (calculated_status == ChangesetStatus.STATUS_APPROVED
and PullRequestModel().is_automatic_merge_enabled(pull_request)):
if calculated_status == ChangesetStatus.STATUS_APPROVED and PullRequestModel().is_automatic_merge_enabled(
pull_request
):
user = pull_request.author.AuthUser()
merge_check = MergeCheck.validate(
pull_request, user, translator=lambda x: x, fail_early=True
)
merge_check = MergeCheck.validate(pull_request, user, translator=lambda x: x, fail_early=True)
if merge_check.merge_possible:
from rhodecode.lib.base import vcs_operation_context
extras = vcs_operation_context(
event.request.environ, repo_name=pull_request.target_repo.repo_name,
username=user.username, action="push",
scm=pull_request.target_repo.repo_type)
from rc_ee.lib.celerylib.tasks import auto_merge_repo
auto_merge_repo.apply_async(
args=(pull_request.pull_request_id, extras), countdown=3
event.request.environ,
repo_name=pull_request.target_repo.repo_name,
username=user.username,
action="push",
scm=pull_request.target_repo.repo_type,
)
from rc_ee.lib.celerylib.tasks import auto_merge_repo
auto_merge_repo.apply_async(args=(pull_request.pull_request_id, extras), countdown=3)
def set_user_lang(event):