[GitHub] [guacamole-client] chisatohasimoto opened a new pull request #413: Add Japanese language support

[GitBox]

chisatohasimoto opened a new pull request #413: Add Japanese language support
URL: https://github.com/apache/guacamole-client/pull/413
 
 
   Added Japanese language support for guacamole-client.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #201: GUACAMOLE-547: Add support for SSH NONE authentication method

[GitBox]

mike-jumper commented on a change in pull request #201: GUACAMOLE-547: Add 
support for SSH NONE authentication method
URL: https://github.com/apache/guacamole-server/pull/201#discussion_r295077837
 
 

 ##
 File path: src/common-ssh/common-ssh/ssh.h
 ##
 @@ -25,6 +25,20 @@
 #include 
 #include 
 
+/**
+ * Handler for retrieving additional credentials.
+ * 
+ * @param client
+ * The Guacamole Client associated with this need for additional
+ * credentials.
+ * 
+ * @param cred_name
+ * The name of the credential being requested, which will be shared
+ * with the client in order to generate a meaningful prompt.
+ * 
 
 Review comment:
   Looks better, but:
   
   > ... which should be a dynamically-allocated such that it can be freed as 
required.
   
   More than that, the returned value _must_ be dynamically-allocated, as the 
caller will eventually attempt to free it with a call to `free()`. For example:
   
   
https://github.com/apache/guacamole-server/blob/5e2ddb890a90e840a419f4878381b0c511cc6f25/src/terminal/terminal/terminal.h#L664-L670
   
   and
   
   
https://github.com/apache/guacamole-server/blob/6dad6cd91933a19bc18095854e93354b6764d763/src/protocols/rdp/tests/fs/normalize_path.c#L174-L179
   
   While we're here: is there a meaning for returning `NULL`? If not, that's 
fine, and the documentation covers what it needs to cover. Well-behaved 
implementations won't return `NULL` unless it's actually documented as a legal 
return value. However, if an implementation returning `NULL` is intended and 
has a specific meaning, that should be documented here, too.


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:
us...@infra.apache.org


With regards,
Apache Git Services

Re: [apache/guacamole-client] GUACAMOLE-304: Expand connection link to encompass entire hover area. (#253)

[Mark Nolan]

Yes, fair comment!

This business of adding extensions is fraught with difficulty ;)

You're right about using inline-block. My preferred solution would be to
use flexbox, but that probably doesn't have quite high enough browser
support for the project (96%).

I can fix my extension by floating my control panel left. The block-sized
 then takes up the rest of the line. It is far from ideal (don't really
like float), but it works *and* leaves the  with block layout.

Incidentally, if you want the same full width behaviour on the settings
page, you could use that approach for the expander. But, again, I think
flexbox would be a more robust solution.

M.
.



On Mon, 17 Jun 2019 at 20:14, Mike Jumper  wrote:

> @manolan1  - this is a pull request which is
> closed, having been merged about a year ago, for an issue which has been
> resolved and released. If you have questions on these changes, please use
> the mailing list.
>
> ... wouldn't it have been better to change it to inline-block?
>
> No. The issue that this PR addressed was the size of the link not actually
> matching the highlighted region, leading to confusion:
>
> https://issues.apache.org/jira/browse/GUACAMOLE-304
>
> inline-block would not have produced the same behavior as block in that
> respect.
>
> I ask because it broke one of my extensions ...
>
> If you would like to propose a change, feel free to open a new JIRA issue
> and pull request with those changes. If there is a better way to achieve
> this which also has a lower potential of breaking third-party styling, I
> see no reason why that would not be merged.
>
> Be sure to describe what specifically broke.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> ,
> or mute the thread
> 
> .
>

[GitHub] [guacamole-server] necouchman commented on a change in pull request #201: GUACAMOLE-547: Add support for SSH NONE authentication method

[GitBox]

necouchman commented on a change in pull request #201: GUACAMOLE-547: Add 
support for SSH NONE authentication method
URL: https://github.com/apache/guacamole-server/pull/201#discussion_r295047012
 
 

 ##
 File path: src/common-ssh/common-ssh/ssh.h
 ##
 @@ -25,6 +25,20 @@
 #include 
 #include 
 
+/**
+ * Handler for retrieving additional credentials.
+ * 
+ * @param client
+ * The Guacamole Client associated with this need for additional
+ * credentials.
+ * 
+ * @param cred_name
+ * The name of the credential being requested, which will be shared
+ * with the client in order to generate a meaningful prompt.
+ * 
 
 Review comment:
   Added that documentation.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #201: GUACAMOLE-547: Add support for SSH NONE authentication method

[GitBox]

mike-jumper commented on a change in pull request #201: GUACAMOLE-547: Add 
support for SSH NONE authentication method
URL: https://github.com/apache/guacamole-server/pull/201#discussion_r294979656
 
 

 ##
 File path: src/common-ssh/common-ssh/ssh.h
 ##
 @@ -25,6 +25,20 @@
 #include 
 #include 
 
+/**
+ * Handler for retrieving additional credentials.
+ * 
+ * @param client
+ * The Guacamole Client associated with this need for additional
+ * credentials.
+ * 
+ * @param cred_name
+ * The name of the credential being requested, which will be shared
+ * with the client in order to generate a meaningful prompt.
+ * 
 
 Review comment:
   Missing docs for the return value.
   
   If the returned string will eventually be freed with a call to `free()` and 
is thus required to be dynamically allocated, that should be documented as well.


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:
us...@infra.apache.org


With regards,
Apache Git Services

Re: Auth Provider from 0.9.14 does not work with 1.0.0

[Mark Nolan]

Probably don't need Jersey. Will have some time to investigate now. I just
knew that if I took that version, it would be compatible, rather than
having to hunt through and work out which version to use. ;)

M.
.


On Tue, 18 Jun 2019 at 19:57, Mike Jumper  wrote:

> On Tue, Jun 18, 2019 at 10:30 AM Mark Nolan  wrote:
>
> > There were no other errors at all!
> >
> > For a moment I was very excited that I might have forgotten to update
> that
> > dependency, but I hadn't been that stupid (wouldn't have been the first
> > time!). I didn't know whether to be relieved or frustrated!
> >
> > Anyway, I created a minimal version and went through it all bit by bit.
> The
> > problem was that I had another version of the JAX-RS api in the pom:
> >
> > 
> > org.glassfish.jersey.media
> > jersey-media-json-jackson
> > 2.27
> > 
> >
> > Need this to get a much more up to date version of Jackson and,
> > conveniently, it also allowed the JAX-RS annotations to compile.
> >
>
> As it's the Guacamole webapp that will be using the base parts of Jersey,
> this may be causing trouble.
>
>
> >
> > This worked fine in 0.9.14, but not in 1.0.0. Not really sure why that
> > should have changed.
> >
>
> My guess would be that including this previously had no effect, with your
> extension actually using the classes exposed by the webapp. Part of the
> changes for 1.0.0 included giving extensions priority for classes on the
> classpath, allowing extensions to use different versions of classes
> internally even if the webapp uses the same library. That may not work as
> expected for something like Jersey.
>
> See:
> http://guacamole.apache.org/releases/1.0.0/#improvements-to-extension-api
> ("... Dependency
> precedence has been updated so that extensions always see the classes they
> bundle, even if the web application bundles a different version of the same
> class. ...")
>
> Are you sure you need to include Jersey in your dependencies? I've normally
> used the following to make JAX-RS annotations available within an extension
> and let the webapp handle the rest:
>
> 
> javax.ws.rs
> jsr311-api
> 1.1.1
> provided
> 
>
> - Mike
>

Re: AngularJS issue upgrading authentication provider from 0.9.14 to 1.0.0

[Mark Nolan]

Perfect!

I was actually quite happy with the interpolation binding (fixing the
identifier on each line) and had no idea it wasn't allowed!

Many thanks,
M.
.


On Tue, 18 Jun 2019 at 19:45, Mike Jumper  wrote:

> On Tue, Jun 18, 2019 at 11:14 AM Mark Nolan  wrote:
>
> > ...
> > The browser console shows this:
> >
> > angular.js:14800 Error: [$parse:syntax]
> >
> >
> http://errors.angularjs.org/1.6.9/$parse/syntax?p0=%7B=invalid%20key=17=xxxAuthGcpStop(%7B%7Bitem.identifier%7D%7D)=%7Bitem.identifier%7D%7D
> > )
> > at angular.js:88
> > at q.throwError (angular.js:15358)
> > at q.object (angular.js:15347)
> > at q.primary (angular.js:15236)
> > at q.unary (angular.js:15224)
> > at q.multiplicative (angular.js:15211)
> > at q.additive (angular.js:15202)
> > at q.relational (angular.js:15193)
> > at q.equality (angular.js:15184)
> > at q.logicalAND (angular.js:15176) " > height="100%" viewBox="0 0 16 24" fit="" focusable="false" xmlns="
> > http://www.w3.org/2000/svg; preserveAspectRatio="xMidYMid meet"
> > class="xxx-icon xxx-enabled"
> > ng-click="xxxAuthGcpStop({{item.identifier}})">"
> >
>
> Did you mean:
>
> ng-click="xxxAuthGcpStop(item.identifier)"
>
> ?
>
> For AngularJS, depending on the directive, you'll find things like
> some-attribute="{{ angular expression }}" and some-attribute="angular
> expression". The ngClick directive is one of the latter. The double braces
> introduce an angular expression, when necessary, but are not valid syntax
> _within_ an angular expression.
>
> - Mike
>

Re: Auth Provider from 0.9.14 does not work with 1.0.0

[Mike Jumper]

On Tue, Jun 18, 2019 at 10:30 AM Mark Nolan  wrote:

> There were no other errors at all!
>
> For a moment I was very excited that I might have forgotten to update that
> dependency, but I hadn't been that stupid (wouldn't have been the first
> time!). I didn't know whether to be relieved or frustrated!
>
> Anyway, I created a minimal version and went through it all bit by bit. The
> problem was that I had another version of the JAX-RS api in the pom:
>
> 
> org.glassfish.jersey.media
> jersey-media-json-jackson
> 2.27
> 
>
> Need this to get a much more up to date version of Jackson and,
> conveniently, it also allowed the JAX-RS annotations to compile.
>

As it's the Guacamole webapp that will be using the base parts of Jersey,
this may be causing trouble.


>
> This worked fine in 0.9.14, but not in 1.0.0. Not really sure why that
> should have changed.
>

My guess would be that including this previously had no effect, with your
extension actually using the classes exposed by the webapp. Part of the
changes for 1.0.0 included giving extensions priority for classes on the
classpath, allowing extensions to use different versions of classes
internally even if the webapp uses the same library. That may not work as
expected for something like Jersey.

See:
http://guacamole.apache.org/releases/1.0.0/#improvements-to-extension-api
("... Dependency
precedence has been updated so that extensions always see the classes they
bundle, even if the web application bundles a different version of the same
class. ...")

Are you sure you need to include Jersey in your dependencies? I've normally
used the following to make JAX-RS annotations available within an extension
and let the webapp handle the rest:


javax.ws.rs
jsr311-api
1.1.1
provided


- Mike

Re: AngularJS issue upgrading authentication provider from 0.9.14 to 1.0.0

[Mike Jumper]

On Tue, Jun 18, 2019 at 11:14 AM Mark Nolan  wrote:

> ...
> The browser console shows this:
>
> angular.js:14800 Error: [$parse:syntax]
>
> http://errors.angularjs.org/1.6.9/$parse/syntax?p0=%7B=invalid%20key=17=xxxAuthGcpStop(%7B%7Bitem.identifier%7D%7D)=%7Bitem.identifier%7D%7D
> )
> at angular.js:88
> at q.throwError (angular.js:15358)
> at q.object (angular.js:15347)
> at q.primary (angular.js:15236)
> at q.unary (angular.js:15224)
> at q.multiplicative (angular.js:15211)
> at q.additive (angular.js:15202)
> at q.relational (angular.js:15193)
> at q.equality (angular.js:15184)
> at q.logicalAND (angular.js:15176) " height="100%" viewBox="0 0 16 24" fit="" focusable="false" xmlns="
> http://www.w3.org/2000/svg; preserveAspectRatio="xMidYMid meet"
> class="xxx-icon xxx-enabled"
> ng-click="xxxAuthGcpStop({{item.identifier}})">"
>

Did you mean:

ng-click="xxxAuthGcpStop(item.identifier)"

?

For AngularJS, depending on the directive, you'll find things like
some-attribute="{{ angular expression }}" and some-attribute="angular
expression". The ngClick directive is one of the latter. The double braces
introduce an angular expression, when necessary, but are not valid syntax
_within_ an angular expression.

- Mike

AngularJS issue upgrading authentication provider from 0.9.14 to 1.0.0

[Mark Nolan]

I am getting multiple parse errors from AngularJS. I know 1.0.0 upgraded
AngularJS and I'm off to look through the notes now, but if anyone knows
what causes this, you could save me a lot of time!

Everything I know about AngularJS I have learned by looking at what
Guacamole does or through writing this extension. I do have a lot of
experience with Angular, so I am somewhat familiar with the concepts. I
don't really have any decent development tools for AngularJS, however, so
everything is a little opaque. Plus, I think it is fairly difficult to
bootstrap a test environment that looks like Guacamole ;)

The authentication provider adds start and stop buttons and a status
indicator. The service itself links these to GCP to start and stop the VMs.
This panel of controls appears to the left of each connection in the list
of connections. The panel does display, but it isn't doing anything on
click (not even erroring).

The browser console shows this:

angular.js:14800 Error: [$parse:syntax]
http://errors.angularjs.org/1.6.9/$parse/syntax?p0=%7B=invalid%20key=17=xxxAuthGcpStop(%7B%7Bitem.identifier%7D%7D)=%7Bitem.identifier%7D%7D
)
at angular.js:88
at q.throwError (angular.js:15358)
at q.object (angular.js:15347)
at q.primary (angular.js:15236)
at q.unary (angular.js:15224)
at q.multiplicative (angular.js:15211)
at q.additive (angular.js:15202)
at q.relational (angular.js:15193)
at q.equality (angular.js:15184)
at q.logicalAND (angular.js:15176) "http://www.w3.org/2000/svg; preserveAspectRatio="xMidYMid meet"
class="xxx-icon xxx-enabled"
ng-click="xxxAuthGcpStop({{item.identifier}})">"

The actual template is huge, so I am reluctant to include the whole thing
here, but this is an excerpt:

http://www.w3.org/2000/svg; preserveAspectRatio="xMidYMid meet"
class="xxx-icon xxx-enabled" ng-click="xxxAuthGcpStop({{item.identifier}})">

Stop Instance



And it is part of a directive that is used like this:




My initial guess was that 1.0.0 changed the use of item.identifier, as that
is what the error message suggested to me, but they have reasonable values
in the document when I look through developer tools. The next most likely
option is that it cannot find the method that is bound to the ng-click.
Unless I find something else, that is my next avenue of investigation.

Thanks,
Mark.
.

Re: Auth Provider from 0.9.14 does not work with 1.0.0

[Mark Nolan]

There were no other errors at all!

For a moment I was very excited that I might have forgotten to update that
dependency, but I hadn't been that stupid (wouldn't have been the first
time!). I didn't know whether to be relieved or frustrated!

Anyway, I created a minimal version and went through it all bit by bit. The
problem was that I had another version of the JAX-RS api in the pom:


org.glassfish.jersey.media
jersey-media-json-jackson
2.27


Need this to get a much more up to date version of Jackson and,
conveniently, it also allowed the JAX-RS annotations to compile.

This worked fine in 0.9.14, but not in 1.0.0. Not really sure why that
should have changed.

I have changed it to the rather tedious:


com.sun.jersey
jersey-json
1.17.1
provided


javax.xml.stream
stax-api


org.codehaus.jackson
jackson-core-asl


org.codehaus.jackson
jackson-mapper-asl


org.codehaus.jackson
jackson-jaxrs


org.codehaus.jackson
jackson-xc





com.fasterxml.jackson.core
jackson-annotations



com.fasterxml.jackson.core
jackson-databind



com.fasterxml.jackson.module
jackson-module-jaxb-annotations


With the Jackson BOM included to get the more recent version.

It is possible that I don't need all those exclusions because the namespace
appears to have changed. When I resolve all my other issues, I'll take a
look at that.

Thanks,
Mark.
.


On Mon, 17 Jun 2019 at 21:44, Mike Jumper  wrote:

> On Sun, Jun 16, 2019 at 7:02 PM Mark Nolan  wrote:
>
> > I have an authentication provider that was working perfectly with 0.9.14.
> >
> > I have updated it to work with 1.0.0, but have got stuck and need some
> > suggestions.
> >
> > I can see everything loading. I can see my UserContext being returned and
> > my Resource as well. I know they are not null. However, no endpoint
> methods
> > of the resource are ever called and I get the following error:
> >
> > 02:53:32.840 [http-nio-8080-exec-2] ERROR o.a.g.rest.RESTExceptionMapper
> -
> > Unexpected internal error: null for uri:
> >
> >
> http://localhost:8080/guacamole-1.0.0/api/session/ext/XXX/hello?token=0F854D6F2B13FA69E3C43E0FBDEA0759316BD93329DCB152CB53B732413D461C
> > 02:53:32.841 [http-nio-8080-exec-2] DEBUG o.a.g.rest.RESTExceptionMapper
> -
> > Unexpected error in REST endpoint.
> > com.sun.jersey.api.NotFoundException: null for uri:
> >
> >
> http://localhost:8080/guacamole-1.0.0/api/session/ext/XXX/hello?token=0F854D6F2B13FA69E3C43E0FBDEA0759316BD93329DCB152CB53B732413D461C
> > at
> >
> >
> com.sun.jersey.server.impl.application.WebApplicationImpl._handleRequest(WebApplicationImpl.java:1512)
> > [jersey-server-1.17.1.jar:1.17.1]
> > at
> >
> >
> com.sun.jersey.server.impl.application.WebApplicationImpl._handleRequest(WebApplicationImpl.java:1442)
> > [jersey-server-1.17.1.jar:1.17.1]
> > at
> >
> >
> com.sun.jersey.server.impl.application.WebApplicationImpl.handleRequest(WebApplicationImpl.java:1391)
> > [jersey-server-1.17.1.jar:1.17.1]
> >
> > (more available on request!)
> >
> > Where I have replaced the authentication provider name with XXX. The
> hello
> > endpoint is just something I threw in for testing, but (again) it works
> > perfectly with 0.9.14.
> >
>
> Are there any other errors earlier in the logs?
>
> Did you update your extension to use the 1.0.0 version of guacamole-ext?
>
> If someone knows what changed in 1.0.0 that could have caused this, that
> > would be great! Otherwise, what I really need are some ideas of what to
> > look at next.
>
>
> I've not seen that specific error before and am not sure what would cause
> it, however the overall changes to the extension API for 1.0.0 are in the
> release notes:
>
> http://guacamole.apache.org/releases/1.0.0/#extension-api-changes
>
> When updating the Java parts of an extension from release X to release Y,
> you generally should only need to update the guacamole-ext dependency to
> match release Y and fix any build errors that result, referencing the
> release notes and guacamole-ext API docs as you go.
>
> - Mike
>

[GitHub] [guacamole-server] necouchman commented on issue #201: GUACAMOLE-547: Add support for SSH NONE authentication method

[GitBox]

necouchman commented on issue #201: GUACAMOLE-547: Add support for SSH NONE 
authentication method
URL: https://github.com/apache/guacamole-server/pull/201#issuecomment-503072533
 
 
   @mike-jumper Okay, fixed up the stuff you requested - let me know if 
anything else needs work.  Also rebased on top of current `staging/1.1.0` 
branch.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #201: GUACAMOLE-547: Add support for SSH NONE authentication method

[GitBox]

necouchman commented on a change in pull request #201: GUACAMOLE-547: Add 
support for SSH NONE authentication method
URL: https://github.com/apache/guacamole-server/pull/201#discussion_r294760735
 
 

 ##
 File path: src/protocols/ssh/ssh.c
 ##
 @@ -130,26 +130,36 @@ static guac_common_ssh_user* 
guac_ssh_get_user(guac_client* client) {
 
 } /* end if key given */
 
