uranusjr opened a new issue, #33684:
URL: https://github.com/apache/airflow/issues/33684

   ### Apache Airflow version
   
   main (development)
   
   ### What happened
   
   Currently the `registration_hash` is a plain nullable field text field:
   
   
https://github.com/apache/airflow/blob/b1a3b4288022c67db22cbc7d24b0c4b2b122453b/airflow/auth/managers/fab/models/__init__.py#L240-L253
   
   But when we try to find a user, this field is queried with `first()`:
   
   
https://github.com/apache/airflow/blob/b1a3b4288022c67db22cbc7d24b0c4b2b122453b/airflow/auth/managers/fab/security_manager/modules/db.py#L259-L264
   
   This means that, in the unlikely event (hence posted here) of a hash 
collision, the security manager may return an incorrect user for the given hash.
   
   ### What you think should happen instead
   
   Either the `registration_hash` field should have `unique` flag on it, or the 
query part should use `one` (or `one_or_none`) instead to prevent selecting an 
incorrect entry.
   
   Ideally I’d prefer using the unique flag, but it’d likely require some 
treatment to null values. Currently the field is nullable, and it is unclear 
whether this is intentional or not. It is also not entirely clear whether it is 
possible to have a null value; the user creation code (in `add_register_user`) 
seems to suggest it is always set, but I am not sure whether out-of-band user 
creation is allowed.
   
   The original Flask-Appbuilder code marks this field as nullable (or rather, 
does not mark it as non-nullable, with unclear intention). 
https://github.com/dpgaspar/Flask-AppBuilder/blob/74f37e21a3c9c7ca7fb3e56f73759e3eaa2ead6b/flask_appbuilder/security/sqla/models.py#L176
   
   ### How to reproduce
   
   n/a
   
   ### Operating System
   
   any
   
   ### Versions of Apache Airflow Providers
   
   _No response_
   
   ### Deployment
   
   Official Apache Airflow Helm Chart
   
   ### Deployment details
   
   _No response_
   
   ### Anything else
   
   _No response_
   
   ### Are you willing to submit PR?
   
   - [X] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of 
Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to