necouchman commented on a change in pull request #308:
URL: https://github.com/apache/guacamole-server/pull/308#discussion_r516244910
##########
File path: src/protocols/rdp/error.c
##########
@@ -18,14 +18,90 @@
*/
#include "error.h"
-#include "rdp.h"
#include <freerdp/freerdp.h>
#include <guacamole/client.h>
#include <guacamole/protocol.h>
#include <guacamole/socket.h>
+#include <winpr/wtypes.h>
-void guac_rdp_client_abort(guac_client* client) {
+/**
+ * Translates the error code returned by freerdp_get_last_error() for the given
+ * RDP instance into a Guacamole status code and human-readable message. If no
+ * error was reported, a successful error code and message will be assigned.
+ *
+ * @param rdp_inst
+ * The FreeRDP client instance handling the RDP connection that failed.
+ *
+ * @param status
+ * Pointer to the variable that should receive the guac_protocol_status
+ * value equivalent to the error returned by freerdp_get_last_error().
+ *
+ * @param message
+ * Pointer to the variable that should receive a static human-readable
+ * message generally describing the error returned by
+ * freerdp_get_last_error().
+ */
+static void guac_rdp_translate_last_error(freerdp* rdp_inst,
+ guac_protocol_status* status, const char** message) {
+
+ UINT32 last_error = freerdp_get_last_error(rdp_inst->context);
+ switch (last_error) {
+
+ /* Normal disconnect */
+ case FREERDP_ERROR_NONE:
+ case FREERDP_ERROR_SUCCESS:
+ *status = GUAC_PROTOCOL_STATUS_SUCCESS;
+ *message = "Disconnected.";
+ break;
+
+ /* Authentication failure */
+ case FREERDP_ERROR_AUTHENTICATION_FAILED:
+ case FREERDP_ERROR_CONNECT_ACCESS_DENIED:
+ case FREERDP_ERROR_CONNECT_ACCOUNT_DISABLED:
+ case FREERDP_ERROR_CONNECT_ACCOUNT_EXPIRED:
+ case FREERDP_ERROR_CONNECT_ACCOUNT_LOCKED_OUT:
+ case FREERDP_ERROR_CONNECT_ACCOUNT_RESTRICTION:
+ case FREERDP_ERROR_CONNECT_CLIENT_REVOKED:
+ case FREERDP_ERROR_CONNECT_LOGON_FAILURE:
+ case FREERDP_ERROR_CONNECT_LOGON_TYPE_NOT_GRANTED:
+ case FREERDP_ERROR_CONNECT_NO_OR_MISSING_CREDENTIALS:
+ case FREERDP_ERROR_CONNECT_PASSWORD_CERTAINLY_EXPIRED:
+ case FREERDP_ERROR_CONNECT_PASSWORD_EXPIRED:
+ case FREERDP_ERROR_CONNECT_PASSWORD_MUST_CHANGE:
+ case FREERDP_ERROR_CONNECT_WRONG_PASSWORD:
+ case FREERDP_ERROR_INSUFFICIENT_PRIVILEGES:
+ case FREERDP_ERROR_SECURITY_NEGO_CONNECT_FAILED:
+ case FREERDP_ERROR_SERVER_DENIED_CONNECTION:
+ case FREERDP_ERROR_SERVER_INSUFFICIENT_PRIVILEGES:
+ case FREERDP_ERROR_SERVER_FRESH_CREDENTIALS_REQUIRED:
+ *status = GUAC_PROTOCOL_STATUS_CLIENT_UNAUTHORIZED;
+ *message = "Authentication failure.";
+ break;
Review comment:
I guess my only question, here, is if it's worth exposing something more
useful to the user about the nature of the failure through Guacamole rather
than bundling all of the credential failures into a single status message? I'm
thinking of the kind of errors that users would receive if they attempted to
connect to a RDP server with something like Microsoft Terminal Services Client
(mstsc) versus Guacamole - I think mstsc exposes more specific information
about these failures to the users - e.g. if your account is locked out, your
told that your account is locked out.
##########
File path: src/protocols/rdp/error.c
##########
@@ -18,14 +18,90 @@
*/
#include "error.h"
-#include "rdp.h"
#include <freerdp/freerdp.h>
#include <guacamole/client.h>
#include <guacamole/protocol.h>
#include <guacamole/socket.h>
+#include <winpr/wtypes.h>
-void guac_rdp_client_abort(guac_client* client) {
+/**
+ * Translates the error code returned by freerdp_get_last_error() for the given
+ * RDP instance into a Guacamole status code and human-readable message. If no
+ * error was reported, a successful error code and message will be assigned.
+ *
+ * @param rdp_inst
+ * The FreeRDP client instance handling the RDP connection that failed.
+ *
+ * @param status
+ * Pointer to the variable that should receive the guac_protocol_status
+ * value equivalent to the error returned by freerdp_get_last_error().
+ *
+ * @param message
+ * Pointer to the variable that should receive a static human-readable
+ * message generally describing the error returned by
+ * freerdp_get_last_error().
+ */
+static void guac_rdp_translate_last_error(freerdp* rdp_inst,
+ guac_protocol_status* status, const char** message) {
+
+ UINT32 last_error = freerdp_get_last_error(rdp_inst->context);
+ switch (last_error) {
+
+ /* Normal disconnect */
+ case FREERDP_ERROR_NONE:
+ case FREERDP_ERROR_SUCCESS:
+ *status = GUAC_PROTOCOL_STATUS_SUCCESS;
+ *message = "Disconnected.";
+ break;
+
+ /* Authentication failure */
+ case FREERDP_ERROR_AUTHENTICATION_FAILED:
+ case FREERDP_ERROR_CONNECT_ACCESS_DENIED:
+ case FREERDP_ERROR_CONNECT_ACCOUNT_DISABLED:
+ case FREERDP_ERROR_CONNECT_ACCOUNT_EXPIRED:
+ case FREERDP_ERROR_CONNECT_ACCOUNT_LOCKED_OUT:
+ case FREERDP_ERROR_CONNECT_ACCOUNT_RESTRICTION:
+ case FREERDP_ERROR_CONNECT_CLIENT_REVOKED:
+ case FREERDP_ERROR_CONNECT_LOGON_FAILURE:
+ case FREERDP_ERROR_CONNECT_LOGON_TYPE_NOT_GRANTED:
+ case FREERDP_ERROR_CONNECT_NO_OR_MISSING_CREDENTIALS:
+ case FREERDP_ERROR_CONNECT_PASSWORD_CERTAINLY_EXPIRED:
+ case FREERDP_ERROR_CONNECT_PASSWORD_EXPIRED:
+ case FREERDP_ERROR_CONNECT_PASSWORD_MUST_CHANGE:
+ case FREERDP_ERROR_CONNECT_WRONG_PASSWORD:
+ case FREERDP_ERROR_INSUFFICIENT_PRIVILEGES:
+ case FREERDP_ERROR_SECURITY_NEGO_CONNECT_FAILED:
+ case FREERDP_ERROR_SERVER_DENIED_CONNECTION:
+ case FREERDP_ERROR_SERVER_INSUFFICIENT_PRIVILEGES:
+ case FREERDP_ERROR_SERVER_FRESH_CREDENTIALS_REQUIRED:
+ *status = GUAC_PROTOCOL_STATUS_CLIENT_UNAUTHORIZED;
+ *message = "Authentication failure.";
+ break;
Review comment:
Okay - I'm fine to leave this off of here, now, and come back later and
fix it up, later, if you think it would be better scoped elsewhere.
----------------------------------------------------------------
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]
