gkneighb commented on code in PR #41843:
URL: https://github.com/apache/superset/pull/41843#discussion_r3565560184
##########
superset/security/manager.py:
##########
@@ -1818,17 +1866,32 @@ def get_table_access_error_object(self, tables:
set["Table"]) -> SupersetError:
},
)
- def get_table_access_link( # pylint: disable=unused-argument
- self, tables: set["Table"]
- ) -> Optional[str]:
+ def get_table_access_link(self, tables: set["Table"]) -> Optional[str]:
"""
Return the access link for the denied SQL tables.
+ The configured ``PERMISSION_INSTRUCTIONS_LINK`` may template the denied
+ table names (and the current username) into the access URL.
+
:param tables: The set of denied SQL tables
:returns: The access URL
"""
- return get_conf().get("PERMISSION_INSTRUCTIONS_LINK")
+ # Build display names from the raw parts: Table.__str__ URL-encodes
+ # each segment, and the renderer encodes the whole value again, so
+ # using it here would double-encode. Sorted for deterministic links.
+ return _render_permission_instructions_link(
+ table_names=",".join(
+ sorted(
+ ".".join(
+ part
+ for part in (table.catalog, table.schema, table.table)
+ if part
+ )
+ for table in tables
+ )
+ ),
+ )
Review Comment:
Declining: the value feeds a human-facing access-request form, not a machine
parser — boundary-precise encoding is exactly what produced the double-encoded
illegible links this change fixed. Table names containing literal dots are rare
and remain identifiable to the requester from context; if an org's form needs
machine-parseable identifiers it can template {datasource_id} instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
