dpgaspar / Flask-AppBuilder

Simple and rapid application development framework, built on top of Flask. includes detailed security, auto CRUD generation for your models, google charts and much more. Demo (login with guest/welcome) - http://flaskappbuilder.pythonanywhere.com/
BSD 3-Clause "New" or "Revised" License
4.72k stars 1.37k forks source link

AUTH_ROLES_MAPPING not working #1641

Open cocampbe opened 3 years ago

cocampbe commented 3 years ago

Environment

Flask-Appbuilder version: 3.3.0

pip freeze output:

adal==1.2.6
aiohttp==3.7.3
alembic==1.5.4
amqp==2.6.1
apache-airflow==2.0.1
apache-airflow-providers-amazon==1.1.0
apache-airflow-providers-apache-spark==1.0.2
apache-airflow-providers-celery==1.0.1
apache-airflow-providers-cncf-kubernetes==1.0.1
apache-airflow-providers-docker==1.0.1
apache-airflow-providers-elasticsearch==1.0.1
apache-airflow-providers-ftp==1.0.1
apache-airflow-providers-google==1.0.0
apache-airflow-providers-grpc==1.0.1
apache-airflow-providers-hashicorp==1.0.1
apache-airflow-providers-http==1.1.0
apache-airflow-providers-imap==1.0.1
apache-airflow-providers-microsoft-azure==1.1.0
apache-airflow-providers-mysql==1.0.1
apache-airflow-providers-postgres==1.0.1
apache-airflow-providers-redis==1.0.1
apache-airflow-providers-sendgrid==1.0.1
apache-airflow-providers-sftp==1.1.0
apache-airflow-providers-slack==2.0.0
apache-airflow-providers-sqlite==1.0.1
apache-airflow-providers-ssh==1.1.0
apispec==3.3.2
appdirs==1.4.4
argcomplete==1.12.2
async-timeout==3.0.1
attrs==20.3.0
azure-batch==10.0.0
azure-common==1.1.26
azure-core==1.11.0
azure-cosmos==3.2.0
azure-datalake-store==0.0.51
azure-identity==1.5.0
azure-keyvault==4.1.0
azure-keyvault-certificates==4.2.1
azure-keyvault-keys==4.3.1
azure-keyvault-secrets==4.2.0
azure-kusto-data==0.0.45
azure-mgmt-containerinstance==1.5.0
azure-mgmt-core==1.2.2
azure-mgmt-datalake-nspkg==3.0.1
azure-mgmt-datalake-store==0.5.0
azure-mgmt-nspkg==3.0.2
azure-mgmt-resource==15.0.0
azure-nspkg==3.0.2
azure-storage-common==2.1.0
azure-storage-file==2.1.0
Babel==2.9.0
bcrypt==3.2.0
billiard==3.6.3.0
boto3==1.15.18
botocore==1.18.18
cached-property==1.5.2
cachetools==4.2.1
cattrs==1.2.0
celery==4.4.7
certifi==2020.12.5
cffi==1.14.5
chardet==3.0.4
click==7.1.2
clickclick==20.10.2
cloudpickle==1.4.1
colorama==0.4.4
colorlog==4.7.2
commonmark==0.9.1
connexion==2.7.0
croniter==0.3.37
cryptography==3.4.5
dask==2021.2.0
defusedxml==0.6.0
dill==0.3.3
distlib==0.3.1
distributed==2.19.0
dnspython==1.16.0
docker==3.7.3
docker-pycreds==0.4.0
docutils==0.16
elasticsearch==7.5.1
elasticsearch-dbapi==0.1.0
elasticsearch-dsl==7.3.0
email-validator==1.1.2
eventlet==0.30.1
filelock==3.0.12
Flask==1.1.2
Flask-AppBuilder==3.3.0
Flask-Babel==1.0.0
Flask-Caching==1.9.0
Flask-JWT-Extended==3.25.1
Flask-Login==0.4.1
Flask-OpenID==1.2.5
Flask-SQLAlchemy==2.4.4
Flask-WTF==0.14.3
flower==0.9.7
gevent==21.1.2
google-ads==7.0.0
google-api-core==1.26.0
google-api-python-client==1.12.8
google-auth==1.26.1
google-auth-httplib2==0.0.4
google-auth-oauthlib==0.4.2
google-cloud-automl==1.0.1
google-cloud-bigquery==2.8.0
google-cloud-bigquery-datatransfer==1.1.1
google-cloud-bigquery-storage==2.2.1
google-cloud-bigtable==1.7.0
google-cloud-container==1.0.1
google-cloud-core==1.6.0
google-cloud-datacatalog==0.7.0
google-cloud-dataproc==1.1.1
google-cloud-dlp==1.0.0
google-cloud-kms==1.4.0
google-cloud-language==1.3.0
google-cloud-logging==1.15.1
google-cloud-memcache==0.3.0
google-cloud-monitoring==1.1.0
google-cloud-os-login==1.0.0
google-cloud-pubsub==1.7.0
google-cloud-redis==1.0.0
google-cloud-secret-manager==1.0.0
google-cloud-spanner==1.19.1
google-cloud-speech==1.3.2
google-cloud-storage==1.36.0
google-cloud-tasks==1.5.0
google-cloud-texttospeech==1.0.1
google-cloud-translate==1.7.0
google-cloud-videointelligence==1.16.1
google-cloud-vision==1.0.0
google-crc32c==1.1.2
google-resumable-media==1.2.0
googleapis-common-protos==1.52.0
graphviz==0.16
greenlet==1.0.0
grpc-google-iam-v1==0.12.3
grpcio==1.35.0
grpcio-gcp==0.2.2
gunicorn==19.10.0
HeapDict==1.0.1
httplib2==0.19.0
humanize==3.2.0
hvac==0.10.8
idna==2.10
importlib-metadata==1.7.0
importlib-resources==1.5.0
inflection==0.5.1
iso8601==0.1.14
isodate==0.6.0
itsdangerous==1.1.0
Jinja2==2.11.3
jmespath==0.10.0
json-merge-patch==0.2
jsonschema==3.2.0
kombu==4.6.11
kubernetes==11.0.0
lazy-object-proxy==1.4.3
ldap3==2.9
libcst==0.3.17
lockfile==0.12.2
Mako==1.1.4
Markdown==3.3.3
MarkupSafe==1.1.1
marshmallow==3.10.0
marshmallow-enum==1.5.1
marshmallow-oneofschema==2.1.0
marshmallow-sqlalchemy==0.23.1
msal==1.9.0
msal-extensions==0.3.0
msgpack==1.0.2
msrest==0.6.21
msrestazure==0.6.4
multidict==5.1.0
mypy-extensions==0.4.3
mysql-connector-python==8.0.22
mysqlclient==1.3.14
natsort==7.1.1
numpy==1.20.1
oauthlib==2.1.0
openapi-spec-validator==0.2.9
packaging==20.9
pandas==1.2.2
pandas-gbq==0.14.1
paramiko==2.7.2
pendulum==2.1.2
portalocker==1.7.1
prison==0.1.3
prometheus-client==0.8.0
proto-plus==1.13.0
protobuf==3.14.0
psutil==5.8.0
psycopg2-binary==2.8.6
py4j==0.10.9
pyarrow==3.0.0
pyasn1==0.4.8
pyasn1-modules==0.2.8
pycparser==2.20
pydata-google-auth==1.1.0
Pygments==2.8.0
PyJWT==1.7.1
PyNaCl==1.4.0
pyOpenSSL==19.1.0
pyparsing==2.4.7
pyrsistent==0.17.3
pysftp==0.2.9
pyspark==3.1.1
python-daemon==2.2.4
python-dateutil==2.8.1
python-editor==1.0.4
python-http-client==3.3.2
python-ldap==3.3.1
python-nvd3==0.15.0
python-slugify==4.0.1
python3-openid==3.2.0
pytz==2020.5
pytzdata==2020.1
PyYAML==5.4.1
redis==3.5.3
requests==2.23.0
requests-oauthlib==1.1.0
rich==9.2.0
rsa==4.7
s3transfer==0.3.4
sendgrid==6.6.0
setproctitle==1.2.2
six==1.15.0
slack-sdk==3.3.2
slackclient==2.9.3
sortedcontainers==2.3.0
SQLAlchemy==1.3.23
SQLAlchemy-JSONField==1.0.0
SQLAlchemy-Utils==0.36.8
sshtunnel==0.1.5
starkbank-ecdsa==1.1.0
statsd==3.3.0
swagger-ui-bundle==0.0.8
tabulate==0.8.7
tblib==1.7.0
tenacity==6.2.0
termcolor==1.1.0
text-unidecode==1.3
toolz==0.11.1
tornado==6.1
typing-extensions==3.7.4.3
typing-inspect==0.6.0
unicodecsv==0.14.1
uritemplate==3.0.1
urllib3==1.25.11
vine==1.3.0
virtualenv==20.4.2
watchtower==0.7.3
websocket-client==0.57.0
Werkzeug==1.0.1
WTForms==2.3.3
yarl==1.6.3
zict==2.0.0
zipp==3.4.0
zope.event==4.5.0
zope.interface==5.2.0

