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

Add support for RBAC in weaviate-cli. #102

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add support for RBAC in weaviate-cli. #102

wants to merge 1 commit into from

Conversation

jfrancoa
Copy link
Collaborator

This commit adds the option to add, remove, get roles. Assigning roles to users, as well as permissions to roles, revoking them (roles as well as permissions).

It also adds the option to specify a user from your config as the user to pass in the client connection. This allows having multiple api-keys assigned to different users without having the need to keep different configs.

Copy link

@orca-security-eu orca-security-eu bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 17 changed files in this pull request and generated 1 suggestion.

Files not reviewed (12)
  • requirements-dev.txt: Language not supported
  • setup.cfg: Language not supported
  • weaviate_cli/defaults.py: Evaluated as low risk
  • weaviate_cli/managers/user_manager.py: Evaluated as low risk
  • test/unittests/test_managers/test_config_manager.py: Evaluated as low risk
  • test/unittests/test_utils.py: Evaluated as low risk
  • cli.py: Evaluated as low risk
  • weaviate_cli/commands/get.py: Evaluated as low risk
  • weaviate_cli/managers/collection_manager.py: Evaluated as low risk
  • weaviate_cli/managers/config_manager.py: Evaluated as low risk
  • weaviate_cli/commands/delete.py: Evaluated as low risk
  • README.md: Evaluated as low risk
Comments skipped due to low confidence (8)

weaviate_cli/commands/add.py:27

  • The 'role' parameter should be defined as 'tuple[str]' instead of 'str' to match its usage.
def add_role_cli(ctx: click.Context, role: str, to_user: str) -> None:

weaviate_cli/commands/add.py:70

  • The 'permission' parameter should be defined as 'tuple[str]' instead of 'str' to match its usage.
def add_permission_cli(ctx: click.Context, permission: str, to_role: str) -> None:

weaviate_cli/managers/role_manager.py:20

  • The 'permissions' parameter should be a 'List' instead of a 'tuple'.
permissions: tuple = CreateRoleDefaults.permission,

weaviate_cli/managers/role_manager.py:51

  • The error message should be more informative, such as 'Error: Role '{name}' does not exist.'
raise Exception(f"Role '{name}' does not exist.")

weaviate_cli/managers/role_manager.py:73

  • The error message could be more descriptive, such as 'Error: Failed to add permission '{permission}' to role '{to_role}'. Details: {e}'
raise Exception(f"Error adding permission '{permission}' to role '{to_role}': {e}")

weaviate_cli/managers/role_manager.py:87

  • The error message could be more descriptive, such as 'Error: Failed to revoke permission '{permission}' from role '{from_role}'. Details: {e}'
raise Exception(f"Error revoking permission '{permission}' from role '{from_role}': {e}")

weaviate_cli/commands/revoke.py:27

  • The 'role' parameter should be a tuple, not a string. Change the type hinting to 'role: tuple[str]'.
def revoke_role_cli(ctx: click.Context, role: str, from_user: str) -> None:

weaviate_cli/utils.py:103

  • Ensure that the parse_permission function handles all possible cases correctly, especially the assignment of the role variable.
role = parts[1] if len(parts) > 1 and "roles" in action else "*"

help="The role to revoke the permission from.",
)
@click.pass_context
def revoke_permission_cli(ctx: click.Context, permission: str, from_role: str) -> None:
Copy link
Preview

Copilot AI Nov 29, 2024

Choose a reason for hiding this comment

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

The 'permission' parameter should be a tuple, not a string. Change the type hinting to 'permission: tuple[str]'.

Suggested change
def revoke_permission_cli(ctx: click.Context, permission: str, from_role: str) -> None:
def revoke_permission_cli(ctx: click.Context, permission: tuple[str], from_role: str) -> None:

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@weaviate-git-bot
Copy link

Great to see you again! Thanks for the contribution.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Slack channel?

This commit adds the option to add, remove, get roles.
Assigning roles to users, as well as permissions to roles,
revoking them (roles as well as permissions).

It also adds the option to specify a user from your
config as the user to pass in the client connection. This
allows having multiple api-keys assigned to different users
without having the need to keep different configs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants