Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ELY-2082] Optimise Tool Help Text #1713

Merged
merged 1 commit into from
Sep 15, 2024
Merged

[ELY-2082] Optimise Tool Help Text #1713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
<version.net.sourceforge.htmlunit.htmlunit>2.40.0</version.net.sourceforge.htmlunit.htmlunit>
<version.org.apache.santuario>2.3.0</version.org.apache.santuario>
<version.org.keycloak.keycloak-services>3.1.0.Final</version.org.keycloak.keycloak-services>
<version.org.aesh>2.7</version.org.aesh>

<test.level>INFO</test.level>
<!-- Checkstyle configuration -->
Expand Down Expand Up @@ -1001,6 +1002,11 @@
<artifactId>jose4j</artifactId>
<version>${version.org.bitbucket.b_c.jose4j}</version>
</dependency>
<dependency>
<groupId>org.aesh</groupId>
<artifactId>aesh</artifactId>
<version>${version.org.aesh}</version>
</dependency>

<!--
Test Scope Only
Expand Down
4 changes: 4 additions & 0 deletions tool/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,10 @@
<artifactId>jboss-logging-processor</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.aesh</groupId>
<artifactId>aesh</artifactId>
</dependency>
<!--
Test Scope Only
-->
Expand Down
21 changes: 13 additions & 8 deletions tool/src/main/java/org/wildfly/security/tool/CredentialStoreCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.CommandLineParser;
import org.apache.commons.cli.DefaultParser;
import org.apache.commons.cli.HelpFormatter;
import org.apache.commons.cli.Option;
import org.apache.commons.cli.OptionGroup;
import org.apache.commons.cli.Options;
Expand All @@ -69,6 +68,10 @@
import org.wildfly.security.password.interfaces.ClearPassword;
import org.wildfly.security.pem.Pem;
import org.wildfly.security.ssh.util.SshUtil;
import org.wildfly.security.tool.help.DescriptionSection;
import org.wildfly.security.tool.help.HelpCommand;
import org.wildfly.security.tool.help.OptionsSection;
import org.wildfly.security.tool.help.UsageSection;

