[GitHub] [guacamole-server] necouchman commented on a change in pull request #228: GUACAMOLE-221: Implement guacd support for prompting

[GitBox <gi...@apache.org>]

necouchman commented on a change in pull request #228:
URL: https://github.com/apache/guacamole-server/pull/228#discussion_r446568856



##########
File path: src/protocols/rdp/rdp.c
##########
@@ -227,10 +227,53 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
 
     rdpContext* context = instance->context;
     guac_client* client = ((rdp_freerdp_context*) context)->client;
-
-    /* Warn if connection is likely to fail due to lack of credentials */
-    guac_client_log(client, GUAC_LOG_INFO,
-            "Authentication requested but username or password not given");
+    guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+    guac_rdp_settings* settings = rdp_client->settings;
+    char* params[4] = {};
+    int i = 0;
+    
+    if (settings->username == NULL || strcmp(settings->username, "") == 0) {
+        params[i] = "username";
+        rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_USERNAME;
+        i++;
+    }
+    
+    if (settings->password == NULL || strcmp(settings->password, "") == 0) {
+        params[i] = "password";
+        rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_PASSWORD;
+        i++;
+    }
+    
+    if (settings->domain == NULL || strcmp(settings->domain, "") == 0) {
+        params[i] = "domain";
+        rdp_client->rdp_credential_flags |= GUAC_RDP_CRED_FLAG_DOMAIN;
+        i++;
+    }
+    
+    /* NULL-terminate the array. */
+    params[i] = NULL;
+    
+    if (i > 0) {
+        /* Lock the client thread. */
+        pthread_mutex_lock(&(rdp_client->rdp_credential_lock));
+        
+        /* Send require params and flush socket. */
+        guac_protocol_send_required(client->socket, (const char**) params);

Review comment:
       Yeah, that makes sense.  However, question on the implementation of that related to the above comment about the `guac_socket_require_keep_alive()` call...
   * Presumably this is done using the `guac_client_for_owner()` function, passing the `client` object, `guac_protocol_send_required` as the callback, and then `(const char**) params` as the data?
   * Should the call to `guac_socket_require_keep_alive()` be limited to the owner, or is it safe to call that on the `client->socket` object, here?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org