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/10/01 13:57:55 UTC
[GitHub] [airflow] ashb opened a new pull request #18670: Don't allow creating users via API unless DB auth type is used
ashb opened a new pull request #18670:
URL: https://github.com/apache/airflow/pull/18670
This mirrors the behaviour of FAB, where for anything but the DB user
type you can edit or delete users, but (even Admins) can only create
users when the AUTH_DB type is in use.
I'm not 100% sold on 403 being the right error code here, but I think it fits this description (even if 403 is _usually_ auth related.)g
> The request contained valid data and was understood by the server, but the server is refusing action. This may be due to the user not having the necessary permissions for a resource or needing an account of some sort, or attempting a prohibited action (e.g. creating a duplicate record where only one is allowed). This code is also typically used if the request provided authentication by answering the WWW-Authenticate header field challenge, but the server did not accept that authentication. The request should not be repeated
A 404 seems wrong, as the URL is right, it's just not allowed by the current configuration. I mean I _guess_ a 5xx error would be better as this is a server error, but none of the 5xx codes feel right.)
--
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 #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933019871
But of course we can make other decision - and force the users to have the righth Group setting and synchronization enabled (however I think this is very limited and is a bit too opinionated).
--
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 #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-973034064
> Authentication via LDAP but authorization (i.e. role mapping) via DB sounds good, but don't we have Authentication via LDAP _and_ DB right now?
I believe not. But If we agree this is the way to go, we can double-check 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-973032443
Authentication via LDAP but authorization (i.e. role mapping) via DB sounds good, but don't we have Authentication via LDAP _and_ DB right now?
--
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 #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933310373
> I would say this should be opt-in, rather than by default with ldap, as I've been in plenty of places where being to create a user manually when using LDAP is viewed as a security risk (cos then the user is not managed centrally.
Just to clarify - adding the user in Airflow DB in this case does NOT create the user in LDAP. It only creates a DB user entry in DB, allowing the user to log-in, but the user must exist in LDAP prior to that. It just merely tells Airflow "This user, when atuthenticatng via LDAP should be able to use Airflow" - so it's only authorization to Airflow, not authentication in LDAP is managed via Airflow UI.
--
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 edited a comment on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933019871
But of course we can make other decision - and force the users to have the right group setting and synchronization enabled (however I think this is very limited and is a bit too opinionated).
--
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 #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-973028979
My thoughts: I think it's ok to support the case where users want to make authentication in LDAP but authorization in Airflow (this was the case before FAB introduced role-based-mapping).
While FAB now supports role-based mapping, it's been changed mid-flight for Airflow 2 (without bumping Airflow version) and I think regardless, it's pretty legitmate case where you want to use Airlfow API to manage who can access Airlow while keeping LDAP password checking (i.e verification).
--
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] mik-laj commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-932616290
In the case of API, when the user is not found in LDAP, the database is checked. In that case, it makes sense to be able to add new users.
https://github.com/apache/airflow/pull/10356#discussion_r471743601
@houqp WDYT?
--
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 #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933019561
Actually I am not sure if this is good approach - see the discussion we had with the user and @dstandish in https://github.com/apache/airflow/discussions/18290, which resulted in opening https://github.com/apache/airflow/issues/18545
Seems that this is perfectly valid case for the users, who are using external authentication (LDAP for example) but rather than relying on synchronizing the users from the auth source (for example using LDAP groups) they prefer to manually add to the Airflow DB only those users that should have access to Airflow. It means that while authentication comes from LDAP, the authorization is based on Airlow DB. Since for every externally authenticated user, there must be a DB entry in Airflow, this works and behaves as expected.
I personally think it is perfectly valid case and we should support it. (hence I turned the discssion back into the issue)
--
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] ashb edited a comment on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
ashb edited a comment on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933503051
> but being able to access Airflow is not determined by belonging to a certain LDAP group - but by presence of the user entry in Airflow DB)
That is not the case. If the user is not found in LDAP the DB is tried.
https://github.com/apache/airflow/blob/866a601b76e219b3c043e1dbbc8fb22300866351/airflow/api/auth/backend/basic_auth.py#L43-L47
This means that (when that API backend is in use) I can create a user only in the Airflow DB and authenticate to the API 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-932294247
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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] github-actions[bot] closed pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #18670:
URL: https://github.com/apache/airflow/pull/18670
--
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 edited a comment on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-934612523
> The more I look at this the more I worry this is a potential security issue.
Agree, the way it is implemented is rather brittle. When are going to vendor-in that part? 2.2 or 2.3 ?
Maybe we can contribute some "fixes" to FAB before.
--
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 edited a comment on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933310373
> I would say this should be opt-in, rather than by default with ldap, as I've been in plenty of places where being to create a user manually when using LDAP is viewed as a security risk (cos then the user is not managed centrally.
Just to clarify - adding the user in Airflow DB in this case does NOT create the user in LDAP. It only creates a DB user entry in Airflow DB, allowing the user to log-in, but the user must exist in LDAP prior to that. It just merely tells Airflow "This user, when atuthenticatng via LDAP should be able to use Airflow" - so it's only authorization to Airflow, not authentication in LDAP is managed via Airflow UI.
--
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 #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933181013
Yep. I believe (from those discussions - i am not FAB expert) this is what `AUTH_USER_REGISTRATION=False' does.
https://flask-appbuilder.readthedocs.io/en/latest/security.html#authentication-methods
It allows the users to self-register. So (providing that I understand it correctly), this is perfectly valid case where the users can be manually added via admin rather than self-register.
That was.much more useful of course when there was no automated role synchronisation (because then you had to manually assign the users to roles if you wanted some people to have different roles.i Fab 3.2 i believe this was changed and you can assign people to roles based on LDAP groups so you do not have to manage the users via Airflow UI at all and LDAP might become a.single source of truth.
But i believe not everyone in their corporate organisation has influence on how groups are defined in LDAP and being able to manually control who has access to Airflow with what role without having to create and configure LDAPl groups for authorisation while still authenticating using LDAP seems like very valid case (at least that what came out of our discussion with @dstandish and the user above).
--
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] uranusjr commented on a change in pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#discussion_r720778409
##########
File path: airflow/api_connexion/endpoints/user_endpoint.py
##########
@@ -70,6 +70,13 @@ def get_users(limit, order_by='id', offset=None):
@security.requires_access([(permissions.ACTION_CAN_CREATE, permissions.RESOURCE_USER)])
def post_user():
"""Create a new user"""
+ import flask
+
+ from airflow.www.fab_security.manager import AUTH_DB
Review comment:
Why not import globally?
--
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] pawsok commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
pawsok commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-947540324
Any thoughts guys? I'd like to upgrade Airflow to the latest version in my organization but I can't do that before changes in the current logic of LDAP authentication.
--
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 #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-968403760
@ashb - any 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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-973022311
Nope, all context here left my head after being on holiday 😁
Any thoughts?
--
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] github-actions[bot] commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-1003799523
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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] ashb commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933301540
> But i believe not everyone in their corporate organisation has influence on how groups are defined in LDAP and being able to manually control who has access to Airflow with what role without having to create and configure LDAPl groups for authorisation while still authenticating using LDAP seems like very valid case (at least that what came out of our discussion with @dstandish and the user above).
I would say this should be opt-in, rather than by default with ldap, as I've been in plenty of places where being to create a user manually when using LDAP is viewed as a security risk (cos then the user is not managed centrally.
So I'd almost say that this should be a new auth type, of `LDAP_AND_DB`.
--
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] houqp commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
houqp commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-932657196
Thanks @mik-laj for mentioning our use-cases. We use Okta to manage regular users and db to manage service accounts. However, we only use the airflow cli to manage these service accounts and don't have the need to create service accounts ad-hoc using APIs because we don't create service accounts often. I don't think anyone is relying on being able to do this through APIs because FAB already disallows this.
BTW, I think 400 is a better code here. 5xx is better suited for unexpected internal server errors. In this case, it's more like a user error.
--
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 edited a comment on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933310373
> I would say this should be opt-in, rather than by default with ldap, as I've been in plenty of places where being to create a user manually when using LDAP is viewed as a security risk (cos then the user is not managed centrally.
Just to clarify - adding the user in Airflow DB in this case does NOT create the user in LDAP. It only creates a DB user entry in Airflow DB, allowing the user to log-in, but the user must exist in LDAP prior to that (and the user is centrally managed in LDAP still including the password of the user- but access to Airflow is not determined by belonging to a certain LDAP group)
It just merely tells Airflow "This user, when atuthenticatng via LDAP (and password matches the LDAP password) should be able to use Airflow" - so it's only authorization to Airflow, not authentication in LDAP is managed via Airflow UI.
--
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] pawsok commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
pawsok commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-942250511
Hi All, As @potiuk mentioned a few messages earlier. In my case, in our organization, there is an LDAP directory that contains a big number of users (let's say 10k users), so I don't want all those users to be automatically added to the Airflow DB. I'd like to have full control over which users are in db (this behaviour is implemented in version 2.0.1 which we're still using and this meets all our needs).
If I try to log in as an LDAP user with an incorrect password I will just retrieve the following message: **Invalid login. Please try again** and that's fine in our case.
And if the user does not exist in LDAP, they simply will not be able to log in and will receive the exact same message.
--
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 #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-947921739
My current thinking is:
1) we should allow the case where authentication comes from external source, but authorization is based on Airlfow Group setting (and presence of the user in Airflow DB). This means allowing to create a (password-less) user in Airflow DB and assigning a role there via UI/CLI.
2) we should make sure that it only works when the authentication using external source of truth (LDAP). I believe it already works like that - we should not allow for Authentication (password verification) come from Airflow DB in this case - but we should verify it
3) we should not introduce LDAP+DB authentication scheme or anything like that. I think separating out Authentication (external source of truth like LDAP) and Authorisation (assignment of the user to a Role based on Airflow DB) makes sense. Authentication coming from two sources is dangerous and not needed.
4) we should clearly document this as "supported case".
--
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 #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-934234962
> > but being able to access Airflow is not determined by belonging to a certain LDAP group - but by presence of the user entry in Airflow DB)
>
> That is not the case. If the user is not found in LDAP the DB is tried.
>
> https://github.com/apache/airflow/blob/866a601b76e219b3c043e1dbbc8fb22300866351/airflow/api/auth/backend/basic_auth.py#L43-L47
>
> This means that (when that API backend is in use) I can create a user only in the Airflow DB and authenticate to the API with it.
I see there is a fallback, but seems those are two different cases. The case I mentioned (and one that would work as I expected) is that the user also exists in LDAP (At least that was what I understand from what user experience was).
For me it looks like the right behaviour to allow the case I described and do not allow LDAP_DB_AUTH (which is indeed the fallback case) would be to only allow to create user in the DB if It also exists in LDAP. That would nicely:
* handle the case when you do not want to turn self-registration on
* will not allow to craeate users if they are not-existing in LDAP
* will keep central authentication in LDAP while also allow to configure authorization in Airflow.
--
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 edited a comment on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933310373
> I would say this should be opt-in, rather than by default with ldap, as I've been in plenty of places where being to create a user manually when using LDAP is viewed as a security risk (cos then the user is not managed centrally.
Just to clarify - adding the user in Airflow DB in this case does NOT create the user in LDAP. It only creates a DB user entry in Airflow DB, allowing the user to log-in, but the user must exist in LDAP prior to that (and the user is centrally managed in LDAP still including the password of the user)
It just merely tells Airflow "This user, when atuthenticatng via LDAP (and password matches the LDAP password) should be able to use Airflow" - so it's only authorization to Airflow, not authentication in LDAP is managed via Airflow UI.
--
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] houqp commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
houqp commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933119071
> It means that while authentication comes from LDAP, the authorization is based on Airlow DB. Since for every externally authenticated user, there must be a DB entry in Airflow, this works and behaves as expected.
When we were using the LDAP backend, I remember when you login through LDAP, it will automatically create the users in the DB? Is there an option to disable this behavior and only allow manual user synchronization between airflow and ldap?
--
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 edited a comment on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933310373
> I would say this should be opt-in, rather than by default with ldap, as I've been in plenty of places where being to create a user manually when using LDAP is viewed as a security risk (cos then the user is not managed centrally.
Just to clarify - adding the user in Airflow DB in this case does NOT create the user in LDAP. It only creates a DB user entry in Airflow DB, allowing the user to log-in, but the user must exist in LDAP prior to that (and the user is centrally managed in LDAP still including the password of the user- but being able to access Airflow is not determined by belonging to a certain LDAP group - but by presence of the user entry in Airflow DB)
It just merely tells Airflow "This user, when atuthenticatng via LDAP (and password matches the LDAP password) should be able to use Airflow" - so it's only authorization to Airflow, not authentication in LDAP is managed via Airflow UI.
--
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] pawsok commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
pawsok commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-942250511
Hi All, As @potiuk mentioned a few messages earlier. In my case, in our organization, there is an LDAP directory that contains a big number of users (let's say 10k users), so I don't want all those users to be automatically added to the Airflow DB. I'd like to have full control over which users are in db (this behaviour is implemented in version 2.0.1 which we're still using and this meets all our needs).
If I try to log in as an LDAP user with an incorrect password I will just retrieve the following message: **Invalid login. Please try again** and that's fine in our case.
And if the user does not exist in LDAP, they simply will not be able to log in and will receive the exact same message.
--
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 #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-947922964
Ash is on vacation now and I guess he needs to take a look and respond as he had a bit different view there - but I guess he will come back to it then and we can proceed with whatever we agree on..
--
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 #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-934612523
> The more I look at this the more I worry this is a potential security issue.
Agree, the way it is implemented is rather brittle. When are going to vendor-in that part? 2.2 ?
--
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] ashb commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933503051
> Just to clarify - adding the user in Airflow DB in this case does NOT create the user in LDAP. It only creates a DB user entry in Airflow DB, allowing the user to log-in, but the user must exist in LDAP prior to that (and the user is centrally managed in LDAP still including the password of the user- but being able to access Airflow is not determined by belonging to a certain LDAP group - but by presence of the user entry in Airflow DB)
That is not the case. If the user is not found in LDAP the DB is tried.
https://github.com/apache/airflow/blob/866a601b76e219b3c043e1dbbc8fb22300866351/airflow/api/auth/backend/basic_auth.py#L43-L47
This means that (when that API backend is in use) I can create a user only in the Airflow DB and authenticate to the API 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: 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 #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-933346326
And another clarification here - I do agree it could be an optional feature, but I think this should also be one of the "supported" configuration options of Airflow (possibly even described in the docs). Apparently - capability of adding the user via UI can be re-added by adding the right permission (this is what https://github.com/apache/airflow/discussions/18290 discussion led to) and I think this might be the right "Feature":
* the same permission should control the UI/API possibility of adding a user
* we should describe this feature as "optional" LDAP seeting and valid use case and how to set the permission to make it works
That would be perfectly fine.
I am not sure though how the permissions are granted/set as this part of FAB/security manager I am not too familiar with, but I think we should make it an optional feature. I was afraid that if we "hard-code" `DB = No adding`, we will make this feature not possible to even configure by the user as opt-in. I just wanted to raise the case that this is a "valid" use case and it should be "possible" to configure (and likely with some guidance from us on how to do 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #18670: Don't allow creating users via API unless DB auth type is used
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #18670:
URL: https://github.com/apache/airflow/pull/18670#issuecomment-934285200
If the user exists in LDAP, but the password is wrong, then `auth_user_ldap` would return None
https://github.com/dpgaspar/Flask-AppBuilder/blob/1fb6d985910d306a821a8da072cd3c93e41311ac/flask_appbuilder/security/manager.py#L1135-L1137
```python
if not self._ldap_bind(ldap, con, bind_username, password):
if user:
self.update_user_auth_stat(user, False)
# Invalid credentials, go away
log.info(LOGMSG_WAR_SEC_LOGIN_FAILED.format(bind_username))
return None
```
The more I look at this the more I worry this is a potential security issue.
--
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