-
Notifications
You must be signed in to change notification settings - Fork 15.5k
List only connections, pools and variables the user has access to #55298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
:param method: the method to filter on | ||
:param session: the session | ||
""" | ||
stmt = select(Connection.conn_id, Team.name).join(Team, Connection.team_id == Team.id, isouter=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgive me for not being up to date on this effort, but I'm a bit concerned this method is misleading. Connections don't just come from the db, so you are really just getting a partial list. Was that considered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And to make it more tricky, secret backends don't even support listing connections from the backend...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries :) I agree this is confusing.
But I am interested by the connections returned by the API get_connections
. This API retrieves connections only from the DB with this code:
connection_select, total_entries = paginated_select(
statement=select(Connection),
filters=[connection_id_pattern],
order_by=order_by,
offset=offset,
limit=limit,
session=session,
)
connections = session.scalars(connection_select)
So I assume the get_connections
API only care about connections from the DB?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks! There is a small Mypy problem that needs some love
6d65e9f
to
af89fe8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks for the PR!
af89fe8
to
b9c5d5e
Compare
Introduce the following methods in auth manager to list only resources (connections, pools and variables) the user has access to. With Fab auth manager you cannot define fine grained access on these resources (e.g. User
X
has access to connectiontest
only) but with different auth managers such as Keycloak you can.get_authorized_connections
filter_authorized_connections
get_authorized_pools
filter_authorized_pools
get_authorized_variables
filter_authorized_variables
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rst
or{issue_number}.significant.rst
, in airflow-core/newsfragments.