251 lines
10 KiB
Markdown
251 lines
10 KiB
Markdown
● Here's the synopsis.
|
|
|
|
What it is
|
|
|
|
Stocks in the Future
|
|
(rubyforgood/stocks-in-the-future) — a Rails 8.1 /
|
|
Ruby 3.4.4 app for a nonprofit that teaches
|
|
middle-schoolers financial literacy. Students earn
|
|
real-ledger, fake-money by attending class and
|
|
getting good grades, then invest that money in a
|
|
simulated stock market tracking real prices. Teachers
|
|
enter grades; admins run everything.
|
|
|
|
Architecture
|
|
|
|
Standard Rails-with-extras, no API layer —
|
|
server-rendered Hotwire (Turbo + Stimulus),
|
|
Tailwind/shadcn, Propshaft, importmap. Postgres.
|
|
Solid Queue for jobs. Devise for auth, Pundit for
|
|
authorization, Discard for soft deletes. Deployment
|
|
is Capistrano → AWS Lightsail (Terraform-managed),
|
|
with SES for mail; Docker for local dev.
|
|
|
|
Layering is more disciplined than typical:
|
|
app/services (business operations), app/policies,
|
|
app/presenters, app/facades, plus a PortfolioPosition
|
|
PORO aggregate. Models stay reasonably thin.
|
|
|
|
Domain shape:
|
|
School → SchoolYear → Quarter(1-4) → GradeBook →
|
|
GradeEntry
|
|
↓
|
|
Classroom ← TeacherClassroom → Teacher
|
|
↓
|
|
ClassroomEnrollment → Student ─ Portfolio ─┬
|
|
PortfolioTransaction (ledger)
|
|
├
|
|
PortfolioStock (lots)
|
|
└
|
|
PortfolioSnapshot (monthly)
|
|
Student → Order → Stock
|
|
|
|
User is STI (User/Student/Teacher) with an admin
|
|
boolean orthogonal to type.
|
|
|
|
Two design decisions worth knowing:
|
|
|
|
1. Transactions are an append-only ledger. portfolios
|
|
deliberately has no cash column — balance is derived
|
|
by summing transactions
|
|
(app/models/portfolio.rb:88), and it subtracts
|
|
pending buy orders plus a pending fee so students
|
|
can't double-spend money already committed.
|
|
2. Orders are batch-executed, not immediate. Students
|
|
queue buy/sell orders; OrderExecutionJob drains
|
|
them, then TransactionFeeProcessor charges a flat
|
|
$1/student/day regardless of order count. Prices
|
|
update once daily from Alpha Vantage, so the price a
|
|
student sees is the price they get.
|
|
|
|
Earnings: $0.20/day attended, $1 perfect-attendance
|
|
bonus, $3 for an A / $2 for a B in reading and math,
|
|
$2 per grade improvement over the prior quarter.
|
|
Admin "finalizes" a gradebook, which is what actually
|
|
mints the deposits (DistributeEarnings).
|
|
|
|
docs/ is unusually good for a volunteer project —
|
|
orders-and-transactions.md and gradebook-earnings.md
|
|
explain the why.
|
|
|
|
State of the code
|
|
|
|
I ran the suite: 675 tests, 1 failure, 2 skips, 0
|
|
errors (~18s).
|
|
|
|
The one failure is not a product bug — it's a time
|
|
bomb. test/controllers/admin/teachers_controller_test
|
|
.rb:143 hardcodes school year "2025 - 2026", but
|
|
Year.current_school_year returns "2026 - 2027" for
|
|
any date after June 2026, so the classroom never
|
|
renders. It began failing on 2026-07-01 and will keep
|
|
failing until the literal is replaced with a
|
|
computed current year.
|
|
|
|
Things I'd flag
|
|
|
|
Two authorization gaps — I verified both with
|
|
throwaway integration tests, then deleted them.
|
|
|
|
1. IDOR on order updates
|
|
(app/controllers/orders_controller.rb:41). set_order
|
|
does an unscoped Order.find(params[:id]), and update
|
|
never calls authorize. OrderPolicy#update? exists and
|
|
is unit-tested, but nothing invokes it — only cancel
|
|
does. Confirmed: an unrelated student changed
|
|
another student's pending order from 1 share to 42
|
|
and got a 302 success. Ownership can't be reassigned
|
|
(order_params excludes user_id) and only pending
|
|
orders qualify, but one student can still rewrite
|
|
another's trade. One-line fix: authorize @order in
|
|
update.
|
|
2. Teachers aren't scoped to their own classrooms in
|
|
StudentsController and
|
|
ClassroomEnrollmentsController. Both use
|
|
ensure_teacher_or_admin — any teacher — then
|
|
Classroom.find(params[:classroom_id]) with no
|
|
membership check. Confirmed: a teacher of classroom A
|
|
reset the password of a student in classroom B.
|
|
Notably
|
|
ClassroomsController#check_classroom_eligibility does
|
|
check membership, so this is an inconsistency rather
|
|
than a deliberate policy.
|
|
|
|
Neither is remotely exploitable — both require an
|
|
authenticated account — and the blast radius is one
|
|
school district's students, but #2 crosses a
|
|
classroom boundary that the app elsewhere treats as
|
|
real.
|
|
|
|
Lower-priority:
|
|
|
|
- Admin::PortfolioTransactionsController has six
|
|
commented-out authorize calls with TODO: FIX.
|
|
Practically fine (BaseController gates admins), but
|
|
the ledger doc says transactions are immutable, and
|
|
this controller offers edit/update/destroy on them —
|
|
that's a stated-invariant violation more than an
|
|
authz one.
|
|
- docs/scheduling.md has drifted from
|
|
config/recurring.yml. The doc says OrderExecutionJob
|
|
runs weekdays at 1 AM and auto-triggers the price
|
|
update; actually it runs every 15 minutes and the
|
|
price job is independently scheduled. It also
|
|
references two docs that don't exist.
|
|
- Admin::BaseController#apply_sorting interpolates
|
|
params[:sort] into reorder. Rails quotes hash keys as
|
|
identifiers so it isn't injectable, but an unknown
|
|
column produces a 500 rather than a fallback.
|
|
- Mid-migration duality: students link to classrooms
|
|
via both legacy users.classroom_id and the newer
|
|
ClassroomEnrollment. ClassroomFacade#students unions
|
|
both. Worth finishing, since some scopes (e.g.
|
|
- docs/scheduling.md has drifted from config/recurring.yml. The doc says OrderExecutionJob runs weekdays at 1
|
|
AM and auto-triggers the price update; actually it runs every 15 minutes and the price job is independently
|
|
scheduled. It also references two docs that don't exist.
|
|
- Admin::BaseController#apply_sorting interpolates params[:sort] into reorder. Rails quotes hash keys as
|
|
identifiers so it isn't injectable, but an unknown column produces a 500 rather than a fallback.
|
|
- Mid-migration duality: students link to classrooms via both legacy users.classroom_id and the newer
|
|
ClassroomEnrollment. ClassroomFacade#students unions both. Worth finishing, since some scopes (e.g.
|
|
Classroom.order_by_student_count) still read only the legacy column.
|
|
- Order#destroy is defined below private, so DELETE /orders/:id routes to a nonexistent action — dead code
|
|
plus a phantom route.
|
|
$2 per grade improvement over the prior quarter.
|
|
Admin "finalizes" a gradebook, which is what actually
|
|
mints the deposits (DistributeEarnings).
|
|
|
|
docs/ is unusually good for a volunteer project —
|
|
orders-and-transactions.md and gradebook-earnings.md
|
|
explain the why.
|
|
|
|
State of the code
|
|
|
|
I ran the suite: 675 tests, 1 failure, 2 skips, 0
|
|
errors (~18s).
|
|
|
|
The one failure is not a product bug — it's a time
|
|
bomb. test/controllers/admin/teachers_controller_test
|
|
.rb:143 hardcodes school year "2025 - 2026", but
|
|
Year.current_school_year returns "2026 - 2027" for
|
|
any date after June 2026, so the classroom never
|
|
renders. It began failing on 2026-07-01 and will keep
|
|
failing until the literal is replaced with a
|
|
computed current year.
|
|
|
|
Things I'd flag
|
|
|
|
Two authorization gaps — I verified both with
|
|
throwaway integration tests, then deleted them.
|
|
|
|
1. IDOR on order updates
|
|
(app/controllers/orders_controller.rb:41). set_order
|
|
does an unscoped Order.find(params[:id]), and update
|
|
never calls authorize. OrderPolicy#update? exists and
|
|
is unit-tested, but nothing invokes it — only cancel
|
|
does. Confirmed: an unrelated student changed
|
|
another student's pending order from 1 share to 42
|
|
and got a 302 success. Ownership can't be reassigned
|
|
(order_params excludes user_id) and only pending
|
|
orders qualify, but one student can still rewrite
|
|
another's trade. One-line fix: authorize @order in
|
|
update.
|
|
2. Teachers aren't scoped to their own classrooms in
|
|
StudentsController and
|
|
ClassroomEnrollmentsController. Both use
|
|
ensure_teacher_or_admin — any teacher — then
|
|
Classroom.find(params[:classroom_id]) with no
|
|
membership check. Confirmed: a teacher of classroom A
|
|
reset the password of a student in classroom B.
|
|
Notably
|
|
ClassroomsController#check_classroom_eligibility does
|
|
check membership, so this is an inconsistency rather
|
|
than a deliberate policy.
|
|
|
|
Neither is remotely exploitable — both require an
|
|
authenticated account — and the blast radius is one
|
|
school district's students, but #2 crosses a
|
|
classroom boundary that the app elsewhere treats as
|
|
real.
|
|
|
|
Lower-priority:
|
|
|
|
- Admin::PortfolioTransactionsController has six
|
|
commented-out authorize calls with TODO: FIX.
|
|
Practically fine (BaseController gates admins), but
|
|
the ledger doc says transactions are immutable, and
|
|
this controller offers edit/update/destroy on them —
|
|
that's a stated-invariant violation more than an
|
|
authz one.
|
|
- docs/scheduling.md has drifted from
|
|
config/recurring.yml. The doc says OrderExecutionJob
|
|
runs weekdays at 1 AM and auto-triggers the price
|
|
update; actually it runs every 15 minutes and the
|
|
price job is independently scheduled. It also
|
|
references two docs that don't exist.
|
|
- Admin::BaseController#apply_sorting interpolates
|
|
params[:sort] into reorder. Rails quotes hash keys as
|
|
identifiers so it isn't injectable, but an unknown
|
|
column produces a 500 rather than a fallback.
|
|
- Mid-migration duality: students link to classrooms
|
|
via both legacy users.classroom_id and the newer
|
|
ClassroomEnrollment. ClassroomFacade#students unions
|
|
both. Worth finishing, since some scopes (e.g.
|
|
Classroom.order_by_student_count) still read only the
|
|
legacy column.
|
|
- Order#destroy is defined below private, so DELETE
|
|
/orders/:id routes to a nonexistent action — dead
|
|
code plus a phantom route.
|
|
- MemorablePasswordGenerator builds passwords from
|
|
Faker::Superhero + rand(1..99) — the file's own TODO
|
|
acknowledges this. Fine for classroom accounts, low
|
|
entropy in principle.
|
|
|
|
Working tree: a 15 MB GITFOLDER.zip containing a full
|
|
.git directory is sitting untracked in the repo
|
|
root, and two .DS_Store files show as deleted.
|
|
.gitignore covers .DS_Store but not the zip. Probably
|
|
a stray artifact from someone's backup — worth
|
|
removing before it gets committed.
|
|
|
|
|