Skip to content

fix: create org membership when creating a user (#100)#105

Open
christianromeni wants to merge 2 commits into
mainfrom
fix/web-ui-user-creation
Open

fix: create org membership when creating a user (#100)#105
christianromeni wants to merge 2 commits into
mainfrom
fix/web-ui-user-creation

Conversation

@christianromeni

Copy link
Copy Markdown
Contributor

Fixes #100

Problem

A user created via the system-users page was inserted with no org_membership row. On login, ResolveUserRole returned an empty org for a system admin, so CreateAPIKey hit a FOREIGN KEY constraint failed on api_keys.org_id and login failed with a 500 (auth.go:159 in the report). A plain member got a 401 "no organization membership". Either way the new user could never log in.

Fix

Invariant: every user always belongs to an organization.

  • CreateUser now requires org_id and creates the user + membership atomically in one transaction (CreateUserWithMembership). A FK violation on a non-existent org rolls back the user insert and returns 400.
  • AuthZ: a non-system-admin caller may only target their own org, and may not assign the org_admin role - only system admins can (mirrors the existing is_system_admin gate).
  • User creation now writes an audit event (user_created, metadata only - never the password).
  • Login gains a defensive guard: a legacy user with an empty org resolves to a generic 401 (logged server-side) instead of a 500, so org-less users left behind by the old bug fail cleanly without leaking account existence.
  • The create-user dialog gains a required organization selector (defaults to the first org; orgs always exist since bootstrap creates one).

Existing data

Users created org-less by the old bug cannot log in (clean 401). Assign them to an org via Organizations → Members; no automatic migration since the target org is ambiguous.

Tests

  • DB: CreateUserWithMembership happy path, FK rollback (no orphan user), role stored, duplicate email.
  • Handler: org_id required for all callers, non-existent org → 400, invalid role → 400, cross-org 403, org_admin cannot assign org_admin, and the end-to-end regression (create user → login → 200, no FK crash).
  • go test -race ./internal/db/ ./internal/api/admin/ and tsc --noEmit clean.

Users created via the system-users page were inserted with no
org_membership row. On login, ResolveUserRole returned an empty org for
system admins, so CreateAPIKey hit a FOREIGN KEY violation on
api_keys.org_id and login failed with a 500; plain members got a 401.

Every user now always belongs to an organization. CreateUser requires
org_id and creates the user and membership atomically in one
transaction (CreateUserWithMembership). org_admin callers may only
target their own org and may not assign the org_admin role - only system
admins can. User creation now writes an audit event.

A defensive login guard turns the legacy empty-org case into a generic
401 (logged server-side) instead of a 500, so pre-existing org-less
users from the old bug fail cleanly. The create-user dialog gains a
required organization selector.

Fixes #100
@snyk-io

snyk-io Bot commented Jun 10, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.09346% with 32 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/api/admin/users.go 44.68% 24 Missing and 2 partials ⚠️
internal/db/users.go 88.46% 3 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

bcrypt rejects inputs longer than 72 bytes with ErrPasswordTooLong, so
CreateUser/UpdateUser would 500 on a valid-looking long password. Cap at
72 bytes and map ErrPasswordTooLong to 400 defensively.
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.

[Bug] Cant login as different user

1 participant