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.

Sign up for GitHub

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

feat(api): better permisisons #2444

Merged
merged 6 commits into from
Nov 12, 2024
Merged

feat(api): better permisisons #2444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
44dad2b
feat: wip
shahargl Nov 12, 2024
307f668
feat: Merge branch 'main' into feature/permissions-improve
shahargl Nov 12, 2024
361fc74
feat: more work
shahargl Nov 12, 2024
97fed57
feat: bump version
shahargl Nov 12, 2024
4449583
Merge branch 'main' into feature/permissions-improve
shahargl Nov 12, 2024
870eef5
Merge branch 'main' into feature/permissions-improve
shahargl Nov 12, 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
35 changes: 29 additions & 6 deletions ee/identitymanager/identity_managers/keycloak/keycloak_identitymanager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@


class KeycloakIdentityManager(BaseIdentityManager):

"""
RESOURCES = {
"preset": {
"table": "preset",
Expand All @@ -38,6 +38,9 @@ class KeycloakIdentityManager(BaseIdentityManager):
"uid": "id",
},
}
"""

RESOURCES = {}

def __init__(self, tenant_id, context_manager: ContextManager, **kwargs):
super().__init__(tenant_id, context_manager, **kwargs)
Expand Down Expand Up @@ -409,6 +412,8 @@ def create_user(
"email": user_email,
"enabled": True,
"firstName": user_name,
"lastName": user_name,
"emailVerified": True,
}
if password:
user_data["credentials"] = [
Expand Down Expand Up @@ -628,7 +633,7 @@ def create_user_policy(self, perm, permission: ResourcePermission) -> None:
# TODO: this is not efficient, we should cache this
users = self.keycloak_admin.get_users({})
user = next(
(user for user in users if user["email"] == perm.id),
(user for user in users if user.get("email") == perm.id),
None,
)
if not user:
Expand Down Expand Up @@ -663,19 +668,27 @@ def create_user_policy(self, perm, permission: ResourcePermission) -> None:
return policy_id

def create_group_policy(self, perm, permission: ResourcePermission) -> None:
group_name = perm.id
group = self.keycloak_admin.get_groups(query={"search": perm.id})
if not group or len(group) > 1:
self.logger.error("Problem with group - should be 1 but got %s", len(group))
raise HTTPException(status_code=400, detail="Problem with group")
group = group[0]
group_id = group["id"]
resp = self.keycloak_admin.connection.raw_post(
f"{self.admin_url}/authz/resource-server/policy/group",
data=json.dumps(
{
"name": f"Allow group {perm.id} to access resource type {permission.resource_type} with name {permission.resource_name}",
"description": json.dumps(
{
"group_id": perm.id,
"group_name": group_name,
"group_id": group_id,
"resource_id": permission.resource_id,
}
),
"logic": "POSITIVE",
"groups": [{"id": perm.id, "extendChildren": False}],
"groups": [{"id": group_id, "extendChildren": False}],
"groupsClaim": "",
}
),
Expand Down Expand Up @@ -838,12 +851,14 @@ def get_permissions(self) -> list[ResourcePermission]:
if resource_id not in resources_to_policies:
resources_to_policies[resource_id] = []
if policy.get("type") == "user":
user_email = details.get("user_email")
resources_to_policies[resource_id].append(
{"id": details.get("user_email"), "type": "user"}
{"id": user_email, "type": "user"}
)
else:
group_name = details.get("group_name")
resources_to_policies[resource_id].append(
{"id": details["group_id"], "type": "group"}
{"id": group_name, "type": "group"}
)
permissions_dto = []
for resource in resources:
Expand All @@ -858,6 +873,7 @@ def get_permissions(self) -> list[ResourcePermission]:
permissions=[
PermissionEntity(
id=policy["id"],
name=policy.get("name", ""),
type=policy["type"],
)
for policy in resources_to_policies.get(resource_id, [])
Expand All @@ -868,6 +884,9 @@ def get_permissions(self) -> list[ResourcePermission]:
except KeycloakGetError as e:
self.logger.error("Failed to fetch permissions from Keycloak: %s", str(e))
raise HTTPException(status_code=500, detail="Failed to fetch permissions")
except Exception as e:
self.logger.error("Failed to fetch permissions from Keycloak: %s", str(e))
raise HTTPException(status_code=500, detail="Failed to fetch permissions")

# TODO: this should use UMA and not evaluation since evaluation needs admin access
def get_user_permission_on_resource_type(
Expand Down Expand Up @@ -916,6 +935,10 @@ def get_user_permission_on_resource_type(
for result in results
if result["status"] == "PERMIT"
]
# there is some bug/limitation in keycloak where if the resource_type does not exist, it returns
# all other objects, so lets handle it by checking if the word "with" is one of the results name
if any("with" in result for result in allowed_resources_ids):
return []
return allowed_resources_ids
except Exception as e:
self.logger.error(
Expand Down
Loading
Loading