Teacher registration accepts an unverified Google or Microsoft account id, an unverified email address, and a reCAPTCHA response it only half checks

[Isaries]

Summary

POST /api/teacher/register is unauthenticated, as it must be, and it establishes
three things about the person registering: that they hold the email address, that
they hold the single sign-on identity they claim, and that they are not a bot. On
develop at 1958196c2 it verifies none of them. The first of the three has
consequences beyond spam.

Found on a TWISE deployment, where open self-service registration was used for
about six months to publish SEO spam disguised as curriculum, until roughly half
of every project on the site belonged to those accounts. Working out why the
registration itself always succeeded is what turned these up. That deployment has
since closed self-service registration behind administrator approval, which is
the mitigation available to anyone else in the meantime.

1. An arbitrary Google or Microsoft account id can be bound to a new account

TeacherRegistrationAPIController.setPassword:

private void setPassword(Map<String, String> teacherFields, TeacherUserDetails tud) {
  String googleUserId = teacherFields.get("googleUserId");
  String microsoftUserId = teacherFields.get("microsoftUserId");
  if (isSet(googleUserId)) {
    tud.setGoogleUserId(googleUserId);
    setRandomPassword(tud);
  } else if (isSet(microsoftUserId)) {
    ...

The value is taken from the request body and stored as the account's single
sign-on identity. No ID token is presented, and nothing is verified.

Sign-in is the part that is done correctly, and that is what makes this matter.
GoogleOpenIdConnectFilter.attemptAuthentication decodes and verifies the ID
token, checks its claims, and then looks the account up by the verified sub:

final Jwt tokenDecoded = JwtHelper.decodeAndVerify(idToken, verifier(kid));
...
verifyClaims(authInfo);
final UserDetails user = userDetailsService.loadUserByGoogleUserId(authInfo.get("sub"));

So an account registered with somebody else's sub in the request body is the
account that answers when that person signs in with Google. The registrant chose
its email address and its other details; the password is random, so the account
is reachable only through that sign-in path and through password recovery to the
address the registrant supplied.

A Google sub is a stable identifier, not a secret. It is disclosed to every
relying party the person signs into, which is the ordinary way it becomes known
to somebody other than Google.

Suggested fix. Require an ID token at registration and verify it exactly as
GoogleOpenIdConnectFilter already does, then take sub from the verified token
rather than from the request body. The verification code exists; registration
simply does not call it. The same applies to microsoftUserId.

2. emailValid is set to true unconditionally

TeacherRegistrationAPIController, in the method that builds the new user
details:

setPassword(teacherFields, tud);
tud.setEmailValid(true);

Nothing confirms the address, so the column records that a check happened rather
than that it passed. Any address completes registration, including one belonging
to somebody else.

Suggested fix. Set it after a confirmation round-trip, or drop the field. A
flag that is always true is worse than no flag, because code and operators read it
as an assurance.

3. reCAPTCHA verification reads success and never score

ControllerUtil.isReCaptchaResponseValid:

JSONObject responseObject = new JSONObject(responseString.toString());
isValid = responseObject.getBoolean("success");

For reCAPTCHA v2 that is correct. For v3 it is not: v3 returns success: true
for any well-formed token and puts its judgement in score. A deployment
configured with a v3 key therefore has no bot protection at all, and no error
anywhere says so — the request succeeds, the log is clean, and the operator
believes the site is protected. The action claim is not checked either, so a
token minted for one action is accepted for another.

Suggested fix. Read score and compare it against a configurable threshold,
check action, and reject on failure. If only v2 is intended to be supported,
saying so where the key is configured would be enough, because the failure mode
is otherwise invisible at every stage of setting it up.

Why these are worth taking together

Each one alone is a missing check. Together they mean an unauthenticated request
can create a teacher account that claims an email address it does not hold and a
single sign-on identity it does not hold, without having demonstrated that a
person is involved. Item 1 is the one with impact beyond spam; items 2 and 3 are
what make the endpoint usable at volume.

Happy to open pull requests for any of these. Item 1 is the one worth doing first
and is mostly a matter of calling verification that is already written.

Related: #343, which is unrelated in cause but was found while cleaning up after
the same incident.