From 6227e767215a9f6b8de73caaee19c05669a10d5c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 20:08:53 +0000 Subject: [PATCH] fix(socrate): RegisterUser and GetUserAsService call the service-account routes Both methods sent a client_credentials token to /api/apps/{id}/users..., routes that need an app admin's user token, so Socrate answered 401. RegisterUser now posts to /api/apps/{id}/service/users (Name still sent). GetUserAsService now reads GET /api/apps/{id}/service/users/{user_id} (added in the Socrate release after v1.5.3); it returns nil, nil only for Socrate's own JSON 404 and an error for a router 404, so a missing route is never read as "no such user". No exported identifier changes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GKRxaeYxyDhmt42cehLsGA --- CHANGELOG.md | 11 +++ docs/CLIENT-INTEGRATION.md | 8 +-- socrate/attribution_test.go | 4 +- socrate/client.go | 28 ++++++-- socrate/client_test.go | 2 +- socrate/service_user_routes_test.go | 100 ++++++++++++++++++++++++++++ 6 files changed, 141 insertions(+), 12 deletions(-) create mode 100644 socrate/service_user_routes_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e571b97..5233d34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,17 @@ All notable changes to backendkit are documented here. Format: ## [Unreleased] +### Fixed + +- **`socrate.Client.RegisterUser` and `GetUserAsService` call the service-account routes.** They + sent a service-account token to `/api/apps/{id}/users…`, routes that need an app admin's user + token, so Socrate answered `401` and neither method worked. `RegisterUser` now posts to + `/api/apps/{id}/service/users` (the route `InviteUserAsService` uses; `Name` is still sent). + `GetUserAsService` now reads `GET /api/apps/{id}/service/users/{user_id}`, added in the + Socrate release after v1.5.3. It returns nil, nil only for Socrate's own 404 (not a member, or no such user); a Socrate + without the route answers with an error that names the version, never a silent "not found". No + exported identifier changes. + ### Changed - Docs: `socrate.ClientConfig.AdminBaseURL` is documented as required in production. The value diff --git a/docs/CLIENT-INTEGRATION.md b/docs/CLIENT-INTEGRATION.md index 405c181..fcf7556 100644 --- a/docs/CLIENT-INTEGRATION.md +++ b/docs/CLIENT-INTEGRATION.md @@ -430,8 +430,8 @@ automatically from `client_id` (cached). | `DeleteUser(ctx, userID)` | JWT | `error` | removes the user's role in this app. | | `ResendVerification(ctx, userID)` | JWT | `error` | re-sends the verification email. | | `ForcePasswordReset(ctx, userID)` | JWT | `error` | triggers a password-reset email. | -| `GetUserAsService(ctx, userID)` | M2M | `*User` | service-account variant; **nil,nil** on 404. | -| `RegisterUser(ctx, CreateUserRequest)` | M2M | `*CreateUserResult` | M2M create+invite; `ErrUserAlreadyExists` on 409. | +| `GetUserAsService(ctx, userID)` | M2M | `*User` | one of the app's members by numeric id (a token's `sub`); **nil,nil** when not a member (Socrate's 404). Needs a Socrate later than v1.5.3; an older one answers with an error, not nil,nil. | +| `RegisterUser(ctx, CreateUserRequest)` | M2M | `*CreateUserResult` | M2M create+invite, with `Name`; `ErrUserAlreadyExists` on 409. | | `InviteUserAsService(ctx, ServiceInviteRequest)` | M2M | `*CreateUserResult` | dedicated M2M invite route; no human JWT needed. | #### Passwordless — Admin port @@ -1184,8 +1184,8 @@ port (8081 in the default deployment). | `ListUsers` / `GetUser` / `CreateUser` | `…/api/apps/{id}/users` | JWT | Admin | | `UpdateUserRole` / `DeleteUser` | `…/api/apps/{id}/users/{uid}` | JWT | Admin | | `ResendVerification` / `ForcePasswordReset` | `…/users/{uid}/…` | JWT | Admin | -| `RegisterUser` / `GetUserAsService` | `…/api/apps/{id}/users…` | M2M | Admin | -| `InviteUserAsService` | `POST …/api/apps/{id}/service/users` | M2M | Admin | +| `RegisterUser` / `InviteUserAsService` | `POST …/api/apps/{id}/service/users` | M2M | Admin | +| `GetUserAsService` | `GET …/api/apps/{id}/service/users/{uid}` | M2M | Admin | | `SendMagicLink` | `POST …/api/apps/{id}/service/magic-link` | M2M | Admin | | `ListApps` … `RotateSecret` | `…/api/admin/apps…` | JWT (admin) | Admin | | `AdminListUsers` … `RevokeUserTokens` | `…/api/admin/users…` | JWT (superadmin) | Admin | diff --git a/socrate/attribution_test.go b/socrate/attribution_test.go index 9591069..dc0cdc5 100644 --- a/socrate/attribution_test.go +++ b/socrate/attribution_test.go @@ -159,7 +159,7 @@ func newAttributionServer(t *testing.T) *attributionServer { _ = json.NewEncoder(w).Encode(socrate.ProfileInfo{Sub: "7"}) case "/oauth/introspect": _ = json.NewEncoder(w).Encode(socrate.IntrospectResponse{Active: true}) - case "/api/apps/42/users/7": + case "/api/apps/42/service/users/7": _ = json.NewEncoder(w).Encode(socrate.User{ID: 7}) default: http.NotFound(w, r) @@ -278,7 +278,7 @@ func TestNonUserCallsNeverSendAttribution(t *testing.T) { if _, err := c.GetCurrentUserProfile(ctx); err != nil { t.Fatalf("GetCurrentUserProfile: %v", err) } - for _, key := range []string{"/oauth/token#client_credentials", "/api/apps/42/users/7", "/oauth/introspect", "/oauth/userinfo"} { + for _, key := range []string{"/oauth/token#client_credentials", "/api/apps/42/service/users/7", "/oauth/introspect", "/oauth/userinfo"} { got := srv.last(t, key) if got.hasXFF || got.ua != "Go-http-client/1.1" { t.Errorf("%s carried attribution: XFF=%q UA=%q", key, got.xff, got.ua) diff --git a/socrate/client.go b/socrate/client.go index 98f6baf..8f8f07d 100644 --- a/socrate/client.go +++ b/socrate/client.go @@ -230,6 +230,15 @@ func (c *Client) getAppIDAsService(ctx context.Context) (string, error) { return c.parseAppID(resp) } +// isSocrateErrorBody reports whether b is a Socrate handler's JSON error +// ({"error": "..."}), as opposed to a router's plain "404 page not found". +func isSocrateErrorBody(b []byte) bool { + var e struct { + Error *string `json:"error"` + } + return json.Unmarshal(b, &e) == nil && e.Error != nil +} + func (c *Client) parseAppID(resp *http.Response) (string, error) { b, err := readBody(resp) if err != nil { @@ -598,15 +607,19 @@ func (c *Client) ForcePasswordReset(ctx context.Context, userID string) error { // User management — service-account methods (Admin port) // ──────────────────────────────────────────────────────────────────────────── -// GetUserAsService retrieves a user by numeric ID using the service-account token. -// Returns nil, nil when the user is not found (404). +// GetUserAsService retrieves one of the app's members by numeric ID using the +// service-account token, via GET /api/apps/{id}/service/users/{user_id} +// (Socrate later than v1.5.3). Returns nil, nil when the user is not a member +// of the app or does not exist (404). A Socrate without that route answers +// with an error rather than nil, nil, so a missing route is never mistaken +// for a missing user. func (c *Client) GetUserAsService(ctx context.Context, userID string) (*User, error) { appID, err := c.getAppIDAsService(ctx) if err != nil { return nil, fmt.Errorf("resolve app ID: %w", err) } resp, err := c.doWithServiceToken(ctx, http.MethodGet, - c.adminURL(fmt.Sprintf("/api/apps/%s/users/%s", appID, url.PathEscape(userID))), nil) + c.adminURL(fmt.Sprintf("/api/apps/%s/service/users/%s", appID, url.PathEscape(userID))), nil) if err != nil { return nil, err } @@ -615,6 +628,10 @@ func (c *Client) GetUserAsService(ctx context.Context, userID string) (*User, er return nil, err } if resp.StatusCode == http.StatusNotFound { + if !isSocrateErrorBody(b) { + return nil, fmt.Errorf("get user HTTP 404 without a Socrate error body: "+ + "GET /api/apps/{id}/service/users/{user_id} needs a Socrate later than v1.5.3: %s", b) + } return nil, nil } if resp.StatusCode != http.StatusOK { @@ -627,7 +644,8 @@ func (c *Client) GetUserAsService(ctx context.Context, userID string) (*User, er return &u, nil } -// RegisterUser creates a user and adds them to the app using the service-account token. +// RegisterUser creates a user and adds them to the app using the service-account token, +// via POST /api/apps/{id}/service/users. Unlike InviteUserAsService it also sends Name. // Socrate dispatches an invite email automatically. // Returns ErrUserAlreadyExists on 409. func (c *Client) RegisterUser(ctx context.Context, req CreateUserRequest) (*CreateUserResult, error) { @@ -636,7 +654,7 @@ func (c *Client) RegisterUser(ctx context.Context, req CreateUserRequest) (*Crea return nil, fmt.Errorf("resolve app ID: %w", err) } resp, err := c.doWithServiceToken(ctx, http.MethodPost, - c.adminURL(fmt.Sprintf("/api/apps/%s/users", appID)), req) + c.adminURL(fmt.Sprintf("/api/apps/%s/service/users", appID)), req) if err != nil { return nil, err } diff --git a/socrate/client_test.go b/socrate/client_test.go index 34528a7..cc51ce7 100644 --- a/socrate/client_test.go +++ b/socrate/client_test.go @@ -230,7 +230,7 @@ func TestGetServiceToken_ExchangesCredentials(t *testing.T) { Apps: []socrate.App{{ID: 5, ClientID: "cid"}}, }) }) - mux.HandleFunc("/api/apps/5/users/7", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/api/apps/5/service/users/7", func(w http.ResponseWriter, r *http.Request) { json.NewEncoder(w).Encode(socrate.User{ID: 7, Email: "svc@x.com"}) }) srv, close := newTestServer(mux) diff --git a/socrate/service_user_routes_test.go b/socrate/service_user_routes_test.go new file mode 100644 index 0000000..82164a0 --- /dev/null +++ b/socrate/service_user_routes_test.go @@ -0,0 +1,100 @@ +package socrate_test + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/ovander/backendkit/socrate" +) + +// serviceRoutesServer is a fake Socrate that serves only the service-account +// routes, and records the path and bearer of every admin-port call. +func serviceRoutesServer(t *testing.T, users http.HandlerFunc) (*socrate.Client, *[]string) { + t.Helper() + var calls []string + mux := http.NewServeMux() + mux.HandleFunc("/oauth/token", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{"access_token": "svc-tok", "expires_in": 3600, "token_type": "Bearer"}) + }) + mux.HandleFunc("/api/apps/3/service/users", func(w http.ResponseWriter, r *http.Request) { + calls = append(calls, r.Method+" "+r.URL.Path+" "+r.Header.Get("Authorization")) + var body map[string]string + _ = json.NewDecoder(r.Body).Decode(&body) + if body["email"] != "new@example.test" || body["name"] != "New" || body["role"] != "user" { + t.Errorf("RegisterUser body = %v", body) + } + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(socrate.CreateUserResult{UserID: 21, Role: "user", EmailSent: true}) + }) + mux.HandleFunc("/api/apps/3/service/users/", func(w http.ResponseWriter, r *http.Request) { + calls = append(calls, r.Method+" "+r.URL.Path+" "+r.Header.Get("Authorization")) + users(w, r) + }) + // The user-token routes answer a service token the way Socrate does. + mux.HandleFunc("/api/apps/3/users", func(w http.ResponseWriter, r *http.Request) { + calls = append(calls, r.Method+" "+r.URL.Path) + http.Error(w, `{"error":"invalid token claims"}`, http.StatusUnauthorized) + }) + mux.HandleFunc("/api/apps/3/users/", func(w http.ResponseWriter, r *http.Request) { + calls = append(calls, r.Method+" "+r.URL.Path) + http.Error(w, `{"error":"invalid token claims"}`, http.StatusUnauthorized) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + c, err := socrate.NewClient(socrate.ClientConfig{ + BaseURL: srv.URL, AdminBaseURL: srv.URL, ClientID: "cid", ClientSecret: "secret", AppID: "3", + }) + if err != nil { + t.Fatal(err) + } + return c, &calls +} + +func TestRegisterUser_UsesTheServiceRoute(t *testing.T) { + c, calls := serviceRoutesServer(t, nil) + res, err := c.RegisterUser(context.Background(), socrate.CreateUserRequest{Email: "new@example.test", Name: "New", Role: "user"}) + if err != nil || res.UserID != 21 || !res.EmailSent { + t.Fatalf("RegisterUser = %+v, %v", res, err) + } + if len(*calls) != 1 || (*calls)[0] != "POST /api/apps/3/service/users Bearer svc-tok" { + t.Fatalf("calls = %v", *calls) + } +} + +func TestGetUserAsService_UsesTheServiceRoute(t *testing.T) { + c, calls := serviceRoutesServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/apps/3/service/users/7" { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":"user not in app"}`)) + return + } + _ = json.NewEncoder(w).Encode(socrate.User{ID: 7, Email: "m@example.test", Role: "admin"}) + }) + u, err := c.GetUserAsService(context.Background(), "7") + if err != nil || u == nil || u.ID != 7 || u.Role != "admin" { + t.Fatalf("GetUserAsService = %+v, %v", u, err) + } + if (*calls)[0] != "GET /api/apps/3/service/users/7 Bearer svc-tok" { + t.Fatalf("calls = %v", *calls) + } + + // Socrate's own 404 (not a member, or no such user) is nil, nil. + u, err = c.GetUserAsService(context.Background(), "8") + if u != nil || err != nil { + t.Fatalf("non-member = %+v, %v; want nil, nil", u, err) + } +} + +// A Socrate without the route answers the router's plain-text 404: that is an +// error, never a silent "no such user". +func TestGetUserAsService_MissingRouteIsAnError(t *testing.T) { + c, _ := serviceRoutesServer(t, http.NotFound) + u, err := c.GetUserAsService(context.Background(), "7") + if u != nil || err == nil || !strings.Contains(err.Error(), "v1.5.3") { + t.Fatalf("missing route = %+v, %v; want an error naming the Socrate version", u, err) + } +}