Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSRF configuration is missing the WTF_ prefix #8915

Closed
zachliu opened this issue May 19, 2020 · 6 comments · Fixed by #8944
Closed

CSRF configuration is missing the WTF_ prefix #8915

zachliu opened this issue May 19, 2020 · 6 comments · Fixed by #8944
Assignees
Labels
kind:bug This is a clearly a bug

Comments

@zachliu
Copy link
Contributor

zachliu commented May 19, 2020

Apache Airflow version:

Kubernetes version (if you are using kubernetes) (use kubectl version):

Environment:

  • Cloud provider or hardware configuration: AWS ECS
  • OS (e.g. from /etc/os-release): Ubuntu 18.04 bionic
  • Kernel (e.g. uname -a): 4.15.0-1065-aws
  • Install tools:
  • Others:

What happened:
I have been trying to update a certain CSRF configuration (WTF_CSRF_TIME_LIMIT) because I've been annoyed by the CSRF token has expired error message whenever I stayed on a page for more than 1 hour and wanted to refresh.

What you expected to happen:
In webserver_config.py there is a CSRF_ENABLED = True. So I added CSRF_TIME_LIMIT = None after that line. But it didn't work. After reading https://github.com/lepture/flask-wtf/blob/v0.14.2/flask_wtf/csrf.py, I realized that we needed WTF_CSRF_TIME_LIMIT in my webserver_config.py. The WTF_ prefix cannot be omitted.

How to reproduce it:

  1. Add CSRF_TIME_LIMIT = None after CSRF_ENABLED = True in webserver_config.py.
  2. Re-deploy.
  3. Go to any DAG's tree view.
  4. Stay there for more than 1 hour.
  5. Click the Refresh button.
  6. See the CSRF token has expired error message

Anything else we need to know:

I already forked Airflow. I guess I'll submit a simple PR to add the WTF_ prefix

@zachliu zachliu added the kind:bug This is a clearly a bug label May 19, 2020
@boring-cyborg
Copy link

boring-cyborg bot commented May 19, 2020

Thanks for opening your first issue here! Be sure to follow the issue template!

@mik-laj
Copy link
Member

mik-laj commented May 19, 2020

#8613
Is it related? Can you check it?

@zachliu
Copy link
Contributor Author

zachliu commented May 19, 2020

it's unrelated

@ashb
Copy link
Member

ashb commented May 20, 2020

I'm not quite sure what you are asking us to do here -- the webserver_config.py that airflow generates does not set a time limit, CSRF_TIME_LIMIT or WTF_CSRF_TIME_LIMIT.

Once airflow has generated that file you are in control of what you put in it -- it lives in your Airflow install, not the airflow code base.

Unless I've misunderstood here, there's nothing for us to do, and you've already found the correct setting to set.

@ashb ashb closed this as completed May 20, 2020
@zachliu
Copy link
Contributor Author

zachliu commented May 20, 2020

I'm not quite sure what you are asking us to do here -- the webserver_config.py that airflow generates does not set a time limit, CSRF_TIME_LIMIT or WTF_CSRF_TIME_LIMIT.

Once airflow has generated that file you are in control of what you put in it -- it lives in your Airflow install, not the airflow code base.

Unless I've misunderstood here, there's nothing for us to do, and you've already found the correct setting to set.

my bad, i should describe it more clearly
the CSRF_ENABLED = True in this file in airflow's code base


should be

WTF_CSRF_ENABLED = True

without WTF_ that line is useless

@ashb ashb reopened this May 20, 2020
ashb added a commit to astronomer/airflow that referenced this issue May 21, 2020
CSRF_ENABLED does nothing.

Thankfully, due to sensible defaults in flask-wtf, CSRF is on by
default, but we should set this correctly.

Fixes apache#8915
@ashb
Copy link
Member

ashb commented May 21, 2020

@zachliu Ahha gotcha, yes. PR to fix that for new installs (we can't do anything about existing/already generated webserver_config.py, but at least it would stop some people tearing their hair out.)

@ashb ashb self-assigned this May 21, 2020
ashb added a commit that referenced this issue May 21, 2020
CSRF_ENABLED does nothing.

Thankfully, due to sensible defaults in flask-wtf, CSRF is on by
default, but we should set this correctly.

Fixes #8915
kaxil pushed a commit that referenced this issue Jun 22, 2020
CSRF_ENABLED does nothing.

Thankfully, due to sensible defaults in flask-wtf, CSRF is on by
default, but we should set this correctly.

Fixes #8915

(cherry picked from commit 16206cd)
potiuk pushed a commit that referenced this issue Jun 29, 2020
CSRF_ENABLED does nothing.

Thankfully, due to sensible defaults in flask-wtf, CSRF is on by
default, but we should set this correctly.

Fixes #8915

(cherry picked from commit 16206cd)
kaxil pushed a commit that referenced this issue Jul 1, 2020
CSRF_ENABLED does nothing.

Thankfully, due to sensible defaults in flask-wtf, CSRF is on by
default, but we should set this correctly.

Fixes #8915

(cherry picked from commit 16206cd)
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this issue Mar 5, 2021
…e#8944)

CSRF_ENABLED does nothing.

Thankfully, due to sensible defaults in flask-wtf, CSRF is on by
default, but we should set this correctly.

Fixes apache#8915

(cherry picked from commit 16206cd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug This is a clearly a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants