Skip to content

test_plotusers: two tests still assert the pre-OIDC cron auth (failing since dec61b4) #209

Description

@sspickle

Summary

Two tests in tests/test_plotusers.py have failed since 2026-06-05. The app is fine — the tests describe an auth scheme the route no longer uses.

FAILED tests/test_plotusers.py::test_update_user_count_appends_new_point - assert 403 == 200
FAILED tests/test_plotusers.py::test_update_user_count_creates_setting_when_missing - assert 403 == 200

Cause

The tests send the old App Engine cron header:

response = client.get('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/admin/update-user-count', headers={'X-Appengine-Cron': 'true'})

but dec61b4 ("add better auth to update count route", 2026-06-05) replaced that check with Cloud Scheduler OIDC verification:

auth_header = flask.request.headers.get('Authorization', '')
if not auth_header.startswith('Bearer '):
    return flask.Response('Forbidden', status=403)
claim = id_token.verify_oauth2_token(auth_header[7:], requests.Request(), audience=_SCHEDULER_AUDIENCE)
if claim.get('email') != _SCHEDULER_SA:
    return flask.Response('Forbidden', status=403)

The tests were written 2026-05-21, two weeks before that change, and were never updated. The 403 is the route correctly rejecting a request with no bearer token.

Second, quieter problem in the same commit

dec61b4 also moved the user-count read from ndb to datastore.Client():

ds_client = datastore.Client()
stat = ds_client.get(ds_client.key('__Stat_Kind__', 'User'))

The tests patch only google.cloud.ndb.Key. So fixing the auth alone will not make them pass — the datastore client needs mocking too, or they will fail further down.

Suggested fix

  1. Patch id_token.verify_oauth2_token to return a claim whose email is _SCHEDULER_SA, and send an Authorization: Bearer <anything> header — mirroring what Cloud Scheduler actually sends.
  2. Mock datastore.Client alongside the existing ndb.Key patch.
  3. Add a case asserting that a request without a valid token still gets a 403 — the property most worth protecting, and currently untested.

Why it matters beyond the red

Two permanently-failing tests train everyone to read "19 passed, 2 failed" as success, which is exactly how a real regression goes unnoticed. That happened during a dependency bump today: the same two failed before and after, and confirming they were pre-existing took a separate run against the old pins.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions