You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/10/12 13:24:20 UTC

[GitHub] [airflow] kaxil opened a new pull request #11468: Mask Password in Log table when creating users via CLI

kaxil opened a new pull request #11468:
URL: https://github.com/apache/airflow/pull/11468


   **Before**:
   
   ![image](https://user-images.githubusercontent.com/8811558/95751440-8622fe00-0c96-11eb-9e2d-e2e0349798bf.png)
   
   
   **After**:
   
   ![image](https://user-images.githubusercontent.com/8811558/95751476-92a75680-0c96-11eb-89d3-fbb4fa6fb499.png)
   
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kaxil edited a comment on pull request #11468: Mask Password in Log table when creating users via CLI

Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on pull request #11468:
URL: https://github.com/apache/airflow/pull/11468#issuecomment-707144934


   > It's also possible to do `--password=foobar` (but not `-p=foobar`) which this PR won't catch.
   
   Fixed in https://github.com/apache/airflow/pull/11468/commits/40b1c0438857926bf7c1b32c70dba73a806edf24
   
   I tested with `-p=foobar` and I was able to create the user with it too
   
   Example:
   
   ```
   root@6b30e511a6ac:/opt/airflow# airflow users  create --role Admin --username admin5 --email admin333 --firstname admin --lastname admin -p=admin
   ```
   
   ![image](https://user-images.githubusercontent.com/8811558/95756247-722eca80-0c9d-11eb-9005-a2ec7dd8add8.png)
   
   So I have handled that case too  and added test for it 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on pull request #11468: Mask Password in Log table when creating users via CLI

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #11468:
URL: https://github.com/apache/airflow/pull/11468#issuecomment-707149275


   Should we add `--conn-password` to this list?
   
   And for bonus points the password part of the URI style?


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kaxil merged pull request #11468: Mask Password in Log table when using the CLI

Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #11468:
URL: https://github.com/apache/airflow/pull/11468


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kaxil commented on pull request #11468: Mask Password in Log table when creating users via CLI

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11468:
URL: https://github.com/apache/airflow/pull/11468#issuecomment-707169823


   > Should we add --conn-password to this list?
   > And for bonus points the password part of the URI style?
   
   Added `--conn-password`, will address URI style later in a separate PR as we can do some cleanup of other parts where we mask Password in Conn URI too
   
   
   > _technically_ this is a password of `=test` I think, (or it's not actually accepted by argparse) and it would have to be given as `-p test`.
   > 
   > But I think as an edge case this is okay if we don't worry about it
   
   Looks like it works fine though 
   ```python
   In [1]: from airflow.cli.cli_parser import get_parser
   In [2]: parser = get_parser()
   In [8]: parser.parse_args(["users", "create","-e" ,"airflow" , "-p=10", "-r", "Admin", "-f", "as", "-l", "asd", "-u", "as"])
   Out[8]: Namespace(email='airflow', firstname='as', func=<function lazy_load_command.<locals>.command at 0x7fd196603dd0>, lastname='asd', password='10', role='Admin', subcommand='create', use_random_password=False, username='as')
   ```


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] kaxil commented on pull request #11468: Mask Password in Log table when creating users via CLI

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11468:
URL: https://github.com/apache/airflow/pull/11468#issuecomment-707144934


   > It's also possible to do `--password=foobar` (but not `-p=foobar`) which this PR won't catch.
   
   Fixed in https://github.com/apache/airflow/pull/11468/commits/40b1c0438857926bf7c1b32c70dba73a806edf24


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org