sha174n commented on PR #33924:
URL: https://github.com/apache/superset/pull/33924#issuecomment-4858207328

   Thanks for this — the mechanism is sound: attributes are set by the 
embedding app at token issuance and carried in the signed guest token (same 
trust level as the existing `username`/RLS clause), and the macro is guarded by 
`is_guest_user()`. Two doc suggestions before it lands:
   
   1. The SQL-templating example interpolates the attribute value directly into 
a `WHERE` clause string. Since the value is returned as-is, it'd be safer to 
show it quoted/parameterized and to note that attribute values are trusted 
(issuer-set). Consider adding an `escape_result` option mirroring `url_param()` 
for the string case.
   2. The `is_admin` example (using an attribute to gate access inside a query) 
encourages building authorization on a raw templated value — I'd drop or 
reframe that one.
   
   Also worth documenting how attributes get set (per @mistercrunch's note), 
since that issuer-trust boundary is what makes the feature safe. Nice addition 
otherwise.
   


-- 
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]

Reply via email to