-/* Otherwise, use password */
-else {
-
-/* Get password if not provided */
-if (settings->password == NULL)
-settings->password = guac_terminal_prompt(ssh_client->term,
-"Password: ", false);
-
-/* Set provided password */
-guac_common_ssh_user_set_password(user, settings->password);
-
-}
-
 /* Clear screen of any prompts */
 guac_terminal_printf(ssh_client->term, "\x1B[H\x1B[J");
 
 return user;
 
 }
 
+/**
+ * A call-back function used to gather additional credentials from a client
+ * during a connection.  It takes the guac_client object and a string to
+ * display to the user, and returns the credentials entered by the user.
+ * 
+ * @param client
+ * The guac_client object associated with the current connection
+ * where additional credentials are required.
+ * 
+ * @param credName
+ * The prompt text to display to the screen when prompting for the
+ * additional credentials.
+ * 
+ * @return 
+ * The string of credentials gathered from the user.
+ */
+char* guac_ssh_get_credential(guac_client *client, char* credName) {
 
 Review comment:
   Changed, here, too.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #201: GUACAMOLE-547: Add support for SSH NONE authentication method

[GitBox]

necouchman commented on a change in pull request #201: GUACAMOLE-547: Add 
support for SSH NONE authentication method
URL: https://github.com/apache/guacamole-server/pull/201#discussion_r294760441
 
 

 ##
 File path: src/common-ssh/ssh.c
 ##
 @@ -304,23 +304,30 @@ static int 
guac_common_ssh_authenticate(guac_common_ssh_session* common_session)
 LIBSSH2_SESSION* session = common_session->session;
 
 /* Get user credentials */
-char* username = user->username;
-char* password = user->password;
 guac_common_ssh_key* key = user->private_key;
 
 /* Validate username provided */
-if (username == NULL) {
+if (user->username == NULL) {
 guac_client_abort(client, GUAC_PROTOCOL_STATUS_CLIENT_UNAUTHORIZED,
 "SSH authentication requires a username.");
 return 1;
 }
 
 /* Get list of supported authentication methods */
-char* user_authlist = libssh2_userauth_list(session, username,
-strlen(username));
+char* user_authlist = libssh2_userauth_list(session, user->username,
+strlen(user->username));
+
+/* If auth list is NULL, then authentication has succeeded with NONE */
+if (user_authlist == NULL) {
+guac_client_log(client, GUAC_LOG_DEBUG,
+"SSH NONE authentication succeeded.");
+return 0;
+}
+
 guac_client_log(client, GUAC_LOG_DEBUG,
 "Supported authentication methods: %s", user_authlist);
 
+
 
 Review comment:
   Ummmbecause...
   
   Removed.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #201: GUACAMOLE-547: Add support for SSH NONE authentication method

[GitBox]

necouchman commented on a change in pull request #201: GUACAMOLE-547: Add 
support for SSH NONE authentication method
URL: https://github.com/apache/guacamole-server/pull/201#discussion_r294759904
 
 

 ##
 File path: src/common-ssh/ssh.c
 ##
 @@ -416,7 +427,7 @@ static int 
guac_common_ssh_authenticate(guac_common_ssh_session* common_session)
 
 guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client,
 const char* hostname, const char* port, guac_common_ssh_user* user, 
int keepalive,
-const char* host_key) {
+const char* host_key, guac_ssh_credential_handler* credential_handler) 
{
 
 Review comment:
   Went with `credential_handler` across the board.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #201: GUACAMOLE-547: Add support for SSH NONE authentication method

[GitBox]

necouchman commented on a change in pull request #201: GUACAMOLE-547: Add 
support for SSH NONE authentication method
URL: https://github.com/apache/guacamole-server/pull/201#discussion_r294759585
 
 

 ##
 File path: src/protocols/ssh/ssh.c
 ##
 @@ -130,26 +130,36 @@ static guac_common_ssh_user* 
guac_ssh_get_user(guac_client* client) {
 
 } /* end if key given */
 
-/* Otherwise, use password */
-else {
-
-/* Get password if not provided */
-if (settings->password == NULL)
-settings->password = guac_terminal_prompt(ssh_client->term,
-"Password: ", false);
-
-/* Set provided password */
-guac_common_ssh_user_set_password(user, settings->password);
-
-}
-
 /* Clear screen of any prompts */
 guac_terminal_printf(ssh_client->term, "\x1B[H\x1B[J");
 
 return user;
 
 }
 
+/**
+ * A call-back function used to gather additional credentials from a client
 
 Review comment:
   Reworded this a bit and added the notes as suggested.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #201: GUACAMOLE-547: Add support for SSH NONE authentication method

[GitBox]

necouchman commented on a change in pull request #201: GUACAMOLE-547: Add 
support for SSH NONE authentication method
URL: https://github.com/apache/guacamole-server/pull/201#discussion_r294759105
 
 

 ##
 File path: src/common-ssh/ssh.c
 ##
 @@ -352,14 +359,18 @@ static int 
guac_common_ssh_authenticate(guac_common_ssh_session* common_session)
 
 }
 
+/* Down to username + password authentication. */
 
 Review comment:
   Down with the rewording.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [guacamole-server] necouchman commented on a change in pull request #201: GUACAMOLE-547: Add support for SSH NONE authentication method

[GitBox]

necouchman commented on a change in pull request #201: GUACAMOLE-547: Add 
support for SSH NONE authentication method
URL: https://github.com/apache/guacamole-server/pull/201#discussion_r294758881
 
 

 ##
 File path: src/common-ssh/common-ssh/ssh.h
 ##
 @@ -99,7 +109,7 @@ void guac_common_ssh_uninit();
  */
 guac_common_ssh_session* guac_common_ssh_create_session(guac_client* client,
 const char* hostname, const char* port, guac_common_ssh_user* user, 
int keepalive,
-const char* host_key);
+const char* host_key, guac_ssh_credential_handler* 
credential_callback);
 
 Review comment:
   Documented the new one, plus all of the missing ones.


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:
us...@infra.apache.org


With regards,
Apache Git Services