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

DFIQ new UI and navigation #3041

Merged
merged 15 commits into from
Mar 28, 2024
Merged

DFIQ new UI and navigation #3041

merged 15 commits into from
Mar 28, 2024

Conversation

berggren
Copy link
Contributor

@berggren berggren commented Feb 21, 2024

This is the new DFIQ UI.

Selected question

Screenshot 2024-03-12 at 15 05 12

Select or create new questions

Screenshot 2024-03-12 at 15 05 25

@berggren berggren added this to the Release: 20240320 milestone Feb 21, 2024
@berggren berggren self-assigned this Feb 21, 2024
@berggren berggren requested a review from jkppr February 27, 2024 07:19
@berggren berggren marked this pull request as ready for review March 12, 2024 14:19
Copy link
Collaborator

@jkppr jkppr left a comment

Choose a reason for hiding this comment

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

Some minor nits, nothing blocking that can't be fixed in a follow-up PR.

Comment on lines +457 to +472
class QuestionTemplateListResource(resources.ResourceMixin, Resource):
"""List all scenarios available."""

@login_required
def get(self):
"""Handles GET request to the resource.

Returns:
A list of JSON representations of the scenarios.
"""
dfiq = load_dfiq_from_config()
if not dfiq:
return jsonify({"objects": []})

scenarios = [scenario.__dict__ for scenario in dfiq.questions]
return jsonify({"objects": scenarios})
Copy link
Collaborator

Choose a reason for hiding this comment

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

The name and docstring do not match here. QuestionTemplateListResource suggests this is a class to list Question Templates. The docstring List all scenarios available. suggests it is listing all available scenarios.

This seems to be a copy of class ScenarioTemplateListResource in line 57, so I assume the title of the class is what this function is supposed to archive.

Please clarify the docstring/comments for this class to make it's task more clear.

let approaches = this.activeQuestion.approaches.map((approach) => JSON.parse(approach.spec_json))
approaches.forEach((approach) => {
approach._view.processors.forEach((processor) => {
processor.analysis.forEach((analysis) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

While testing I ran into a situation where this line resulted in the following error:

Error in render: "TypeError: processor.analysis.forEach is not a function"
found in
---> <TsQuestionCard> at src/components/Scenarios/QuestionCard.vue

To reproduce:

  1. Load dfiq questions into Timesketch
  2. Select one of the DFIQ questions as your context that has suggested queries e.g. Has there been any executions of PsExec?
  3. Click "change question" and in the DFIQ list, click the same question again (text, not plus). This should trigger the error.

Looks like there are DFIQ questions that have another layer in the processor that is not an array (with this example above it is processor.analysis.timesketch.array. So it could also be me having an old version of DFIQ questions. Therefore I don't consider this blocking the PR!

Copy link
Collaborator

Choose a reason for hiding this comment

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

When I select a question from the predefined DFIQ questions, it gets added to the left side Questions column. However, it also stays in the right side DFIQ list. This results in a situation where you can add the same question multiple times. And since there is no option to edit questions yet, it can get confusing.
image

@jkppr jkppr merged commit 1827a14 into master Mar 28, 2024
27 checks passed
@jkppr jkppr deleted the dfiq-navigation branch March 28, 2024 14:40
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.

2 participants