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

2.3.3 (jQuery upgrade) breaks CSRF on some Safari, Chrome #1362

Closed
mb-m opened this issue Apr 28, 2020 · 9 comments
Closed

2.3.3 (jQuery upgrade) breaks CSRF on some Safari, Chrome #1362

mb-m opened this issue Apr 28, 2020 · 9 comments

Comments

@mb-m
Copy link

mb-m commented Apr 28, 2020

Environment

Flask-Appbuilder version: 2.3.3 (confirmed equivalent code works with 2.3.2)

Describe the actual results

I'm not sure exactly how the code is called - but this is visible in the airflow pause dag checkbox (trying to change the state with 2.3.3 causes a INFO - The CSRF token is missing. log line.

In the JS Console in the browser
jQuery.Deferred exception: e.hasOwnProperty is not a function. (In e.hasOwnProperty(i)', 'e.hasOwnProperty' is undefined)
The stack trace appears to be something like:

(anonymous function) -- bootstrap.min.js:6:23497
(anonymous function) -- bootstrap.min.js:6:22381
m -- bootstrap.min.js:6:21898
(anonymous function) -- bootstrap.min.js:6:30926
each -- jquery-latest.js:157
(anonymous function) -- base.a49848a5126c0cfc249c.js:111
e -- jquery-latest.js:1341
(anonymous function) -- jquery-latest.js:1348

I also see:
TypeError: e.hasOwnProperty s not a function. (In 'e.hasOwnProperty(i)', 'e.hasOwnProperty' is undefined) with (anonymous function) -- jquery-latest.js:1399

I'm afraid my JS-fu is very weak, so I've not been able to delve further

Steps to reproduce

Airflow build with 2.3.3, attempt to pause/unpause a DAG on at least Safari (Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.1 Safari/605.1.15), Chrome Version 81.0.4044.122, Chrome version 80.0.3987.116 (Official Build) (64-bit)

@dpgaspar
Copy link
Owner

dpgaspar commented Apr 28, 2020

Hi @mb-m,

Testing on superset, I get the same JS error on 2.3.2 and 2.3.3, and no CSRF issues, I think that the CSRF issues are not related with that JS error. Can you post a bit more information, server logs etc...

the JS error is related with a regression related with tooltip texts. going to address it (but, unrelated with CSRF)

#1350

@mb-m
Copy link
Author

mb-m commented Apr 28, 2020

@dpgaspar that's interesting, because the CSRF definitely doesn't get submitted in the request to pause/unpause a DAG (either on the main page of the airflow UI or the per DAG page), so I just posted what was in the console. Airflow returns a 400 Bad Request to that URL, and logs the info line above, but other than that doesn't really say much. I had the request up in the Develop window from my browser, and there wasn't any CSRF in headers or in payload.

I'm not sure if it's just on that checkbox where I think the form gets generated dynamically (I couldn't find it in the DOM from a quick scan). This was based on a 1.10.9 install of airflow, but as soon as I limited flask-appbuilder to <= 2.3.2, the pause/unpause was working again.

I might well have jumped the gun on the exceptions - right now I'm at the end of my day - so I'm going to sign off for the moment, but since I could replicate it on my local airflow build, I can try and delve in in more detail.

To be honest, most of the support I do on airflow for my company is backend - I'm not nearly as aware as I should be about the front-end, UI and HTML generation.

@kaxil
Copy link

kaxil commented Apr 28, 2020

Airflow issue: apache/airflow#8613 | apache/airflow#8599

@dpgaspar
Copy link
Owner

dpgaspar commented Apr 29, 2020

Ok, thanks @kaxil @mb-m going to revert JQuery bump and release a candidate 2.3.4rc1 do you have availability to test it?

Note also found that 3.5.0 has the following issue that impacts rendering tooltips (minor issue, but an issue also): jquery/jquery#4665

We probably need to wait for JQuery 3.5.1

@dpgaspar
Copy link
Owner

dpgaspar commented Apr 29, 2020

@mb-m
Copy link
Author

mb-m commented Apr 29, 2020

@dpgaspar for what its worth a quick test clicking around my local airflow instance with 2.3.4rc1 looks good (it definitely doesn't have this CSRF bug).

Thanks for responding so fast!

@dpgaspar
Copy link
Owner

No problem @mb-m, thks for reporting and taking the time to test. I'll probably release this today

@dpgaspar
Copy link
Owner

ok, released: https://pypi.org/project/Flask-AppBuilder/

Going to close the issue, feel free to reopen if you find any problem related with this

@kaxil
Copy link

kaxil commented May 4, 2020

Thanks @dpgaspar for releasing 2.3.4 quickly. I can confirm this does solve the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants