Skip to content

refactor(proto): consolidate to single Authorizer service; method names mirror GraphQL ops#619

Closed
lakhansamani wants to merge 3 commits into
feat/test-coveragefrom
feat/single-authorizer-service
Closed

refactor(proto): consolidate to single Authorizer service; method names mirror GraphQL ops#619
lakhansamani wants to merge 3 commits into
feat/test-coveragefrom
feat/single-authorizer-service

Conversation

@lakhansamani
Copy link
Copy Markdown
Contributor

Stacked on top of #618. Per your direction: collapse the nine per-resource proto services into one Authorizer service whose method names match the GraphQL operation names 1:1.

What changed

The 9 services (Meta / User / Session / MagicLink / EmailVerification / PasswordReset / OtpChallenge / Token / Authz) become one Authorizer service with 19 RPCs:

`Signup`, `Login`, `Logout`, `MagicLinkLogin`, `VerifyEmail`, `ResendVerifyEmail`, `VerifyOtp`, `ResendOtp`, `ForgotPassword`, `ResetPassword`, `Profile`, `UpdateProfile`, `DeactivateAccount`, `Revoke`, `Session`, `ValidateJwtToken`, `ValidateSession`, `Meta`, `Permissions`

Trade-offs noted

Pros: mirrors GraphQL surface users already know; single typed client per language; no resource-name conventions to learn.
Cons (vs the AIP-121/122 design we had): loses `List/Get/Create` symmetry per resource; doesn't scale as cleanly past 50+ methods. Acceptable for an auth surface where most ops are stateless verbs and admin services stay on GraphQL.

Design decisions called out

  1. Buf STANDARD's `RPC_REQUEST_RESPONSE_UNIQUE` stays enforced. Methods whose underlying payload is shared (`AuthResponse`, `User`, `Meta`) wrap it in per-RPC `SignupResponse`/`LoginResponse`/`MetaResponse`/etc. One extra accessor on the client side; one extra field per response — but lint passes and each response can evolve independently.
  2. `SERVICE_SUFFIX` lint excepted in `buf.yaml`. The single service is intentionally named `Authorizer`, not `AuthorizerService` — it IS the authorizer.
  3. REST mapping rule: `GET /v1/{method}` for genuinely-empty queries (Meta, Profile, Permissions, Logout); `POST /v1/{method}` for everything else. Path uses snake_case method name (`POST /v1/magic_link_login`, `/v1/forgot_password`, ...) so REST clients see the same identifier they'd see in GraphQL.
  4. MCP-exposed tools (proto annotation `mcp_tool.exposed=true`): `meta`, `profile`, `session`, `permissions`. Credential-bearing methods stay unexposed.

Files

  • Deleted: `proto/authorizer/{meta,user,session,verification,token,authz}/v1/**` and their generated counterparts; `internal/grpcsrv/handlers/{meta,stubs}.go`.
  • New: `proto/authorizer/v1/{authorizer,types}.proto`, `gen/go/authorizer/v1/**` regenerated, `internal/grpcsrv/handlers/authorizer.go` (Meta real, 18 stubs).
  • Updated: `proto/buf.yaml`, `internal/grpcsrv/server.go`, `internal/gateway/mount.go`, and the 7 test files that referenced old per-service types.
  • Unchanged: `internal/mcp/{scanner,schema,server}.go` (annotation-driven; discovered tools from any proto package without modification), `internal/grpcsrv/transport/grpc_metadata.go`, `internal/service/`, `cmd/root.go`.

Test plan

  • `go build ./...` clean
  • `buf lint proto` clean
  • All 17 packages green: integration suite 65s, no regression
  • `TestAuthorizerStubsReturnUnimplemented` locks down the 18 not-yet-migrated methods
  • `TestMCPListAndCallMeta` confirms 4 MCP tools (meta, profile, session, permissions) auto-discovered from annotations
  • CI green on this stacked PR

🤖 Generated with Claude Code

lakhansamani and others added 3 commits May 30, 2026 15:11
…es mirror GraphQL ops

User direction: collapse the nine per-resource proto services (Meta,
User, Session, MagicLink, EmailVerification, PasswordReset, OtpChallenge,
Token, Authz) into a single Authorizer service with method names that
match the GraphQL operation names 1:1.

Trade-offs
==========
Pros:
- Surface mirrors what existing GraphQL users already know (Signup, Login,
  Logout, Meta, Profile, Session, Permissions, ValidateJwtToken, ...).
- Single typed client per language; SDK discovery is trivial.
- No resource-name conventions to learn; method = verb.

Cons (vs the prior AIP-121/122 resource-oriented design):
- Loses List/Get/Create symmetry per resource — acceptable for an auth
  surface where most ops are stateless verbs.
- Adding new resources scales worse than per-service. Acceptable: the
  public auth surface is fairly stable; admin services that DO want
  resource orientation already live on GraphQL (out of scope here).

Design choices called out
=========================
1. Buf STANDARD's RPC_REQUEST_RESPONSE_UNIQUE remains enforced. Methods
   whose underlying payload is shared (AuthResponse, User, Meta) wrap
   that payload in per-RPC SignupResponse/LoginResponse/MetaResponse/etc.
   One extra accessor on the client side; one extra field per response.

2. SERVICE_SUFFIX lint excepted in buf.yaml. The single service is
   intentionally named "Authorizer" rather than "AuthorizerService" —
   it IS the authorizer, not "one of many authorizer services".

3. REST mapping:
     - GET /v1/{method}    for genuinely-empty queries (Meta, Profile,
                           Permissions, Logout)
     - POST /v1/{method}   for everything else
   Path uses snake_case method name (POST /v1/magic_link_login,
   /v1/forgot_password, etc.) so REST clients see the same identifier
   they'd see in GraphQL.

4. MCP-exposed tools (proto annotation mcp_tool.exposed=true): meta,
   profile, session, permissions. Credential-bearing methods (Signup,
   Login, password/OTP/reset) stay unexposed.

Files
=====
DELETED
  proto/authorizer/{meta,user,session,verification,token,authz}/v1/**
  gen/go/authorizer/{meta,user,session,verification,token,authz}/v1/**
  internal/grpcsrv/handlers/{meta,stubs}.go

NEW
  proto/authorizer/v1/authorizer.proto  (single service, 19 RPCs)
  proto/authorizer/v1/types.proto       (shared User, AuthResponse, Meta, Permission)
  gen/go/authorizer/v1/**               (regenerated)
  internal/grpcsrv/handlers/authorizer.go  (Meta real, rest Unimplemented)

UPDATED
  proto/buf.yaml         — except SERVICE_SUFFIX
  internal/grpcsrv/server.go    — register single Authorizer service
  internal/gateway/mount.go     — register single Authorizer handler
  internal/integration_tests/grpc_meta_test.go        — Authorizer.Meta
  internal/integration_tests/grpc_surface_test.go     — 18 Authorizer methods
  internal/integration_tests/rest_meta_test.go        — wrapped response shape
  internal/integration_tests/mcp_test.go              — discovers 4 MCP tools
  internal/integration_tests/mcp_stubs_test.go        — permissions stub
  internal/mcp/schema_test.go                         — uses authorizerv1
  internal/grpcsrv/interceptors/interceptors_test.go  — uses authorizerv1

UNCHANGED
  internal/mcp/scanner.go, schema.go, server.go      — annotation-driven;
    discovers tools from any proto package without modification
  internal/grpcsrv/transport/grpc_metadata.go        — generic gRPC bridge
  internal/service/                                  — transport-agnostic
  cmd/root.go                                        — same wiring shape

Tests
=====
All 17 packages green:
  ok  internal/integration_tests        65s
  ok  internal/grpcsrv/{interceptors,transport}
  ok  internal/mcp / service / cookie / parsers / ...
TestAuthorizerStubsReturnUnimplemented locks down the contract for all 18
not-yet-migrated methods; TestMCPListAndCallMeta verifies the four
MCP-exposed tools surface from the single service.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per review: keep buf STANDARD's SERVICE_SUFFIX rule enforced rather than
excepted. The service is now `AuthorizerService`; symbols change to
`AuthorizerServiceClient`, `AuthorizerServiceServer`,
`RegisterAuthorizerServiceServer`, `RegisterAuthorizerServiceHandler`,
`UnimplementedAuthorizerServiceServer`, `NewAuthorizerServiceClient`.

Method names + REST paths + MCP tool names are unchanged — only the
service identifier moves. The Go handler type stays `AuthorizerHandler`
(we don't repeat "Service" at the call site in Go).

Files:
  proto/buf.yaml                — removed SERVICE_SUFFIX from `except`
  proto/authorizer/v1/authorizer.proto  — service AuthorizerService { }
  gen/go/authorizer/v1/**       — regenerated
  gen/openapi/authorizer.swagger.json  — regenerated
  internal/grpcsrv/handlers/authorizer.go      — UnimplementedAuthorizerServiceServer
  internal/grpcsrv/server.go                   — RegisterAuthorizerServiceServer
  internal/gateway/mount.go                    — RegisterAuthorizerServiceHandler
  internal/integration_tests/grpc_meta_test.go — NewAuthorizerServiceClient
  internal/integration_tests/grpc_surface_test.go — test renamed; client renamed
  internal/integration_tests/{rest_meta,mcp_stubs,mcp}_test.go — comment refs

Tests: full SQLite integration suite (70s) + all 17 packages green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two changes:
  1. Ran `buf format -w proto` — sorts imports alphabetically, normalises
     multi-line validator option blocks, single-space comment trailers.
  2. CI: `bufbuild/buf-action@v1` now runs `buf format -d --exit-code`
     (format: true) so any future drift fails the proto job.

`buf lint proto` is clean. The only generated-code change is a 10-line
reshuffle in authorizer.pb.go from the import reordering.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lakhansamani
Copy link
Copy Markdown
Contributor Author

Superseded by #620, which consolidates this stack into a single PR against main. All blocking review findings from this PR were addressed in #620; see its body for the per-finding traceback.

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