Describe the expected results

User logs in and gets role based on group membership.

        stringOverride: |-
          from airflow import configuration as conf
          from flask_appbuilder.security.manager import AUTH_LDAP

          SQLALCHEMY_DATABASE_URI = conf.get('core', 'SQL_ALCHEMY_CONN')

          AUTH_TYPE = AUTH_LDAP
          AUTH_LDAP_SERVER = "ldaps://ldap.example.com:636"
          # AUTH_LDAP_USE_TLS = True
          AUTH_LDAP_TLS_DEMAND = True
          AUTH_LDAP_ALLOW_SELF_SIGNED = True
          AUTH_LDAP_TLS_CACERTFILE = '/etc/ca/ldap_ca.crt'

          # registration configs
          AUTH_USER_REGISTRATION = True  # allow users who are not already in the FAB DB
          AUTH_USER_REGISTRATION_ROLE = "User"  # this role will be given in addition to any AUTH_ROLES_MAPPING
          AUTH_LDAP_FIRSTNAME_FIELD = "givenName"
          AUTH_LDAP_LASTNAME_FIELD = "sn"
          AUTH_LDAP_EMAIL_FIELD = "mail"  # if null in LDAP, email is set to: "{username}@email.notfound"

          AUTH_LDAP_USERNAME_FORMAT = "cn=%s,ou=admins,ou=ra,dc=example,dc=com"  # %s is replaced with the provided username

          AUTH_LDAP_SEARCH = "ou=admins,ou=ra,dc=example,dc=com"  # the LDAP search base (if non-empty, a search will ALWAYS happen)
          AUTH_LDAP_UID_FIELD = "cn"  # the username field

          AUTH_ROLES_MAPPING = {
            "cn=airflow-admins,ou=groups,dc=example,dc=com": ["Admin"],
          }

          AUTH_LDAP_GROUP_FIELD = "memberOf"

          AUTH_ROLES_SYNC_AT_LOGIN = True

          PERMANENT_SESSION_LIFETIME = 1800

