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

chore: Add test for flask wtf csrf exempt config #17468

Closed

Conversation

etr2460
Copy link
Member

@etr2460 etr2460 commented Nov 17, 2021

SUMMARY

Follow up from #17429 to add a test to ensure this doesn't break again

TESTING INSTRUCTIONS

CI, ensure that when i add a view that doesn't exist onto the exempt list the test fails

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

to: @john-bodley @ofekisr @villebro

@etr2460 etr2460 force-pushed the erik-ritter--add-csrf-exempt-test branch 3 times, most recently from 19b2a07 to 1e8dc34 Compare November 17, 2021 22:58
@etr2460
Copy link
Member Author

etr2460 commented Nov 17, 2021

Any ideas why tests are failing here? My new test is working at least :/

@ofekisr
Copy link
Contributor

ofekisr commented Nov 18, 2021

Any ideas why tests are failing here? My new test is working at least :/

you should debug it, contact me if you need help

@etr2460 etr2460 force-pushed the erik-ritter--add-csrf-exempt-test branch from 3147c02 to 7c50790 Compare November 18, 2021 19:57
all_view_functions = {
f"{view.__module__}.{view.__name__}" for view in app.view_functions.values()
}
for exempt_api in app.config["WTF_CSRF_EXEMPT_LIST"]:
Copy link
Contributor

@ofekisr ofekisr Nov 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rare use case, but you should check against the default config (superset.config) and not from the test config even if the test config does not contain "WTF_CSRF_EXEMPT_LIST" key

@etr2460 etr2460 force-pushed the erik-ritter--add-csrf-exempt-test branch from 7c50790 to a034bc4 Compare December 27, 2021 04:55
@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

@stale stale bot added the inactive Inactive for >= 30 days label Apr 16, 2022
@stale stale bot closed this Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inactive Inactive for >= 30 days size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants