Extending AirflowSecurityManager

[Victor Noël <vi...@crazydwarves.org>]

Hi,

I'm upgrading from 1.10.1 to 1.10.2 and I had a custom SecurityManager that
was extending flask_appbuilder.security.sqla.manager.SecurityManager but it
seems that now I need to extend AirflowSecurityManager.

It's fine by me but I would like to understand something: if I want to add
Admin role to some users based on some condition, until now I was
overriding add_user to add the extra role on user creation if the condition
was true.

But when I read the code of AirflowSecurityManager, I see there is a
get_user_roles method: can I extend it so that I can just check the
condition here and add Admin to the list of roles. The advantage would be
to not have to add the role when the user is created (and thus if I change
something in the condition, I don't need to delete and recreate the user).

In another words: can I be sure that airflow will always go through this
method to check the role of a user and not bypass the
AirflowSecurityManager to check the db directly?

Thanks for any help!

Victor

Re: Extending AirflowSecurityManager

[Victor Noël <vi...@crazydwarves.org>]

Thank you for your answer,

In practice, I tested, and the behaviour of the app is quite strange: after
logging, I am properly logged and I suppose identified as an Admin (since
`get_user_roles` is now returning Admin for my user with this change), some
of the button that I couldn't see if I wasn't an admin were shown, but the
Security tab at the top is missing for example.

Then if I click one of the button such as manually starting a job, it just
doesn't do anything.

So I suppose that it's not working as expected, there must be some place
where the answer from `get_user_roles` is not exploited or something like
that.

Any idea how this could be improved or if I should create a ticket for this?

Le sam. 26 janv. 2019 à 00:06, Tao Feng <fe...@gmail.com> a écrit :

> I think you could extend that method. We shouldn't call db directly unless
> FAB doesn't provide the api to do so.
>
> On Wed, Jan 23, 2019 at 8:18 AM Victor Noël <vi...@crazydwarves.org>
> wrote:
>
>> Hi,
>>
>> I'm upgrading from 1.10.1 to 1.10.2 and I had a custom SecurityManager
>> that
>> was extending flask_appbuilder.security.sqla.manager.SecurityManager but
>> it
>> seems that now I need to extend AirflowSecurityManager.
>>
>> It's fine by me but I would like to understand something: if I want to add
>> Admin role to some users based on some condition, until now I was
>> overriding add_user to add the extra role on user creation if the
>> condition
>> was true.
>>
>> But when I read the code of AirflowSecurityManager, I see there is a
>> get_user_roles method: can I extend it so that I can just check the
>> condition here and add Admin to the list of roles. The advantage would be
>> to not have to add the role when the user is created (and thus if I change
>> something in the condition, I don't need to delete and recreate the user).
>>
>> In another words: can I be sure that airflow will always go through this
>> method to check the role of a user and not bypass the
>> AirflowSecurityManager to check the db directly?
>>
>> Thanks for any help!
>>
>> Victor
>>
>

Re: Extending AirflowSecurityManager

[Tao Feng <fe...@gmail.com>]

I think you could extend that method. We shouldn't call db directly unless
FAB doesn't provide the api to do so.

On Wed, Jan 23, 2019 at 8:18 AM Victor Noël <vi...@crazydwarves.org>
wrote:

> Hi,
>
> I'm upgrading from 1.10.1 to 1.10.2 and I had a custom SecurityManager that
> was extending flask_appbuilder.security.sqla.manager.SecurityManager but it
> seems that now I need to extend AirflowSecurityManager.
>
> It's fine by me but I would like to understand something: if I want to add
> Admin role to some users based on some condition, until now I was
> overriding add_user to add the extra role on user creation if the condition
> was true.
>
> But when I read the code of AirflowSecurityManager, I see there is a
> get_user_roles method: can I extend it so that I can just check the
> condition here and add Admin to the list of roles. The advantage would be
> to not have to add the role when the user is created (and thus if I change
> something in the condition, I don't need to delete and recreate the user).
>
> In another words: can I be sure that airflow will always go through this
> method to check the role of a user and not bypass the
> AirflowSecurityManager to check the db directly?
>
> Thanks for any help!
>
> Victor
>