auto-updating config via websocket | setting: automatically include limit in query for tags/services #189

findus · 2024-11-12T07:36:02Z

~~Config option still missing~~

Websocket connection to update setting for all connected users on the fly
Introduces config endpoint that lets us configure settings on the fly and send it over instantly to all connected clients

@peace-maker not sure if https://www.youtube.com/watch?v=cDA3_5982h8 moment, but I tried.

#181

peace-maker · 2024-11-12T23:35:49Z

We try to make as much configurable during runtime without restarting pkappa2. So some config page and rest endpoints to change the config values would be good. You don't have to add the UI part yet to configure it.

The current way the ltime:-1h: expression is inserted into the search box when clicking on Help or All Streams confused me at first because the query isn't run yet. The All Streams page still shows all streams instead of only the ones from the last hour as shown in the search box. I'd expect the query to be applied. Can you try how it feels when actually including that constraint on the All Streams view?
The interaction when clearing the search box and submitting the empty query after having searched another query should be special. Right now the ltime expression is inserted right away again but the search is still the empty query. It should always be possible to send the empty query to get all streams (paginated of course).

I think it's useful to add it to the other tags too. When clicking on a tag like flag_in it should show the query like tag:flag_in ltime:-1h:.

We'll have to test if this is annoying or intuitive to use during a CTF.

findus · 2024-11-17T18:05:14Z

@peace-maker came up with a solution, now on result-view creation it gets checked if setting option is is enabled, if yes + query is empty the limit gets auto applied.

Also if any link is clicked in the sidebar the router sends an additional queryparam that indicates exactly that, then the limit also gets auto applied.

Opening links with already defined queries in url just are just getting placed as is.

Only remaining issue: searching with empty query not possible atm, limit always get applied every time now.

peace-maker

Nice, thank you for working on this! The empty query could be allowed by not interacting with an manually entered empty query. So after clearing the searchbox and pressing enter, you wouldn't insert the time query part.

I think adding another websocket event to notify all clients of the changed config would be useful instead of polling, but that can be done later.

A note on the Settings page that changes are global and affect all users would hopefully avoid confusion on people trying to disable the autofill for themselves.

web/src/components/SearchBox.vue

web/src/apiClient.ts

cmd/pkappa2/main.go

internal/index/manager/manager.go

web/src/components/SearchBox.vue

web/src/components/Settings.vue

web/src/stores/index.ts

peace-maker

I haven't tried running the code yet, so this is still just visual inspection. Looks good overall.

I had the idea of turning the config option into an arbitrary suffix to add to the query, so you can change the currently hardcoded "ltime:-1h:". I'm not sure it'd help usability though and this can be switched to that option when a different query was desired during a CTF. Just for future reference.

There are multiple linter warnings still yelling about.

cmd/pkappa2/main.go

internal/index/manager/manager.go

web/src/apiClient.ts

web/src/components/Results.vue

frontend glue code load config on component load get config from server config flag

yarn format

Fixed conditional vue-if

lint

Removed obsolete code SetClientConfig now returns nothing linting <3

linting <3333

lint

dont return config after updating

peace-maker · 2024-12-03T13:14:00Z