Describe the actual results

I have airflow deployed in k8s using the airflow helm chart. Airflow version is 2.0.1. User logs in and only has the "User" role. The user is in the airflow-admins group but the role is not mapping. I only get the role defined in AUTH_USER_REGISTRATION_ROLE. If it matters, the ldap provider is AD. The config is pretty much a copy/paste from the FAQ section of the helm chart page.

https://github.com/airflow-helm/charts/tree/main/charts/airflow#how-to-authenticate-airflow-users-with-ldapoauth

cocampbe commented 3 years ago

Well this is mildly humorous. The AUTH_ROLES_MAPPING seems to be case sensitive. I Changed the cn to match what is in AD and it worked.

AUTH_ROLES_MAPPING = {
            "CN=Airflow-admins,ou=Groups,DC=example,DC=com": ["Admin"],
          }

I wonder if it would make sense to convert the groups to all lowercase and compare. I haven't checked the code.

thesuperzapper commented 3 years ago

@cocampbe the lines which are causing this to be case sensitive are here in _ldap_calculate_user_roles()

The issue is we are directly comparing the string provided as the key of the AUTH_ROLES_MAPPING dict with what was returned by LDAP, clearly if these strings are different cases the get_roles_from_keys() function will not return the correct roles.

It seems like most of the time, attributes in AD are case-insenstive, but there are probably cases were assuming case-insensitivity might lead to a security issue.

We could possibly make the default behaviour to assume case-insentivity, and have a flag to reenable case-sensitivity in role mapping if the user wants.

@dpgaspar what are your thoughts?

dpgaspar commented 3 years ago

@thesuperzapper a flag sounds good, but would prefer to just improve documentation

cocampbe commented 3 years ago

@dpgaspar I agree that the docs are a good way to go. If it had been documented, I would have likely picked up on my issue earlier.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Feel free to reopen it if it's still relevant to you. Thank you

thesuperzapper commented 3 years ago

Bumping for bot.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Feel free to reopen it if it's still relevant to you. Thank you

eladkal commented 1 year ago

I have the exact same issue and so far couldn't resolve it.

My setting is:

          AUTH_ROLES_MAPPING = {
            "DC=f839,DC=msl": ["Admin"],
            "CN=833/name,OU=Users,OU=787,OU=7180,DC=f839,DC=msl": ["Admin"]
          }

Logs show: [2023-10-24T10:08:31.001+000] {manager.py:999} DEBUG - Calculated new roles for user='CN=833/name,OU=Users,OU=787,OU=7180,DC=f839,DC=msl' as: Public]

I don't understand why it choose the role of Public if the map says Admin. I tried also with ou it didn't help.

dpgaspar commented 1 year ago

Hi @eladkal,

Take a look at #2149

Do note that AUTH_ROLES_MAPPING maps the extracted user attributes against memberOf by default. for example using the provided LDAP server that you can spin up with docker-compose, Alice has the following user attributes:

  {
    'sn': [b'Doe'],
    'givenName': [b'Alice'],
    'mail': [b'alice@example.org'],
    'memberOf': [b'cn=readers,ou=groups,dc=example,dc=org', b'cn=staff,ou=groups,dc=example,dc=org']
  }

