fix(login): redirect user from login page if already logged in (#6689)
* fix(login): redirect user from login page if already logged in the user should not be able to access the login page if a user session already exists. closes #6500. Signed-off-by: Chinmay Pai <chinmaydpai@gmail.com> * fix(test-website): fix website test what is the point in writing tests if they don't really work/function as intended? Signed-off-by: Chinmay Pai <chinmaydpai@gmail.com> * fix(regex): do not replace '\' in rules that defeats the entire purpose of creating rules, wtf? Signed-off-by: Chinmay Pai <chinmaydpai@gmail.com> * fix(test_website): change user using set_user() Signed-off-by: Chinmay Pai <chinmaydpai@gmail.com> * redirect: prefix string with r to escape string literals Signed-off-by: Chinmay Pai <chinmaydpai@gmail.com>
This commit is contained in:
parent
ba72414a3d
commit
825b0120fa
5 changed files with 22 additions and 16 deletions
|
|
@ -13,45 +13,47 @@ def set_request(**kwargs):
|
|||
class TestWebsite(unittest.TestCase):
|
||||
|
||||
def test_page_load(self):
|
||||
frappe.set_user('Guest')
|
||||
set_request(method='POST', path='login')
|
||||
response = render.render()
|
||||
|
||||
self.assertTrue(response.status_code, 200)
|
||||
self.assertEquals(response.status_code, 200)
|
||||
|
||||
html = frappe.safe_decode(response.get_data())
|
||||
|
||||
self.assertTrue('/* login-css */' in html)
|
||||
self.assertTrue('// login.js' in html)
|
||||
self.assertTrue('<!-- login.html -->' in html)
|
||||
frappe.set_user('Administrator')
|
||||
|
||||
def test_redirect(self):
|
||||
import frappe.hooks
|
||||
frappe.hooks.website_redirects = [
|
||||
dict(source='/testfrom', target='://testto1'),
|
||||
dict(source='/testfromregex.*', target='://testto2'),
|
||||
dict(source='/testsub/(.*)', target='://testto3/\1')
|
||||
dict(source=r'/testfrom', target=r'://testto1'),
|
||||
dict(source=r'/testfromregex.*', target=r'://testto2'),
|
||||
dict(source=r'/testsub/(.*)', target=r'://testto3/\1')
|
||||
]
|
||||
frappe.cache().delete_key('app_hooks')
|
||||
frappe.cache().delete_key('website_redirects')
|
||||
|
||||
set_request(method='GET', path='/testfrom')
|
||||
response = render.render()
|
||||
self.assertTrue(response.status_code, 301)
|
||||
self.assertTrue(response.headers.get('Location'), '://testto1')
|
||||
self.assertEquals(response.status_code, 301)
|
||||
self.assertEquals(response.headers.get('Location'), r'://testto1')
|
||||
|
||||
set_request(method='GET', path='/testfromregex/test')
|
||||
response = render.render()
|
||||
self.assertTrue(response.status_code, 301)
|
||||
self.assertTrue(response.headers.get('Location'), '://testto2')
|
||||
self.assertEquals(response.status_code, 301)
|
||||
self.assertEquals(response.headers.get('Location'), r'://testto2')
|
||||
|
||||
set_request(method='GET', path='/testsub/me')
|
||||
response = render.render()
|
||||
self.assertTrue(response.status_code, 301)
|
||||
self.assertTrue(response.headers.get('Location'), '://testto3/me')
|
||||
self.assertEquals(response.status_code, 301)
|
||||
self.assertEquals(response.headers.get('Location'), r'://testto3/me')
|
||||
|
||||
set_request(method='GET', path='/test404')
|
||||
response = render.render()
|
||||
self.assertTrue(response.status_code, 404)
|
||||
self.assertEquals(response.status_code, 404)
|
||||
|
||||
delattr(frappe.hooks, 'website_redirects')
|
||||
frappe.cache().delete_key('app_hooks')
|
||||
|
|
|
|||
|
|
@ -48,7 +48,7 @@ def update_controller_context(context, controller):
|
|||
ret = module.get_context(context)
|
||||
if ret:
|
||||
context.update(ret)
|
||||
except (frappe.PermissionError, frappe.DoesNotExistError):
|
||||
except (frappe.PermissionError, frappe.DoesNotExistError, frappe.Redirect):
|
||||
raise
|
||||
except:
|
||||
if not frappe.flags.in_migrate:
|
||||
|
|
|
|||
|
|
@ -16,7 +16,8 @@ def resolve_redirect(path):
|
|||
{"source": "/from", "target": "/main"},
|
||||
|
||||
# use regex
|
||||
{"source": "/from/(.*)", "target": "/main/\1"}
|
||||
{"source": r"/from/(.*)", "target": r"/main/\1"}
|
||||
# use r as a string prefix if you use regex groups or want to escape any string literal
|
||||
]
|
||||
'''
|
||||
redirects = frappe.get_hooks('website_redirects')
|
||||
|
|
@ -31,7 +32,7 @@ def resolve_redirect(path):
|
|||
for rule in redirects:
|
||||
pattern = rule['source'].strip('/ ') + '$'
|
||||
if re.match(pattern, path):
|
||||
redirect_to = re.sub(pattern, rule['target'].replace('\\', '\\\\'), path)
|
||||
redirect_to = re.sub(pattern, rule['target'], path)
|
||||
frappe.flags.redirect_location = redirect_to
|
||||
frappe.cache().hset('website_redirects', path, redirect_to)
|
||||
raise frappe.Redirect
|
||||
|
|
|
|||
|
|
@ -67,6 +67,9 @@ def render(path=None, http_status_code=None):
|
|||
except frappe.PermissionError as e:
|
||||
data, http_status_code = render_403(e, path)
|
||||
|
||||
except frappe.Redirect as e:
|
||||
raise e
|
||||
|
||||
except Exception:
|
||||
path = "error"
|
||||
data = render_page(path)
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ from frappe.utils.html_utils import get_icon_html
|
|||
no_cache = True
|
||||
|
||||
def get_context(context):
|
||||
if frappe.session.user != "Guest" and frappe.session.data.user_type=="System User":
|
||||
frappe.local.flags.redirect_location = "/desk"
|
||||
if frappe.session.user != "Guest":
|
||||
frappe.local.flags.redirect_location = "/" if frappe.session.data.user_type=="Website User" else "/desk"
|
||||
raise frappe.Redirect
|
||||
|
||||
# get settings from site config
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue