You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by necouchman <gi...@git.apache.org> on 2017/08/12 01:11:14 UTC

[GitHub] incubator-guacamole-client pull request #181: GUACAMOLE-38: Implement on-dem...

GitHub user necouchman opened a pull request:

    https://github.com/apache/incubator-guacamole-client/pull/181

    GUACAMOLE-38: Implement on-demand connections

    This is an extension that implements the on-demand (ad-hoc, "QuickConnect") feature, which places a connection bar on the home page and allows users to type in a URI to establish a connection.  Everything is in-memory, so connections do not persist after logout, and there are no records kept of the connections.
    
    The original JIRA issue was closed as a duplicate of parameter prompting, but reopened it as I think they're different enough, and went ahead and wrote it, anyway, and figured I'd see if there was any interest in merging it into the code.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/necouchman/incubator-guacamole-client GUACAMOLE-38

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-guacamole-client/pull/181.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #181
    
----
commit cbed0832b625c8bd1126adf4f192da07d9d641ff
Author: Nick Couchman <vn...@apache.org>
Date:   2017-06-06T15:09:01Z

    GUACAMOLE-38: Initial implementation of QuickConnect feature

commit 11e65629814dfb002c03107c5ce3bd1d0c41e260
Author: Nick Couchman <vn...@apache.org>
Date:   2017-07-21T16:23:17Z

    GUACAMOLE-38: Fix issue with Settings -> Users page.

commit d2ad1956360172768e41f82ca8f542c776a5ccaf
Author: Nick Couchman <vn...@apache.org>
Date:   2017-07-21T16:53:31Z

    GUACAMOLE-38: Code cleanup - remove unneeded code, debugging, etc.  Provide proper documentation.

commit 558d119f1371ef92f6947f8fd967fc4dc231ca8c
Author: Nick Couchman <vn...@apache.org>
Date:   2017-07-25T19:49:15Z

    GUACAMOLE-38: Fix issue related to submitting via Enter key.

commit 2954fdf077d5e5d5852683f3542c458d15ca982f
Author: Nick Couchman <vn...@apache.org>
Date:   2017-08-12T00:22:03Z

    GUACAMOLE-38: Add licenses to QuickConnect extension.

