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

fix: 1154 Add LOGOUT_REDIRECT_URL setting #1749

Merged
merged 10 commits into from
Dec 9, 2021

Conversation

blag
Copy link
Contributor

@blag blag commented Nov 30, 2021

Description

This pull request addresses #1154, by redirecting to LOGOUT_REDIRECT_URL in the /logout view if LOGOUT_REDIRECT_URL is set in the config, falling back to self.appbuilder.get_url_for_index.

It also adds LOGOUT_REDIRECT_URL to the Configuration keys table in docs/config.rst, and adds a unit test.

This PR is the exact same changes as #1160, but reopened since that one didn't seem to garner any feedback or interest once it was closed by the stale bot. This PR will cleanly merge, and will likely also solve apache/airflow#17279, so there is already people waiting on this fix to be merged.

ADDITIONAL INFORMATION

  • Has associated issue: fixes Make redirect after logout configurable #1154 (which was automatically closed by the stalebot because it was "stale", even though the issue was not fixed)
  • Is CRUD MVC related.
  • Is Auth, RBAC security related.
  • Changes the security db schema.
  • Introduces new feature
  • Removes existing feature

docs/config.rst Outdated Show resolved Hide resolved
Co-authored-by: Jed Cunningham <66968678+jedcunningham@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Dec 4, 2021

Codecov Report

Merging #1749 (c5fb3d2) into master (319b6be) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1749   +/-   ##
=======================================
  Coverage   76.88%   76.88%           
=======================================
  Files          56       56           
  Lines        8136     8136           
=======================================
  Hits         6255     6255           
  Misses       1881     1881           
Flag Coverage Δ
python 76.88% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
flask_appbuilder/security/views.py 62.16% <100.00%> (ø)

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 319b6be...c5fb3d2. Read the comment docs.

@blag
Copy link
Contributor Author

blag commented Dec 6, 2021

Commenting so the stalebot doesn't mark this as stale. This PR is in a mergable state, it just needs a review.
@dpgaspar - Are you going to have time to review and merge this? Or do you need some additional help managing this project?

@dpgaspar dpgaspar merged commit afdbf69 into dpgaspar:master Dec 9, 2021
@blag blag deleted the 1154-use-LOGOUT_REDIRECT_URL branch December 9, 2021 17:26
@blag
Copy link
Contributor Author

blag commented Dec 10, 2021

Thank you!

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

Successfully merging this pull request may close these issues.

Make redirect after logout configurable
4 participants