Skip to content

fix!: delete_user takes id, not email - #12

Merged
lakhansamani merged 3 commits into
mainfrom
fix/delete-user-id-only
Aug 7, 2026
Merged

fix!: delete_user takes id, not email#12
lakhansamani merged 3 commits into
mainfrom
fix/delete-user-id-only

Conversation

@lakhansamani

Copy link
Copy Markdown
Contributor

Mirrors the server-side breaking change in authorizerdev/authorizer#753.

_delete_user was keyed on email. Email is not an identifier every account has — a phone-only signup has none — so an email-keyed delete could not reach those accounts at all, and there was no second identifier accepted. Those accounts were permanent.

Change

 @dataclass
 class DeleteUserRequest(_Request):
-    email: str
+    id: str

Both sync and async delete_user serialise via req.to_dict(), so the dataclass is the whole surface — no client, dispatch, or query changes needed. The method docstrings never named email, so they stand.

Both integration call sites already had the id in scope, so migrating them needed no extra lookup:

  • test_live.py:553user_id was already bound from signup.user.id for the org-member assertions
  • test_live.py:694 — uses signup.user.id directly

BREAKING

DeleteUserRequest(email=...)DeleteUserRequest(id=...). Requires server 2.4.0+; the server removed the field rather than accepting both.

⚠️ Dependency pin worth a look. This repo declares authorizer-proto>=0.2.0rc0 with no upper bound. The companion regeneration (authorizerdev/authorizer-proto-python#2, bumped to 0.2.0rc1) is itself breaking, so the floating minimum means an older SDK release will also pull it and break at the call site. This PR fixes the call site here, but the spec does not prevent the skew. Suggest a bound.

Verified

Run in a clean venv with .[dev]:

  • pytest — 117 passed
  • mypy src — no issues in 12 source files
  • ruff check src tests — all checks passed
  • tests/integration/test_live.py parse-checked (needs a live server to run)

Mirrors authorizerdev/authorizer#753. Email is not an identifier every
account has — a phone-only signup has none — so an email-keyed delete could
not reach those accounts at all, and there was no second way in.

Both integration call sites already had the id in scope (user_id and
signup.user.id), so no extra lookup was needed to migrate them.

BREAKING CHANGE: DeleteUserRequest.email is replaced by DeleteUserRequest.id.
Requires server 2.4.0+ and authorizer-proto 0.2.0rc1+.
rc1 replaced DeleteUserRequest.email with .id and this SDK builds the id form,
so the floor moves with the break. The previous >=0.2.0rc0 floated across it:
an older SDK release would resolve rc1 and fail at the delete call.
@lakhansamani

Copy link
Copy Markdown
Contributor Author

Pushed 581077e — raised the authorizer-proto floor to >=0.2.0rc1, now that it is published on PyPI.

This closes the version-skew hole I flagged in the PR description. The old >=0.2.0rc0 floated across the break: an older SDK release would happily resolve rc1 and then fail at the delete call, because rc0 has email and rc1 has id. The floor now moves with the break.

Re-verified against the real published package, not the local checkout

Fresh venv, pip install -e ".[dev,grpc]", resolving from PyPI:

authorizer-proto        Version: 0.2.0rc1
>>> [f.name for f in admin_pb2.DeleteUserRequest.DESCRIPTOR.fields]
['id']
  • pytest — 117 passed
  • mypy src — clean
  • ruff check src tests — clean

Ready to merge from my side.

The live job pinned rc.13, which still has DeleteUserRequest.email. Since
delete_user now sends id, that server rejects it with 'unknown field' — real
version skew, not a test bug. rc.16 is the first RC with the id-only field.
@lakhansamani
lakhansamani merged commit 26c1676 into main Aug 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant