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.

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

Feature/change tactis and techniques resources #3346

Conversation

eze9252
Copy link
Contributor

@eze9252 eze9252 commented Jun 7, 2021

Merge whit mitre intellingence section

Closes #3290

Comment on lines 150 to 164
getArrayFormatted(arrObj) {
try {
arrObj.map(obj => {
return ( <EuiToolTip
position="top"
content={"Open " + obj.name + " details in a new page"}>
<EuiLink href="" external target="_blank">
{obj.name}
</EuiLink>
</EuiToolTip>)
})
}
catch{
return ""
}
Copy link
Member

Choose a reason for hiding this comment

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

Should this function return something or there is some I can't undestand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is obsolete. I delete now

@Desvelao
Copy link
Member

Desvelao commented Jun 8, 2021

Testing:

Clicking in the link of flyout redirect to the Intelligence section
image
image

Should we display the resource information (the flyout) instead of applying a filter?

Comment on lines 306 to 311
openIntelligence(e,redirectTo,itemId){
sessionStorage.setItem("tabRedirect", redirectTo);
sessionStorage.setItem("idToRedirect", itemId);
this.props.onSelectedTabChanged('intelligence');
}

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could do the redirection through a query param in the URL instead of using the sessionStorage.

Copy link
Contributor Author

@eze9252 eze9252 Jun 8, 2021

Choose a reason for hiding this comment

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

Yes we should show the flyout, it was my mistake. I fix this

import {
EuiBasicTableColumn,
EuiButtonIcon,
EuiFlyout,
Copy link
Contributor

Choose a reason for hiding this comment

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

Different indetation inside the import

let contador = 0;
array.forEach((element) => {
//The character limit of a request is 8190, if the characters of our request exceed 8100, we divide said call into several to avoid errors
if(namesContatenated[contador].length + element.length >= 8100)
Copy link
Contributor

Choose a reason for hiding this comment

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

Apply prettier

useEffect(() => {
const redirectTab = sessionStorage.getItem("tabRedirect")
const idToRedirect = sessionStorage.getItem("idToRedirect")
if(redirectTab && idToRedirect){
Copy link
Contributor

Choose a reason for hiding this comment

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

Apply prettier



useEffect(() => {
const redirectTab = sessionStorage.getItem("tabRedirect")
Copy link
Contributor

Choose a reason for hiding this comment

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

semicolon


useEffect(() => {
const redirectTab = sessionStorage.getItem("tabRedirect")
const idToRedirect = sessionStorage.getItem("idToRedirect")
Copy link
Contributor

Choose a reason for hiding this comment

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

semicolon

Copy link
Contributor

@gabiwassan gabiwassan left a comment

Choose a reason for hiding this comment

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

Some suggestions 👍🏼


export const ModuleMitreAttackIntelligenceAllResources = withGuard(({didSearch}) => !didSearch, ModuleMitreAttackIntelligenceAllResourcesWelcome)(({ results, loading }) => {
const [isDetailsOpen, setIsDetailsOpen] = useState(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should you use types for useStates like:
const [isDetailsOpen, setIsDetailsOpen] = useState<boolean>(false);
Same for the item if it possible create an interface WzItem { ... } and assigne that.

},[]);

const rowPropsFlyout = useCallback((item) => ({
// 'data-test-subj': `row-${file}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Clear this comments 🤓

Comment on lines 128 to 129
// maxWidth="70%"
// className=""
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments 🤓

@@ -177,29 +181,47 @@ export const Techniques = withWindowSize(class Techniques extends Component {
}
}

async getMitreTechniques () {
const data = await WzRequest.apiReq('GET', '/mitre/techniques', {}).then(res => res).catch(err => mitreTechniques);
const result = (((data || {}).data || {}).data || {}).affected_items;
Copy link
Contributor

Choose a reason for hiding this comment

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

it will throw if data comes from the catch
.catch(err => mitreTechniques);
I think there is needed a better error handling

Comment on lines 213 to 214
.filter(techniqueID => this.state.filteredTechniques ? this.state.filteredTechniques.includes(techniqueID.id) : techniqueID.id)
.map(techniqueID => {
Copy link
Contributor

Choose a reason for hiding this comment

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

replace techniqueID with technique

Comment on lines 306 to 311
openIntelligence(e,redirectTo,itemId){
sessionStorage.setItem("tabRedirect", redirectTo);
sessionStorage.setItem("idToRedirect", itemId);
this.props.onSelectedTabChanged('intelligence');
window.location.href = window.location+`&tabRedirect=${redirectTo}&idToRedirect=${itemId}`
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why it is storing and sending in the URL the same data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the storing was not used, now I delete it

@CPAlejandro
Copy link
Contributor

I have just found a problem that can be fixed paginating the query of techniques
image

Copy link
Contributor

@CPAlejandro CPAlejandro left a comment

Choose a reason for hiding this comment

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

I have found these 3 errors:

  • When you try to navigate from Framework flyout to a technique, the response flyout stay loading
    image

  • When you try to navigate from Framework flyout to a RuleID, sometimes, there's a bad query to /Mitre from an old endpoint
    image
    image

  • When you did the last error and you go against Mitre/Framework, sometimes appears a No data notice, it refreshes itself and shows properly info

Copy link
Contributor

@CPAlejandro CPAlejandro left a comment

Choose a reason for hiding this comment

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

LGTM!
Testing: ✅
CR: ✅

Sometimes when I enter to /Mitre/Framework appears a warning that says No data noticed but it refresh itself and show the info later than 2-3 seconds

@CPAlejandro
Copy link
Contributor

When you pinned an agent, go to another window, come back, and unpinned this agent, the Framework panel loads two times.
https://user-images.githubusercontent.com/17100196/121674926-fa76a100-cab2-11eb-89f8-31cac0cd4789.mp4

const tactic = Object.values(tacticsObject).find(obj => obj.id === element);
return { id:tactic.references[0].external_id, name: tactic.name};
};
return tactics.reduce((tacticsObj, element) => [...tacticsObj, getTactic(element)], []);
Copy link
Member

Choose a reason for hiding this comment

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

You could use .map instead of .reduce here.

Comment on lines 145 to 147
const tacticsObj = this.findTacticName(tactics)

this.setState({techniqueData: { name, mitre_version, tacticsObj }, loading: false })
Copy link
Member

Choose a reason for hiding this comment

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

Semicolon

@@ -29,14 +29,14 @@ import {
EuiLoadingSpinner,
} from '@elastic/eui';
import { FlyoutTechnique } from './components/flyout-technique/';
import { mitreTechniques, getElasticAlerts, IFilterParams } from '../../lib'
import { getElasticAlerts, IFilterParams } from '../../lib'
Copy link
Member

Choose a reason for hiding this comment

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

Semicolon

Comment on lines 211 to 221
let mitreTechniques = [];
if (totalItems && output.data && output.data.data && totalItems > 500) {
params.offset = 0;
mitreTechniques.push(...output.data.data.affected_items);
while (mitreTechniques.length < totalItems && params.offset < totalItems) {
params.offset += params.limit;
const tmpData = await this.getMitreTechniques(params)
mitreTechniques.push(...tmpData.data.data.affected_items);
}
}
this.setState({ mitreTechniques: mitreTechniques, isSearching: false })
Copy link
Member

@Desvelao Desvelao Jun 14, 2021

Choose a reason for hiding this comment

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

When the totalItems is lower than 500, the mitreTechniques variable seems to be an empty array. Is this a bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, is a bug. Before validation I have to save output.data.data.affected_items first.

Comment on lines 227 to 232
const mitreObj = this.state.mitreTechniques.filter(item => item.id === element);
if(mitreObj.length != 0){
const mitreTechniqueName = mitreObj[0].name;
const mitreTechniqueID = mitreObj[0].references.filter(item => item.source === "mitre-attack")[0].external_id;
mitreTechniqueID ? techniquesObj.push({ id : mitreTechniqueID, name: mitreTechniqueName}) : '';
}
Copy link
Member

Choose a reason for hiding this comment

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

To search one element, you could be used the .find instead of .filter.

useEffect(() => {
const urlParams = new URLSearchParams(location.href);
const redirectTab = urlParams.get('tabRedirect');
const idToRedirect = urlParams.get("idToRedirect")
Copy link
Member

Choose a reason for hiding this comment

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

Semicolon

Comment on lines 31 to 33
getMitreItemToRedirect(endpoint)
urlParams.delete('tabRedirect')
urlParams.delete('idToRedirect')
Copy link
Member

Choose a reason for hiding this comment

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

Semicolon

Comment on lines 43 to 47
...item,
['references.external_id']: item?.references?.find(reference => reference.source === 'mitre-attack')?.external_id
}))
setIsDetailsOpen(true)
setDetails(data[0])
Copy link
Member

Choose a reason for hiding this comment

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

Semicolon

Comment on lines 376 to 388
let tacticsObj = []
const data = await WzRequest.apiReq('GET', '/mitre/tactics', {
params: {
tactic_ids: tactics.toString()
}
})
const formattedData = (((((data || {}).data).data || {}).affected_items || []) || {});
formattedData && formattedData.forEach(item => {
tacticsObj.push(item.name);
});
return tacticsObj;
}catch(error){
return []
Copy link
Member

Choose a reason for hiding this comment

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

Semicolon



const getMitreItemToRedirect = (endpoint) => {
WzRequest.apiReq('GET', endpoint, {}).then(res => {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you could replace the .then/.catch block with try/catch that is trending to use.

…ature/change-tactis-and-techniques-resources
@frankeros frankeros self-requested a review June 14, 2021 13:40
Copy link
Member

@Desvelao Desvelao left a comment

Choose a reason for hiding this comment

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

LGTM!

@eze9252 eze9252 merged commit 4ef11a9 into feature/3291-mitre-attack-intelligence-section Jun 15, 2021
@eze9252 eze9252 deleted the feature/change-tactis-and-techniques-resources branch June 15, 2021 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants