mike-jumper commented on a change in pull request #228:
URL: https://github.com/apache/guacamole-server/pull/228#discussion_r446575629
##########
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:
> So, should the `guac_protocol_send_required()` function be re-worked
to take `guac_user` instead of a `guac_socket`?
No, I think the `guac_protocol_send_*()` family of functions should remain
unaware of the concerns of `guac_user`, `guac_client`, etc. They are intended
as low-level functions for sending instructions over a `guac_socket`.
> Or should I implement some intermediate function in `user.c` that uses
this `guac_protocol_send_required()` function and just passes through the
`guac_user->socket`?
This would be the way, yes.
As this would be a common need, if you're looking to avoid having to do this
everywhere, I think it could make sense to provide a convenience function like
`guac_client_owner_send_required()` or similar.
----------------------------------------------------------------
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:
[email protected]