import static org.wildfly.security.tool.Params.ALIAS_PARAM;
import static org.wildfly.security.tool.Params.CREATE_CREDENTIAL_STORE_PARAM;
Expand Down Expand Up @@ -959,13 +962,15 @@ protected Set<String> aliases() {
*/
@Override
public void help() {
HelpFormatter help = new HelpFormatter();
help.setWidth(WIDTH);
help.printHelp(ElytronToolMessages.msg.cmdHelp(getToolCommand(), CREDENTIAL_STORE_COMMAND),
ElytronToolMessages.msg.cmdLineCredentialStoreHelpHeader().concat(ElytronToolMessages.msg.cmdLineActionsHelpHeader()),
options,
"",
true);
OptionsSection optionsSection = new OptionsSection(ElytronToolMessages.msg.cmdLineActionsHelpHeader(), options);
UsageSection usageSection = new UsageSection(CREDENTIAL_STORE_COMMAND, null);
DescriptionSection descriptionSection = new DescriptionSection(ElytronToolMessages.msg.cmdLineCredentialStoreHelpHeader());
HelpCommand helpCommand = HelpCommand.HelpCommandBuilder.builder()
.description(descriptionSection)
.usage(usageSection)
.options(optionsSection)
.build();
helpCommand.printHelp();
}

static Map<String, String> parseCredentialStoreProperties(final String attributeString) {
Expand Down
37 changes: 28 additions & 9 deletions tool/src/main/java/org/wildfly/security/tool/ElytronTool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,17 @@
import org.apache.commons.cli.AlreadySelectedException;
import org.apache.commons.cli.Option;
import org.wildfly.security.WildFlyElytronProvider;
import org.wildfly.security.tool.help.CommandsSection;
import org.wildfly.security.tool.help.DescriptionSection;
import org.wildfly.security.tool.help.HelpCommand;
import org.wildfly.security.tool.help.OptionsSection;
import org.wildfly.security.tool.help.UsageSection;

import java.security.Security;
import java.util.HashMap;
import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;

/**
* Elytron Tool main class which drives all registered commands.
Expand Down Expand Up @@ -131,15 +138,27 @@ private static void configureLogManager() {
}

private void generalHelp() {
System.out.print(ElytronToolMessages.msg.generalHelpTitle());
System.out.println();
for (Command c: commandRegistry.values()) {
if (scriptName != null) {
c.setToolCommand(scriptName);
}
c.help();
System.out.println();
}
DescriptionSection descriptionSection = new DescriptionSection(ElytronToolMessages.msg.cmdElytronToolDescription());
UsageSection usageSection = new UsageSection(null, null);
OptionsSection optionsSection = new OptionsSection(ElytronToolMessages.msg.generalHelpOptionsOpening(), null);

// Using SortedMap so commands are in alphabetical order
SortedMap<String, String> commandsMap = new TreeMap<>();
commandsMap.put(CredentialStoreCommand.CREDENTIAL_STORE_COMMAND, ElytronToolMessages.msg.cmdLineCredentialStoreHelpHeader());
commandsMap.put(VaultCommand.VAULT_COMMAND, ElytronToolMessages.msg.cmdVaultHelpHeader());
commandsMap.put(FileSystemRealmCommand.FILE_SYSTEM_REALM_COMMAND, ElytronToolMessages.msg.cmdFileSystemRealmHelpHeader());
commandsMap.put(FileSystemEncryptRealmCommand.FILE_SYSTEM_ENCRYPT_COMMAND, ElytronToolMessages.msg.cmdFileSystemEncryptHelpHeader());
fjuma marked this conversation as resolved.
Show resolved Hide resolved
commandsMap.put(MaskCommand.MASK_COMMAND, ElytronToolMessages.msg.cmdMaskHelpHeader());
commandsMap.put(FileSystemRealmIntegrityCommand.FILE_SYSTEM_REALM_INTEGRITY_COMMAND, ElytronToolMessages.msg.cmdFileSystemIntegrityHelpHeader());
CommandsSection commandsSection = new CommandsSection(commandsMap);

HelpCommand helpCommand = HelpCommand.HelpCommandBuilder.builder()
.description(descriptionSection)
.usage(usageSection)
.options(optionsSection)
.commands(commandsSection)
.build();
helpCommand.printHelp();
}

Command findCommand(String commandName) {
Expand Down
35 changes: 20 additions & 15 deletions tool/src/main/java/org/wildfly/security/tool/ElytronToolMessages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,25 +105,25 @@ public interface ElytronToolMessages extends BasicLogger {
"Provider must be installed through java.security file or through service loader from properly packaged jar file on classpath.")
String cmdLineCustomCredentialStoreProviderDesc();

@Message(id = NONE, value = "Create credential store (Action)")
@Message(id = NONE, value = "* Create credential store")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering if we should have the * at the end instead of the beginning just to be consistent with the way Action was displayed before. Maybe it stands more at the beginning though?

@Skyllarr @petrberan What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there @fjuma , thank you for your review, I'll try to get those resolved asap

Just wanted to point here the UX solution behind the *. The problem with (Action) being at the end is that since all those lines are not the same length, it's very hard for us to find out which command is action and which not, as it requires to read every single line up until the very end of said line. Us humans being very lazy, we don't generally do that :) For that reason, I'd put it in the beginning of the line.

However, since (Action) is just another word, it doesn't stand out enough to be noticed at first. By replacing (Action) with * it's something extra that moves the entire line a little bit, showing exactly which command is action and which not.

In the end, it's all about what you want exactly. If you want user to know which command is action and which not, putting * in the beginning is better. If you prefer user to read the command easily and don't really care about them knowing whether it's an action or not, it could stay in the back

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @petrberan, thanks for the explanation! Putting the * at the beginning makes sense then.

String cmdLineCreateCredentialStoreDesc();

@Message(id = NONE, value = "Credential store type")
String cmdLineCredentialStoreTypeDesc();

@Message(id = NONE, value = "Add new alias to the credential store (Action)")
@Message(id = NONE, value = "* Add new alias to the credential store")
String cmdLineAddAliasDesc();

@Message(id = NONE, value = "Remove alias from the credential store (Action)")
@Message(id = NONE, value = "* Remove alias from the credential store")
String cmdLineRemoveAliasDesc();

@Message(id = NONE, value = "Check if alias exists within the credential store (Action)")
@Message(id = NONE, value = "* Check if alias exists within the credential store")
String cmdLineCheckAliasDesc();

@Message(id = NONE, value = "Display all aliases (Action)")
@Message(id = NONE, value = "* Display all aliases")
String cmdLineAliasesDesc();

@Message(id = NONE, value = "Display all types of stored credentials for given alias (Action)")
@Message(id = NONE, value = "* Display all types of stored credentials for given alias")
String cmdLineAliasTypes();

@Message(id = NONE, value = "Generate private and public key pair and store them as a KeyPairCredential")
Expand Down Expand Up @@ -159,7 +159,7 @@ public interface ElytronToolMessages extends BasicLogger {
@Message(id = NONE, value = "Print summary, especially command how to create this credential store")
String cmdLinePrintSummary();

@Message(id = NONE, value = "Get help with usage of this command (Action)")
@Message(id = NONE, value = "* Get help with usage of this command")
String cmdLineHelp();

@Message(id = NONE, value = "Alias \"%s\" exists")
Expand Down Expand Up @@ -281,7 +281,7 @@ public interface ElytronToolMessages extends BasicLogger {
@Message(id = NONE, value = "CLI command to add new credential store:%n")
String cliCommandToNewCredentialStore();

@Message(id = NONE, value = "Bulk conversion with options listed in description file. All options have no default value and should be set in the file. (Action)%n" +
@Message(id = NONE, value = "* Bulk conversion with options listed in description file. All options have no default value and should be set in the file.%n" +
"All options are required with the exceptions:%n" +
" - \"properties\" option%n - \"type\" option (defaults to \"KeyStoreCredentialStore\")%n - \"credential-store-provider\" option%n - \"other-providers\" option%n" +
" - \"salt\" and \"iteration\" options can be omitted when plain-text password is used%n" +
Expand Down Expand Up @@ -406,7 +406,7 @@ public interface ElytronToolMessages extends BasicLogger {
String longOptionDescription(String option, String longOption);

// filesystem-realm command
@Message(id = NONE, value = "'FileSystemRealm' command is used to convert legacy properties files and scripts to an Elytron FileSystemRealm.")
@Message(id = NONE, value = "\"filesystem-realm\" command is used to convert legacy properties files and scripts to an Elytron FileSystemRealm.")
String cmdFileSystemRealmHelpHeader();

@Message(id = NONE, value = "The relative or absolute path to the users file.")
Expand Down Expand Up @@ -489,7 +489,7 @@ public interface ElytronToolMessages extends BasicLogger {
@Message(id = NONE, value = "Name of the security-domain to be configured.")
String cmdFileSystemRealmSecurityDomainNameDesc();

@Message(id = NONE, value = "Bulk conversion with options listed in description file. Optional options have default values, required options do not. (Action) %n" +
@Message(id = NONE, value = "* Bulk conversion with options listed in description file. Optional options have default values, required options do not.%n" +
"The options fileSystemRealmName and securityDomainName are optional. %n" +
"These optional options have default values of: converted-properties-filesystem-realm and converted-properties-security-domain. %n" +
"Values are required for the following options: users-file, roles-file, and output-location. %n" +
Expand All @@ -498,7 +498,7 @@ public interface ElytronToolMessages extends BasicLogger {
"Blocks of options must be separated by a blank line.")
String cmdFileSystemRealmBulkConvertDesc();

@Message(id = NONE, value = "Bulk conversion with options listed in description file. Optional options have default values, required options do not. (Action) %n" +
@Message(id = NONE, value = "* Bulk conversion with options listed in description file. Optional options have default values, required options do not. %n" +
"The options realm-name, hash-encoding, levels, secret-key, create, populate, keystore, type, password, password-env, and key-pair are optional. %n" +
"Values are required for the following options: input-location, output-location, and credential-store. %n" +
"The default values of realm-name, hash-encoding, hash-charset, levels, secret-key, create, and populate are encrypted-filesystem-realm, BASE64, UTF-8, 2, key, true, and true respectively. %n" +
Expand All @@ -508,7 +508,7 @@ public interface ElytronToolMessages extends BasicLogger {
"Blocks of options must be separated by a blank line.")
String cmdFileSystemRealmEncryptBulkConvertDesc();

@Message(id = NONE, value = "Bulk conversion with options listed in description file. (Action)" +
@Message(id = NONE, value = "* Bulk conversion with options listed in description file. " +
"Optional options have defaults and can be skipped ([type, default_or_NULL]), required options do not (<type>). %n" +
"One of either password or password-env is required. %n" +
"Blocks of options must be separated by a blank line; order is not important. Syntax: %n" +
Expand All @@ -519,7 +519,7 @@ public interface ElytronToolMessages extends BasicLogger {
String cmdFileSystemRealmIntegrityBulkConvertDesc();

// filesystem-realm encrypt command
@Message(id = NONE, value = "'FileSystemRealmEncrypt' command is used to convert non-empty, un-encrypted FileSystemSecurityRealm(s) to encrypted FileSystemSecurityRealm(s) with a SecretKey.")
@Message(id = NONE, value = "\"filesystem-realm-encrypt\" command is used to convert non-empty, un-encrypted FileSystemSecurityRealm(s) to encrypted FileSystemSecurityRealm(s) with a SecretKey.")
String cmdFileSystemEncryptHelpHeader();

@Message(id = NONE, value = "Secret Key was not found in the Credential Store at %s, and populate option was not set. Skipping descriptor file block number %d.")
Expand Down Expand Up @@ -669,7 +669,7 @@ public interface ElytronToolMessages extends BasicLogger {
@Message(id = NONE, value = "Should file %s be overwritten? (y/n) ")
String shouldFileBeOverwritten(String file);

@Message(id = NONE, value = "\nSome of the parameters below are mutually exclusive actions which are marked with (Action) in the description.")
@Message(id = NONE, value = "Some of the parameters below are mutually exclusive actions which are marked with * in the description.")
String cmdLineActionsHelpHeader();

@Message(id = NONE, value = "Key size (bits).")
Expand Down Expand Up @@ -738,8 +738,13 @@ public interface ElytronToolMessages extends BasicLogger {
@Message(id = NONE, value = "No Credential Store location or Secret Key Alias specified.")
MissingOptionException missingCredentialStoreSecretKey();

@Message(id = NONE, value = "To get list of options for a specific command, please specify the command by using ./elytron-tool.sh [command] --help")
String generalHelpOptionsOpening();

@Message(id = NONE, value = "A tool that assists with Elytron configuration")
String cmdElytronToolDescription();

// Numeric Errors
@Message(id = 35, value = "Only one of '%s' and '%s' can be specified at the same time")
IllegalArgumentException mutuallyExclusiveOptions(String first, String second);

}
21 changes: 13 additions & 8 deletions tool/src/main/java/org/wildfly/security/tool/FileSystemEncryptRealmCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,17 @@
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.CommandLineParser;
import org.apache.commons.cli.DefaultParser;
import org.apache.commons.cli.HelpFormatter;
import org.apache.commons.cli.Option;
import org.apache.commons.cli.OptionGroup;
import org.apache.commons.cli.Options;
import org.wildfly.security.auth.realm.FileSystemRealmUtil;
import org.wildfly.security.auth.realm.FileSystemSecurityRealm;
import org.wildfly.security.auth.realm.FileSystemSecurityRealmBuilder;
import org.wildfly.security.password.spec.Encoding;
import org.wildfly.security.tool.help.DescriptionSection;
import org.wildfly.security.tool.help.HelpCommand;
import org.wildfly.security.tool.help.OptionsSection;
import org.wildfly.security.tool.help.UsageSection;

/**
* Elytron-Tool command to convert un-encrypted FileSystemRealms into an encrypted realm with the use of a SecretKey.
Expand Down Expand Up @@ -576,13 +579,15 @@ public void execute(String[] args) throws Exception {
*/
@Override
public void help() {
HelpFormatter help = new HelpFormatter();
help.setWidth(WIDTH);
help.printHelp(ElytronToolMessages.msg.cmdHelp(getToolCommand(), FILE_SYSTEM_ENCRYPT_COMMAND),
ElytronToolMessages.msg.cmdFileSystemEncryptHelpHeader(),
options,
"",
true);
OptionsSection optionsSection = new OptionsSection(ElytronToolMessages.msg.cmdLineActionsHelpHeader(), options);
UsageSection usageSection = new UsageSection(FILE_SYSTEM_ENCRYPT_COMMAND, null);
DescriptionSection descriptionSection = new DescriptionSection(ElytronToolMessages.msg.cmdFileSystemEncryptHelpHeader());
HelpCommand helpCommand = HelpCommand.HelpCommandBuilder.builder()
.description(descriptionSection)
.usage(usageSection)
.options(optionsSection)
.build();
helpCommand.printHelp();
}

/**
Expand Down
21 changes: 13 additions & 8 deletions tool/src/main/java/org/wildfly/security/tool/FileSystemRealmCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.CommandLineParser;
import org.apache.commons.cli.DefaultParser;
import org.apache.commons.cli.HelpFormatter;
import org.apache.commons.cli.Option;
import org.apache.commons.cli.Options;
import org.apache.commons.lang3.ArrayUtils;
Expand All @@ -63,6 +62,10 @@
import org.wildfly.security.password.interfaces.DigestPassword;
import org.wildfly.security.password.spec.DigestPasswordSpec;
import org.wildfly.security.password.spec.PasswordSpec;
import org.wildfly.security.tool.help.DescriptionSection;
import org.wildfly.security.tool.help.HelpCommand;
import org.wildfly.security.tool.help.OptionsSection;
import org.wildfly.security.tool.help.UsageSection;

/**
* Elytron-Tool command to convert legacy properties file into a FileSystemRealm.
Expand Down Expand Up @@ -298,13 +301,15 @@ public void execute(String[] args) throws Exception {
*/
@Override
public void help() {
HelpFormatter help = new HelpFormatter();
help.setWidth(WIDTH);
help.printHelp(ElytronToolMessages.msg.cmdHelp(getToolCommand(), FILE_SYSTEM_REALM_COMMAND),
ElytronToolMessages.msg.cmdFileSystemRealmHelpHeader().concat(ElytronToolMessages.msg.cmdLineActionsHelpHeader()),
options,
"",
true);
OptionsSection optionsSection = new OptionsSection(ElytronToolMessages.msg.cmdLineActionsHelpHeader(), options);
UsageSection usageSection = new UsageSection(FILE_SYSTEM_REALM_COMMAND, null);
DescriptionSection descriptionSection = new DescriptionSection(ElytronToolMessages.msg.cmdFileSystemRealmHelpHeader());
HelpCommand helpCommand = HelpCommand.HelpCommandBuilder.builder()
.description(descriptionSection)
.usage(usageSection)
.options(optionsSection)
.build();
helpCommand.printHelp();
}

@Override
Expand Down
Loading
Loading