Issue #1656: improve the registration of txt4 controller

- if the SAME robot issue a registration request
- with the same token
- that has been approved in the past
- then accept this token as approved

OpenRobertaRobot/src/main/java/de/fhg/iais/roberta/robotCommunication/RobotCommunicator.java

@@ -14,6 +14,7 @@
 import de.fhg.iais.roberta.robotCommunication.RobotCommunicationData.State;
 import de.fhg.iais.roberta.util.Key;
 import de.fhg.iais.roberta.util.dbc.Assert;
+import de.fhg.iais.roberta.util.dbc.DbcException;
 
 /**
  * class, that synchronizes the communication between the bricks and the web-app. Thread-safe. See class {@link RobotCommunicationData} for further
@@ -37,11 +38,11 @@ public RobotCommunicator() {
      *
      * @return true if the ticket has been accepted
      */
-    public boolean addNewRegistration(RobotCommunicationData newRobotCommunicationData) {
-        String token = newRobotCommunicationData.getToken();
+    RegistrationRequest addNewRegistration(RobotCommunicationData newRobotCommunicationData) {
+        String newToken = newRobotCommunicationData.getToken();
         String newIdentificator = newRobotCommunicationData.getRobotIdentificator();
-        Assert.isTrue(token != null && newIdentificator != null);
-        RobotCommunicationData existingRobotCommunicationData = this.allStates.get(token);
+        Assert.isTrue(newToken != null && newIdentificator != null);
+        RobotCommunicationData existingRobotCommunicationData = this.allStates.get(newToken);
         if ( existingRobotCommunicationData != null ) {
             String existingIdentificator = existingRobotCommunicationData.getRobotIdentificator();
             if ( existingIdentificator == null
@@ -50,34 +51,45 @@ public boolean addNewRegistration(RobotCommunicationData newRobotCommunicationDa
                 || existingIdentificator.equals("unknown")
             ) {
                 LOG.info("ROBOT_RC: token already used. New token required");
-                return false;
+                return RegistrationRequest.TOKEN_INVALID;
             }
         }
         for ( String storedToken : this.allStates.keySet() ) {
             RobotCommunicationData storedState = this.allStates.get(storedToken);
             if ( newIdentificator.equals(storedState.getRobotIdentificator())
                 && !newIdentificator.equals("usb")
-                && !newIdentificator.equals("unknown") ) {
-                LOG.info("ROBOT_RC: token approval request for robot [" + newIdentificator + "], but an old request is pending. Start abort old request");
-                this.allStates.remove(storedToken);
-                storedState.abort(); // notifyAll() executed
-                LOG.info("ROBOT_RC: token approval request for robot [" + newIdentificator + "], but an old request is pending. End abort old request.");
+                && !newIdentificator.equals("unknown") ) //
+            {
+                if ( storedState.getToken().equals(newToken) ) {
+                    // TODO: keep? Experimental for Fischertechnik. A register for a token that was the same as a past registration. Keep running!
+                    LOG.info("ROBOT_RC: token approved for robot [" + newIdentificator + "]. Token  " + newToken + " unchanged");
+                    return RegistrationRequest.REPEATED_REGISTRATION_REQUEST;
+                } else {
+                    this.allStates.remove(storedToken);
+                    storedState.abort(); // notifyAll() executed
+                    LOG.info("ROBOT_RC: token approval request for robot [" + newIdentificator + "]. An old request is pending and aborted.");
+                }
             }
         }
-        this.allStates.put(token, newRobotCommunicationData);
-        return true;
+        this.allStates.put(newToken, newRobotCommunicationData);
+        return RegistrationRequest.NEW_REGISTRATION_REQUEST;
     }
 
     public boolean robotWantsTokenToBeApproved(RobotCommunicationData newRobotCommunicationData) {
-        if ( addNewRegistration(newRobotCommunicationData) ) {
-            return newRobotCommunicationData.robotTokenAgreementRequest(); // this will freeze the request until another thread issues a notifyAll()
-        } else {
-            try {
-                Thread.sleep(1000); // to avoid a DOS attack (either by a bad implementation of the robot-server protocol or by an attacker), we sleep
-            } catch ( InterruptedException e ) {
-                // ignore the interrupt
-            }
-            return false;
+        switch ( addNewRegistration(newRobotCommunicationData) ) {
+            case TOKEN_INVALID:
+                try {
+                    Thread.sleep(1000); // to avoid server congestion if a robot uses a bad implementation of the robot-server protocol
+                } catch ( InterruptedException e ) {
+                    // ignore the interrupt
+                }
+                return false;
+            case NEW_REGISTRATION_REQUEST:
+                return newRobotCommunicationData.robotTokenAgreementRequest();  // this will freeze the request until another thread issues a notifyAll()
+            case REPEATED_REGISTRATION_REQUEST:
+                return true; // was already approved. Second approvement would irritate the user
+            default:
+                throw new DbcException("invalid registration request");
         }
     }
 
@@ -286,4 +298,10 @@ public boolean stop(String token, String robotName) {
         }
         return false;
     }
+
+    public enum RegistrationRequest {
+        TOKEN_INVALID,
+        NEW_REGISTRATION_REQUEST,
+        REPEATED_REGISTRATION_REQUEST;
+    }
 }

OpenRobertaRobot/src/test/java/de/fhg/iais/roberta/robotCommunication/RobotCommunicatorTest.java

@@ -0,0 +1,42 @@
+package de.fhg.iais.roberta.robotCommunication;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import de.fhg.iais.roberta.util.dbc.DbcException;
+
+public class RobotCommunicatorTest {
+    static RobotCommunicationData badRegistration = new RobotCommunicationData(null, null, null, null, null, null, null, null, null);
+    static RobotCommunicationData goodRegistration1 = new RobotCommunicationData("12345678", null, "00:11:22:33:44:55", null, null, null, null, null, null);
+    static RobotCommunicationData goodRegistration2 = new RobotCommunicationData("12345678", null, "13:05:98:29:12:99", null, null, null, null, null, null);
+
+    @Test
+    public void testFirstCanRegister() throws Exception {
+        RobotCommunicator rc = new RobotCommunicator();
+        expect(rc.addNewRegistration(goodRegistration1), RobotCommunicator.RegistrationRequest.NEW_REGISTRATION_REQUEST);
+    }
+
+    @Test(expected = DbcException.class)
+    public void testBadRegistrationIsRejected() throws Exception {
+        RobotCommunicator rc = new RobotCommunicator();
+        rc.addNewRegistration(badRegistration);
+    }
+
+    @Test
+    public void testRegisterTwiceWithSameIp() {
+        RobotCommunicator rc = new RobotCommunicator();
+        rc.addNewRegistration(goodRegistration1);
+        expect(rc.addNewRegistration(goodRegistration1), RobotCommunicator.RegistrationRequest.REPEATED_REGISTRATION_REQUEST);
+    }
+
+    @Test
+    public void testCantRegisterTwice() {
+        RobotCommunicator rc = new RobotCommunicator();
+        rc.addNewRegistration(goodRegistration1);
+        expect(rc.addNewRegistration(goodRegistration2), RobotCommunicator.RegistrationRequest.TOKEN_INVALID);
+    }
+
+    private void expect(RobotCommunicator.RegistrationRequest result, RobotCommunicator.RegistrationRequest expected) {
+        Assert.assertEquals(expected, result);
+    }
+}

OpenRobertaServer/src/main/java/de/fhg/iais/roberta/javaServer/restServices/robot/RobotCommand.java

@@ -61,7 +61,6 @@ public Response handle(JSONObject requestEntity) throws JSONException, Interrupt
         }
 
         String robot = r.getWithDefault("?", "robot");
