Expose what a bulk export needs from the assessment API - #149
Open
summitt wants to merge 1 commit into
Open
Conversation
Migrating a FACTION instance elsewhere means reading the whole inventory, and
three gaps made that impossible through the API:
- VulnerabilityDTO carried a finding's severity but not its state. Added
Status, Opened, Closed, DevClosed, StagingClosed and the category, so a
consumer can reproduce a finding rather than just describe it.
- Nothing listed every assessment. /queue is open assessments assigned to the
caller and /completed wants a date range, so a full inventory meant guessing
at the union of two partial views. GET /assessments/all returns what the
caller can see, in one call.
- The report download served one variant, preferring the PDF with no way to
ask for anything else. It now takes type=docx|pdf and retest=true, and
/report/{aid}/info lists what a report actually holds so a caller can choose
before pulling a blob. A named type that is absent 404s rather than
substituting another format, which would otherwise be filed as the wrong
document type on the far end.
GET /users/all rounds this out: the directory is how an integration resolves
the user ids that appear on assessments, and there was no way to read it.
Also fixes an NPE this surfaced. Vulnerability.getAssessmentId() is a
primitive getter over a nullable Long, so any finding not attached to an
assessment threw on read — that took down /assessments/vulns/{aid}, not just
the new paths. It now returns 0, the "no assessment" value callers already
test for (see AppBootstrapListener), and the DTO omits the field rather than
pointing consumers at assessment 0. AssessmentDTO's start/end are guarded for
the same reason: a bulk listing sweeps up drafts that never had dates set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S72uFdcjbJfhgEwwWxnfKX
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if (finalReport == null) { | ||
| return Response.status(404).entity(String.format(Support.ERROR, "No final report available for this assessment")).build(); | ||
| } | ||
| return invokeDownloadReport(u, assessment, aid, type, retest); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reading a whole FACTION instance through the API — to migrate it, archive it, or feed another system — is not currently possible. Three gaps stand in the way, and this closes them. Everything here is additive; no existing response loses a field.
A finding's state was not in the payload
VulnerabilityDTOcarried severity, text and CVSS, but not whether the finding was open. AddedStatus,Opened,Closed,DevClosed,StagingClosed, and the category (Category,CategoryId). Dates cross the wire as epoch-millisecond strings, matching the existingCreated. Unset fields stay out of the payload rather than appearing as null.Nothing listed every assessment
/queuereturns open assessments assigned to the caller;/completedneeds a date range. A full inventory meant guessing at the union of two partial views, and still missing in-progress work assigned to someone else.GET /assessments/allreturns what the caller can see in one call, using the same scoping the UI applies — managers with unrestricted access get everything, everyone else gets their own subset.The report download served one variant, with no way to choose
It preferred the PDF and fell back to whatever was first. Now:
GET /assessments/report/{aid}?type=docx|pdf&retest=truepicks a variant and can reach the retest report. With notypethe old behaviour is unchanged.GET /assessments/report/{aid}/infolists what a report actually holds, so a caller can decide before pulling a multi-megabyte blob. It also reports whether the PDF is password protected — the password itself is deliberately not exposed.A named type that is absent returns 404 rather than substituting another format. Handing back a PDF to a caller that asked for a DOCX would get it filed as the wrong document type on the far end. Downloads now also carry a real
Content-Typeinstead of defaulting to octet-stream.GET /users/allrounds this out: the directory is how an integration resolves the user ids that appear on assessments, and there was no way to read it. Admin or manager only; password hashes and tokens are never included.A live NPE this surfaced
Vulnerability.getAssessmentId()is a primitivelonggetter over a nullableLongfield, so any finding not attached to an assessment threw on read. That is not limited to the new endpoints — it takes down the existing/assessments/vulns/{aid}too.It now returns
0, the "no assessment" value callers already test for (AppBootstrapListenerdoesgetAssessmentId() > 0), and the DTO omits the field rather than pointing consumers at assessment 0.AssessmentDTO'sstart/endare guarded for the same reason: a bulk listing sweeps up drafts that never had dates set, and one of them would otherwise fail the whole page.Testing
New
MigrationExportAPITest— 16 tests over variant selection (including legacy reports with no variant list, and the refusal to substitute a format), the info listing, retest downloads, and the DTO lifecycle fields. Full suite: 927 pass, 0 failures.🤖 Generated with Claude Code
https://claude.ai/code/session_01S72uFdcjbJfhgEwwWxnfKX