NM-272: Migrate Pending Users and User Invites

[VishalDalwadi]

Describe your changes

Provide Issue ticket number if applicable/not in title

Provide testing steps

Checklist before requesting a review

• My changes affect only 10 files or less.
• I have performed a self-review of my code and tested it.
• If it is a new feature, I have added thorough tests, my code is <= 1450 lines.
• If it is a bugfix, my code is <= 200 lines.
• My functions are <= 80 lines.
• I have had my code reviewed by a peer.
• My unit tests pass locally.
• Netmaker is awesome.

[tenki-reviewer]

VishalDalwadi does not have a Code Reviewer seat assigned. All seats are in use — you can purchase additional seats in billing.

[abhishek9686]

@tenki-reviewer

[tenki-reviewer]

Tenki Code Review - Complete

Files Reviewed: 16
Findings: 3

By Severity:

• 🟠 Medium: 2
• 🟡 Low: 1

---

This PR migrates PendingUser and UserInvite from a key-value store to the SQL schema, refactoring several functions and introducing a new v1.5.2 migration step. The main concern is that the new schema.UserInvite struct silently drops the NetworkRoles field that existed in models.UserInvite, causing invited users to lose their intended network role assignments.

Files Reviewed (16 files)

controllers/user.go
database/database.go
logic/users.go
migrate/migrate_schema.go
migrate/migrate_v1_5_1.go
migrate/migrate_v1_5_2.go
pro/auth/azure-ad.go
pro/auth/github.go
pro/auth/google.go
pro/auth/headless_callback.go
pro/auth/oidc.go
pro/controllers/users.go
pro/logic/user_mgmt.go
schema/models.go
schema/pending_users.go
schema/user_invites.go

[tenki-reviewer]

Risk: 🟡 Medium (45/100) — 2 medium findings, 1 low · 700 LOC across 16 files

---

Overview

This PR completes the SQL schema migration for PendingUser and UserInvite models, adding new schema.PendingUser and schema.UserInvite GORM models, a new migrateV1_5_2 migration step, and refactoring the business logic and OAuth callbacks to use the new schema types directly.

Functional Regression: NetworkRoles Silently Dropped

The most impactful issue is a silent data loss regression in schema/user_invites.go. The old models.UserInvite struct contained a NetworkRoles field (map[schema.NetworkID]map[schema.UserRoleID]struct{}). The new schema.UserInvite omits this field entirely. This affects:

• New invites created via inviteUsers in pro/controllers/users.go: the inviteReq.NetworkRoles from the request body is now silently discarded.
• Existing invite migration in migrate/migrate_v1_5_2.go: NetworkRoles from KV-store invites is not mapped to the new struct.
• User signup from invite in PrepareOauthUserFromInvite and userInviteSignUp: network role assignment is never applied.

Transaction Commit Semantics Change

In the refactored migrate/migrate_schema.go, the single outer transaction now covers both v1.5.1 and v1.5.2 migration steps. In the old code, commit = true was set immediately after v1.5.1 succeeded. Now if v1.5.2 fails, the deferred Rollback() will also undo the v1.5.1 job marker, causing v1.5.1 to attempt re-execution on the next restart — which could result in duplicate key errors against existing SQL rows.

Raw SQL Table Name Concatenation

The fetchAll function in migrate/migrate_schema.go (also present in the original migrate_v1_5_1.go) builds a raw SQL query by direct string concatenation of tableName. Current callers pass only hardcoded database.*_TABLE_NAME constants, so there is no active injection risk, but the function has no guard against misuse.