potiuk commented on code in PR #24399:
URL: https://github.com/apache/airflow/pull/24399#discussion_r903442803
##########
airflow/www/fab_security/manager.py:
##########
@@ -1042,6 +1048,12 @@ def auth_user_ldap(self, username, password):
ldap.set_option(ldap.OPT_X_TLS_CERTFILE,
self.auth_ldap_tls_certfile)
if self.auth_ldap_tls_keyfile:
ldap.set_option(ldap.OPT_X_TLS_KEYFILE,
self.auth_ldap_tls_keyfile)
+ if self.auth_ldap_allow_self_signed:
+ ldap.set_option(ldap.OPT_X_TLS_REQUIRE_CERT,
ldap.OPT_X_TLS_ALLOW)
+ ldap.set_option(ldap.OPT_X_TLS_NEWCTX, 0)
+ elif self.auth_ldap_tls_demand:
+ ldap.set_option(ldap.OPT_X_TLS_REQUIRE_CERT,
ldap.OPT_X_TLS_DEMAND)
+ ldap.set_option(ldap.OPT_X_TLS_NEWCTX, 0)
Review Comment:
This i part of synchronization with FAB 4.1.1 changes.
I basically strictly follow the comment about Security Manager being
strictly coupled with https://github.com/apache/airflow/blob/main/setup.cfg#L108
Not my way of doing it - but seems we chose the way to synchronize manually
all the changes in security manager at teh very moment (and in the same commit)
when we update FAB version (hence the == and the comment in setup,cfg).
Following that, this commit contains all the relevat changes in the Security
Manager that were implemented in FAB since last "upgrade" that are currently
released in FAB 4.1.1.
Those were three changes i found relevant:
* LDAP sequence (apparently a fix) -
https://github.com/apache/airflow/blob/main/setup.cfg#L108 (this is the change
you commented on)
* KeyCloak provider added
https://github.com/dpgaspar/Flask-AppBuilder/commit/b21010249a33f657897d26b0a7bec93908290266)
* Upgrading pyJWT
https://github.com/dpgaspar/Flask-AppBuilder/commit/2e2931f2fc470a4f1756a0c39e0cc04b86818d84
All of them bring our Security Manager (as far as I can tell) to the parity
with the FAB one (there are other modifications and removals of unused parts in
our Security Manager before - so they are not identical, but assuming previous
changes properly syncrhronized to the previous version of FAB - those are the
only relevant changes since.
Again - not the way I'd do it, but I guess we need to live with it
--
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]
