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 2019/12/26 22:17:47 UTC

[GitHub] [airflow] tooptoop4 opened a new pull request #6912: [AIRFLOW-6352] security - ui - add login timeout

tooptoop4 opened a new pull request #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912
 
 
   Make sure you have checked _all_ steps below.
   
   ### Jira
   
   - [ ] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-6352\] My Airflow PR"
     - https://issues.apache.org/jira/browse/AIRFLOW-6352
     - In case you are fixing a typo in the documentation you can prepend your commit with \[AIRFLOW-6352\], code changes always need a Jira issue.
     - In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)).
     - In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   
   ### Description
   
   - [ ] Here are some details about my PR, including screenshots of any UI changes:
   
   ### Tests
   
   - [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
   
   ### Commits
   
   - [ ] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
     1. Subject is separated from body by a blank line
     1. Subject is limited to 50 characters (not including Jira issue reference)
     1. Subject does not end with a period
     1. Subject uses the imperative mood ("add", not "adding")
     1. Body wraps at 72 characters
     1. Body explains "what" and "why", not "how"
   
   ### Documentation
   
   - [ ] In case of new functionality, my PR adds documentation that describes how to use it.
     - All the public functions and the classes in the PR contain docstrings that explain what it does
     - If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] potiuk commented on a change in pull request #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#discussion_r362108831
 
 

 ##########
 File path: airflow/config_templates/default_airflow.cfg
 ##########
 @@ -370,10 +370,13 @@ default_wrap = False
 # on webserver startup
 update_fab_perms = True
 
+# Minutes of non-activity before logged out from UI
+# 0 means never get forcibly logged out
 
 Review comment:
   @mik-laj - > where do you think the documentation should be added ? Maybe you can provide some pointers? Most of the config options are documented in comments of the default_airlfow.cfg. Any other specific place you think this documentation should be added (and where all the previous options are documented?)
   
   

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


With regards,
Apache Git Services

[GitHub] [airflow] tooptoop4 commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
tooptoop4 commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569315727
 
 
   @potiuk make sense, I've now guarded with config

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


With regards,
Apache Git Services

[GitHub] [airflow] mik-laj commented on a change in pull request #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#discussion_r362102039
 
 

 ##########
 File path: airflow/config_templates/default_airflow.cfg
 ##########
 @@ -370,10 +370,13 @@ default_wrap = False
 # on webserver startup
 update_fab_perms = True
 
