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

feat: limit oauth client to roles (backport #26193) #26196

Merged
merged 4 commits into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
94 changes: 0 additions & 94 deletions cypress/integration/dashboard_links.js

This file was deleted.

2 changes: 1 addition & 1 deletion frappe/core/doctype/user/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ frappe.ui.form.on("User", {
frm.set_df_property("enabled", "read_only", 0);
}

if (frappe.session.user !== "Administrator") {
if (frm.doc.name !== "Administrator") {
frm.toggle_enable("email", frm.is_new());
}
},
Expand Down
2 changes: 1 addition & 1 deletion frappe/core/doctype/user/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def validate(self):
self.password_strength_test()

if self.name not in STANDARD_USERS:
self.validate_email_type(self.email)
self.email = self.name
self.validate_email_type(self.name)
self.add_system_manager_role()
self.populate_role_profile_roles()
Expand Down
10 changes: 10 additions & 0 deletions frappe/desk/reportview.py
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,16 @@ def delete_bulk(doctype, items):
if undeleted_items and len(items) != len(undeleted_items):
frappe.clear_messages()
delete_bulk(doctype, undeleted_items)
elif undeleted_items:
frappe.msgprint(
_("Failed to delete {0} documents: {1}").format(len(undeleted_items), ", ".join(undeleted_items)),
realtime=True,
title=_("Bulk Operation Failed"),
)
else:
frappe.msgprint(
_("Deleted all documents successfully"), realtime=True, title=_("Bulk Operation Successful")
)


@frappe.whitelist()
Expand Down
11 changes: 9 additions & 2 deletions frappe/integrations/doctype/oauth_client/oauth_client.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"client_id",
"app_name",
"user",
"allowed_roles",
"cb_1",
"client_secret",
"skip_authorization",
Expand Down Expand Up @@ -114,10 +115,16 @@
"in_standard_filter": 1,
"label": "Response Type",
"options": "Code\nToken"
},
{
"fieldname": "allowed_roles",
"fieldtype": "Table MultiSelect",
"label": "Allowed Roles",
"options": "OAuth Client Role"
}
],
"links": [],
"modified": "2023-07-17 07:06:35.765981",
"modified": "2024-04-29 12:07:07.946980",
"modified_by": "Administrator",
"module": "Integrations",
"name": "OAuth Client",
Expand All @@ -141,4 +148,4 @@
"states": [],
"title_field": "app_name",
"track_changes": 1
}
}
13 changes: 13 additions & 0 deletions frappe/integrations/doctype/oauth_client/oauth_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import frappe
from frappe import _
from frappe.model.document import Document
from frappe.permissions import SYSTEM_USER_ROLE


class OAuthClient(Document):
Expand All @@ -13,8 +14,10 @@ class OAuthClient(Document):
from typing import TYPE_CHECKING

if TYPE_CHECKING:
from frappe.integrations.doctype.oauth_client_role.oauth_client_role import OAuthClientRole
from frappe.types import DF

allowed_roles: DF.TableMultiSelect[OAuthClientRole]
app_name: DF.Data
client_id: DF.Data | None
client_secret: DF.Data | None
Expand All @@ -32,6 +35,7 @@ def validate(self):
if not self.client_secret:
self.client_secret = frappe.generate_hash(length=10)
self.validate_grant_and_response()
self.add_default_role()

def validate_grant_and_response(self):
if (
Expand All @@ -45,3 +49,12 @@ def validate_grant_and_response(self):
"Combination of Grant Type (<code>{0}</code>) and Response Type (<code>{1}</code>) not allowed"
).format(self.grant_type, self.response_type)
)

def add_default_role(self):
if not self.allowed_roles:
self.append("allowed_roles", {"role": SYSTEM_USER_ROLE})

def user_has_allowed_role(self) -> bool:
"""Returns true if session user is allowed to use this client."""
allowed_roles = {d.role for d in self.allowed_roles}
return bool(allowed_roles & set(frappe.get_roles()))
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import frappe


def execute():
"""Set default allowed role in OAuth Client"""
for client in frappe.get_all("OAuth Client", pluck="name"):
doc = frappe.get_doc("OAuth Client", client)
if doc.allowed_roles:
continue
row = doc.append("allowed_roles", {"role": "All"}) # Current default
row.db_insert()
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
"actions": [],
"creation": "2024-04-29 12:08:19.459404",
"doctype": "DocType",
"editable_grid": 1,
"engine": "InnoDB",
"field_order": [
"role"
],
"fields": [
{
"fieldname": "role",
"fieldtype": "Link",
"in_list_view": 1,
"label": "Role",
"options": "Role"
}
],
"index_web_pages_for_search": 1,
"istable": 1,
"links": [],
"modified": "2024-04-29 12:16:48.018031",
"modified_by": "Administrator",
"module": "Integrations",
"name": "OAuth Client Role",
"owner": "Administrator",
"permissions": [],
"sort_field": "creation",
"sort_order": "DESC",
"states": []
}
23 changes: 23 additions & 0 deletions frappe/integrations/doctype/oauth_client_role/oauth_client_role.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Copyright (c) 2024, Frappe Technologies and contributors
# For license information, please see license.txt

# import frappe
from frappe.model.document import Document


class OAuthClientRole(Document):
# begin: auto-generated types
# This code is auto-generated. Do not modify anything in this block.

from typing import TYPE_CHECKING

if TYPE_CHECKING:
from frappe.types import DF

parent: DF.Data
parentfield: DF.Data
parenttype: DF.Data
role: DF.Link | None
# end: auto-generated types

pass
9 changes: 5 additions & 4 deletions frappe/oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ def validate_client_id(self, client_id, request, *args, **kwargs):
# Simple validity check, does client exist? Not banned?
cli_id = frappe.db.get_value("OAuth Client", {"name": client_id})
if cli_id:
request.client = frappe.get_doc("OAuth Client", client_id).as_dict()
return True
else:
return False
client = frappe.get_doc("OAuth Client", client_id)
if client.user_has_allowed_role():
request.client = client.as_dict()
return True
return False

def validate_redirect_uri(self, client_id, redirect_uri, request, *args, **kwargs):
# Is the client allowed to use the supplied redirect_uri? i.e. has
Expand Down
1 change: 1 addition & 0 deletions frappe/patches.txt
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,4 @@ frappe.core.doctype.data_import.patches.remove_stale_docfields_from_legacy_versi
frappe.patches.v15_0.validate_newsletter_recipients
frappe.patches.v15_0.sanitize_workspace_titles
frappe.custom.doctype.property_setter.patches.remove_invalid_fetch_from_expressions
frappe.integrations.doctype.oauth_client.patches.set_default_allowed_role_in_oauth_client
Loading