What I had built
The project is upgrading CircuitVerse's LMS integration from LTI 1.1 to LTI 1.3
(issue #7405). LTI is the
standard that lets a tool like CircuitVerse live inside Canvas or Moodle: a student
clicks an assignment in their course, and they land inside CircuitVerse already signed
in, with their grade flowing back to the gradebook.
CircuitVerse currently speaks LTI 1.1, which authenticates a launch with an OAuth 1.0
signature over a shared secret. Canvas has deprecated that for new installations. LTI
1.3 replaces it with OpenID Connect and signed JWTs.
At the start of week 2 I opened #7488,
titled [POC] Feat/lti 1 3 upgrade. It was the whole thing. Migrations for a new
deployments table, a JWT validator, an OIDC login endpoint, a rewritten launch, key
management, a JWKS endpoint, tool configuration, user provisioning, and a rewritten
test suite. It worked. I had a real Canvas container talking to a local CircuitVerse
and a real user landing signed in.
It was closed the next day.
Sitting with that
My first instinct was that this was a communication failure — that if I had explained
the design better, or written a longer description, it would have gone differently. I
spent a chunk of this week drafting exactly that: a better writeup of the same pull
request.
Somewhere in the middle of writing it I opened the diff and tried to review it as if
someone else had sent it to me. That was the moment it clicked.
The change touched authentication. It auto-provisioned user accounts. It added a code
path that signs somebody in based on a token that arrived from an external server. To
approve it, a maintainer would have to hold all of that in their head at once and be
confident that no part of it was wrong. Not "probably fine" — confident, because the
failure mode is somebody signing in as somebody else.
I could not have reviewed it myself, and I wrote it.
That is not a communication problem. No description makes a change like that
reviewable. The size is the problem, and a better cover letter would only have made
it more persuasive without making it more correct.
The thing I had actually gotten wrong
I had been treating the pull request as a delivery mechanism — a box you put finished
work in. Get the feature working, put it in the box, hand it over.
For a project where a maintainer has to put their name on the merge, that is backwards.
The pull request is the unit of review, and review is the thing that has to be possible.
Which means the size and shape of a change is not a packaging decision made at the end.
It is a design decision made at the start, and it constrains how you build.
A 2,000 line change that works is worth less than a 150 line change that a reviewer can
be confident about, because only one of them can actually land. Code that cannot merge
is not really finished, no matter how well it runs on my machine.
What I did with the rest of the week
Two smaller pull requests were sitting in the queue and I left them there rather than
poking at them:
#7502, a NameError fix in
the existing 1.1 code, and
#7510, which swapped in an
lti-advantage gem. Both had been open since the previous week.
Instead I went back to the specification. Not skimming for the parts I needed to make
Canvas work — reading it properly, in order, including the parts I had implemented by
copying from other people's tools.
That turned out to matter. There is a real difference between "I made the handshake
succeed" and "I understand what each step of the handshake is defending against." I had
the first. Writing the OIDC login by pattern-matching against blog posts, I could tell
you the state parameter is required. I could not have told you crisply what an
attacker does if you get it wrong.
Going through it deliberately, the shape of the protocol became much clearer. The
handshake has two legs, and each carries a specific defence:
- The platform asks the tool to begin a login. The tool generates a nonce and a state, and redirects back to the platform's authorization endpoint.
- The platform authenticates the user and POSTs an
id_tokenback to the tool. The tool verifies the token's signature against the platform's published keys, checks the nonce it issued, and only then signs anyone in.
The state proves the launch answers an initiation we started. The nonce ties the
token to that one initiation, so a captured token cannot be replayed. The signature
proves the platform, and only the platform, minted the token. Three separate defences,
and I had implemented all three as one undifferentiated blob of code.
Once I saw them as separate defences, they started to look like separate pieces of
work. Which is, I suspect, the point I was supposed to reach.
What I am carrying into next week
Rebuilding, and rebuilding differently. Not the same code re-uploaded in smaller
chunks, but actually reconsidered — because reading the spec properly turned up things
my POC had glossed over, particularly around binding a token to the specific deployment
that issued it.
The plan is to start from the pieces that stand alone and can be judged on their own
terms. A storage model for platform registrations does not need the launch to exist to
be reviewable. A JWT validator can be unit tested against forged tokens without any
controller at all.
I will be honest that the week felt bad while I was in it. There is a particular
discomfort in having working code and no way to ship it, and in reporting a week with
an empty commit graph. But I would rather find out in week 3 that my approach to
shipping was wrong than in week 10.
Next week: rebuilding the protocol layer, and discovering that the gem I had been
counting on does less than its name suggests.
Top comments (0)