+# Minutes of non-activity before logged out from UI
+# 0 means never get forcibly logged out
 
 Review comment:
   Can you add some documentation about this option?   If there is no information about this feature in the documentation, very few people will be able to use 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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/947fb64f7f16dc65a24f8d337ed1adc0fb28e464?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [947fb64...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/947fb64f7f16dc65a24f8d337ed1adc0fb28e464?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [947fb64...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] potiuk edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569255228
 
 
   Agree @RosterIn with security. Internal security should not be neglected. 
   
   It's just that security is never an on/off switch and "let's apply all the possible security practices" is not always best choice.
   
   There are often multiple layers of security in different places so this logout might not be needed (for example when you have individual client certificates individually issued to your users and verified in proxy standing in front of Airflow.). This is often case in corporate environments (such as Airflow often is installed at) but almost unheard of in public-facing websites.
   
   There is always a delicate balance "convenience vs. security" and sometimes enforcing some "best practices" for security with some inconveniences built in gives the opposite result. People tend to bypass security inconveniences by introducing even more insecure workarounds. For example in this case, I can very easily imagine a data engineer wanting a dashboard installing "auto-refresh" browser plugin to refresh the airflow dashboard every 20 minutes. Been there, done that. Such plugins are often vectors of attack on their own.
   
   So yeah I agree with force_log_out_after conf value. I think having a separate conf entry for that is much better choice and gives freedom to admins to set their policy rules as they find best for their users. 

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


With regards,
Apache Git Services

[GitHub] [airflow] NBardelot commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
NBardelot commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-598290139
 
 
   @tooptoop4 sadly it doesn't work as-is (see https://issues.apache.org/jira/browse/AIRFLOW-6865).

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] potiuk edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569255228
 
 
   Agree @RosterIn with security. Internal security should not be neglected. 
   
   It's just that security is never an on/off switch and "let's apply all the possible security practices" is not always best choice.
   
   There are often multiple layers of security in different places so this logout might not be needed (for example when you have individual client certificates individually issued to your users and verified in proxy standing in front of Airflow.). There is always a delicate balance "convenience vs. security" and sometimes enforcing some "best practices" for security with some inconveniences built in gives the opposite result. People tend to bypass security inconveniences by introducing even more insecure workarounds. For example in this case, I can very easily imagine a data engineer wanting a dashboard installing "auto-refresh" browser plugin to refresh the airflow dashboard every 20 minutes. Been there, done that. Such plugins are often vectors of attack on their own.
   
   So yeah I agree with force_log_out_after conf value. I think having a separate conf entry for that is much better choice and gives freedom to admins to set their policy rules as they find best for their users. 

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **decrease** coverage by `0.01%`.
   > The diff coverage is `60%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##           master   #6912      +/-   ##
   =========================================
   - Coverage   84.72%   84.7%   -0.02%     
   =========================================
     Files         679     679              
     Lines       38505   38528      +23     
   =========================================
   + Hits        32623   32635      +12     
   - Misses       5882    5893      +11
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `94.57% <60%> (-2.91%)` | :arrow_down: |
   | [airflow/contrib/hooks/cassandra\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL2Nhc3NhbmRyYV9ob29rLnB5) | `0% <0%> (-100%)` | :arrow_down: |
   | [airflow/models/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvX19pbml0X18ucHk=) | `91.3% <0%> (-8.7%)` | :arrow_down: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `82% <0%> (-0.5%)` | :arrow_down: |
   | [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `76.08% <0%> (ø)` | :arrow_up: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.91% <0%> (+0.43%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...b86c7ac](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] tooptoop4 commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
tooptoop4 commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-595847453
 
 
   @mik-laj any idea how this works? these lines are missing from app.py in 1.10.9
   import datetime
   import flask
   import flask_login

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


With regards,
Apache Git Services

[GitHub] [airflow] RosterIn commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
RosterIn commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569252689
 
 
   I think it can become handy.
   @potiuk Security isn't always about the "outside" world. It's also for inner one. Companies may use more than one instance of airflow. For example I have airflow install unique for HR data - this is extremely sensitive. I would welcome such feature for this instance but I wouldn't welcome it for my other airflow instances.
   
   So I think this shouldn't be enforced for everyone.
   This can be optional feature that users can set by themselves from `airflow.cfg`
   I also recommend not to hard code 60 min. Let users decide what they want. It can be in the same setting:
   
   `force_log_out_after = 60` if set this will timeout the session and also use the number to determine the time
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/947fb64f7f16dc65a24f8d337ed1adc0fb28e464?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [947fb64...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] dephusluke commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
dephusluke commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-599646401
 
 
   I'm trying also to understand the origin of this bug. The imports appear in the file so how could they be missed?
   
   I see also there is discussion about it in the commit https://github.com/apache/airflow/commit/0721d62bf1b23a471d7fe97ccf108db9f92403ef
   
   @tooptoop4 have you figured it out?

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **decrease** coverage by `0.01%`.
   > The diff coverage is `60%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##           master   #6912      +/-   ##
   =========================================
   - Coverage   84.72%   84.7%   -0.02%     
   =========================================
     Files         679     679              
     Lines       38505   38528      +23     
   =========================================
   + Hits        32623   32635      +12     
   - Misses       5882    5893      +11
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `94.57% <60%> (-2.91%)` | :arrow_down: |
   | [airflow/contrib/hooks/cassandra\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL2Nhc3NhbmRyYV9ob29rLnB5) | `0% <0%> (-100%)` | :arrow_down: |
   | [airflow/models/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvX19pbml0X18ucHk=) | `91.3% <0%> (-8.7%)` | :arrow_down: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `82% <0%> (-0.5%)` | :arrow_down: |
   | [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `76.08% <0%> (ø)` | :arrow_up: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.91% <0%> (+0.43%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...b86c7ac](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] potiuk commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569255228
 
 
   Agree @RosterIn with security. Internal security should not be neglected. 
   
   It's just that security is never an on/off switch and "let's apply all the possible security practices" is good choice.
   
   There are often multiple layers of security in different places so this logout might not be needed (for example when you have individual client certificates individually issued to your users and verified in proxy standing in front of Airflow.). There is always a delicate balance "convenience vs. security" and sometimes enforcing some "best practices" for security with some inconveniences built in gives the opposite result. People tend to bypass security inconveniences by introducing even more insecure workarounds. For example in this case, I can very easily imagine a data engineer wanting a dashboard installing "auto-refresh" browser plugin to refresh the airflow dashboard every 20 minutes. Been there, done that. Such plugins are often vectors of attack on their own.
   
   So yeah I agree with force_log_out_after conf value. I think having a separate conf entry for that is much better choice and gives freedom to admins to set their policy rules as they find best for their users. 

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


With regards,
Apache Git Services

[GitHub] [airflow] mik-laj commented on a change in pull request #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#discussion_r362112340
 
 

 ##########
 File path: airflow/config_templates/default_airflow.cfg
 ##########
 @@ -370,10 +370,13 @@ default_wrap = False
 # on webserver startup
 update_fab_perms = True
 
+# Minutes of non-activity before logged out from UI
+# 0 means never get forcibly logged out
 
 Review comment:
   Related discussion: https://github.com/apache/airflow/pull/6952#discussion_r362102402
   
   I think, we can add short description in 'security.rst' file or a new file in `howto` directory. 

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] potiuk commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569215888
 
 
   Same here @tooptoop4 -> I love the small fixes you provide but static checks are failing for them (and could be prevented easily by pre-commit).

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


