SQOOP-941: Sqoop2: Do not send sensitive values from server to client

(Abraham Elmahrek via Jarek Jarcec Cecho)

client/src/main/java/org/apache/sqoop/client/request/ConnectionRequest.java

@@ -49,7 +49,9 @@ public ConnectionBean read(String serverUrl, Long xid) {
   public ValidationBean create(String serverUrl, MConnection connection) {
 
     ConnectionBean connectionBean = new ConnectionBean(connection);
-    JSONObject connectionJson = connectionBean.extract();
+
+    // Extract all form inputs including sensitive inputs
+    JSONObject connectionJson = connectionBean.extract(false);
 
     String response = super.post(serverUrl + RESOURCE,
                                  connectionJson.toJSONString());
@@ -63,7 +65,9 @@ public ValidationBean create(String serverUrl, MConnection connection) {
   public ValidationBean update(String serverUrl, MConnection connection) {
 
     ConnectionBean connectionBean = new ConnectionBean(connection);
-    JSONObject connectionJson = connectionBean.extract();
+
+    // Extract all form inputs including sensitive inputs
+    JSONObject connectionJson = connectionBean.extract(false);
 
     String response = super.put(serverUrl + RESOURCE
                                   + connection.getPersistenceId(),

client/src/main/java/org/apache/sqoop/client/request/JobRequest.java

@@ -49,7 +49,9 @@ public JobBean read(String serverUrl, Long xid) {
   public ValidationBean create(String serverUrl, MJob job) {
 
     JobBean jobBean = new JobBean(job);
-    JSONObject jobJson = jobBean.extract();
+
+    // Extract all form inputs including sensitive inputs
+    JSONObject jobJson = jobBean.extract(false);
 
     String response = super.post(serverUrl + RESOURCE,
       jobJson.toJSONString());
@@ -63,7 +65,9 @@ public ValidationBean create(String serverUrl, MJob job) {
   public ValidationBean update(String serverUrl, MJob job) {
 
     JobBean jobBean = new JobBean(job);
-    JSONObject jobJson = jobBean.extract();
+
+    // Extract all form inputs including sensitive inputs
+    JSONObject jobJson = jobBean.extract(false);
 
     String response = super.put(serverUrl + RESOURCE + job.getPersistenceId(),
                                 jobJson.toJSONString());

common/src/main/java/org/apache/sqoop/json/ConnectionBean.java

@@ -96,7 +96,7 @@ public ResourceBundle getFrameworkBundle() {
 
   @Override
   @SuppressWarnings("unchecked")
-  public JSONObject extract() {
+  public JSONObject extract(boolean skipSensitive) {
     JSONArray array = new JSONArray();
 
     for(MConnection connection : connections) {
@@ -108,9 +108,9 @@ public JSONObject extract() {
       object.put(UPDATED, connection.getLastUpdateDate().getTime());
       object.put(CONNECTOR_ID, connection.getConnectorId());
       object.put(CONNECTOR_PART,
-        extractForms(connection.getConnectorPart().getForms()));
+        extractForms(connection.getConnectorPart().getForms(), skipSensitive));
       object.put(FRAMEWORK_PART,
-        extractForms(connection.getFrameworkPart().getForms()));
+        extractForms(connection.getFrameworkPart().getForms(), skipSensitive));
 
       array.add(object);
     }

common/src/main/java/org/apache/sqoop/json/ConnectorBean.java

@@ -62,7 +62,7 @@ public Map<Long, ResourceBundle> getResourceBundles() {
 
   @SuppressWarnings("unchecked")
   @Override
-  public JSONObject extract() {
+  public JSONObject extract(boolean skipSensitive) {
 
     JSONArray array = new JSONArray();
 
@@ -73,11 +73,11 @@ public JSONObject extract() {
       object.put(NAME, connector.getUniqueName());
       object.put(CLASS, connector.getClassName());
       object.put(VERSION, connector.getVersion());
-      object.put(CON_FORMS, extractForms(connector.getConnectionForms().getForms()));
+      object.put(CON_FORMS, extractForms(connector.getConnectionForms().getForms(), skipSensitive));
 
       JSONObject jobForms = new JSONObject();
       for (MJobForms job : connector.getAllJobsForms().values()) {
-        jobForms.put(job.getType().name(), extractForms(job.getForms()));
+        jobForms.put(job.getType().name(), extractForms(job.getForms(), skipSensitive));
       }
       object.put(JOB_FORMS, jobForms);
 

common/src/main/java/org/apache/sqoop/json/FrameworkBean.java

@@ -64,13 +64,13 @@ public ResourceBundle getResourceBundle() {
 
   @SuppressWarnings("unchecked")
   @Override
-  public JSONObject extract() {
+  public JSONObject extract(boolean skipSensitive) {
     JSONArray conForms =
-      extractForms(framework.getConnectionForms().getForms());
+      extractForms(framework.getConnectionForms().getForms(), skipSensitive);
     JSONObject jobForms = new JSONObject();
 
     for (MJobForms job : framework.getAllJobsForms().values()) {
-      jobForms.put(job.getType().name(), extractForms(job.getForms()));
+      jobForms.put(job.getType().name(), extractForms(job.getForms(), skipSensitive));
     }
 
     JSONObject result = new JSONObject();

common/src/main/java/org/apache/sqoop/json/JobBean.java

@@ -98,7 +98,7 @@ public ResourceBundle getFrameworkBundle() {
 
   @Override
   @SuppressWarnings("unchecked")
-  public JSONObject extract() {
+  public JSONObject extract(boolean skipSensitive) {
     JSONArray array = new JSONArray();
 
     for(MJob job : jobs) {
@@ -112,9 +112,9 @@ public JSONObject extract() {
       object.put(CONNECTION_ID, job.getConnectionId());
       object.put(CONNECTOR_ID, job.getConnectorId());
       object.put(CONNECTOR_PART,
-        extractForms(job.getConnectorPart().getForms()));
+        extractForms(job.getConnectorPart().getForms(), skipSensitive));
       object.put(FRAMEWORK_PART,
-        extractForms(job.getFrameworkPart().getForms()));
+        extractForms(job.getFrameworkPart().getForms(), skipSensitive));
 
       array.add(object);
     }

common/src/main/java/org/apache/sqoop/json/JsonBean.java

@@ -21,14 +21,14 @@
 
 public interface JsonBean {
 
-  JSONObject extract();
+  JSONObject extract(boolean skipSensitive);
 
   void restore(JSONObject jsonObject);
 
   public static final JsonBean EMPTY_BEAN = new JsonBean() {
 
     @Override
-    public JSONObject extract() {
+    public JSONObject extract(boolean skipSensitive) {
       return new JSONObject();
     }
 

common/src/main/java/org/apache/sqoop/json/SubmissionBean.java

@@ -61,7 +61,7 @@ public SubmissionBean() {
 
   @Override
   @SuppressWarnings("unchecked")
-  public JSONObject extract() {
+  public JSONObject extract(boolean skipSensitive) {
     JSONObject ret = new JSONObject();
 
     ret.put(JOB, submission.getJobId());

common/src/main/java/org/apache/sqoop/json/ThrowableBean.java

@@ -54,7 +54,7 @@ public Throwable getThrowable() {
 
   @Override
   @SuppressWarnings("unchecked")
-  public JSONObject extract() {
+  public JSONObject extract(boolean skipSensitive) {
     JSONObject result = new JSONObject();
 
     result.put(MESSAGE, throwable.getMessage());
@@ -77,7 +77,7 @@ public JSONObject extract() {
     Throwable cause = throwable.getCause();
     if(cause != null) {
       ThrowableBean causeBean = new ThrowableBean(cause);
-      result.put(CAUSE, causeBean.extract());
+      result.put(CAUSE, causeBean.extract(skipSensitive));
     }
 
     return result;

common/src/main/java/org/apache/sqoop/json/ValidationBean.java

@@ -73,7 +73,7 @@ public Long getId() {
   }
 
   @SuppressWarnings("unchecked")
-  public JSONObject extract() {
+  public JSONObject extract(boolean skipSensitive) {
     JSONObject object = new JSONObject();
 
     // Optionally transfer id

common/src/main/java/org/apache/sqoop/json/VersionBean.java

@@ -54,7 +54,7 @@ public VersionBean() {
 
   @SuppressWarnings("unchecked")
   @Override
-  public JSONObject extract() {
+  public JSONObject extract(boolean skipSensitive) {
     JSONObject result = new JSONObject();
     result.put(VERSION, version);
     result.put(REVISION, revision);

common/src/main/java/org/apache/sqoop/json/util/FormSerialization.java

@@ -64,11 +64,11 @@ public final class FormSerialization {
    * @return JSON object with serialized form of the list.
    */
   @SuppressWarnings("unchecked")
-  public static JSONArray extractForms(List<MForm> mForms) {
+  public static JSONArray extractForms(List<MForm> mForms, boolean skipSensitive) {
     JSONArray forms = new JSONArray();
 
     for (MForm mForm : mForms) {
-      forms.add(extractForm(mForm));
+      forms.add(extractForm(mForm, skipSensitive));
     }
 
     return forms;
@@ -78,10 +78,11 @@ public static JSONArray extractForms(List<MForm> mForms) {
    * Transform given form to JSON Object.
    *
    * @param mForm Given MForm instance
+   * @param skipSensitive conditionally add sensitive input values
    * @return Serialized JSON object.
    */
   @SuppressWarnings("unchecked")
-  public static JSONObject extractForm(MForm mForm) {
+  public static JSONObject extractForm(MForm mForm, boolean skipSensitive) {
     JSONObject form = new JSONObject();
     form.put(ID, mForm.getPersistenceId());
     form.put(FORM_NAME, mForm.getName());
@@ -91,7 +92,6 @@ public static JSONObject extractForm(MForm mForm) {
 
     for (MInput<?> mInput : mForm.getInputs()) {
       JSONObject input = new JSONObject();
-      mInputs.add(input);
       input.put(ID, mInput.getPersistenceId());
       input.put(FORM_INPUT_NAME, mInput.getName());
       input.put(FORM_INPUT_TYPE, mInput.getType().toString());
@@ -111,9 +111,12 @@ public static JSONObject extractForm(MForm mForm) {
       }
 
       // Serialize value if is there
-      if(!mInput.isEmpty()) {
+      // Skip if sensitive
+      if (!mInput.isEmpty() && !(skipSensitive && ((MStringInput)mInput).isMasked())) {
         input.put(FORM_INPUT_VALUE, mInput.getUrlSafeValueString());
       }
+
+      mInputs.add(input);
     }
 
     return form;

common/src/test/java/org/apache/sqoop/json/TestConnectionBean.java

@@ -20,6 +20,7 @@
 import org.apache.sqoop.model.MConnection;
 import org.apache.sqoop.model.MStringInput;
 import org.json.simple.JSONObject;
+import org.json.simple.JSONArray;
 import org.json.simple.JSONValue;
 import org.junit.Test;
 
@@ -49,7 +50,7 @@ public void testSerialization() {
 
     // Serialize it to JSON object
     ConnectionBean bean = new ConnectionBean(connection);
-    JSONObject json = bean.extract();
+    JSONObject json = bean.extract(false);
 
     // "Move" it across network in text form
     String string = json.toJSONString();
@@ -71,4 +72,47 @@ public void testSerialization() {
       .getForms().get(0).getInputs().get(0);
     assertEquals("Hi there!", targetInput.getValue());
   }
+
+  @Test
+  public void testSensitivityFilter() {
+    Date created = new Date();
+    Date updated = new Date();
+    MConnection connection = getConnection("ahoj");
+    connection.setName("Connection");
+    connection.setPersistenceId(666);
+    connection.setCreationDate(created);
+    connection.setLastUpdateDate(updated);
+
+    // Fill some data at the beginning
+    MStringInput input = (MStringInput) connection.getConnectorPart().getForms()
+      .get(0).getInputs().get(0);
+    input.setValue("Hi there!");
+
+    // Serialize it to JSON object
+    ConnectionBean bean = new ConnectionBean(connection);
+    JSONObject json = bean.extract(false);
+    JSONObject jsonFiltered = bean.extract(true);
+
+    // Sensitive values should exist
+    JSONArray all = (JSONArray)json.get("all");
+    JSONObject allItem = (JSONObject)all.get(0);
+    JSONArray connectors = (JSONArray)allItem.get("connector");
+    JSONObject connector = (JSONObject)connectors.get(0);
+    JSONArray inputs = (JSONArray)connector.get("inputs");
+    assertEquals(3, inputs.size());
+    // Inputs are ordered when creating connection
+    JSONObject password = (JSONObject)inputs.get(2);
+    assertTrue(password.containsKey("value"));
+
+    // Sensitive values should not exist
+    all = (JSONArray)jsonFiltered.get("all");
+    allItem = (JSONObject)all.get(0);
+    connectors = (JSONArray)allItem.get("connector");
+    connector = (JSONObject)connectors.get(0);
+    inputs = (JSONArray)connector.get("inputs");
+    assertEquals(3, inputs.size());
+    // Inputs are ordered when creating connection
+    password = (JSONObject)inputs.get(2);
+    assertFalse(password.containsKey("value"));
+  }
 }

common/src/test/java/org/apache/sqoop/json/TestConnectorBean.java

@@ -55,7 +55,7 @@ public void testSerialization() {
 
     // Serialize it to JSON object
     ConnectorBean bean = new ConnectorBean(connectors, bundles);
-    JSONObject json = bean.extract();
+    JSONObject json = bean.extract(false);
 
     // "Move" it across network in text form
     String string = json.toJSONString();

common/src/test/java/org/apache/sqoop/json/TestFrameworkBean.java

@@ -44,7 +44,7 @@ public void testSerialization() {
 
     // Serialize it to JSON object
     FrameworkBean bean = new FrameworkBean(framework, getResourceBundle());
-    JSONObject json = bean.extract();
+    JSONObject json = bean.extract(false);
 
     // "Move" it across network in text form
     String string = json.toJSONString();

common/src/test/java/org/apache/sqoop/json/TestJobBean.java

@@ -50,7 +50,7 @@ public void testSerialization() throws ParseException {
 
     // Serialize it to JSON object
     JobBean bean = new JobBean(job);
-    JSONObject json = bean.extract();
+    JSONObject json = bean.extract(false);
 
     // "Move" it across network in text form
     String string = json.toJSONString();

common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java

@@ -158,7 +158,7 @@ public void testTransferCounters() {
    */
   private MSubmission transfer(MSubmission submission) {
     SubmissionBean bean = new SubmissionBean(submission);
-    JSONObject json = bean.extract();
+    JSONObject json = bean.extract(false);
 
     String string = json.toString();
 

common/src/test/java/org/apache/sqoop/json/TestThrowableBean.java

@@ -31,7 +31,7 @@ public void testSerialization() {
 
     // Serialize it to JSON object
     ThrowableBean bean = new ThrowableBean(ex);
-    JSONObject json = bean.extract();
+    JSONObject json = bean.extract(false);
 
     // "Move" it across network in text form
     String string = json.toJSONString();

common/src/test/java/org/apache/sqoop/json/TestUtil.java

@@ -75,10 +75,12 @@ public static MConnectionForms getConnectionForms() {
 
     input = new MStringInput("username", false, (short) 10);
     input.setPersistenceId(2);
+    input.setValue("test");
     inputs.add(input);
 
-    input = new MStringInput("password", false, (short) 10);
+    input = new MStringInput("password", true, (short) 10);
     input.setPersistenceId(3);
+    input.setValue("test");
     inputs.add(input);
 
     form = new MForm("connection", inputs);

common/src/test/java/org/apache/sqoop/json/TestValidationBean.java

@@ -40,7 +40,7 @@ public void testSerialization() {
       getValidation(Status.FINE),
       getValidation(Status.UNACCEPTABLE)
     );
-    JSONObject json = bean.extract();
+    JSONObject json = bean.extract(false);
 
     // "Move" it across network in text form
     String string = json.toJSONString();
@@ -78,7 +78,7 @@ public void testId() {
       getValidation(Status.FINE)
     );
     bean.setId((long) 10);
-    JSONObject json = bean.extract();
+    JSONObject json = bean.extract(false);
 
     // "Move" it across network in text form
     String string = json.toJSONString();

server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java

@@ -108,7 +108,7 @@ private void sendSuccessResponse(RequestContext ctx, JsonBean bean)
     response.setStatus(HttpServletResponse.SC_OK);
     setContentType(response);
     setHeaders(response, SqoopResponseCode.SQOOP_1000);
-    String responseString = bean.extract().toJSONString();
+    String responseString = bean.extract(true).toJSONString();
     response.getWriter().write(responseString);
     response.getWriter().flush();
   }
@@ -139,7 +139,7 @@ private void sendErrorResponse(RequestContext ctx, Exception ex)
       ThrowableBean throwableBean = new ThrowableBean(ex);
 
       response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
-      response.getWriter().write(throwableBean.extract().toJSONString());
+      response.getWriter().write(throwableBean.extract(true).toJSONString());
     } else {
       response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
     }