fix: Save next URL on failed login attempt (#1936)

* Save next URL on failed login attempt

* Save next URL also for LDAP

* Add test for DB auth

* Add test for LDAP auth

* Fix formatting

* Fix formatting

---------

Co-authored-by: Daniel Vaz Gaspar <danielvazgaspar@gmail.com>

flask_appbuilder/base.py

@@ -610,6 +610,11 @@ def security_converge(self, dry: bool = False) -> Dict[str, Any]:
             return {}
         return self.sm.security_converge(self.baseviews, self.menu.menu, dry)
 
+    def get_url_for_login_with(self, next_url: str = None) -> str:
+        if self.sm.auth_view is None:
+            return ""
+        return url_for("%s.%s" % (self.sm.auth_view.endpoint, "login"), next=next_url)
+
     @property
     def get_url_for_login(self) -> str:
         if self.sm.auth_view is None:

flask_appbuilder/security/views.py

@@ -514,15 +514,15 @@ def login(self):
             return redirect(self.appbuilder.get_url_for_index)
         form = LoginForm_db()
         if form.validate_on_submit():
+            next_url = get_safe_redirect(request.args.get("next", ""))
             user = self.appbuilder.sm.auth_user_db(
                 form.username.data, form.password.data
             )
             if not user:
                 flash(as_unicode(self.invalid_login_message), "warning")
-                return redirect(self.appbuilder.get_url_for_login)
+                return redirect(self.appbuilder.get_url_for_login_with(next_url))
             login_user(user, remember=False)
-            next_url = request.args.get("next", "")
-            return redirect(get_safe_redirect(next_url))
+            return redirect(next_url)
         return self.render_template(
             self.login_template, title=self.title, form=form, appbuilder=self.appbuilder
         )
@@ -537,15 +537,15 @@ def login(self):
             return redirect(self.appbuilder.get_url_for_index)
         form = LoginForm_db()
         if form.validate_on_submit():
+            next_url = get_safe_redirect(request.args.get("next", ""))
             user = self.appbuilder.sm.auth_user_ldap(
                 form.username.data, form.password.data
             )
             if not user:
                 flash(as_unicode(self.invalid_login_message), "warning")
-                return redirect(self.appbuilder.get_url_for_login)
+                return redirect(self.appbuilder.get_url_for_login_with(next_url))
             login_user(user, remember=False)
-            next_url = request.args.get("next", "")
-            return redirect(get_safe_redirect(next_url))
+            return redirect(next_url)
         return self.render_template(
             self.login_template, title=self.title, form=form, appbuilder=self.appbuilder
         )

flask_appbuilder/tests/security/test_auth_ldap.py

@@ -10,7 +10,6 @@
 import ldap
 from mockldap import MockLdap
 
-
 from ..const import USERNAME_ADMIN, USERNAME_READONLY
 
 logging.basicConfig(format="%(asctime)s:%(levelname)s:%(name)s:%(message)s")
@@ -1187,3 +1186,32 @@ def test__indirect_bind__registered__multi_role__with_role_sync(self):
                 self.call_bind_alice,
             ],
         )
+
+    def test_login_failed_keep_next_url(self):
+        """
+        LDAP: Keeping next url after failed login attempt
+        """
+
+        self.app.config["AUTH_LDAP_SEARCH"] = "ou=users,o=test"
+        self.app.config["AUTH_LDAP_USERNAME_FORMAT"] = "uid=%s,ou=users,o=test"
+        self.app.config["AUTH_USER_REGISTRATION"] = True
+        self.app.config["AUTH_USER_REGISTRATION_ROLE"] = "Public"
+        self.app.config["WTF_CSRF_ENABLED"] = False
+        self.app.config["SECRET_KEY"] = "thisismyscretkey"
+
+        self.appbuilder = AppBuilder(self.app, self.db.session)
+        client = self.app.test_client()
+        client.get("/logout/")
+
+        response = client.post(
+            "/login/?next=/users/userinfo/",
+            data=dict(username="natalie", password="wrong_natalie_password"),
+            follow_redirects=False,
+        )
+        response = client.post(
+            response.location,
+            data=dict(username="natalie", password="natalie_password"),
+            follow_redirects=False,
+        )
+
+        assert response.location == "http://localhost/users/userinfo/"

flask_appbuilder/tests/security/test_mvc_security.py

@@ -203,11 +203,31 @@ def test_db_login_invalid_control_characters_next_url(self):
             self.client,
             USERNAME_ADMIN,
             PASSWORD_ADMIN,
-            next_url=u"\u0001" + "sample.com",
+            next_url="\u0001" + "sample.com",
             follow_redirects=False,
         )
         assert response.location == "http://localhost/"
 
+    def test_db_login_failed_keep_next_url(self):
+        """
+        Test Security Keeping next url after failed login attempt
+        """
+        self.browser_logout(self.client)
+        response = self.browser_login(
+            self.client,
+            USERNAME_ADMIN,
+            f"wrong_{PASSWORD_ADMIN}",
+            next_url="/users/list/",
+            follow_redirects=False,
+        )
+        response = self.client.post(
+            response.location,
+            data=dict(username=USERNAME_ADMIN, password=PASSWORD_ADMIN),
+            follow_redirects=False,
+        )
+
+        assert response.location == "http://localhost/users/list/"
+
     def test_auth_builtin_roles(self):
         """
         Test Security builtin roles readonly