Fix to Werkzeug ProxyFix; expose ProxyFix configuration items

[ericandrewmeadows]

CATEGORY

Choose one

• Bug Fix
• Enhancement (new features, refinement)
• Refactor
• Add tests
• Build / Development Environment
• Documentation

SUMMARY

Custom Security Manager appeared to be broken when behind an AWS ALB, but when I dug further, the Werkzeug version was bumped. This caused an invalid configuration for the proxy in that the host was not passed on. Because of this, along with the change of the library path, and the inclusion of the additional library bump in accordance with apache/airflow#5571, I have validated that the regression that prevented CSM from working in v0.33.0rc1 -> v0.34.0rc [1 & 2] has been resolved.

TEST PLAN

I deployed internally the fix, and validated that the port is not being returned in the header for the destination after hitting the root IP of the Superset instance.

ADDITIONAL INFORMATION

• Has associated issue:
  • Fixes CUSTOM_SECURITY_MANAGER not working in 0.34.0rc1 #8062
  • Fixes Investigate broken Custom Security Manager #8110
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

REVIEWERS

[ericandrewmeadows]

This was validated to fix the issue with CSM, when ENABLE_PROXY_FIX is set to True.

[dpgaspar]

black config.py

FYI Airflow followup fix on: apache/airflow#5563
Docs: https://werkzeug.palletsprojects.com/en/0.15.x/middleware/proxy_fix/?highlight=proxyfix#werkzeug.middleware.proxy_fix.ProxyFix

[villebro]

Thanks for putting in the work to fix this @ericandrewmeadows . Once this gets merged I think we can start picking cherries for a 0.34.1.

[ericandrewmeadows]

👍 - fair to enforce all on since the side effect is minimal.

An aside: why don't we force line length to 79 max? I know it's an outdated standard set by punch cards...but most linters complain.

[ericandrewmeadows]

@villebro - you're welcome. I just happened to come across the Airflow item after I solved it. I updated the docs - thanks again!

[ericandrewmeadows]

@dpgaspar / @villebro - what's the process for merging this?

[villebro]

LGTM, but I don't feel comfortable merging before @dpgaspar approves this, as he is more knowledgeable on this topic than I am.

[ericandrewmeadows]

I figured - yeah - it may have just slipped under his radar.

[ericandrewmeadows]

@dpgaspar - can this be merged?

[mistercrunch]

Few notes:

• do we know how this maps to previous default settings from the previous implementation?
• can we add an entry in UPDATING.md to tell people "Changes around a new version of ProxyFix in Werkzeuf, if you use config ENABLE_PROXY_FIX = True, you'll want to review the newly introduced PROXY_FIX_CONFIG config key and make sure it's set as expected for your environment"

[ericandrewmeadows]

1. I first fixed my instance, then @dpgaspar pointed out the defaults from Airflow so I followed that convention. For me, I use the following settings:

ENABLE_PROXY_FIX = True
PROXY_FIX_CONFIG = {
    "x_for": 1,
    "x_proto": 0,
    "x_host": 1,
    "x_port": 0,
    "x_prefix": 0,
}

2. I'll add that to UPDATING.md

[mistercrunch]

Otherwise LGTM

[codecov-io]

Codecov Report

Merging #8117 into master will increase coverage by 0.13%.
The diff coverage is 33.33%.

@@            Coverage Diff            @@
##           master   #8117      +/-   ##
=========================================
+ Coverage   66.06%   66.2%   +0.13%     
=========================================
  Files         479     479              
  Lines       22930   22930              
  Branches     2524    2524              
=========================================
+ Hits        15148   15180      +32     
+ Misses       7648    7616      -32     
  Partials      134     134

Impacted Files	Coverage Δ
superset/__init__.py	75% <0%> (-0.79%)	⬇️
superset/config.py	88.7% <100%> (+0.06%)	⬆️
superset/assets/src/chart/chartAction.js	49.63% <0%> (ø)	⬆️
superset/sql_lab.py	77.51% <0%> (+0.04%)	⬆️
superset/views/core.py	71.53% <0%> (+0.27%)	⬆️
superset/models/core.py	81.76% <0%> (+0.64%)	⬆️
superset/db_engine_specs/mysql.py	92.15% <0%> (+49.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee24539...cc556e6. Read the comment docs.

[ericandrewmeadows]

@mistercrunch - Should be good to be merged.