[GitHub] [airflow] jedcunningham commented on pull request #18222: make current_user_has_permissions backwards compatible

2021-09-14 Thread GitBox


jedcunningham commented on pull request #18222:
URL: https://github.com/apache/airflow/pull/18222#issuecomment-919408132


   > it feels misleading for that 403 page to say "user has no roles and/**or** 
permissions" if we really are only checking roles.
   
   It's actually checking both given the assumption of user -> roles -> 
permissions, so it seems like decent language to me.  However I'm certainly 
open to alternate wording here, maybe "user has no permissions"?


-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] jedcunningham commented on pull request #18222: make current_user_has_permissions backwards compatible

2021-09-14 Thread GitBox


jedcunningham commented on pull request #18222:
URL: https://github.com/apache/airflow/pull/18222#issuecomment-919193243


   I mean, both Airflow's `get_current_user_permissions` and FAB overall assume 
permissions come from roles. If someone is trying to subclass away roles, seems 
reasonable to me to need to adjust a few new methods every so often 🤷‍♂️. It 
was written as it is so it can short-circuit vs merging all permissions if a 
user has multiple roles.
   
   @jhtimmins, what are your thoughts here?


-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org