commit 2deb8e2e8fe2de632698749f2410bcad67a9dd5a
Author: Nick Couchman <vn...@apache.org>
Date:   2017-08-12T00:45:39Z

    GUACAMOLE-38: Clean up unnecessary imports and tweak style issues.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-guacamole-client pull request #181: GUACAMOLE-38: Implement on-dem...

Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/181#discussion_r140417241
  
    --- Diff: extensions/guacamole-auth-quickconnect/src/main/resources/controllers/quickconnectController.js ---
    @@ -0,0 +1,186 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +/**
    + * The controller for making ad-hoc (quick) connections
    + */
    +angular.module('guacQuickConnect').controller('quickconnectController', ['$scope', '$injector', '$log',
    +        function manageConnectionController($scope, $injector, $log) {
    +
    +    // Required types
    +    var ClientIdentifier    = $injector.get('ClientIdentifier');
    +    var Connection          = $injector.get('Connection');
    +
    +    // Required services
    +    var $location                = $injector.get('$location');
    +    var $routeParams             = $injector.get('$routeParams');
    +    var guacNotification         = $injector.get('guacNotification');
    +    var connectionService        = $injector.get('connectionService');
    +    var schemaService            = $injector.get('schemaService');
    +
    +    /**
    +     * An action to be provided along with the object sent to showStatus which
    +     * closes the currently-shown status dialog.
    +     */
    +    var ACKNOWLEDGE_ACTION = {
    +        name        : "MANAGE_CONNECTION.ACTION_ACKNOWLEDGE",
    +        // Handle action
    +        callback    : function acknowledgeCallback() {
    +            guacNotification.showStatus(false);
    +        }
    +    };
    +
    +    $scope.uri = null;
    +
    +    $scope.selectedDataSource = 'quickconnect';
    +
    +    /**
    +     * Saves the connection, creating a new connection or updating the existing
    +     * connection.
    +     */
    +    $scope.quickConnect = function quickConnect() {
    +
    +        // Construct parameters from URI...
    +        /**
    +         * Parse URL into the following components:
    +         * [0] - Full URL
    +         * [3] - Protocol
    +         * [5] - Username
    +         * [7] - Password
    +         * [8] - Hostname
    +         * [10] - Port
    +         * [11] - Path
    +         * [13] - Document
    +         * [15] - Parameters
    +         * [17] - JS Route
    +         */
    +        var regexURL = /^(((rdp|ssh|telnet|vnc)?):\/)?\/?((.*?)(:(.*?)|)@)?([^:\/\s]+)(:([^\/]*))?((\/\w+\/)*)([-\w.\/]+[^#?\s]*)?(\?([^#]*))?(#(.*))?$/g;
    --- End diff --
    
    Whoa.


---

[GitHub] incubator-guacamole-client pull request #181: GUACAMOLE-38: Implement on-dem...

Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/181#discussion_r141797398
  
    --- Diff: extensions/guacamole-auth-quickconnect/src/main/resources/controllers/quickconnectController.js ---
    @@ -0,0 +1,186 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +/**
    + * The controller for making ad-hoc (quick) connections
    + */
    +angular.module('guacQuickConnect').controller('quickconnectController', ['$scope', '$injector', '$log',
    +        function manageConnectionController($scope, $injector, $log) {
    +
    +    // Required types
    +    var ClientIdentifier    = $injector.get('ClientIdentifier');
    +    var Connection          = $injector.get('Connection');
    +
    +    // Required services
    +    var $location                = $injector.get('$location');
    +    var $routeParams             = $injector.get('$routeParams');
    +    var guacNotification         = $injector.get('guacNotification');
    +    var connectionService        = $injector.get('connectionService');
    +    var schemaService            = $injector.get('schemaService');
    +
    +    /**
    +     * An action to be provided along with the object sent to showStatus which
    +     * closes the currently-shown status dialog.
    +     */
    +    var ACKNOWLEDGE_ACTION = {
    +        name        : "MANAGE_CONNECTION.ACTION_ACKNOWLEDGE",
    +        // Handle action
    +        callback    : function acknowledgeCallback() {
    +            guacNotification.showStatus(false);
    +        }
    +    };
    +
    +    $scope.uri = null;
    +
    +    $scope.selectedDataSource = 'quickconnect';
    +
    +    /**
    +     * Saves the connection, creating a new connection or updating the existing
    +     * connection.
    +     */
    +    $scope.quickConnect = function quickConnect() {
    +
    +        // Construct parameters from URI...
    +        /**
    +         * Parse URL into the following components:
    +         * [0] - Full URL
    +         * [3] - Protocol
    +         * [5] - Username
    +         * [7] - Password
    +         * [8] - Hostname
    +         * [10] - Port
    +         * [11] - Path
    +         * [13] - Document
    +         * [15] - Parameters
    +         * [17] - JS Route
    +         */
    +        var regexURL = /^(((rdp|ssh|telnet|vnc)?):\/)?\/?((.*?)(:(.*?)|)@)?([^:\/\s]+)(:([^\/]*))?((\/\w+\/)*)([-\w.\/]+[^#?\s]*)?(\?([^#]*))?(#(.*))?$/g;
    --- End diff --
    
    On a similar note, any reason why you chose to handle this client-side vs. server-side? I'm not sure whether one is better than the other at the moment, but I am curious. Might be worth considering.


---

[GitHub] incubator-guacamole-client pull request #181: GUACAMOLE-38: Implement on-dem...

Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/181#discussion_r142018422
  
    --- Diff: extensions/guacamole-auth-quickconnect/src/main/resources/controllers/quickconnectController.js ---
    @@ -0,0 +1,186 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +/**
    + * The controller for making ad-hoc (quick) connections
    + */
    +angular.module('guacQuickConnect').controller('quickconnectController', ['$scope', '$injector', '$log',
    +        function manageConnectionController($scope, $injector, $log) {
    +
    +    // Required types
    +    var ClientIdentifier    = $injector.get('ClientIdentifier');
    +    var Connection          = $injector.get('Connection');
    +
    +    // Required services
    +    var $location                = $injector.get('$location');
    +    var $routeParams             = $injector.get('$routeParams');
    +    var guacNotification         = $injector.get('guacNotification');
    +    var connectionService        = $injector.get('connectionService');
    +    var schemaService            = $injector.get('schemaService');
    +
    +    /**
    +     * An action to be provided along with the object sent to showStatus which
    +     * closes the currently-shown status dialog.
    +     */
    +    var ACKNOWLEDGE_ACTION = {
    +        name        : "MANAGE_CONNECTION.ACTION_ACKNOWLEDGE",
    +        // Handle action
    +        callback    : function acknowledgeCallback() {
    +            guacNotification.showStatus(false);
    +        }
    +    };
    +
    +    $scope.uri = null;
    +
    +    $scope.selectedDataSource = 'quickconnect';
    +
    +    /**
    +     * Saves the connection, creating a new connection or updating the existing
    +     * connection.
    +     */
    +    $scope.quickConnect = function quickConnect() {
    +
    +        // Construct parameters from URI...
    +        /**
    +         * Parse URL into the following components:
    +         * [0] - Full URL
    +         * [3] - Protocol
    +         * [5] - Username
    +         * [7] - Password
    +         * [8] - Hostname
    +         * [10] - Port
    +         * [11] - Path
    +         * [13] - Document
    +         * [15] - Parameters
    +         * [17] - JS Route
    +         */
    +        var regexURL = /^(((rdp|ssh|telnet|vnc)?):\/)?\/?((.*?)(:(.*?)|)@)?([^:\/\s]+)(:([^\/]*))?((\/\w+\/)*)([-\w.\/]+[^#?\s]*)?(\?([^#]*))?(#(.*))?$/g;
    --- End diff --
    
    I think the reason I did it that way was because I was trying to leverage as much as possible of the existing REST endpoints for creating/saving the connections, which seems to be easier if the connection attributes and parameters are parsed out on the client-side and then fed to the existing API.
    
    I suppose I could leverage the extension-specific REST API and feed the entire connection string to a REST endpoint and then parse it out there.  Or is there some other way I'm not thinking about that already exists to get the data to the server-side for parsing by the extension?


---

[GitHub] incubator-guacamole-client pull request #181: GUACAMOLE-38: Implement on-dem...

Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/181#discussion_r141673590
  
    --- Diff: extensions/guacamole-auth-quickconnect/src/main/resources/controllers/quickconnectController.js ---
    @@ -0,0 +1,186 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +/**
    + * The controller for making ad-hoc (quick) connections
    + */
    +angular.module('guacQuickConnect').controller('quickconnectController', ['$scope', '$injector', '$log',
    +        function manageConnectionController($scope, $injector, $log) {
    +
    +    // Required types
    +    var ClientIdentifier    = $injector.get('ClientIdentifier');
    +    var Connection          = $injector.get('Connection');
    +
    +    // Required services
    +    var $location                = $injector.get('$location');
    +    var $routeParams             = $injector.get('$routeParams');
    +    var guacNotification         = $injector.get('guacNotification');
    +    var connectionService        = $injector.get('connectionService');
    +    var schemaService            = $injector.get('schemaService');
    +
    +    /**
    +     * An action to be provided along with the object sent to showStatus which
    +     * closes the currently-shown status dialog.
    +     */
    +    var ACKNOWLEDGE_ACTION = {
    +        name        : "MANAGE_CONNECTION.ACTION_ACKNOWLEDGE",
    +        // Handle action
    +        callback    : function acknowledgeCallback() {
    +            guacNotification.showStatus(false);
    +        }
    +    };
    +
    +    $scope.uri = null;
    +
    +    $scope.selectedDataSource = 'quickconnect';
    +
    +    /**
    +     * Saves the connection, creating a new connection or updating the existing
    +     * connection.
    +     */
    +    $scope.quickConnect = function quickConnect() {
    +
    +        // Construct parameters from URI...
    +        /**
    +         * Parse URL into the following components:
    +         * [0] - Full URL
    +         * [3] - Protocol
    +         * [5] - Username
    +         * [7] - Password
    +         * [8] - Hostname
    +         * [10] - Port
    +         * [11] - Path
    +         * [13] - Document
    +         * [15] - Parameters
    +         * [17] - JS Route
    +         */
    +        var regexURL = /^(((rdp|ssh|telnet|vnc)?):\/)?\/?((.*?)(:(.*?)|)@)?([^:\/\s]+)(:([^\/]*))?((\/\w+\/)*)([-\w.\/]+[^#?\s]*)?(\?([^#]*))?(#(.*))?$/g;
    --- End diff --
    
    Obviously it would be better if the available protocols there were parsed from a REST API check of supported protocols instead of hard-coded.


---

[GitHub] incubator-guacamole-client pull request #181: GUACAMOLE-38: Implement on-dem...

Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/181#discussion_r142047642
  
    --- Diff: extensions/guacamole-auth-quickconnect/src/main/resources/controllers/quickconnectController.js ---
    @@ -0,0 +1,186 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +/**
    + * The controller for making ad-hoc (quick) connections
    + */
    +angular.module('guacQuickConnect').controller('quickconnectController', ['$scope', '$injector', '$log',
    +        function manageConnectionController($scope, $injector, $log) {
    +
    +    // Required types
    +    var ClientIdentifier    = $injector.get('ClientIdentifier');
    +    var Connection          = $injector.get('Connection');
    +
    +    // Required services
    +    var $location                = $injector.get('$location');
    +    var $routeParams             = $injector.get('$routeParams');
    +    var guacNotification         = $injector.get('guacNotification');
    +    var connectionService        = $injector.get('connectionService');
    +    var schemaService            = $injector.get('schemaService');
    +
    +    /**
    +     * An action to be provided along with the object sent to showStatus which
    +     * closes the currently-shown status dialog.
    +     */
    +    var ACKNOWLEDGE_ACTION = {
    +        name        : "MANAGE_CONNECTION.ACTION_ACKNOWLEDGE",
    +        // Handle action
    +        callback    : function acknowledgeCallback() {
    +            guacNotification.showStatus(false);
    +        }
    +    };
    +
    +    $scope.uri = null;
    +
    +    $scope.selectedDataSource = 'quickconnect';
    +
    +    /**
    +     * Saves the connection, creating a new connection or updating the existing
    +     * connection.
    +     */
    +    $scope.quickConnect = function quickConnect() {
    +
    +        // Construct parameters from URI...
    +        /**
    +         * Parse URL into the following components:
    +         * [0] - Full URL
    +         * [3] - Protocol
    +         * [5] - Username
    +         * [7] - Password
    +         * [8] - Hostname
    +         * [10] - Port
    +         * [11] - Path
    +         * [13] - Document
    +         * [15] - Parameters
    +         * [17] - JS Route
    +         */
    +        var regexURL = /^(((rdp|ssh|telnet|vnc)?):\/)?\/?((.*?)(:(.*?)|)@)?([^:\/\s]+)(:([^\/]*))?((\/\w+\/)*)([-\w.\/]+[^#?\s]*)?(\?([^#]*))?(#(.*))?$/g;
    --- End diff --
    
    > I'm not sure whether one is better than the other at the moment, but I am curious.
    
    Well, since Java already has a URI class and is capable of parsing out the URI without the...colorful...regex, I think I might be leaning toward redoing it server-side.  JavaScript does have some basic parsing available through the DOM, and some external libraries available for it, but it doesn't seem nearly as robust as the Java library.


---

[GitHub] incubator-guacamole-client pull request #181: GUACAMOLE-38: Implement on-dem...

Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:

    https://github.com/apache/incubator-guacamole-client/pull/181#discussion_r140637578
  
    --- Diff: extensions/guacamole-auth-quickconnect/src/main/resources/controllers/quickconnectController.js ---
    @@ -0,0 +1,186 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +
    +/**
    + * The controller for making ad-hoc (quick) connections
    + */
    +angular.module('guacQuickConnect').controller('quickconnectController', ['$scope', '$injector', '$log',
    +        function manageConnectionController($scope, $injector, $log) {
    +
    +    // Required types
    +    var ClientIdentifier    = $injector.get('ClientIdentifier');
    +    var Connection          = $injector.get('Connection');
    +
    +    // Required services
    +    var $location                = $injector.get('$location');
    +    var $routeParams             = $injector.get('$routeParams');
    +    var guacNotification         = $injector.get('guacNotification');
    +    var connectionService        = $injector.get('connectionService');
    +    var schemaService            = $injector.get('schemaService');
    +
    +    /**
    +     * An action to be provided along with the object sent to showStatus which
    +     * closes the currently-shown status dialog.
    +     */
    +    var ACKNOWLEDGE_ACTION = {
    +        name        : "MANAGE_CONNECTION.ACTION_ACKNOWLEDGE",
    +        // Handle action
    +        callback    : function acknowledgeCallback() {
    +            guacNotification.showStatus(false);
    +        }
    +    };
    +
    +    $scope.uri = null;
    +
    +    $scope.selectedDataSource = 'quickconnect';
    +
    +    /**
    +     * Saves the connection, creating a new connection or updating the existing
    +     * connection.
    +     */
    +    $scope.quickConnect = function quickConnect() {
    +
    +        // Construct parameters from URI...
    +        /**
    +         * Parse URL into the following components:
    +         * [0] - Full URL
    +         * [3] - Protocol
    +         * [5] - Username
    +         * [7] - Password
    +         * [8] - Hostname
    +         * [10] - Port
    +         * [11] - Path
    +         * [13] - Document
    +         * [15] - Parameters
    +         * [17] - JS Route
    +         */
    +        var regexURL = /^(((rdp|ssh|telnet|vnc)?):\/)?\/?((.*?)(:(.*?)|)@)?([^:\/\s]+)(:([^\/]*))?((\/\w+\/)*)([-\w.\/]+[^#?\s]*)?(\?([^#]*))?(#(.*))?$/g;
    --- End diff --
    
    Ha.  Yeah.  If you have suggestions for better ways to parse out a URI, I'm open.  That regex right there is the result of a combo of Google searches and working with the regex on regex101.com.


---