Skip to content

added user role update event#8674

Open
RjManhas wants to merge 1 commit into
logto-io:masterfrom
RjManhas:master
Open

added user role update event#8674
RjManhas wants to merge 1 commit into
logto-io:masterfrom
RjManhas:master

Conversation

@RjManhas

@RjManhas RjManhas commented Apr 19, 2026

Copy link
Copy Markdown

Summary

Added new webhook events for when user or a user org roles are updated.

Testing

I tested it by building a docker compose, and testing api routes etc.

Checklist

  • [x ] .changeset
  • [x ] unit tests
  • [x ] integration tests
  • [ x] necessary TSDoc comments

@github-actions

Copy link
Copy Markdown

COMPARE TO master

Total Size Diff ⚠️ 📈 +15.76 KB

Diff by File
Name Diff
.gitattributes 📈 +288 Bytes
Dockerfile 📈 +310 Bytes
docker-compose.local.yml 📈 +1.36 KB
package.json 📈 +24 Bytes
packages/core/src/libraries/hook/context-manager.ts 📈 +387 Bytes
packages/core/src/routes/admin-user/role.ts 📈 +1.17 KB
packages/core/src/routes/organization/index.ts 📈 +24 Bytes
packages/core/src/routes/organization/user/index.ts 📈 +955 Bytes
packages/core/src/routes/organization/user/role-relations.ts 📈 +2.14 KB
packages/core/src/routes/role.user.ts 📈 +1.21 KB
packages/integration-tests/src/tests/api/hook/hook.trigger.data.test.ts 📈 +3.54 KB
packages/integration-tests/src/tests/api/hook/test-cases.ts 📈 +3.93 KB
packages/schemas/src/foundations/jsonb-types/hooks.ts 📈 +465 Bytes

@wangsijie wangsijie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 PR Review

This PR adds role-update webhook events and richer payloads for management routes, plus local Docker setup changes.

  • 🔒 Security: clean
  • 🏗️ Architecture: 0 high, 3 medium
  • 👨‍💻 Engineering: clean

Verdict: ⚠️ Needs attention

Comment thread packages/schemas/src/foundations/jsonb-types/hooks.ts
Comment thread packages/schemas/src/foundations/jsonb-types/hooks.ts
Comment thread package.json
@simeng-li

Copy link
Copy Markdown
Contributor

@RjManhas, please check the failed CI jobs

@simeng-li simeng-li added the pending-verification Something is still under investigation label May 14, 2026
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale label Jun 14, 2026
@wangsijie

Copy link
Copy Markdown
Contributor

Thanks for working on this. I do not think this PR fully solves the original problem of capturing user role changes.

The new User.Roles.Updated and Organization.UserRoles.Updated events are only emitted from a subset of explicit Management API role-relation routes. There are still several paths that change a user's effective roles but would not emit these new events, for example:

  • default user roles assigned during user creation
  • organization roles assigned through JIT provisioning
  • organization roles assigned when accepting an organization invitation
  • first-admin / tenant organization provisioning
  • role or organization role deletion, where relation rows are removed through foreign-key cascade

So consumers listening to these new events would still miss role changes. There is also the opposite issue: some routes may emit an Updated event even when the submitted role set does not actually change the relation rows.

Because of this, I do not think adding manual webhook triggers in these route handlers is enough to solve the original problem. If we want to guarantee coverage for role changes, the event should probably be emitted from a more centralized mutation path for user-role and organization-user-role relations, and role deletion should snapshot affected users before deletion so we can emit updates after cascade removal.

@github-actions github-actions Bot removed the stale label Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pending-verification Something is still under investigation size/xl

Development

Successfully merging this pull request may close these issues.

3 participants