This week produced the single most important line of the project so far, and a decision
I would reverse four days later. It also took a detour into a completely different
feature, which turned out to be a better use of time than it sounds.
The claim you must not trust
The commit is
fix(lti): key 1.3 identity on validated sub, not the email claim.
Here is the problem. When a launch arrives, the id_token contains claims about the
user — a subject identifier (sub), usually a name, and often an email address. The
natural way to sign somebody in is to take that email and look them up:
user = User.find_by(email: payload["email"])
That is what my POC did, and it is wrong in a way that took me a while to see, because
it works perfectly in every test you would think to write.
The token's signature proves the platform sent it. It does not prove the platform
verified anything inside it. The email claim is whatever the LMS has in its user record
— self-asserted, frequently unverified, sometimes editable by the user in their own
profile settings.
So if a CircuitVerse account exists with the email maintainer@circuitverse.org, and
somebody can get an LMS to emit a launch with that email claim, matching on email hands
them that account. The token is valid. The signature checks out. And they are signed in
as somebody else.
The fix is to identify people by the sub claim, which is the platform's own stable
identifier for that user and is not user-editable, scoped to the deployment it came
from:
User.find_or_create_by!(provider: "lti", uid: "#{deployment.id}:#{payload['sub']}")
The scoping matters as much as the choice of claim. Two different institutions can both
have a user with sub of 12345. Without the deployment prefix, those two people
collide into one CircuitVerse account.
Email still gets used — when provisioning a brand new account, we have to put something
in the email column. But it is used as an attribute of a user we have already
identified, never as the thing that identifies them. That distinction is the whole
lesson: authentication data and profile data are different, even when they arrive in
the same signed envelope.
The companion commit, fix(lti): return 401 for tokens missing required claims, comes
from the same place. If sub is missing, there is no identity to key on, and the right
answer is to reject the launch rather than fall back to something more convenient.
The cookie I weakened
The other commit from this week is
Add Canvas placements to tool config and SameSite=None session cookie, and I want to
write about it honestly because I undid half of it the following week.
The problem is real. When the platform POSTs the id_token back to CircuitVerse, that
is a cross-site POST — the request originates from Canvas's domain and lands on
ours. Browsers do not send SameSite=Lax cookies on cross-site POSTs. That is the
entire point of SameSite.
I had stored the login state in the session. So when the launch came back, the session
cookie was not sent, the session was empty, and the launch failed with no state to
check against.
The fix I reached for was to set SameSite=None on the session cookie, which makes the
browser send it on cross-site requests. It worked immediately.
It is also a change to how every cookie in the application behaves, made to serve one
endpoint. Every page on CircuitVerse would have a session cookie that browsers are
willing to attach to cross-site requests, weakening a default that exists specifically
to blunt CSRF — for a feature that is off by default and used by a small fraction of
users.
I did not see it that way when I wrote it. I saw a blocked feature and a config flag
that unblocked it. Seeing it required asking a different question: not "does this make
the launch work" but "what else does this change, for people not using this feature at
all?" More on how I fixed it next week.
The other half of the commit has aged better: Canvas placements in the tool
configuration document. Placements are how a tool declares where it wants to appear in
the LMS — course navigation, assignment selection. It is the difference between a tool
an admin can register and a tool an instructor can actually find.
The detour: subgroups
Midweek I switched to something unrelated: subgroups within a group, so an instructor
can split a class into smaller teams. Commits
feat(groups): add self-referential subgroups data layer and
feat(groups): subgroup management UI, controller and API scoping.
The design decision worth recording: a subgroup is just a Group with a
parent_group_id. That is it. No new model, no parallel hierarchy.
The alternative was a dedicated Subgroup model. It looks cleaner on a diagram and it
is much worse in practice, because assignments, membership, grades, notifications and
authorization policies are all written against Group. A new model means every one of
those systems needs to learn about a second kind of container, and every one of them is
a place to forget. A self-referential association means all of that machinery works
unchanged, and the entire feature is a nullable foreign key plus scoping.
Why work on this at all mid-project? Partly because it had been asked for. But the
honest reason is that I was waiting on review for the LTI work, and I had learned the
week before that pushing on a branch nobody has agreed to review does not move anything
forward. Having a second track meant review latency stopped being dead time.
What I would tell myself
Two things.
The signature does not vouch for the contents. A signed token proves who sent it,
not that what it says is true. Everything inside still needs to be evaluated on whether
the sender had any business asserting it.
"It works" is where you start asking questions, not where you stop. The
SameSite=None change worked on the first try. Working was exactly what stopped me
looking at it.
Next week: undoing the cookie change properly, and two pull requests getting closed on
the same day.
Top comments (0)