-
Notifications
You must be signed in to change notification settings - Fork 56
add login screen, redirect when unauthorized #302
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
base: develop
Are you sure you want to change the base?
Conversation
intelmq_manager/static/js/static.js
Outdated
function requireLogin(destinationUrl){ | ||
let currentPath = window.location.pathname.substring(1) | ||
let loginUrl = "/login.html" | ||
|
||
if ( ! ["login.html", "index.html"].includes(currentPath)){ | ||
loginUrl += "?r=" + encodeURIComponent(destinationUrl || currentPath) | ||
} | ||
window.location.href = loginUrl | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, this function won't work in multiple cases. For example, IntelMQ manager in all instances I manage is deployed under a path /intelmq-manager
. In addition, the main page is accessible directly under the path, without redirecting to index.html
.
I've checked how the code would behave on the main page on one of my instances (in the browser's console):
> let currentPath = window.location.pathname.substring(1)
> let loginUrl = "/login.html"
> currentPath
"intelmq-manager/"
> window.location.href = loginUrl
# Redirects to the main domain, not under the manager's path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have modified the redirects so that all work with relative paths
intelmq_manager/static/js/static.js
Outdated
|
||
let url = new URL(window.location.href) | ||
let redirect = new URLSearchParams(url.search).get('r') | ||
window.location.replace(redirect || "/index.html"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a minimum precaution, we should verify the redirect target is related to the current path, to avoid an open redirect issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was assuming this wouldn't be an issue as the redirect happens only after the user is authenticated, anyways I have made a slight change, so that the redirect target is treated as relative path to the current location.
This PR is taking out the login modal from base.mako into a new template login.mako. The redirect happens as part of the authenticatedAjax function.
#295