pzampino commented on a change in pull request #476:
URL: https://github.com/apache/knox/pull/476#discussion_r697699834



##########
File path: 
gateway-applications/src/main/resources/applications/knoxauth/app/js/knoxauth.js
##########
@@ -19,70 +19,74 @@ var loginPageSuffix = "/knoxauth/login.html";
 var webssoURL = "/api/v1/websso?originalUrl=";
 var userAgent = navigator.userAgent.toLowerCase();
 
-function get(name){
-    //KNOX-820 changing the regex so that multiple query params get included 
with the 'originalUrl'
-   if(name=(new 
RegExp('[?&]'+encodeURIComponent(name)+'=([^]*)')).exec(location.search))
-      return decodeURIComponent(name[1]);
+function get(name) {
+       //KNOX-820 changing the regex so that multiple query params get 
included with the 'originalUrl'
+       if ((name = (new RegExp('[?&]' + encodeURIComponent(name) + 
'=([^]*)')).exec(location.search))) {
+               return decodeURIComponent(name[1]);
+       }
 }
 
 function testSameOrigin(url) {
-    var loc = window.location,
-        a = document.createElement('a');
-    a.href = url;
-    return a.hostname == loc.hostname &&
-           a.port == loc.port &&
-           a.protocol == loc.protocol;
+       var loc = window.location,
+               a = document.createElement('a');
+       a.href = url;
+       return a.hostname == loc.hostname &&
+               a.port == loc.port &&
+               a.protocol == loc.protocol;
 }
 
 function redirect(redirectUrl) {
-  try { window.location.replace(redirectUrl); } 
-  catch(e) { window.location = redirectUrl; }
+       try {
+               window.location.replace(redirectUrl);
+       } catch (e) {
+               window.location = redirectUrl;
+       }
 }
 
 var keypressed = function(event) {
-    if (event.keyCode == 13) {
-        login();
-    }
-}
+       if (event.keyCode == 13) {
+               login();
+       }
+};
 
 var login = function() {
-    var pathname = window.location.pathname;
-    var topologyContext = pathname.replace(loginPageSuffix, "");;
-    var loginURL = topologyContext + webssoURL;
-    var form = document.forms[0];
-    var username = form.username.value;
-    var password = form.password.value;
-    var _login = function() {
-    var originalUrl = get("originalUrl");
-    var idpUrl = loginURL + originalUrl;
-    var redirectUrl = originalUrl;
-      //Instantiate HTTP Request
-        var request = ((window.XMLHttpRequest) ? new XMLHttpRequest() : new 
ActiveXObject("Microsoft.XMLHTTP"));
-        request.open("POST", idpUrl, true);
-        request.setRequestHeader("Authorization", "Basic " + btoa(username + 
":" + password))
-        request.send(null);
+       var pathname = window.location.pathname;
+       var topologyContext = pathname.replace(loginPageSuffix, "");
+       var loginURL = topologyContext + webssoURL;
+       var form = document.forms[0];
+       var username = form.username.value;
+       var password = form.password.value;
+       var _login = function() {
+               var originalUrl = get("originalUrl");
+               var idpUrl = loginURL + originalUrl;
+               var redirectUrl = originalUrl;
+               //Instantiate HTTP Request
+               var request = ((window.XMLHttpRequest) ? new XMLHttpRequest() : 
new ActiveXObject("Microsoft.XMLHTTP"));
+               request.open("POST", idpUrl, true);
+               request.setRequestHeader("Authorization", "Basic " + 
btoa(username + ":" + password));
+               request.send(null);
 
-      //Process Response
-        request.onreadystatechange = function(){
-            if (request.readyState == 4) {
-                if (request.status==0 || request.status==200 || 
request.status==204 || request.status==307 || request.status==303) {
-                  if (testSameOrigin(originalUrl) == false) {
-                    redirectUrl = "redirecting.html?originalUrl=" + 
originalUrl;
-                  }
-                  redirect(redirectUrl);
-                } else {
-                  $('#errorBox').show();
-                  $('#signInLoading').hide();
-                  $('#signIn').removeAttr('disabled');
-                  if (request.status==401) {
-                    $('#errorBox .errorMsg').text("The username or password 
you entered is incorrect.");
-                  } else {
-                    $('#errorBox .errorMsg').text("Response from " + 
request.responseURL + " - " + request.status + ": " + request.statusText);
-                  }
-                }
-            }
-        }
-    }
+               //Process Response
+               request.onreadystatechange = function() {
+                       if (request.readyState == 4) {
+                               if (request.status == 0 || request.status == 
200 || request.status == 204 || request.status == 307 || request.status == 303) 
{
+                                       if (testSameOrigin(originalUrl) == 
false) {
+                                               redirectUrl = 
"redirecting.html?originalUrl=" + originalUrl;
+                                       }
+                                       redirect(redirectUrl);
+                               } else {
+                                       $('#errorBox').show();
+                                       $('#signInLoading').hide();
+                                       $('#signIn').removeAttr('disabled');
+                                       if (request.status == 401) {
+                                               $('#errorBox 
.errorMsg').text("The username or password you entered is incorrect.");
+                                       } else {
+                                               $('#errorBox 
.errorMsg').text("Response from " + request.responseURL + " - " + 
request.status + ": " + request.statusText);
+                                       }
+                               }
+                       }
+               };
+       };
 
-    _login();
-}
+       _login();
+};

Review comment:
       Should there not be a newline at the end of the file?

##########
File path: 
gateway-applications/src/main/resources/applications/knoxauth/app/logout.jsp
##########
@@ -62,15 +75,41 @@
                 whitelist = "";
             }
         }
+
+        boolean validRedirect = false;
         String origUrl = request.getParameter("originalUrl");
         String del = "?";
-        if (origUrl.contains("?")) {
+        if (origUrl != null && origUrl.contains("?")) {
           del = "&";
         }
-        boolean validRedirect = RegExUtils.checkWhitelist(whitelist, origUrl);
-        if (("1".equals(request.getParameter("returnToApp"))) && 
validRedirect) {
-          response.setStatus(response.SC_MOVED_PERMANENTLY);
-          response.setHeader("Location",originalUrl + del + "refresh=1");
+        if (origUrl != null) {
+          validRedirect = RegExUtils.checkWhitelist(whitelist, origUrl);
+        }
+        if (("1".equals(request.getParameter("returnToApp")))) {
+          if (validRedirect) {
+               response.setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY);
+               response.setHeader("Location",originalUrl + del + "refresh=1");
+            return;
+          }
+        }
+               else if (("1".equals(request.getParameter("globalLogout")))) {

Review comment:
       nit: else if on a new line with extra indentation?

##########
File path: 
gateway-applications/src/main/resources/applications/knoxauth/app/logout.jsp
##########
@@ -62,15 +75,41 @@
                 whitelist = "";
             }
         }
+
+        boolean validRedirect = false;
         String origUrl = request.getParameter("originalUrl");
         String del = "?";
-        if (origUrl.contains("?")) {
+        if (origUrl != null && origUrl.contains("?")) {
           del = "&";
         }
-        boolean validRedirect = RegExUtils.checkWhitelist(whitelist, origUrl);
-        if (("1".equals(request.getParameter("returnToApp"))) && 
validRedirect) {
-          response.setStatus(response.SC_MOVED_PERMANENTLY);
-          response.setHeader("Location",originalUrl + del + "refresh=1");
+        if (origUrl != null) {

Review comment:
       nit: validRedirect is only used if returnToApp == "1", so this check 
could all go into that block.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to