trustification / trustify

Apache License 2.0
8 stars 15 forks source link

Advisory search: sort by `average_severity` does not work #383

Closed carlosthe19916 closed 1 month ago

carlosthe19916 commented 1 month ago

Some Context

Given the Advisories UI table (image below), I'd like to sort Advisories by Severity.

Screenshot from 2024-06-06 13-40-02

{
    "items": [
        {
            "identifier": "CVE-2023-20862",
            "hashes": [
                "sha256:e6a2bc1084c77809af965dc4facb0c28aca67210f054eb8de4778190b8c6347e"
            ],
            "issuer": {
                "id": 2,
                "name": "Red Hat Product Security",
                "cpe_key": null,
                "website": null
            },
            "published": "2023-04-19T00:00:00Z",
            "modified": "2023-11-14T22:09:12Z",
            "title": "spring-security: Empty SecurityContext Is Not Properly Saved Upon Logout",
            "average_severity": "medium",... this is the field I want to SortBy
            "average_score": 6.3,
            "vulnerabilities": [
                {
                    "identifier": "CVE-2023-20862",
                    "non_normative": {
                        "non_normative": true,
                        "identifier": "CVE-2023-20862",
                        "title": "spring-security: Empty SecurityContext Is Not Properly Saved Upon Logout",
                        "discovered": "2023-07-31T00:00:00Z",
                        "released": "2023-04-19T00:00:00Z"
                    },
                    "severity": "medium",
                    "score": 6.3
                }
            ]
        },
    ],
    "total": 2595
}

Here is the problem

When I try to search Advisories and order them by average_severity I get an error. The request I am executing is:

GET /api/v1/advisory?sort=average_severity:asc&limit=10&offset=0

And this is the exact error I get from the backend 400 Bad Request:

{
    "error": "Query error",
    "message": "query syntax error: Invalid field name for sort: 'average_severity'"
}

This might be just me not knowing how to properly create the query for sorting, or it might be a real bug.

Could you help me with this please?

carlosthe19916 commented 1 month ago

On a side note. The query for sorting by published works so my queries are grammatically correct

GET /api/v1/advisory?sort=published:asc&limit=10&offset=0
bobmcwhirter commented 1 month ago

The average_severity string is a synthetic string generated based upon the average_score just as a helper.

Can the UI treat sort-by-severity as exactly sort-by-score?

Else, I presume it'll sort lexographically on the strings "high" "medium" "low" which... isn't very sensible.

jcrossley3 commented 1 month ago

This one will be tricky. Because the results are post-processed after being fetched from the db. Unless it's possible to move that processing logic into the db (as a view or some stored proc), I'm not sure how to make the pagination work correctly if sorting on one of those calculated fields.

bobmcwhirter commented 1 month ago

After package?q= I'll look into if we can't SQL calculate that column (or at least the score, if not the severity word) so perhaps it'd be sortable using your stuff @jcrossley3

jcrossley3 commented 1 month ago

Cool. In general, we probably need to be conscious of the fact that folks might try to sort on any field they see in a JSON response. If those fields are calculated, expectations may not be met.

bobmcwhirter commented 1 month ago

I think this harkens back to the Sikula discussion a bit. Since we DTO, things in JSON may not be in the DB, or at least not in the JSON-shaped format.

In this case, I suspect I can sort out some SQL aggregate/sum/average to give us a named column, even if synthetic, but we may hit a wall at some point.

carlosthe19916 commented 1 month ago

Can the UI treat sort-by-severity as exactly sort-by-score?

Technically possible. But I believe just like me, any other client will try to pursue my same journey: "sort by average_severity"

Generally speaking i think this specific thing should be addressed in the server side and not on the client side.

Filtering

@bobmcwhirter @jcrossley3 I just tried to also add filtering to the table "filter by average_severity" which is a very valid use case for a client: E.g. "give me all advisories with high average_severity". And I have hit the same issue: "query syntax error: Invalid field name for filter: 'average_severity'".

I thought about opening another issue as the title of this issue refers only to SORTING, but I think whatever is the solution it might apply to both Sorting and Filtering.

jcrossley3 commented 1 month ago

Just so we're clear: our filtering/sorting/pagination stuff ONLY works on database queries.

Any required filtering/sorting/pagination of DTO's will need to be done by hand within the API request handlers.

bobmcwhirter commented 1 month ago

WIP: https://github.com/trustification/trustify/pull/399

carlosthe19916 commented 1 month ago

@bobmcwhirter I confirm SortBy average_score works. But FilterBy average_severity does not work. The title of this issue is exclusively for SORTING so I am happy to close this issue and open a new one for Filtering by average_severity

carlosthe19916 commented 1 month ago

Closing this issue as, we can Sort by average_score