mike-jumper commented on code in PR #840: URL: https://github.com/apache/guacamole-client/pull/840#discussion_r1171954389
########## guacamole/src/main/frontend/src/images/question.svg: ########## @@ -0,0 +1,64 @@ +<?xml version="1.0" encoding="UTF-8" standalone="no"?> +<svg + width="64" + height="64" + viewBox="0 0 64 64" + version="1.1" + id="svg10" + sodipodi:docname="question.svg" + inkscape:version="1.1.2 (0a00cf5339, 2022-02-04)" + xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"; + xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"; + xmlns="http://www.w3.org/2000/svg"; + xmlns:svg="http://www.w3.org/2000/svg";> + <defs + id="defs14" /> + <sodipodi:namedview + id="namedview12" + pagecolor="#ffffff" + bordercolor="#666666" + borderopacity="1.0" + inkscape:pageshadow="2" + inkscape:pageopacity="0.0" + inkscape:pagecheckerboard="0" + showgrid="false" + inkscape:zoom="8" + inkscape:cx="38.5625" + inkscape:cy="31.0625" + inkscape:window-width="2048" + inkscape:window-height="1025" + inkscape:window-x="0" + inkscape:window-y="28" + inkscape:window-maximized="1" + inkscape:current-layer="g8" /> + <g + style="stroke-width:1.04766" + id="g8"> + <g + style="font-style:normal;font-weight:400;font-size:142.558px;line-height:100%;font-family:Sans;letter-spacing:0;word-spacing:0;fill:#000000;fill-opacity:1;stroke:none;stroke-width:3.20748px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1" + id="g6"> + <path + d="m 169.301,354.693 c -60.57527,-41.3594 -30.28763,-20.6797 0,0 z m -0.627,90.839 c -60.15727,-101.91873 -30.07863,-50.95937 0,0 z" + style="font-style:normal;font-variant:normal;font-weight:900;font-stretch:normal;font-size:142.558px;line-height:100%;font-family:Roboto;-inkscape-font-specification:'Roboto Heavy';text-align:center;text-anchor:middle;fill:#000000;stroke-width:3.20748px" + transform="matrix(0.3118,0,0,0.31173,-24.457,-91.229)" + aria-label="!" + id="path4" + sodipodi:nodetypes="cccc" /> + </g> + <path + id="path2540" + style="fill:#000000" + d="M 32,0.75 A 31.25,31.25 0 0 0 0.75,32 31.25,31.25 0 0 0 32,63.25 31.25,31.25 0 0 0 63.25,32 31.25,31.25 0 0 0 32,0.75 Z m 0,5 A 26.25,26.25 0 0 1 58.25,32 26.25,26.25 0 0 1 32,58.25 26.25,26.25 0 0 1 5.75,32 26.25,26.25 0 0 1 32,5.75 Z" /> + <text + xml:space="preserve" + style="font-style:normal;font-weight:normal;font-size:40px;line-height:1.25;font-family:sans-serif;fill:#000000;fill-opacity:1;stroke:none" + x="17.083984" + y="52.78125" + id="text4900"><tspan + id="tspan4898" + x="17.083984" + y="52.78125" + sodipodi:role="line" + style="font-style:normal;font-variant:normal;font-weight:normal;font-stretch:normal;font-size:56px;font-family:sans-serif;-inkscape-font-specification:'sans-serif, Normal';font-variant-ligatures:normal;font-variant-caps:normal;font-variant-numeric:normal;font-variant-east-asian:normal">?</tspan></text> Review Comment: Embedding the "?" as actual text will result in this SVG being dependent on the fonts installed on the user's system. It should instead be converted to a path. I'd also recommend running it through `svgo` to trim down the size: https://www.npmjs.com/package/svgo ########## guacamole/src/main/frontend/src/translations/en.json: ########## @@ -200,38 +200,52 @@ "DIALOG_HEADER_ERROR" : "@:APP.DIALOG_HEADER_ERROR", "DIALOG_HEADER_SUCCESS" : "Success", - "ERROR_AMBIGUOUS_CSV_HEADER" : "Ambiguous CSV Header \"{HEADER}\" could be either a connection attribute or parameter", - "ERROR_AMBIGUOUS_PARENT_GROUP" : "Both group and parentIdentifier may be not specified at the same time", - "ERROR_ARRAY_REQUIRED" : "The provided file must contain a list of connections", - "ERROR_DUPLICATE_CSV_HEADER" : "Duplicate CSV Header: {HEADER}", - "ERROR_EMPTY_FILE" : "The provided file is empty", - "ERROR_INVALID_CSV_HEADER" : "Invalid CSV Header \"{HEADER}\" is neither an attribute or parameter", - "ERROR_INVALID_MIME_TYPE" : "Unsupported file type: \"{TYPE}\"", - "ERROR_DETECTED_INVALID_TYPE" : "Unsupported file type. Please make sure the file is valid CSV, JSON, or YAML.", - "ERROR_INVALID_GROUP" : "No group matching \"{GROUP}\" found", - "ERROR_INVALID_GROUP_IDENTIFIER" : "No connection group with identifier \"{IDENTIFIER}\" found", - "ERROR_NO_FILE_SUPPLIED" : "Please select a file to import", - "ERROR_PARSE_FAILURE_CSV" : "Please make sure your file is valid CSV. Parsing failed with error \"{ERROR}\". ", - "ERROR_PARSE_FAILURE_JSON" : "Please make sure your file is valid JSON. Parsing failed with error \"{ERROR}\". ", - "ERROR_PARSE_FAILURE_YAML" : "Please make sure your file is valid YAML. Parsing failed with error \"{ERROR}\". ", - "ERROR_REQUIRED_NAME" : "No connection name found in the provided file", - "ERROR_REQUIRED_PROTOCOL" : "No connection protocol found in the provided file", + "ERROR_AMBIGUOUS_CSV_HEADER" : "Ambiguous CSV Header \"{HEADER}\" could be either a connection attribute or parameter", + "ERROR_AMBIGUOUS_PARENT_GROUP" : "Both group and parentIdentifier may be not specified at the same time", + "ERROR_ARRAY_REQUIRED" : "The provided file must contain a list of connections", + "ERROR_DETECTED_INVALID_TYPE" : "Unsupported file type. Please make sure the file is valid CSV, JSON, or YAML.", + "ERROR_DUPLICATE_CONNECTION_IN_FILE" : "Duplicate connection in file at \"{PATH}\"", + "ERROR_DUPLICATE_CSV_HEADER" : "Duplicate CSV Header: {HEADER}", + "ERROR_EMPTY_FILE" : "The provided file is empty", + "ERROR_INVALID_CSV_HEADER" : "Invalid CSV Header \"{HEADER}\" is neither an attribute or parameter", + "ERROR_INVALID_MIME_TYPE" : "Unsupported file type: \"{TYPE}\"", + "ERROR_INVALID_GROUP" : "No group matching \"{GROUP}\" found", + "ERROR_INVALID_GROUP_IDENTIFIER" : "No connection group with identifier \"{IDENTIFIER}\" found", + "ERROR_NO_FILE_SUPPLIED" : "Please select a file to import", + "ERROR_PARSE_FAILURE_CSV" : "Please make sure your file is valid CSV. Parsing failed with error \"{ERROR}\". ", + "ERROR_PARSE_FAILURE_JSON" : "Please make sure your file is valid JSON. Parsing failed with error \"{ERROR}\". ", + "ERROR_PARSE_FAILURE_YAML" : "Please make sure your file is valid YAML. Parsing failed with error \"{ERROR}\". ", + "ERROR_REJECT_UPDATE_CONNECTION" : "Disallowed update to existing connection at \"{PATH}\"", + "ERROR_REQUIRED_NAME" : "No connection name found in the provided file", + "ERROR_REQUIRED_PROTOCOL" : "No connection protocol found in the provided file", "FIELD_PLACEHOLDER_FILTER" : "@:APP.FIELD_PLACEHOLDER_FILTER", + "FIELD_HEADER_REPLACE_CONNECTIONS" : "Replace/Update existing connections", + "FIELD_HEADER_REPLACE_PERMISSIONS" : "Reset permissions", + + "FIELD_OPTION_DUPLICATE_REPLACE" : "Replace duplicates", + "FIELD_OPTION_DUPLICATE_IGNORE" : "Ignore duplicates", + "FIELD_OPTION_DUPLICATE_ERROR" : "Disallow duplicates", + "HELP_CSV_DESCRIPTION" : "A connection import CSV file has one connection record per row. Each column will specify a connection field. At minimum the connection name and protocol must be specified.", "HELP_CSV_EXAMPLE" : "name,protocol,hostname,group,users,groups,guacd-encryption (attribute)\nconn1,vnc,conn1.web.com,ROOT,guac user 1;guac user 2,Connection 1 Users,none\nconn2,rdp,conn2.web.com,ROOT/Parent Group,guac user 1,,ssl\nconn3,ssh,conn3.web.com,ROOT/Parent Group/Child Group,guac user 2;guac user 3,,\nconn4,kubernetes,,,,,", "HELP_CSV_MORE_DETAILS" : "The CSV header for each row specifies the connection field. The connection group ID that the connection should be imported into may be directly specified with \"parentIdentifier\", or the path to the parent group may be specified using \"group\" as shown below. In most cases, there should be no conflict between fields, but if needed, an \" (attribute)\" or \" (parameter)\" suffix may be added to disambiguate. Lists of user or user group identifiers must be semicolon-separated.¹", - "HELP_FILE_TYPE_DESCRIPTION" : "Three file types are supported for connection import: CSV, JSON, and YAML. The same data may be specified by each file type. This must include the connection name and protocol. Optionally, a connection group location, a list of users and/or user groups to grant access, connection parameters, or connection protocols may also be specified. Any users or user groups that do not exist in the current data source will be automatically created.", + "HELP_FILE_TYPE_DESCRIPTION" : "Three file types are supported for connection import: CSV, JSON, and YAML. The same data may be specified by each file type. This must include the connection name and protocol. Optionally, a connection group location, a list of users and/or user groups to grant access, connection parameters, or connection protocols may also be specified. Any users or user groups that do not exist in the current data source will be automatically created. Note that any existing connection permissions will not be removed for updated connections.", "HELP_FILE_TYPE_HEADER" : "File Types", "HELP_JSON_DESCRIPTION" : "A connection import JSON file is a list of connection objects. At minimum the connection name and protocol must be specified in each connection object.", "HELP_JSON_EXAMPLE" : "[\n \\{\n \"name\": \"conn1\",\n \"protocol\": \"vnc\",\n \"parameters\": \\{ \"hostname\": \"conn1.web.com\" \\},\n \"parentIdentifier\": \"ROOT\",\n \"users\": [ \"guac user 1\", \"guac user 2\" ],\n \"groups\": [ \"Connection 1 Users\" ],\n \"attributes\": \\{ \"guacd-encryption\": \"none\" \\}\n \\},\n \\{\n \"name\": \"conn2\",\n \"protocol\": \"rdp\",\n \"parameters\": \\{ \"hostname\": \"conn2.web.com\" \\},\n \"group\": \"ROOT/Parent Group\",\n \"users\": [ \"guac user 1\" ],\n \"attributes\": \\{ \"guacd-encryption\": \"none\" \\}\n \\},\n \\{\n \"name\": \"conn3\",\n \"protocol\": \"ssh\",\n \"parameters\": \\{ \"hostname\": \"conn3.web.com\" \\},\n \"group\": \"ROOT/Parent Group/Child Group\",\n \"users\": [ \"guac user 2\", \"guac user 3\" ]\n \\},\n \\{\n \"name\": \"conn4\",\n \"protocol\": \"kubernetes\"\n \\}\n]", - "HELP_JSON_MORE_DETAILS" : "The connection group ID that the connection should be imported into may be directly specified with a \"parentIdentifier\" field, or the path to the parent group may be specified using a \"group\" field as shown below. An array of user and user group identifiers to grant access to may be specified per connection.", + "HELP_JSON_MORE_DETAILS" : "The connection group ID that the connection should be imported into may be directly specified with a \"parentIdentifier\" field, or the path to the parent group may be specified using a \"group\" field as shown below. An array of user and user group identifiers to grant access to may be specified per connection.", + "HELP_REPLACE_CONNECTION_TITLE" : "Replacing existing connections", + "HELP_REPLACE_CONNECTION_CONTENT" : "Checking this box will allow existing connections to be updated, if an imported connection has the same name and parent group as an existing connection. If unchecked, attempts to update existing connections will be treated as an error.", Review Comment: I'd recommend: > Entirely replace/update existing connections if their names and parent connection groups match the values in the provided file. If unchecked, attempting to import a connection with the same name and parent connection group of an existing connection will be considered an error. ########## guacamole/src/main/frontend/src/app/import/types/ConnectionImportConfig.js: ########## @@ -0,0 +1,103 @@ +/* + * 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. + */ + +/** + * Service which defines the ConnectionImportConfig class. + */ +angular.module('import').factory('ConnectionImportConfig', [ + function defineConnectionImportConfig() { + + /** + * A representation of any user-specified configuration when + * batch-importing connections. + * + * @constructor + * @param {ConnectionImportConfig|Object} [template={}] + * The object whose properties should be copied within the new + * ConnectionImportConfig. + */ + const ConnectionImportConfig = function ConnectionImportConfig(template) { + + // Use empty object by default + template = template || {}; + + /** + * The mode for handling connections that match existing connections. + * + * @type ConnectionImportConfig.ReplaceConnectionMode + */ + this.replaceConnectionMode = template.replaceConnectionMode + || ConnectionImportConfig.ReplaceConnectionMode.REJECT; + + /** + * The mode for handling permissions on existing connections that are + * being updated. Only meaningful if the importer is configured to + * replace existing connections. + * + * @type ConnectionImportConfig.ExistingPermissionMode + */ + this.existingPermissionMode = template.existingPermissionMode + || ConnectionImportConfig.ExistingPermissionMode.ADD; + + }; + + /** + * Valid modes for the behavior of the importer when attempts are made to + * update existing connections. + */ + ConnectionImportConfig.ReplaceConnectionMode = { + + /** + * Any attempt to update existing connections will cause the entire + * import to be rejected with an error. + */ + REJECT : "REJECT", + + /** + * Replace/update any existing connections. + */ + REPLACE : "REPLACE" + + }; + + /** + * Valid modes for the behavior of the importer with respect to connection + * permissions when existing connections are being replaced. + */ + ConnectionImportConfig.ExistingPermissionMode = { + + /** + * Any new permissions specified in the imported connection will be + * added to the existing connection, without removing any existing + * permissions. + */ + ADD : "ADD", + + /** + * Any existing permissions will be removed, ensuring that only the + * users or groups specified in the import file will be granted to the + * replaced connection after import. + */ + REPLACE : "REPLACE" Review Comment: Recommend renaming `ADD` to `PRESERVE` so that the mode is consistently worded from the same perspective (an action performed upon existing permissions). To me, `REPLACE` makes sense as an "existing permission mode" (we're replacing existing permissions), but not `ADD` (we're not adding existing permissions). ########## guacamole/src/main/frontend/src/app/import/types/ParseResult.js: ########## @@ -65,10 +69,19 @@ angular.module('import').factory('ParseResult', [function defineParseResult() { */ this.groups = template.users || {}; + /** + * A map of connection index within the patch array, to connection group + * path for that connection, of the form "ROOT/Parent/Child". + * + * @type {Object.<String, String>} + */ + this.groupPaths = template.groupPaths || {}; + /** * An array of errors encountered while parsing the corresponding - * connection (at the same array index). Each connection should have a - * an array of errors. If empty, no errors occurred for this connection. + * connection (at the same array index in the patches array). Each + * connection should have a an array of errors. If empty, no errors Review Comment: "a an array" -> "an array" ########## guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryResource.java: ########## @@ -590,7 +591,56 @@ public void executeOperation(boolean atomic, Directory<InternalType> directory) } - // Append each identifier to the list, to be removed atomically + else if (op == APIPatch.Operation.replace) { + + // The identifier of the object to be replaced + String identifier = path.substring(1); + + InternalType original = null; + + try { + + // Fetch the object to be updated + original = directory.get(identifier); Review Comment: `get()` has its own success/failure event type that should be dispatched separately from `update()`. See: https://github.com/apache/guacamole-client/blob/05c675bb215111643fc1213f661b07867a98aeb0/guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryResource.java#L747-L757 ########## guacamole/src/main/frontend/src/translations/en.json: ########## @@ -200,38 +200,52 @@ "DIALOG_HEADER_ERROR" : "@:APP.DIALOG_HEADER_ERROR", "DIALOG_HEADER_SUCCESS" : "Success", - "ERROR_AMBIGUOUS_CSV_HEADER" : "Ambiguous CSV Header \"{HEADER}\" could be either a connection attribute or parameter", - "ERROR_AMBIGUOUS_PARENT_GROUP" : "Both group and parentIdentifier may be not specified at the same time", - "ERROR_ARRAY_REQUIRED" : "The provided file must contain a list of connections", - "ERROR_DUPLICATE_CSV_HEADER" : "Duplicate CSV Header: {HEADER}", - "ERROR_EMPTY_FILE" : "The provided file is empty", - "ERROR_INVALID_CSV_HEADER" : "Invalid CSV Header \"{HEADER}\" is neither an attribute or parameter", - "ERROR_INVALID_MIME_TYPE" : "Unsupported file type: \"{TYPE}\"", - "ERROR_DETECTED_INVALID_TYPE" : "Unsupported file type. Please make sure the file is valid CSV, JSON, or YAML.", - "ERROR_INVALID_GROUP" : "No group matching \"{GROUP}\" found", - "ERROR_INVALID_GROUP_IDENTIFIER" : "No connection group with identifier \"{IDENTIFIER}\" found", - "ERROR_NO_FILE_SUPPLIED" : "Please select a file to import", - "ERROR_PARSE_FAILURE_CSV" : "Please make sure your file is valid CSV. Parsing failed with error \"{ERROR}\". ", - "ERROR_PARSE_FAILURE_JSON" : "Please make sure your file is valid JSON. Parsing failed with error \"{ERROR}\". ", - "ERROR_PARSE_FAILURE_YAML" : "Please make sure your file is valid YAML. Parsing failed with error \"{ERROR}\". ", - "ERROR_REQUIRED_NAME" : "No connection name found in the provided file", - "ERROR_REQUIRED_PROTOCOL" : "No connection protocol found in the provided file", + "ERROR_AMBIGUOUS_CSV_HEADER" : "Ambiguous CSV Header \"{HEADER}\" could be either a connection attribute or parameter", + "ERROR_AMBIGUOUS_PARENT_GROUP" : "Both group and parentIdentifier may be not specified at the same time", + "ERROR_ARRAY_REQUIRED" : "The provided file must contain a list of connections", + "ERROR_DETECTED_INVALID_TYPE" : "Unsupported file type. Please make sure the file is valid CSV, JSON, or YAML.", + "ERROR_DUPLICATE_CONNECTION_IN_FILE" : "Duplicate connection in file at \"{PATH}\"", + "ERROR_DUPLICATE_CSV_HEADER" : "Duplicate CSV Header: {HEADER}", + "ERROR_EMPTY_FILE" : "The provided file is empty", + "ERROR_INVALID_CSV_HEADER" : "Invalid CSV Header \"{HEADER}\" is neither an attribute or parameter", + "ERROR_INVALID_MIME_TYPE" : "Unsupported file type: \"{TYPE}\"", + "ERROR_INVALID_GROUP" : "No group matching \"{GROUP}\" found", + "ERROR_INVALID_GROUP_IDENTIFIER" : "No connection group with identifier \"{IDENTIFIER}\" found", + "ERROR_NO_FILE_SUPPLIED" : "Please select a file to import", + "ERROR_PARSE_FAILURE_CSV" : "Please make sure your file is valid CSV. Parsing failed with error \"{ERROR}\". ", + "ERROR_PARSE_FAILURE_JSON" : "Please make sure your file is valid JSON. Parsing failed with error \"{ERROR}\". ", + "ERROR_PARSE_FAILURE_YAML" : "Please make sure your file is valid YAML. Parsing failed with error \"{ERROR}\". ", + "ERROR_REJECT_UPDATE_CONNECTION" : "Disallowed update to existing connection at \"{PATH}\"", Review Comment: As written, this makes sense to me only in the context of an update, but: * A connection import isn't an update unless the "Replace/Update ..." box is checked. * This error doesn't occur if the "Replace/Update" box is checked. Assuming the name can be included, recommend rephrasing to: `"Connection \"{NAME}\" already exists at \"{PATH}\"."` ########## guacamole/src/main/frontend/src/app/import/types/ConnectionImportConfig.js: ########## @@ -0,0 +1,103 @@ +/* + * 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. + */ + +/** + * Service which defines the ConnectionImportConfig class. + */ +angular.module('import').factory('ConnectionImportConfig', [ + function defineConnectionImportConfig() { + + /** + * A representation of any user-specified configuration when + * batch-importing connections. + * + * @constructor + * @param {ConnectionImportConfig|Object} [template={}] + * The object whose properties should be copied within the new + * ConnectionImportConfig. + */ + const ConnectionImportConfig = function ConnectionImportConfig(template) { + + // Use empty object by default + template = template || {}; + + /** + * The mode for handling connections that match existing connections. + * + * @type ConnectionImportConfig.ReplaceConnectionMode + */ + this.replaceConnectionMode = template.replaceConnectionMode + || ConnectionImportConfig.ReplaceConnectionMode.REJECT; + + /** + * The mode for handling permissions on existing connections that are + * being updated. Only meaningful if the importer is configured to + * replace existing connections. + * + * @type ConnectionImportConfig.ExistingPermissionMode + */ + this.existingPermissionMode = template.existingPermissionMode + || ConnectionImportConfig.ExistingPermissionMode.ADD; + + }; + + /** + * Valid modes for the behavior of the importer when attempts are made to + * update existing connections. + */ + ConnectionImportConfig.ReplaceConnectionMode = { Review Comment: Similar to `ExistingPermissionMode`, I suggest renaming this to `ExistingConnectionMode`. ########## guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryResource.java: ########## @@ -590,7 +591,56 @@ public void executeOperation(boolean atomic, Directory<InternalType> directory) } - // Append each identifier to the list, to be removed atomically + else if (op == APIPatch.Operation.replace) { + + // The identifier of the object to be replaced + String identifier = path.substring(1); + + InternalType original = null; + + try { + + // Fetch the object to be updated + original = directory.get(identifier); + + // Apply the changes to the original object + translator.applyExternalChanges( + original, patch.getValue()); + + // Update the directory + directory.update(original); + + replacedObjects.add(original); Review Comment: If the requested object does not exist, `original` will be `null` here. ########## guacamole/src/main/frontend/src/app/import/controllers/importConnectionsController.js: ########## @@ -688,9 +637,42 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$ }; /** - * The name of the file that's currently being uploaded, or has yet to - * be imported, if any. + * Display a modal with the given title and text keys. + * + * @param {String} titleKey + * The translation key to use for the title of the modal. + * + * @param {String} contentKey + * The translation key to use for the text contents of the modal. */ - $scope.fileName = null; + const showModal = (titleKey, contentKey) => guacNotification.showStatus({ + + // The provided modal contents + title: titleKey, + text: { key: contentKey }, + + // Add a button to hide the modal + actions : [{ + name : 'IMPORT.ACTION_ACKNOWLEDGE', + callback : () => guacNotification.showStatus(false) + }] + + }); + + /** + * Display a modal with information about the existing connection + * replacement option. + */ + $scope.showConnectionReplaceHelp = () => showModal( + 'IMPORT.HELP_REPLACE_CONNECTION_TITLE', + 'IMPORT.HELP_REPLACE_CONNECTION_CONTENT'); + + /** + * Display a modal with information about the existing connection permission + * replacement option. + */ + $scope.showPermissionReplaceHelp = () => showModal( + 'IMPORT.HELP_REPLACE_PERMISSION_TITLE', + 'IMPORT.HELP_REPLACE_PERMISSION_CONTENT'); Review Comment: What about showing this information as a tooltip visible on hover (such as with the `title` attribute), rather than a modal dialog? ########## guacamole/src/main/frontend/src/app/import/types/ConnectionImportConfig.js: ########## @@ -0,0 +1,103 @@ +/* + * 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. + */ + +/** + * Service which defines the ConnectionImportConfig class. + */ +angular.module('import').factory('ConnectionImportConfig', [ + function defineConnectionImportConfig() { + + /** + * A representation of any user-specified configuration when + * batch-importing connections. + * + * @constructor + * @param {ConnectionImportConfig|Object} [template={}] + * The object whose properties should be copied within the new + * ConnectionImportConfig. + */ + const ConnectionImportConfig = function ConnectionImportConfig(template) { + + // Use empty object by default + template = template || {}; + + /** + * The mode for handling connections that match existing connections. + * + * @type ConnectionImportConfig.ReplaceConnectionMode + */ + this.replaceConnectionMode = template.replaceConnectionMode + || ConnectionImportConfig.ReplaceConnectionMode.REJECT; + + /** + * The mode for handling permissions on existing connections that are + * being updated. Only meaningful if the importer is configured to + * replace existing connections. + * + * @type ConnectionImportConfig.ExistingPermissionMode + */ + this.existingPermissionMode = template.existingPermissionMode + || ConnectionImportConfig.ExistingPermissionMode.ADD; + + }; + + /** + * Valid modes for the behavior of the importer when attempts are made to + * update existing connections. + */ + ConnectionImportConfig.ReplaceConnectionMode = { + + /** + * Any attempt to update existing connections will cause the entire + * import to be rejected with an error. Review Comment: Here, too, I don't think we should presume the intent to update. I suggest instead: > Connections that have the same name and parent group as an existing connection will cause the entire import to be rejected with an error. ########## guacamole/src/main/frontend/src/app/import/types/ConnectionImportConfig.js: ########## @@ -0,0 +1,103 @@ +/* + * 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. + */ + +/** + * Service which defines the ConnectionImportConfig class. + */ +angular.module('import').factory('ConnectionImportConfig', [ + function defineConnectionImportConfig() { + + /** + * A representation of any user-specified configuration when + * batch-importing connections. + * + * @constructor + * @param {ConnectionImportConfig|Object} [template={}] + * The object whose properties should be copied within the new + * ConnectionImportConfig. + */ + const ConnectionImportConfig = function ConnectionImportConfig(template) { + + // Use empty object by default + template = template || {}; + + /** + * The mode for handling connections that match existing connections. + * + * @type ConnectionImportConfig.ReplaceConnectionMode + */ + this.replaceConnectionMode = template.replaceConnectionMode + || ConnectionImportConfig.ReplaceConnectionMode.REJECT; + + /** + * The mode for handling permissions on existing connections that are + * being updated. Only meaningful if the importer is configured to + * replace existing connections. + * + * @type ConnectionImportConfig.ExistingPermissionMode + */ + this.existingPermissionMode = template.existingPermissionMode + || ConnectionImportConfig.ExistingPermissionMode.ADD; + + }; + + /** + * Valid modes for the behavior of the importer when attempts are made to + * update existing connections. Review Comment: I don't think we should presume the intent to update. I suggest: > Valid modes for the behavior of the importer when an imported connection already exists. ########## guacamole/src/main/frontend/src/translations/en.json: ########## @@ -200,38 +200,52 @@ "DIALOG_HEADER_ERROR" : "@:APP.DIALOG_HEADER_ERROR", "DIALOG_HEADER_SUCCESS" : "Success", - "ERROR_AMBIGUOUS_CSV_HEADER" : "Ambiguous CSV Header \"{HEADER}\" could be either a connection attribute or parameter", - "ERROR_AMBIGUOUS_PARENT_GROUP" : "Both group and parentIdentifier may be not specified at the same time", - "ERROR_ARRAY_REQUIRED" : "The provided file must contain a list of connections", - "ERROR_DUPLICATE_CSV_HEADER" : "Duplicate CSV Header: {HEADER}", - "ERROR_EMPTY_FILE" : "The provided file is empty", - "ERROR_INVALID_CSV_HEADER" : "Invalid CSV Header \"{HEADER}\" is neither an attribute or parameter", - "ERROR_INVALID_MIME_TYPE" : "Unsupported file type: \"{TYPE}\"", - "ERROR_DETECTED_INVALID_TYPE" : "Unsupported file type. Please make sure the file is valid CSV, JSON, or YAML.", - "ERROR_INVALID_GROUP" : "No group matching \"{GROUP}\" found", - "ERROR_INVALID_GROUP_IDENTIFIER" : "No connection group with identifier \"{IDENTIFIER}\" found", - "ERROR_NO_FILE_SUPPLIED" : "Please select a file to import", - "ERROR_PARSE_FAILURE_CSV" : "Please make sure your file is valid CSV. Parsing failed with error \"{ERROR}\". ", - "ERROR_PARSE_FAILURE_JSON" : "Please make sure your file is valid JSON. Parsing failed with error \"{ERROR}\". ", - "ERROR_PARSE_FAILURE_YAML" : "Please make sure your file is valid YAML. Parsing failed with error \"{ERROR}\". ", - "ERROR_REQUIRED_NAME" : "No connection name found in the provided file", - "ERROR_REQUIRED_PROTOCOL" : "No connection protocol found in the provided file", + "ERROR_AMBIGUOUS_CSV_HEADER" : "Ambiguous CSV Header \"{HEADER}\" could be either a connection attribute or parameter", + "ERROR_AMBIGUOUS_PARENT_GROUP" : "Both group and parentIdentifier may be not specified at the same time", + "ERROR_ARRAY_REQUIRED" : "The provided file must contain a list of connections", + "ERROR_DETECTED_INVALID_TYPE" : "Unsupported file type. Please make sure the file is valid CSV, JSON, or YAML.", + "ERROR_DUPLICATE_CONNECTION_IN_FILE" : "Duplicate connection in file at \"{PATH}\"", + "ERROR_DUPLICATE_CSV_HEADER" : "Duplicate CSV Header: {HEADER}", + "ERROR_EMPTY_FILE" : "The provided file is empty", + "ERROR_INVALID_CSV_HEADER" : "Invalid CSV Header \"{HEADER}\" is neither an attribute or parameter", + "ERROR_INVALID_MIME_TYPE" : "Unsupported file type: \"{TYPE}\"", + "ERROR_INVALID_GROUP" : "No group matching \"{GROUP}\" found", + "ERROR_INVALID_GROUP_IDENTIFIER" : "No connection group with identifier \"{IDENTIFIER}\" found", + "ERROR_NO_FILE_SUPPLIED" : "Please select a file to import", + "ERROR_PARSE_FAILURE_CSV" : "Please make sure your file is valid CSV. Parsing failed with error \"{ERROR}\". ", + "ERROR_PARSE_FAILURE_JSON" : "Please make sure your file is valid JSON. Parsing failed with error \"{ERROR}\". ", + "ERROR_PARSE_FAILURE_YAML" : "Please make sure your file is valid YAML. Parsing failed with error \"{ERROR}\". ", + "ERROR_REJECT_UPDATE_CONNECTION" : "Disallowed update to existing connection at \"{PATH}\"", + "ERROR_REQUIRED_NAME" : "No connection name found in the provided file", + "ERROR_REQUIRED_PROTOCOL" : "No connection protocol found in the provided file", "FIELD_PLACEHOLDER_FILTER" : "@:APP.FIELD_PLACEHOLDER_FILTER", + "FIELD_HEADER_REPLACE_CONNECTIONS" : "Replace/Update existing connections", + "FIELD_HEADER_REPLACE_PERMISSIONS" : "Reset permissions", + + "FIELD_OPTION_DUPLICATE_REPLACE" : "Replace duplicates", + "FIELD_OPTION_DUPLICATE_IGNORE" : "Ignore duplicates", + "FIELD_OPTION_DUPLICATE_ERROR" : "Disallow duplicates", + "HELP_CSV_DESCRIPTION" : "A connection import CSV file has one connection record per row. Each column will specify a connection field. At minimum the connection name and protocol must be specified.", "HELP_CSV_EXAMPLE" : "name,protocol,hostname,group,users,groups,guacd-encryption (attribute)\nconn1,vnc,conn1.web.com,ROOT,guac user 1;guac user 2,Connection 1 Users,none\nconn2,rdp,conn2.web.com,ROOT/Parent Group,guac user 1,,ssl\nconn3,ssh,conn3.web.com,ROOT/Parent Group/Child Group,guac user 2;guac user 3,,\nconn4,kubernetes,,,,,", "HELP_CSV_MORE_DETAILS" : "The CSV header for each row specifies the connection field. The connection group ID that the connection should be imported into may be directly specified with \"parentIdentifier\", or the path to the parent group may be specified using \"group\" as shown below. In most cases, there should be no conflict between fields, but if needed, an \" (attribute)\" or \" (parameter)\" suffix may be added to disambiguate. Lists of user or user group identifiers must be semicolon-separated.¹", - "HELP_FILE_TYPE_DESCRIPTION" : "Three file types are supported for connection import: CSV, JSON, and YAML. The same data may be specified by each file type. This must include the connection name and protocol. Optionally, a connection group location, a list of users and/or user groups to grant access, connection parameters, or connection protocols may also be specified. Any users or user groups that do not exist in the current data source will be automatically created.", + "HELP_FILE_TYPE_DESCRIPTION" : "Three file types are supported for connection import: CSV, JSON, and YAML. The same data may be specified by each file type. This must include the connection name and protocol. Optionally, a connection group location, a list of users and/or user groups to grant access, connection parameters, or connection protocols may also be specified. Any users or user groups that do not exist in the current data source will be automatically created. Note that any existing connection permissions will not be removed for updated connections.", "HELP_FILE_TYPE_HEADER" : "File Types", "HELP_JSON_DESCRIPTION" : "A connection import JSON file is a list of connection objects. At minimum the connection name and protocol must be specified in each connection object.", "HELP_JSON_EXAMPLE" : "[\n \\{\n \"name\": \"conn1\",\n \"protocol\": \"vnc\",\n \"parameters\": \\{ \"hostname\": \"conn1.web.com\" \\},\n \"parentIdentifier\": \"ROOT\",\n \"users\": [ \"guac user 1\", \"guac user 2\" ],\n \"groups\": [ \"Connection 1 Users\" ],\n \"attributes\": \\{ \"guacd-encryption\": \"none\" \\}\n \\},\n \\{\n \"name\": \"conn2\",\n \"protocol\": \"rdp\",\n \"parameters\": \\{ \"hostname\": \"conn2.web.com\" \\},\n \"group\": \"ROOT/Parent Group\",\n \"users\": [ \"guac user 1\" ],\n \"attributes\": \\{ \"guacd-encryption\": \"none\" \\}\n \\},\n \\{\n \"name\": \"conn3\",\n \"protocol\": \"ssh\",\n \"parameters\": \\{ \"hostname\": \"conn3.web.com\" \\},\n \"group\": \"ROOT/Parent Group/Child Group\",\n \"users\": [ \"guac user 2\", \"guac user 3\" ]\n \\},\n \\{\n \"name\": \"conn4\",\n \"protocol\": \"kubernetes\"\n \\}\n]", - "HELP_JSON_MORE_DETAILS" : "The connection group ID that the connection should be imported into may be directly specified with a \"parentIdentifier\" field, or the path to the parent group may be specified using a \"group\" field as shown below. An array of user and user group identifiers to grant access to may be specified per connection.", + "HELP_JSON_MORE_DETAILS" : "The connection group ID that the connection should be imported into may be directly specified with a \"parentIdentifier\" field, or the path to the parent group may be specified using a \"group\" field as shown below. An array of user and user group identifiers to grant access to may be specified per connection.", + "HELP_REPLACE_CONNECTION_TITLE" : "Replacing existing connections", + "HELP_REPLACE_CONNECTION_CONTENT" : "Checking this box will allow existing connections to be updated, if an imported connection has the same name and parent group as an existing connection. If unchecked, attempts to update existing connections will be treated as an error.", + "HELP_REPLACE_PERMISSION_TITLE" : "Replacing connection permissions", + "HELP_REPLACE_PERMISSION_CONTENT" : "If replacement of existing connections is enabled, checking this box will allow full replacement of connection permissions. If checked, access permission will only be granted to users and groups specified in the import file for this connection. If unchecked, specified users and groups will be granted access in addition to any existing permissions.", Review Comment: I'd recommend: > Fully reset the permissions granted for all connections in the provided file to the permissions specified in that file. If no permissions are specified, all relevant connection permissions will be revoked. If unchecked, existing permissions are preserved, and any permissions specified in the file will be added. -- 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. To unsubscribe, e-mail: dev-unsubscr...@guacamole.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
