Document attr behavior: type and value are required but category is set by default

[chrisinmtown]

The PyMISP API should document somewhere that an attribute must have a type and a value, but if no category is supplied, a default value is set based on the type. I think this is only done at the server after calling add_attribute but not 100% certain. Maybe in the MISPAttribute class init method?

And if you don't mind educating me on one of the finer points here, please explain where those .type and .value fields are defined as valid. I checked classes in mispevent.py and abstract.py but could not find those field/attribute definitions, must be blind today.

[Rafiot]

It is a bit hidden, the default values set by PyMISP for the attributes are defined there: https://github.com/MISP/PyMISP/blob/main/pymisp/data/describeTypes.json#L527

And the file is opened once globally there: https://github.com/MISP/PyMISP/blob/main/pymisp/abstract.py#L37

[chrisinmtown]

Thanks @Rafiot I think you have beautifully confirmed the need for documentation here lol

[chrisinmtown]

Maybe a dumb question, but it appears that the constructor/init method doesn't accept any values, that we all have to build a minimal attribute like this:

            new_attribute = MISPAttribute()
            new_attribute.value = '1.2.3.4'
            new_attribute.type = 'ip-dst'

why not like this?

            new_attribute = MISPAttribute(value='1.2.3.4', type='ip-dst')

Is this in agreement with design principles you're following in PyMISP? Should I write an issue requesting this enhancement?

[Rafiot]

The reason I didn't implement it this way is because for other classes, we can have a lot more parameters: if you take MISPObject for example, or MISPEvent, adding all the possible parameters is a nightmare. The expected way to populate the data is to use from_dict.

In the case of MISPAttribute, you would have a problem when someone will ask how to add tags for example. That's opening a pandora box I'd rather not open.

The compromise I went for is to allow creating attributes in one shot in the add_attribute methods (and the missing parameters fallback to the sane default).

In short, adding that feature only makes sense if all the classes support the same thing, otherwise, it breaks the logic of the library and ultimately makes it more error prone.

[chrisinmtown]

I propose this minimal docstring change

diff --git a/pymisp/mispevent.py b/pymisp/mispevent.py
index 8a23a1d..63e7339 100644
--- a/pymisp/mispevent.py
+++ b/pymisp/mispevent.py
@@ -189,7 +189,8 @@ class MISPAttribute(AbstractMISP):
                              'first_seen', 'last_seen'}
 
     def __init__(self, describe_types: Optional[Dict] = None, strict: bool = False):
-        """Represents an Attribute
+        """Represents an Attribute. The type and value fields are required;
+        if the category is not set, the server will set the category based on the type.
 
         :param describe_types: Use it if you want to overwrite the default describeTypes.json file (you don't)
         :param strict: If false, fallback to sane defaults for the attribute type if the ones passed by the user are incorrect

[chrisinmtown]

@Rafiot please consider the docstring proposal, if you can add something like it then this issue can be closed, thanks