I didn't notice a difference in query speed when appending ltime:-1h: to tag/service queries during saarctf. So I'm not sure this does anything in the current state :(

Maybe a suffix in the v-text-input which gets added to the query transparently could work? I'm not sure how that interacts with sharing links to query results. Would the ltime be in the query or would it be omitted since the config is global and it would be added again when loading the page? I'd like to make it very clear visually what query is actually run.
Or include it in the query in the link but strip it from the text field value using the parser if the config option is enabled? That could fail horribly for more complex queries using parentheses or negation and would need heavy testing. But it wouldn't rely on the state of the config value when clicking on links.

How should the submenu time limit buttons work together with the global auto append setting?

peace-maker

From the technical side this looks good now minus the remaining comments. But since the envisioned performance gain doesn't exist, it's a lot of code that doesn't improve the experience :(

@spq do you have ideas on how to implement appending the time limitation ltime to any query the user might do by default, without them having to remember to click a button or type it themselves?

peace-maker · 2024-12-03T13:17:50Z

web/src/components/Results.vue

@@ -353,6 +360,12 @@ watch(route, () => {
  fetchStreams();
 });

+onBeforeMount(() => {


The config is only fetched when looking at query results once. It doesn't update when you start on the landing page and click on one of the tags, so this should be moved to Navigation.vue. That way clicking on Settings right away without searching for anything first updates the button state correctly.

peace-maker · 2024-12-03T13:19:29Z

web/src/stores/index.ts

@@ -28,6 +30,7 @@ export const useRootStore = defineStore("root", {
      tags: null,
      converters: null,
      pcapOverIPEndpoints: null,
+      clientConfig: null,


We can add the config defaults here instead of allowing null. This makes a bunch of null checks obsolete too and simplifies the code. It also allows the websocket update messages to work even when we didn't fetch the clientconfig REST endpoint yet.

findus · 2024-12-03T13:25:00Z

@peace-maker

Right now the ltime query param only gets added to the links inside the navbar, so that should not clash with more complex queries.

Could we just repurpose the PR to something like: Boilerplate code for config functionality and remove the ltime feature from this branch?

spq · 2025-01-10T09:25:08Z

internal/index/manager/manager.go

+		c <- mgr.clientConfig
+		close(c)
+	}
+	res := <-c


return <-res

spq · 2025-01-10T09:27:05Z

cmd/pkappa2/main.go

why do we name the "clientconfig" that way? why not config and open it up to store runtime controllable server-side config for later?

findus marked this pull request as ready for review November 12, 2024 10:56

peace-maker linked an issue Nov 17, 2024 that may be closed by this pull request

Config option to search in last hour by default #181

Open

peace-maker requested changes Nov 17, 2024

View reviewed changes

findus requested a review from peace-maker November 17, 2024 23:33

peace-maker added enhancement New feature or request javascript Pull requests that update Javascript code go Pull requests that update Go code labels Nov 18, 2024

peace-maker reviewed Nov 18, 2024

View reviewed changes

cmd/pkappa2/main.go Outdated Show resolved Hide resolved

internal/index/manager/manager.go Outdated Show resolved Hide resolved

web/src/apiClient.ts Outdated Show resolved Hide resolved

web/src/components/Results.vue Outdated Show resolved Hide resolved

findus requested a review from peace-maker November 19, 2024 10:22

findus force-pushed the limit-auto-fill branch 6 times, most recently from 2c17232 to 57a7529 Compare November 20, 2024 09:55

findus added 5 commits November 20, 2024 11:06

auto include limit in query

05cf13f

set default query only if route is unset

38edd47

Clientconfig in backend:

f365f6b

frontend glue code load config on component load get config from server config flag

post request to set config on the fly

44087a9

Config view

fb76b6e

yarn format

findus force-pushed the limit-auto-fill branch from 57a7529 to 8318fd8 Compare November 20, 2024 10:07

findus added 7 commits November 20, 2024 11:17

Test logic to auto append limit to query

fb0e992

Only add limit on empty queries

fa6d918

mark navigation router events so limit gets inserted after switch

d26cfcb

empty queries are working again

07c32cc

Renamed "Settings" to "Global Settings"

086a934

Fixes from first Review

9d542cb

only send fromNavigation when auto insert is active

30dba3f

findus added 10 commits November 20, 2024 11:17

Fixed go compile error,

faa0642

Fixed conditional vue-if

only add manualSearch query if feature is active

d09dc0e

lint

Review fixes:

dffbeb9

Removed obsolete code SetClientConfig now returns nothing linting <3

fix: manager_test removed clientconfig in constructor

155ad76

linting <3333

hardcode query in navigation view

a869a4f

lint

generated type guards

3fb0c89

Config now reactive

1edd8aa

watch on store object instead of listening on computed property

dccc989

dont return config after updating

clientconfig now pointer

c659924

Removed unneeded for-loop from settings

14a0fe3

findus force-pushed the limit-auto-fill branch from 8318fd8 to 14a0fe3 Compare November 20, 2024 10:17

findus changed the title ~~auto include limit in query~~ auto-updating config via websocket | setting: automatically include limit in query for tags/services Nov 20, 2024

findus mentioned this pull request Dec 2, 2024

Shift select filters #211

Merged

peace-maker requested changes Dec 3, 2024

View reviewed changes

spq reviewed Jan 10, 2025

View reviewed changes

internal/index/manager/manager.go

c <- mgr.clientConfig

close(c)

}

res := <-c

Copy link

Owner

spq Jan 10, 2025

Choose a reason for hiding this comment

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

return <-res

spq reviewed Jan 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

auto-updating config via websocket | setting: automatically include limit in query for tags/services #189

auto-updating config via websocket | setting: automatically include limit in query for tags/services #189

findus commented Nov 12, 2024 •

edited

Loading

peace-maker commented Nov 12, 2024

findus commented Nov 17, 2024

peace-maker left a comment

peace-maker left a comment

peace-maker commented Dec 3, 2024

peace-maker left a comment

peace-maker Dec 3, 2024

peace-maker Dec 3, 2024

findus commented Dec 3, 2024

spq Jan 10, 2025

spq Jan 10, 2025

auto-updating config via websocket | setting: automatically include limit in query for tags/services #189

Are you sure you want to change the base?

auto-updating config via websocket | setting: automatically include limit in query for tags/services #189

Conversation

findus commented Nov 12, 2024 • edited Loading

peace-maker commented Nov 12, 2024

findus commented Nov 17, 2024

peace-maker left a comment

Choose a reason for hiding this comment

peace-maker left a comment

Choose a reason for hiding this comment

peace-maker commented Dec 3, 2024

peace-maker left a comment

Choose a reason for hiding this comment

peace-maker Dec 3, 2024

Choose a reason for hiding this comment

peace-maker Dec 3, 2024

Choose a reason for hiding this comment

findus commented Dec 3, 2024

spq Jan 10, 2025

Choose a reason for hiding this comment

spq Jan 10, 2025

Choose a reason for hiding this comment

findus commented Nov 12, 2024 •

edited

Loading