Skip to content

Develop#39

Merged
0xmanhnv merged 3 commits into
mainfrom
develop
Apr 9, 2026
Merged

Develop#39
0xmanhnv merged 3 commits into
mainfrom
develop

Conversation

@0xmanhnv
Copy link
Copy Markdown
Collaborator

@0xmanhnv 0xmanhnv commented Apr 9, 2026

No description provided.

0xmanhnv added 3 commits April 9, 2026 06:53
Several gaps around the new member-suspension feature:

1. Audit log used the wrong action type. SuspendMember and
   ReactivateMember both wrote ActionMemberRemoved, so compliance
   reports could not distinguish "paused" from "deleted". Add
   ActionMemberSuspended / ActionMemberReactivated and wire them in.

2. Re-invitation could not handle a suspended member. CreateInvitation
   and AddMember (admin paths) and AcceptInvitation /
   AcceptInvitationWithRefreshToken (user paths) all checked existing
   membership but only knew "yes / no". After my Option A fix that
   exposes membership.status to the service layer, detect the suspended
   case and return a clear, action-oriented error:

     - admin: "this user has a suspended membership in this tenant —
       reactivate them via the Members page instead of sending a new
       invitation"
     - user:  "your access to this team is suspended — please contact
       an administrator to be reactivated"

   This protects the suspension audit trail (re-invitation can't
   silently bypass it) and gives the operator a usable next step.

3. Suspended-only-tenant login was unhelpful. After I taught
   GetUserMemberships to filter out suspended rows, a user whose only
   membership had been suspended was bounced into /onboarding/create-team
   with no explanation. Add GetUserSuspendedMemberships, surface the
   list as LoginResult.SuspendedTenants, expose it in the JSON
   LoginResponse as suspended_tenants, and let the UI show "your access
   to {tenant} is suspended" instead of routing to onboarding.

Includes an integration test (tests/integration/member_suspension_test.go)
that creates a tenant + user + active membership, suspends, and asserts
that ListMembersWithUserInfo / SearchMembersWithUserInfo /
GetMemberByEmail / GetMemberStats all reflect the new status. This is a
regression guard for the bug where the queries silently selected
users.status instead of tenant_members.status.
These methods existed on the User entity, on UserService, and as a
RequireActiveUser middleware, but no handler or route ever called
them — member access is managed at the membership level via
TenantService.SuspendMember / ReactivateMember. The OSS product has
no admin path that suspends a user globally.

Removed:
- User.Suspend() / Activate() / Deactivate() / IsSuspended()
- UserService.SuspendUser() / ActivateUser() (and their tests)
- middleware.RequireActiveUser()
- The createSuspendedUserForTest helper that propped up the dead tests

Kept (intentionally):
- StatusInactive / StatusSuspended constants on the user entity
- The login-time check `u.Status() == user.StatusSuspended` in
  auth_service.go
- The "suspended user attempted access" warn log in user_sync middleware

Rationale: an operator can still flip users.status='suspended' directly
in SQL for incident response, and the login check honours that as
defense-in-depth. The constants and the read path stay so a non-default
value round-trips through the parser without coercion.
Shell test scripts belong under scripts/tests/ alongside the rest of
the e2e and integration shell harness — tests/ is for Go test files
that are picked up by `go test`. The lone test_notification_api.sh
under tests/scripts/ was a leftover from before scripts/tests/ existed
and confused the layout. Removed the now-empty tests/scripts/ folder.
@0xmanhnv 0xmanhnv merged commit cc5d97a into main Apr 9, 2026
22 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