Skip to content

feat: serve Submission and AttachmentUpload - #27

Open
mattdjenkinson wants to merge 1 commit into
feat/votesfrom
feat/submissions
Open

mattdjenkinson wants to merge 1 commit into
feat/votesfrom
feat/submissions

Conversation

@mattdjenkinson

Copy link
Copy Markdown
Contributor

Serves Submission and AttachmentUpload. Closes #8, except to-security-report, which needs SecurityReport storage and moves to #9.

Stacked on #26 (votes), which is on #25, #23, #22 and #16.

Submissions

  • Create: user context only, with a verified email. The rate limit is --submission-limit (5) per --submission-window (10m). It's counted in Postgres, so it holds across replicas, and 429s carry a Retry-After for when the oldest submission in the window ages out.
  • Names are always generated (sub- plus random hex). They're unique across users so staff can fetch by name, which means a chosen name could reveal that someone else's submission exists.
  • Users can't change or withdraw a submission. Staff can edit metadata and delete; spec is immutable.
  • What submitters see: phase, postRef, and the decision with reason and mergedInto. They don't see triage, the reviewer, or the staff-edited public title and body. Staff see everything, with the owner in the feedback.miloapis.com/owner annotation from feat: serve Vote and Subscription #26 (kubectl get submissions -o wide).
  • Status: feedback-controller writes status.triage (fraud score in 0..1, valid kind). feedback-sync writes status.postRef, once. Anyone else, staff included, is refused.
  • Review: POST submissions/{name}/review with a SubmissionReview, from staff in the platform context only (never sync or the controller).
    • Decisions: Approved needs publicTitle and publicBody; Merged needs mergedInto, which must be a visible, unmerged post; Rejected needs a reason.
    • Reviews only apply to a submission in PendingReview; reviewing it again returns Conflict.
    • The reviewer and time are recorded.
  • Votes from decisions:
    • Merged: the submitter gets a Subscription (reason: merged) and a Vote (source taken from the submission's client), and the post is re-tallied, all in the review's transaction. An existing vote is left alone.
    • Approved: the same happens with reason: submitted when sync sets postRef, since the post doesn't exist before that.

Attachments

  • AttachmentUpload (create-only, user context, verified email, 20 per 10 minutes per user per replica) stores a file in --attachments-bucket.
    • Bucket URLs use gocloud: gs:// in clusters, file:// or mem:// locally.
    • Allowed types are PNG, JPEG, GIF, WebP, PDF, plain text and JSON; nothing a browser would run.
    • The declared type must match the content (sniffed for binary types, UTF-8 for text).
    • Size limit is --attachment-max-size, 2 MiB by default, which keeps the base64 body under the apiserver's 3 MiB limit.
    • The response leaves out the data.
  • Attaching: a submission lists uploadRefs. The server copies the caller's own uploads under the submission and takes each file's name and type from the upload, ignoring the client's. Someone else's ref is Invalid.
  • Download: GET submissions/{name}/attachment?ref= serves the file to the submitter and staff (others get NotFound). It's sent as an attachment with nosniff, a sandbox CSP and no-store.
  • Lifecycle: deleting a submission deletes its files. Unattached uploads are left for a bucket lifecycle rule (delete uploads/ after a day), which Deploy to staging #13 should add with the bucket.
  • Without a bucket, uploads return 503 and submissions without attachments still work.

Other changes

  • Query parameters: the API group decoded query parameters with metav1.ParameterCodec, which doesn't know the group's own option types, so ?ref= failed with "no kind is registered". It now uses a codec built from the server's scheme, tested for both AttachmentOptions and list options.
  • AttachmentOptions has its own OpenAPIModelName. Without it, the one promoted from the embedded TypeMeta would make its OpenAPI definition TypeMeta's.
  • Shared packages: the email check moved to internal/identity and the rate limiter to internal/ratelimit.

Testing

  • Postgres integration tests:
    • generated names, phase and status set by the server;
    • redacted versus staff views, and isolation between users;
    • every rejected create;
    • who can update, delete and write status;
    • reject, merge and approve paths, including permissions, validation, Conflict on a second review, votes and subscriptions with the right reasons, and the post re-tallied;
    • the rate limit, including expiry;
    • attachments: type, sniffing and name checks; someone else's ref; metadata from the upload; downloads by submitter, staff and others; files deleted with the submission;
    • the upload rate limit.
  • Kind cluster with an in-memory bucket:
    • upload and submit; SVG refused; client-set attachment name and type replaced;
    • downloads by Alice and staff returned the bytes, Bob got NotFound (after the parameter codec fix above);
    • the controller's triage applied while its phase change was dropped;
    • Alice's view hid triage, and her review was Forbidden;
    • the staff merge gave Alice a portal vote and a merged subscription and re-tallied the post, and a second review got Conflict;
    • the submission limit returned 429 with Retry-After.

Submissions are private feedback waiting for a staff decision.

- Users create them in the user context with a verified email, limited to
  5 per 10 minutes, counted in Postgres. Names are always generated since
  they are unique across users. Users can't change or withdraw them.
- Submitters read their own without triage, the reviewer or the public
  draft; staff read all of them in the platform context.
- feedback-controller writes status.triage; feedback-sync writes
  status.postRef, once.
- submissions/{name}/review takes a staff decision: Approved, Merged or
  Rejected, once, from the platform context only. A merge gives the
  submitter a Vote and Subscription in the same transaction; an approval
  does when sync links the post.
- AttachmentUpload stores a file in a private bucket (gocloud: gs, file,
  mem) with type, size and content checks. Submissions copy their uploads
  and take names and types from them. submissions/{name}/attachment?ref=
  serves a file to the submitter and staff as a sandboxed download.
- The API group now decodes query parameters with its own scheme, so
  AttachmentOptions works.
- The email check and rate limiter move to shared packages.

to-security-report needs SecurityReport storage and comes with #9.

Part of datum-cloud/enhancements#943. Closes #8.
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.

1 participant