Fix and simplify get_permitted_dag_ids in auth manager

[vincbeck]

There is a bug currently in FAB auth manager. An error is thrown when trying to list DAGs with this following error:

AttributeError: 'NoneType' object has no attribute 'is_anonymous'

This PR fixes the issue but more importantly, it simplifies the methods get_permitted_dag_ids and filter_permitted_dag_ids which were overly complicated for no reason.

I also created tests for get_permitted_dag_ids in FAB auth manager.

---

^ 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 newsfragments.

[pierrejeambrun]

Just a question, it looks like before filter_permitted_dag_ids and get_permitted_dag_ids would return a dag id if we had read or write permission on it.

Now this is not the case anymore, we absolutely need the read. Are we sure that existing installation always set "GET" access when they grant "PUT" access to a user, could that cause disruption ? For instance certain legacy endpoint would not specify the method, defaulting to GET or PUT.

[vincbeck]

Just a question, it looks like before filter_permitted_dag_ids and get_permitted_dag_ids would return a dag id if we had read or write permission on it.

Now this is not the case anymore, we absolutely need the read. Are we sure that existing installation always set "GET" access when they grant "PUT" access to a user, could that cause disruption ? For instance certain legacy endpoint would not specify the method, defaulting to GET or PUT.

No, if you pass method="PUT", you do not need read access, only the edit access. GET is the default value for method but it does not mean you must have it

[pierrejeambrun]

Maybe I got it wrong but that doesn't answer my question, before calling get_permitted_dag_ids without specifying the method would then default to ["PUT", "GET"], now this will default to "GET" only. Which is not the same is it ?

[vincbeck]

Maybe I got it wrong but that doesn't answer my question, before calling get_permitted_dag_ids without specifying the method would then default to ["PUT", "GET"], now this will default to "GET" only. Which is not the same is it ?

Oh! Got it! Yes you're right, but to be honest I think before was very wrong. Before, if you do not pass any methods to get_permitted_dag_ids, default was ["PUT", "GET"]. Then we can imagine a user having no read permissions but all permissions on edit. Before, get_permitted_dag_ids was returning all the DAGs because the user had edit permissions on all DAGs. But then the user would click on one DAGs and would end up with an access denied because they do not have read permissions.

So I think we are solving a "bug" even though I am not sure this use case even exist (users having edit permissions but not read)

[pierrejeambrun]

Makes sense !