fortify / fcli

fcli is a command-line utility for interacting with various Fortify products
https://fortify.github.io/fcli/
Other
30 stars 16 forks source link

--add-users, --rm-users #339

Open xakrurychle opened 1 year ago

xakrurychle commented 1 year ago

--add-users fails completely when two names are specified at once, --rm-users passes but only deletes the first one mentioned.

$ fcli ssc appversion update FCLIV3:FCLIV3UPDATED --add-users "abel, karim"

java.lang.IllegalArgumentException: The following auth entities cannot be found: karim at com.fortify.cli.ssc.entity.user.helper.SSCAuthEntitySpecPredicate.checkUnmatched(SSCAuthEntitySpecPredicate.java:71) at com.fortify.cli.ssc.entity.user.helper.SSCAuthEntitiesHelper.getAuthEntities(SSCAuthEntitiesHelper.java:45) at com.fortify.cli.ssc.entity.appversion_user.helper.SSCAppVersionAuthEntitiesUpdateBuilder.add(SSCAppVersionAuthEntitiesUpdateBuilder.java:90) at com.fortify.cli.ssc.entity.appversion.cli.cmd.SSCAppVersionUpdateCommand.getUserUpdateRequest(SSCAppVersionUpdateCommand.java:86) at com.fortify.cli.ssc.entity.appversion.cli.cmd.SSCAppVersionUpdateCommand.getJsonNode(SSCAppVersionUpdateCommand.java:63) at com.fortify.cli.ssc.output.cli.cmd.AbstractSSCJsonNodeOutputCommand.getJsonNode(AbstractSSCJsonNodeOutputCommand.java:23) at com.fortify.cli.common.output.cli.cmd.AbstractOutputCommand.run(AbstractOutputCommand.java:33) at picocli.CommandLine.executeUserObject(CommandLine.java:2104) at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2539) at picocli.CommandLine$RunLast.handle(CommandLine.java:2531) at picocli.CommandLine$RunLast.handle(CommandLine.java:2493) at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2351) at picocli.CommandLine$RunLast.execute(CommandLine.java:2495) at picocli.CommandLine.execute(CommandLine.java:2248) at com.fortify.cli.app.FortifyCLI.execute(FortifyCLI.java:74) at com.fortify.cli.app.FortifyCLI.main(FortifyCLI.java:56)

$ fcli ssc appversion update FCLIV3:FCLIV3UPDATED --rm-users "abel, karim" WARN: The following auth entities are not being removed as they are not assigned to the application version: karim Id Application name Name Issue template name Created by Action 10028 FCLIV3 FCLIV3UPDATED Prioritized High Risk Issue Template krystof UPDATED

fcli version 0.20230629.082654-dev_develop, built on 2023-06-29 08:27:42

rsenden commented 1 year ago

@xakrurychle I assume this error is caused by having a space after the comma; correct syntax would be --add-users abel,karim (no need for the quotes anymore). Can you please try?

Potentially fcli could just trim whitespace before or after each user name, which would allow the example commands above to succeed.

However, we wouldn't be able to handle usernames with leading or trailing spaces anymore. SSC (23.1.1) apparently does allow for creating usernames with leading or trailing spaces, but it's very unlikely that anyone would purposely do this, and anyway SSC itself doesn't properly handle such usernames; I guess the SSC login page trims the username; I've been unable to log in to the SSC web interface with a username containing leading or trailing spaces. I guess this could be considered an SSC bug; SSC probably should disallow creating usernames with leading or trailing spaces in the first place.

For consistency, if we implement this enhancement. we should do the same for all other fcli options that accept a list of comma-separated values. We need to check whether there's a generic way for handling this for all options, or whether we need to implement trimming functionality in all commands that accept such options.

MikeTheSnowman commented 1 year ago

@rsenden, +1 for auto-trimming of whitespace after a comma, provided that the comma separated list of values is surrounded by quotes. Though, I imagine that if it is surrounded in quotes, that will be transparent to us. I also believe that leading whitespace for an SSC username is a bug and it's something that we probably shouldn't encourage anyways.

rsenden commented 1 year ago

@MikeTheSnowman Indeed, whether the value is surrounded by quotes is transparent to us as this is handled by the shell; we just see a single option value that potentially contains spaces. Preferably we should handle trimming of comma-separated values in a generic way (automatically applying this behavior to all options that accept comma-separated values), but this requires some research; not sure whether this is possible. If not, we'd need to implement this for every individual option.

xakrurychle commented 1 year ago

fcli ssc appversion update FCLIV3:FCLIV3UPDATED --add-users "abel, karim"

Hi, provided there are no spaces in the list of the users passed as params to either --add-users or --rm-users, everything works fine. What I was disputing at that time was that--rm-users worked (though only partially as it deleted the first name but not the second) while --add-users did not work at all.

rsenden commented 1 year ago

Related to, and potentially superseded by #369