[backend/frontend] Add new IMEI, ICCID and IMSI Observables (#3182)#14237
[backend/frontend] Add new IMEI, ICCID and IMSI Observables (#3182)#14237
Conversation
|
thanks @lndrtrbn ! The e2e test you added is a good start. We need to expand the suite and test more observables types. |
...t/src/private/components/observations/stix_cyber_observables/StixCyberObservableCreation.jsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
1. Global approach
To manage IMEI, ICCID and IMSI values correctness in frontend: code does not make any implicit transformation, user has to enter a valid value without any dashes. If any other character than number is entered or length is invalid, the input is red with an error message on it, the form cannot be submitted.
It's what you have done already for ICCID and IMSI, need to do the same for IMEI.
2. Client Python
EDIT: this part is done
Code looks good to me, just the documentation in constans.py can be extended a bit to be similar to other observables.
For example for IMEI:
"""IMEI observable.
Represents an International Mobile Equipment Identity which is a phone serial number.
:param value: The IMEI value (required)
:type value: str
:param spec_version: STIX specification version, fixed to "2.1"
:type spec_version: str
:param object_marking_refs: List of marking definition references
:type object_marking_refs: list
"""3. Frontend
EDIT: this part is done
Most of the changes are good, in the STIX Cyber Observable creation form (StixCyberObservableCreation.jsx):
- Simplify the regex that validate the IMEI value, only digits allowed, regex should be the same in frontend and backend,
- Remove the transformation of IMEI value in the submit function.
Add e2e tests that, for each new observable type: create one, create multiple in bulk.
For e2e tests, I have added a commit that contains an example of observable creation, I will also add a test for bulk creation for example.
When modifications are done, verify if some translations are no more used and remove them.
4. Backend
Verify that stix converter is ok in stix-2-1-converter.ts. I'll ask confirmation but I think the following is enough (@SouadHadjiat do you confirm?):
const convertIMEIToStix = (instance: StoreCyberObservable, type: string): SCO.StixIMEI => {
assertType(ENTITY_IMEI, type);
const stixCyberObject = buildStixCyberObservable(instance);
return {
...stixCyberObject,
value: instance.value,
// No need for the rest as it's already present in extension
// create by the function buildStixCyberObservable
};
};If my thoughts are confirmed, make the changes for the 3 observables.
On the same topic of stix conversion in stix-2-1-sco.d.ts. If I am correct, classes can be simplified to:
export interface StixIMEI extends StixCyberObject {
value: string;
}ABout tests, the test suite on observable CRUD operation is good. It would be nice to put them in separate files instead of all in stixCyberObservable-test.js as it is already a huge file.
I suggest having 3 files: imei-test.ts, imsi-test.ts, iccid-test.ts in the same folder as stixCyberObservable-test.js.
Note that new files are in TypeScript.
And on each file add some tests on the new relationships that can be created. There is examples in the file stixCoreRelationship-test.js.
As you can see there is not that much to change, mainly tests to add.
Thank you! You can ping me here when modifications are done.
...t/src/private/components/observations/stix_cyber_observables/StixCyberObservableCreation.jsx
Outdated
Show resolved
Hide resolved
...t/src/private/components/observations/stix_cyber_observables/StixCyberObservableCreation.jsx
Outdated
Show resolved
Hide resolved
| }, | ||
| }; | ||
| }; | ||
| const convertIMSIToStix = (instance: StoreCyberObservable, type: string): SCO.StixIMSI => { |
There was a problem hiding this comment.
Same remark for simplification
opencti-platform/opencti-graphql/tests/03-integration/02-resolvers/stixCyberObservable-test.js
Outdated
Show resolved
Hide resolved
|
|
||
| // Custom object extension - IMEI | ||
| // value | ||
| export interface StixIMEI extends StixCyberObject { |
There was a problem hiding this comment.
@SouadHadjiat cannot it be simplified with:
export interface StixIMEI extends StixCyberObject {
value: string;
}as the other props are already defined in extension of StixCyberObject
|
|
||
| // Custom object extension - ICCID | ||
| // value | ||
| export interface StixICCID extends StixCyberObject { |
|
|
||
| // Custom object extension - IMSI | ||
| // value | ||
| export interface StixIMSI extends StixCyberObject { |
There was a problem hiding this comment.
Pull request overview
This pull request adds support for three new observable types - IMEI (International Mobile Equipment Identity), ICCID (Integrated Circuit Card Identifier), and IMSI (International Mobile Subscriber Identity) - to the OpenCTI platform. This is a mirror of PR #13588 with identified issues being addressed. The implementation includes the observable types themselves, validation logic, GraphQL schema definitions, frontend UI components, and Python client support. Additionally, it defines relationships between these observables and with existing types like phone numbers and MAC addresses.
Changes:
- Added three new observable types (IMEI, ICCID, IMSI) with validation, schema definitions, and conversion logic across backend, frontend, and Python client
- Implemented bidirectional and unidirectional relationships between the new observables and existing types (phone numbers, MAC addresses)
- Added comprehensive backend integration tests for create, read, update, and delete operations on the new observables
Reviewed changes
Copilot reviewed 37 out of 39 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| opencti-platform/opencti-graphql/src/utils/syntax.js | Added regex validators for IMEI (15-16 digits), ICCID (18-22 digits), and IMSI (14-15 digits) |
| opencti-platform/opencti-graphql/src/schema/stixCyberObservable.ts | Registered new observable type constants |
| opencti-platform/opencti-graphql/src/database/stix.ts | Added relationship mappings between new observables and existing types |
| opencti-platform/opencti-graphql/config/schema/opencti.graphql | Added GraphQL type definitions and input types for IMEI, ICCID, and IMSI |
| opencti-platform/opencti-graphql/tests/03-integration/02-resolvers/stixCyberObservable-test.js | Added comprehensive CRUD tests for all three new observable types |
| opencti-platform/opencti-front/src/private/components/observations/stix_cyber_observables/StixCyberObservableCreation.jsx | Added form validation and IMEI special character handling |
| opencti-platform/opencti-front/src/utils/Colors.js | Added color mappings for new observable types |
| opencti-platform/opencti-front/lang/front/*.json | Added translations for new observable types and validation messages in 7 languages |
| opencti-platform/opencti-front/tests_e2e/observables/emailMessage.spec.ts | Added E2E test file (unrelated to this PR) |
| client-python/pycti/utils/constants.py | Added custom observable class definitions for IMEI, ICCID, and IMSI |
| client-python/pycti/entities/opencti_stix_cyber_observable.py | Added creation logic for new observable types |
...nt/src/private/components/observations/stix_cyber_observables/StixCyberObservableDetails.tsx
Outdated
Show resolved
Hide resolved
opencti-platform/opencti-graphql/tests/03-integration/02-resolvers/stixCyberObservable-test.js
Outdated
Show resolved
Hide resolved
2e1b10d to
1ec253d
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14237 +/- ##
==========================================
- Coverage 32.36% 25.71% -6.66%
==========================================
Files 3097 3097
Lines 210976 211165 +189
Branches 38233 35460 -2773
==========================================
- Hits 68280 54296 -13984
- Misses 142696 156869 +14173
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2ea5a15 to
1262efe
Compare
9e4176f to
c21f2e0
Compare
Proposed changes
Related issues
Checklist
Further comments
This PR initial state is a mirror of #13588 by @scarletmerlin123
We identified some problems that need to be addressed before merging this new feature again.