-        String macaddr = r.getWithDefault("?", "macaddr");
         String robotName = r.getWithDefault("?", "robotname", "brickname");
         String batteryvoltage = r.getWithDefault("?", "battery");
         String menuversion = r.getWithDefault("?", "menuversion");
@@ -72,6 +71,7 @@ public Response handle(JSONObject requestEntity) throws JSONException, Interrupt
         JSONObject response;
         switch (cmd) {
             case CMD_REGISTER:
+                String macaddr = r.get("macaddr");
                 LOG.info("ROBOT_PROTOCOL: robot [" + macaddr + "] send token " + token + " for user approval");
                 RobotCommunicationData state =
                         new RobotCommunicationData(token, robot, macaddr, robotName, batteryvoltage, menuversion, runtimeVersion, pluginName, firmwareversion);
@@ -91,8 +91,7 @@ public Response handle(JSONObject requestEntity) throws JSONException, Interrupt
                     return Response.serverError().build();
                 } else {
                     if (!command.equals(CMD_REPEAT) || logPush) {
-                        LOG.info("ROBOT_PROTOCOL: the command " + command + " is pushed to robot [" + macaddr
-                                + "] for token " + token + " [count: " + counter + "]");
+                        LOG.info("ROBOT_PROTOCOL: the command " + command + " is pushed to robot with token " + token + " [count: " + counter + "]");
                     }
                     response = new JSONObject().put(CMD, command);
                     response.put(SUBTYPE, this.brickCommunicator.getSubtype());