So a valid mapping could be:

AUTH_ROLES_MAPPING = {
   "cn=staff,ou=groups,dc=example,dc=org": ["Role1"]
}
eladkal commented 1 year ago

Thank you @dpgaspar

I was able eventually to trace the issue. In our case name was non english letters so it had to be unicode string as: u"CN=833/שם,OU=Users,OU=787,OU=7180,DC=f839,DC=msl"


Another issue we faced is that the current code is expected exact match of all OU. This is not a bug in FAB but is a feature request.

https://github.com/dpgaspar/Flask-AppBuilder/blob/f591ee543b5b36db94fcacf8a45def404c7656a6/flask_appbuilder/security/manager.py#L341-L352

while in my case we expected for a partial match. Thus setting:

AUTH_ROLES_MAPPING = {
   "cn=staff": ["Role1"]
}

would work for user that has record of: cn=staff,ou=groups,dc=example,dc=org in LDAP. This require introducing new configuration for partial match and changing the code I pasted above to support it.

harshavardhanbale commented 1 year ago

@dpgaspar @thesuperzapper @eladkal @cocampbe

My airflow version 2.2.4

''' dc=example,dc=com (5) ---> cn=admin +--> ou=groups (50+) +--> ou=samba (1) +--> ou=users (50+) ---> sambaDomainName=example '''

"""Default configuration for the Airflow webserver""" import os

from airflow.www.fab_security.manager import AUTH_LDAP

basedir = os.path.abspath(os.path.dirname(file))

WTF_CSRF_ENABLED = True

AUTH_TYPE = AUTH_LDAP AUTH_LDAP_FIRSTNAME_FIELD = "givenName" AUTH_LDAP_LASTNAME_FIELD = "sn" AUTH_LDAP_EMAIL_FIELD = "mail" AUTH_ROLE_ADMIN = 'Viewer'

AUTH_USER_REGISTRATION = True AUTH_USER_REGISTRATION_ROLE = 'Viewer'

AUTH_LDAP_SERVER = 'ldaps://ldap.example.com:636' AUTH_LDAP_SEARCH = 'ou=users,dc=example,dc=com'

AUTH_LDAP_BIND_USER = 'cn=Yaswanth Amasa,ou=users,dc=example,dc=com' AUTH_LDAP_BIND_PASSWORD = 'example_password'

AUTH_ROLES_MAPPING = { "CN=devops,ou=groups,DC=example,DC=com": ["Admin"], }

AUTH_LDAP_GROUP_FIELD = "memberUid"

AUTH_LDAP_USE_TLS = False AUTH_LDAP_ALLOW_SELF_SIGNED = False AUTH_ROLES_SYNC_AT_LOGIN = True PERMANENT_SESSION_LIFETIME = 1800

'''When Configuring this in webserver_config.py in airflow All the members including devops group have Viewer access, But I want Admin access to devops group and rest public access or else I dont want to allow then to view also. Kindly Please Help me Email: harshavardhanbalentr@gmail.com'''

I dont have memberOf attribute only memberUid image

harshavardhanbale commented 1 year ago

I have the exact same issue and so far couldn't resolve it.

My setting is:

          AUTH_ROLES_MAPPING = {
            "DC=f839,DC=msl": ["Admin"],
            "CN=833/name,OU=Users,OU=787,OU=7180,DC=f839,DC=msl": ["Admin"]
          }

Logs show: [2023-10-24T10:08:31.001+000] {manager.py:999} DEBUG - Calculated new roles for user='CN=833/name,OU=Users,OU=787,OU=7180,DC=f839,DC=msl' as: Public]

I don't understand why it choose the role of Public if the map says Admin. I tried also with ou it didn't help.

harshavardhanbale commented 1 year ago

Where to see logs

romsharon98 commented 1 year ago

Try to open the log in the webserver and look how the user(from devops group) look like when it authenticate via LDAP. In Elad case (I was with him :) ) we saw in the logs the groups the user maps to in LDAP then we copy paste (notice it should be exactly as the one in the LDAP) one of them that we want, in your case you should look for the devops group.

harshavardhanbale commented 1 year ago

Try to open the log in the webserver and look how the user(from devops group) look like when it authenticate via LDAP. In Elad case (I was with him :) ) we saw in the logs the groups the user maps to in LDAP then we copy paste (notice it should be exactly as the one in the LDAP) one of them that we want, in your case you should look for the devops group.

harshavardhanbale commented 1 year ago

Yeah Thank you, I think the problem is memberOf attribute https://github.com/dpgaspar/Flask-AppBuilder/issues/1641#issuecomment-1783050264

Shall you help me in this case I dont have memberOf attribute, I have memberUid attribute