With regards,
Apache Git Services

[GitHub] [airflow] potiuk edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569238068
 
 
   This change introduces a new behaviour (logout after 60 minutes).  While it is good for  security reasons (obviously) the UI of Airflow has a little bit different use patterns/characteristics than typical user-facing apps. 
   
   It's mostly internal use, with very small number of users, it's already behind a VPN and I guess often witht some kind of client certificates being verified by web seervers. I can imagine in those cases prolonged session persistency might be important feature for users using Apache Airflow. In many cases UI of Airflow can be used in a fashion similar to "operational dashboard" rather than the typical case of "login/do something/logout".
   
   Since we have no auto-refresh yet, using Airflow as dashboard with 60 minutes logout session would not be super user-friendly. 
   
   I'd love to hear what others think about it, but I believe at the very least UPDATING.md should mention that new behaviour if we agree this is a good thing to introduce 60 minutes (or another period) timeout.
   

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


With regards,
Apache Git Services

[GitHub] [airflow] potiuk merged pull request #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912
 
 
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] potiuk commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569238068
 
 
   This change introduces a new behaviour (logout after 60 minutes).  While it is good for  security reasons (obviously) the UI of Airflow has a little bit different use patterns/characteristics. 
   
   It's mostly internal use, with very small number of users, it's already behind a VPN and I guess often witht some kind of client certificates being verified by web seervers. I can imagine in those cases prolonged session persistency might be important feature for users using Apache Airflow. In many cases UI of Airflow can be used in a fashion similar to "operational dashboard" rather than the typical case of "login/do something/logout".
   
   Since we have no auto-refresh yet, using Airflow as dashboard with 60 minutes logout session would not be super user-friendly. 
   
   I'd love to hear what others think about it, but I believe at the very least UPDATING.md should mention that new behaviour if we agree this is a good thing to introduce 60 minutes (or another period) timeout.
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/6fffa5b0d7840727a96dc1765a0166656bc7ea52?src=pr&el=desc) will **increase** coverage by `0.06%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6912      +/-   ##
   ==========================================
   + Coverage   84.72%   84.78%   +0.06%     
   ==========================================
     Files         679      679              
     Lines       38505    39092     +587     
   ==========================================
   + Hits        32623    33145     +522     
   - Misses       5882     5947      +65
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `97.63% <100%> (+0.15%)` | :arrow_up: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `80.43% <0%> (-2.07%)` | :arrow_down: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `79.91% <0%> (+3.83%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [6fffa5b...e139d63](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569240839
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=h1) Report
   > Merging [#6912](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/947fb64f7f16dc65a24f8d337ed1adc0fb28e464?src=pr&el=desc) will **decrease** coverage by `0.01%`.
   > The diff coverage is `60%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/6912/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff            @@
   ##           master   #6912      +/-   ##
   =========================================
   - Coverage   84.72%   84.7%   -0.02%     
   =========================================
     Files         679     679              
     Lines       38505   38528      +23     
   =========================================
   + Hits        32623   32635      +12     
   - Misses       5882    5893      +11
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/www/app.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvYXBwLnB5) | `94.57% <60%> (-2.91%)` | :arrow_down: |
   | [airflow/contrib/hooks/cassandra\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL2Nhc3NhbmRyYV9ob29rLnB5) | `0% <0%> (-100%)` | :arrow_down: |
   | [airflow/models/\_\_init\_\_.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvX19pbml0X18ucHk=) | `91.3% <0%> (-8.7%)` | :arrow_down: |
   | [airflow/contrib/hooks/spark\_submit\_hook.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9jb250cmliL2hvb2tzL3NwYXJrX3N1Ym1pdF9ob29rLnB5) | `82% <0%> (-0.5%)` | :arrow_down: |
   | [airflow/jobs/backfill\_job.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9qb2JzL2JhY2tmaWxsX2pvYi5weQ==) | `91.59% <0%> (-0.29%)` | :arrow_down: |
   | [airflow/www/views.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy93d3cvdmlld3MucHk=) | `76.08% <0%> (ø)` | :arrow_up: |
   | [airflow/executors/base\_executor.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvYmFzZV9leGVjdXRvci5weQ==) | `96.05% <0%> (ø)` | :arrow_up: |
   | [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/6912/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.91% <0%> (+0.43%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=footer). Last update [947fb64...b86c7ac](https://codecov.io/gh/apache/airflow/pull/6912?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [airflow] tooptoop4 commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout

Posted by GitBox <gi...@apache.org>.
tooptoop4 commented on issue #6912: [AIRFLOW-6352] security - ui - add login timeout
URL: https://github.com/apache/airflow/pull/6912#issuecomment-569230600
 
 
   travis unrelated?

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


With regards,
Apache Git Services