You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/12/02 09:42:17 UTC
[GitHub] [airflow] lwyszomi opened a new pull request #19963: Add possibility to create user in the Remote User mode Auth.
lwyszomi opened a new pull request #19963:
URL: https://github.com/apache/airflow/pull/19963
This fix gives a possibility to create user in the Remote User Auth. I know that we have open discussion about the same issue in the LDAP mode, but in the Remote User mode I think this option should be available.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19963: Add possibility to create user in the Remote User mode Auth.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19963:
URL: https://github.com/apache/airflow/pull/19963#issuecomment-986330459
I think some common approach here should be worked out. I am for adding it, but I am not as "deeply" in the rules and configuration - I believe for the new UI we are porting FAB permission model to Airlfow. so maybe that's a good time to discuss and agree something there @ashb @jithimmins @bbovenzi @ryanahamilton ?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19963: Add possibility to create user in the Remote User mode Auth.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19963:
URL: https://github.com/apache/airflow/pull/19963#issuecomment-1030886206
Hey @jedcunningham - I think that one should be easy to cherry-pick to 2.1.4 and it is a bit of regression (not entirely - this was a change in FAB rather than in Airflow). I marked it as 2.1.4 but maybe you can decide whether to cherry-pick it to 2.2.4 or not.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on pull request #19963: Add possibility to create user in the Remote User mode Auth.
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on pull request #19963:
URL: https://github.com/apache/airflow/pull/19963#issuecomment-1020807513
This is a sensible change, and looks like it was supported in the original version of `UserRemoteUserModelView` in Flask Appbuilder.
https://github.com/dpgaspar/Flask-AppBuilder/blob/985b1b7c1d77d38e667199573455a4a3b6f40ac9/flask_appbuilder/security/views.py#L286
It shouldn't be necessary to modify `_class_permission_name` or `class_permission_name_mapping` though, since those values remain unchanged from the base class.
It may be tricky to test though. @kosteev or @lwyszomi can one of y'all verify that the change works as intended and that you can actually add a new user and then log in with that user?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on pull request #19963: Add possibility to create user in the Remote User mode Auth.
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #19963:
URL: https://github.com/apache/airflow/pull/19963#issuecomment-1021301520
@jhtimmins I duplicated both values from base Class to easly show that this is needed in the `UserRemoteUserModelView`. We have similar implementation in the `CustomUserDBModelView` one of the parameters are copy pasted for Mixin class, second is changed.
Change is already tested and all works fine.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kosteev commented on pull request #19963: Add possibility to create user in the Remote User mode Auth.
Posted by GitBox <gi...@apache.org>.
kosteev commented on pull request #19963:
URL: https://github.com/apache/airflow/pull/19963#issuecomment-1014268592
@potiuk @jithimmins Can you, please, review this fix/change one more time.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins merged pull request #19963: Add possibility to create user in the Remote User mode Auth.
Posted by GitBox <gi...@apache.org>.
jhtimmins merged pull request #19963:
URL: https://github.com/apache/airflow/pull/19963
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on pull request #19963: Add possibility to create user in the Remote User mode Auth.
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #19963:
URL: https://github.com/apache/airflow/pull/19963#issuecomment-1021301520
@jhtimmins I duplicated both values from base Class to easly show that this is needed in the `UserRemoteUserModelView`. We have similar implementation in the `CustomUserDBModelView` one of the parameters are copy pasted for Mixin class, second is changed.
Change is already tested and all works fine.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on pull request #19963: Add possibility to create user in the Remote User mode Auth.
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on pull request #19963:
URL: https://github.com/apache/airflow/pull/19963#issuecomment-1020807513
This is a sensible change, and looks like it was supported in the original version of `UserRemoteUserModelView` in Flask Appbuilder.
https://github.com/dpgaspar/Flask-AppBuilder/blob/985b1b7c1d77d38e667199573455a4a3b6f40ac9/flask_appbuilder/security/views.py#L286
It shouldn't be necessary to modify `_class_permission_name` or `class_permission_name_mapping` though, since those values remain unchanged from the base class.
It may be tricky to test though. @kosteev or @lwyszomi can one of y'all verify that the change works as intended and that you can actually add a new user and then log in with that user?
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kosteev edited a comment on pull request #19963: Add possibility to create user in the Remote User mode Auth.
Posted by GitBox <gi...@apache.org>.
kosteev edited a comment on pull request #19963:
URL: https://github.com/apache/airflow/pull/19963#issuecomment-1014268592
@potiuk @jhtimmins Can you, please, review this fix/change one more time.
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org