Merge pull request #91 from davidradl/git89

#89 amend login logic to handle incorrect server name more elegantly

cra-client/src/auth/NoServerName.js

@@ -29,7 +29,7 @@ const NoServerName = () => {
   return (
     <div>
       <Egeriacolor />
-      <div>Please supply a server name:</div>
+      <div>Please supply a VALID server name:</div>
       <Grid>
         <Row>
           <Column

cra-client/src/auth/login.js

@@ -13,8 +13,8 @@ import {
   Form,
   FormGroup,
   TextInput,
-  Button
-} from 'carbon-components-react';
+  Button,
+} from "carbon-components-react";
 
 const Login = () => {
   const identificationContext = useContext(IdentificationContext);
@@ -23,29 +23,32 @@ const Login = () => {
   const [errorMsg, setErrorMsg] = useState();
   let history = useHistory();
 
-  const handleOnClick = e => {
+  const handleOnClick = (e) => {
     console.log("login handleClick(()");
     e.preventDefault();
-    const url = identificationContext.getBrowserURL('login') + "?" + new URLSearchParams({
-      username: userId,
-      password: password
-  });
+    const url =
+      identificationContext.getBrowserURL("login") +
+      "?" +
+      new URLSearchParams({
+        username: userId,
+        password: password,
+      });
     fetch(url, {
       method: "post",
       headers: {
         Accept: "application/json",
-        "Content-Type": "application/json"
-      }
+        "Content-Type": "application/json",
+      },
     })
-      .then(res => res.json())
-      .then(res => {
+      .then((res) => res.json())
+      .then((res) => {
         if (res.status === "success") {
           console.log("login worked " + JSON.stringify(res));
           identificationContext.setUserId(userId);
           identificationContext.setUser(res.user);
           identificationContext.setAuthenticated(true);
-          // TODO original url prop. 
-          const path = identificationContext.getBrowserURL(''); 
+          // TODO original url prop.
+          const path = identificationContext.getBrowserURL("");
           history.push(path);
         } else {
           if (res.errno) {
@@ -54,13 +57,15 @@ const Login = () => {
             setErrorMsg("Login Failed");
           }
         }
-      })
-      .catch(res => {
-        setErrorMsg("Create Failed - logic error");
-      });
+        })
+        .catch(res => {
+          alert("The URL is not valid. You will be asked to put in a valid Server Name.");
+          history.replace('/');
+        }
+      );
   };
 
-  const handleOnChange = event => {
+  const handleOnChange = (event) => {
     const value = event.target.value;
     sessionStorage.setItem("egeria-userId", value);
     setUserId(value);
@@ -99,10 +104,10 @@ const Login = () => {
                   id="login-password"
                   name="password"
                   value={password}
-                  onChange={e => setPassword(e.target.value)}
+                  onChange={(e) => setPassword(e.target.value)}
                   placeholder="Password"
                   required
-                  />
+                />
               </FormGroup>
               <Button
                 type="submit"
@@ -122,4 +127,3 @@ const Login = () => {
 };
 
 export default Login;
-

cra-server/db/users.js

@@ -41,7 +41,7 @@ var records = [
 
 exports.findById = function (id, cb) {
   process.nextTick(function () {
-    console.log("findByUserId");
+    // console.log("findByUserId");
     for (var i = 0, len = records.length; i < len; i++) {
       var record = records[i];
       if (record.id === id) {
@@ -54,7 +54,7 @@ exports.findById = function (id, cb) {
 
 exports.findByUsername = function(username, cb) {
 process.nextTick(function() {
-console.log("findByUsername");
+// console.log("findByUsername");
 for (var i = 0, len = records.length; i < len; i++) {
   var record = records[i];
   if (record.username === username) {

cra-server/functions/getServerInfoFromEnv.js

@@ -22,7 +22,7 @@ const getServerInfoFromEnv = () => {
           );
         } else {
           const serverName = envVariable.substr(env_prefix.length);
-          console.log("Found server name " + serverName);
+          // console.log("Found server name " + serverName);
           const serverDetailsStr = env[envVariable];
           const serverDetails = JSON.parse(serverDetailsStr);
           if (

cra-server/functions/serverNameMiddleware.js

@@ -17,42 +17,75 @@
  *   /coco1/abc/de => /abc/de?servername=coco1
  *   /display.ico => /display.ico
  *
+ * This middleware also validates that the server name that has been supplied is valid (i.e. is defined in our list of servers).
+ * So we can have a better user experience, the login screen is displayed for /<server name> and /<server name>/login even if the <server name>
+ * is invalid. The request will fail on the login post if the <server name> is not known.      
  */
 const serverNameMiddleWare = (req, res, next) => {
-
+  // Disabling logging as CodeQL does not like user supplied values being logged.
   // console.log("before " + req.url);
   const segmentArray = req.url.split("/");
+  // the supplied url always starts with a '/' so there will always be at least 2 segments
+  // '/' goes to "" and ""
+  // "/ddd" goes to "" and "ddd"
   const segmentNumber = segmentArray.length;
+  let noServerfound = false;
+  const servers = req.app.get("servers");
 
   if (segmentNumber > 1) {
+    // the supplied url always starts with a /
     const segment1 = segmentArray.slice(1, 2).join("/");
     // Disabling logging as CodeQL does not like user supplied values being logged.
-    // console.log("segment1 " + segment1);
+    // console.log("A segment1 " + segment1);
+    const lastSegment = segmentArray.slice(-1);
+    const lastSegmentStr = lastSegment[0];
+    // Disabling logging as CodeQL does not like user supplied values being logged.
+    // console.log("Last segment is " + lastSegmentStr);
 
-    if (segment1 != "servers" && segment1 != "open-metadata" && segment1 != "user") {
+    if (
+      segment1 != "servers" &&
+      segment1 != "open-metadata" &&
+      segment1 != "user" &&
+       // we want the login screen to be displayed with the get - so we can properly handle the invalid server name - so don't check the server in this case   
+      !(segmentNumber == 2 && req.method === 'GET')
+    ) {
       // in a production scenario we are looking at login, favicon.ico and bundle.js for for now look for those in the last segment
       // TODO once we have development webpack, maybe the client should send a /js/ or a /static/ segment after the servername so we know to keep the subsequent segments.
+      // console.log("req.method "+ req.method); 
+      if (lastSegmentStr.startsWith("login") && req.method === 'POST') {
+        // segment1 should be the serverName - so validate that it is
 
-      const lastSegment = segmentArray.slice(-1);
-      // Disabling logging as CodeQL does not like user supplied values being logged.
-      // console.log("Last segment is " + lastSegment);
-      if (
-        lastSegment == "bundle.js" ||
-        lastSegment == "favicon.ico" ||
-        lastSegment == "login"
-      ) {
+        if (servers[segment1] === undefined) {
+          //Not in the array of servers
+          noServerfound = true;
+        }
+      }
+      if (lastSegmentStr == "bundle.js" || lastSegmentStr == "favicon.ico") {
         req.url = "/" + lastSegment;
       } else {
-        // remove the server name and pass through
-        req.url = "/" + segmentArray.slice(2).join("/");
+        // we want the login screen to be displayed with the get - so we can properly handle the invalid server name - so don't check the server in this case  
+        if (servers[segment1] === undefined && !((lastSegmentStr.startsWith("login") && req.method === 'GET'))) {
+          //Not in the array of servers
+          noServerfound = true;
+        } else {
+          // remove the server name and pass through
+          req.url = "/" + segmentArray.slice(2).join("/");
+          // we have a valid server name 
+          req.query.serverName = segment1;
+        } 
       }
-      req.query.serverName = segment1;
     }
   }
-  // Disabling logging as CodeQL does not like user supplied values being logged.
-  // console.log("after " + req.url);
-  next();
 
-}
+  if (noServerfound) {
+    // Disabling logging as CodeQL does not like user supplied values being logged.
+    // Invalid tenant - this is forbidden
+    res.status(403).send("Error, forbidden URL. Please supply a valid Server Name in the URL.");
+  } else {
+    // Disabling logging as CodeQL does not like user supplied values being logged.
+    // console.log("after " + req.url);
+    next();
+  }
+};
 
-module.exports = serverNameMiddleWare;
+module.exports = serverNameMiddleWare;

cra-server/index.js

@@ -36,6 +36,7 @@ app.set('servers', servers);
 if (env === 'production') {
   app.use(express.static(path.join(__dirname, '../cra-client/build')));
 }
+
 // This middleware method takes off the first segment which is the serverName and puts it into a query parameter
 app.use((req, res, next) => serverNameMiddleWare(req, res, next));
 

cra-server/router/routes.js

@@ -98,7 +98,7 @@ const joinedPath = path.join(
  */
 router.get("/login", loginLimiter, (req, res) => {
   // Disabling logging as CodeQL does not like user supplied values being logged.
-  //  console.log("/login called " + joinedPath);
+  console.log("/login called " + joinedPath);
   res.sendFile(joinedPath);
 });