Repository navigation
fix(backend): stop users reading and overwriting each other's data - #164
Merged
Merged
Conversation
Malformed JSON, bad path ids, 405, unknown routes, AccessDeniedException and ResponseStatusException all fell through the catch-all and came back as 500. Adds NotFoundException -> 404 and logs the real 500s.
- GET /jobs/{id}/documents returned every user's generated docs for a job
- PUT on experience, projects, education, socials, strengths, languages and
profile skills saved the path id with the caller's user id, so anyone who
knew an id could overwrite (and take over) another user's row. Now loads by
id + owner first, 404 otherwise
- PDF templates: get/export/update only system or own templates
- prompt templates: no duplicating, favouriting or generating with someone
else's private template
- CV analysis ignores cv versions the caller doesn't own
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
1 participant
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.
Went through every endpoint that takes an id and checked it's actually scoped to the logged-in user. A few weren't:
GET /jobs/{id}/documentsreturned everyone's generated CVs and cover letters for that job, since jobs are shared. Now filtered by user in the query.PUTon experience, projects, education, socials, strengths, languages and profile skills saved the path id with the caller's user id stamped on it, so knowing someone else's id let you overwrite their row and take it over. They now load by id + owner first and 404 if it isn't yours (same 404 for "doesn't exist", so ids can't be probed).GET /{id}, export and update only work on system templates or your own.AccessDeniedExceptionandResponseStatusExceptionwere all turning into 500s. They're 4xx now, and real 500s get logged.Deletes were already scoped, and I added tests to keep them that way.
Tests: service tests with two users for each fix, plus
@WebMvcTests for the error mapping and the cross-user cases../gradlew :backend:testpasses locally (576 tests), and so do spotlessCheck and spotbugs.Not fixed here: company research notes live on the shared
companiesrow, so all users read and overwrite the same notes. Making them per user needs a migration, so it'll be its own PR.Merge order: no conflicts with #155, #157, #158, #160, #161 or #162. If #155 lands first,
CrossUserAccessWebMvcTestneeds a@MockitoBean ManageCustomSectionsUseCase, same as #162'sProfileSectionControllerTest. A Testcontainers test for the new documents query can go in once #158 is merged.The red dependency-audit check is the jackson-databind CVEs already on main, fixed in #157.