Discussion: Changes to include API Key or replace User/Pass

[namachieli]

I have a desire to utilize this pack in place of a BASH script i wrote that functions similarly. To do so, I need to be able to use an API Key for authentication, and I wanted to begin the discussion. (paging @userlocalhost )

In looking at the code for register_st2_config_to_zabbix.py and st2_dispatch.py I believe that it wouldn't be difficult to add API Key functionality even when considering the positional args limitation.

A few points I would like to consider however:

• Would it be preferable to replace User/Pass with API Key only?
• If both can be defined, which should take priority for being used?
• Updating string inputs the be key::val so that positional arguments no longer apply
  • This allows additional args to be passed to the api dynamically as a dictionary instead of list.
  • requires some string parsing and a delimiter like ::
• Media Type - Script Parameters have a max limitation of 255 characters. That is to say, the combined total of all parameters must not exceed 255, not just any single field.
  • I believe that User Macros should be used to help alleviate this limitation
  • st2_dispatch.py#L59 Apparently it stores it in the DB in a single field with VARCHAR(255) as a string as well.
  • Additional benefit that these macros are hidden behind "Super Admin" permissions in the WebGui
• Update event_handler.yaml alert_message to be an object
  • Add logic to in st2_dispatch.py to check for JSON blob or string, and format accordingly.

I would love to hear thoughts and opinions on this, thanks!

[userlocalhost]

Thank you for your supposing.

I need to be able to use an API Key for authentication.

That's great! I affirm your idea to be able to use API key for authentication when st2_dispatch.py is called.

If both can be defined, which should take priority for being used?

I thinks that should be same with st2-auth's specification (I need to confirm which has priority).

Updating string inputs the be key::val so that positional arguments no longer apply

• This allows additional args to be passed to the api dynamically as a dictionary instead of list.

I'm not sure it's a good way. First of all, the reason why I implement it like this is because of the limitation of Zabbix. The order of parameters in a Media type is corresponding to the one of AlertScript. Even in the current design, we can add parameters dynamically by small fix of st2_dispatch.py which allows to receive additional arguments and send them to the StackStorm. And we can also send a structural type value (like dict) by serializing. So I can't agree to implement a new feature to parse complex input parameter to dict to keep implementation simple.

[namachieli]

I'm not sure it's a good way.

Totally understandable. Seeing how it would be possible and assuming you knew and had good reasons why you chose not to, I'm fine with keeping inputs simple.

Thoughts on just migrating to API Key only? This would reduce input complexity and enhance security.
Thoughts on modifying the trigger param alert_message from string to object, and adding logic to sanitize the field in st2_dispatch.py?

[userlocalhost]

To implement a feature to be able to authenticate with either password and API key, I think adding a parameter in MediaType for specifying API key and modifying st2_dispatch.py to receive and handle it are the easiest way to do it.

[namachieli]

So I was playing around with this some and I have some additional data/notes to share.

• All Script Parameters are stored in the database as a single string, delimited by a LF (\n) character. (Not CRLF)
  • Maximum number of characters that can be entered in total for a single script is 255 - (n Parameters) - (sum of entered characters)
  • This means 8 parameters with a total character count of 247, will reach the 255 varchar limit of the DB column.
• Script Parameters can be entered with flags and those flags are parseable by the script
  • when a parameter is entered as -a value the resulting serialized key is "a": " value" (Note the leading space before the value). -avalue is serialized as "a": "value"
  • Tested with some dummy flags and found that a mix of flagged args and positional args do behave as desired.
  • Each flagged arg was assigned the appropriate key, and all remaining positional args were parsed as such.
  • All behaviors noted are consistent with the logic as written for st2_dispatch.py (working as designed)
• User defined macros are not available to Script Parameters

Given the weirdness in which parameters would need to be input for any key/value logic to succeed (which i agree is not simple and not desireable), I propose the following change to parameters

(positionally)

• JSON Dict containing possible keys of api_url, auth_url, st2_userid, st2_passwd, st2_apikey
• {ALERT.SENDTO}
• {ALERT.SUBJECT}
• {ALERT.MESSAGE}

While this makes the first field a little more complex, it does so in a common way, that is easily expandable over time as new keys or information needs to be passed. I can modify st2_dispatch.py to assume this dict as the first arg, and add logic to consider the combination of keys that are required at a minimum, and tie breaker if both are present.

This also maintains the ability to have custom sendto values on a per user basis, and a dynamic number of additional string arguments parsed into a list.

Thoughts @userlocalhost ?

[namachieli]

I intend to move forward with a PR to modify script parameters as outlined above (JSON Blob with 3 macros).

[userlocalhost]

First of all, I agree with your feature to be able to use API Key for the authentication of StackStorm.

While this makes the first field a little more complex, it does so in a common way, that is easily expandable over time as new keys or information needs to be passed.

But I doubt that your idea that changes script parameters of Media Type from string to dict really makes user happy.

I concern that it becomes more difficult for user to change script parameters. Please imagine the situation when user want to change script parameters (e.g. st2_apikey, api_url etc). User have to read long serialized dict value and understand dict structure, then change them with taking care of keeping JSON format. Once user inadvertently delete some character (like double quotation " or comma ,) unconsciously, user must suffer from parse error and it probably makes user annoy.

Please understand that I don't intend to stand in your way. Basically, I think how to implement feature is left to the programmer who implement it. But this would make major change of user-experience. So we should be more careful about it.

My opinion about how to implement it is https://github.com/StackStorm-Exchange/stackstorm-zabbix/issues/30#issuecomment-485231175 (It simply adds another parameter for API Key at the Script parameters on the MediaType of StackStorm in Zabbix).

[userlocalhost]

Or, one of the most conservative way to implement it is to make password parameter of MediaType interchangeable with API Key. In this case, backward compatibility would be keeped because we don't have to change the format of MediaType. And PR would also be simple because you only have to change st2_dispatch.py to check passed parameter is whether it is an API Key or Password.

[namachieli]

But this would make major change of user-experience. So we should be more careful about it.

What about allowing for backwards compatibility? We check for first args to be JSON Dict

• if true we take the next 3 args as well known, and the rest as list of strings.
• if false we leave existing logic in place.

This allows the default to behave as it does today and allows backwards compatibility, and simplicity for users that want it that way. It also enables advanced usage for users that want to do that.

Please understand that I don't intend to stand in your way.

I do, but I want to find a way that works for more than just my way, and I value your input as pack creator. :)

one of the most conservative way to implement it is to make password parameter of MediaType interchangeable with API Key

Considering that, I'm not sure this is a good way. Whiles its not difficult to recognize a fixed length API key, it creates odd cases where a password of exactly that length are misinterpreted, or if the core st2 API Key logic changes, it requires someone to remember to update this pack.

I would love other people input as well.

[namachieli]

one of the most conservative way to implement it is to make password parameter of MediaType interchangeable with API Key

Something i realized is once you add the full length url for api and auth, and try to add an API key for password, you start to approach the 255 max total character limit.

I have a slightly longer than average url for my stackstorm instance (multi domain naming) and I would exceed the 255 character limit this way.

using a json dict doesn't necessarily solve this, but with simple structure and short key names it can help.

[namachieli]

I still have some other cleanup and documentation updates, but you can take a look at the meat of the changes so far and review while I update README, CHANGELOG, and tweak some other bits.

c061c59302a41b6ac4d5eac71b3470e39b914ce7