Skip to content

security(teams): authorize /teams from the session, not the global users.role_id (#830) #844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alex-dembele merged 3 commits into from
Oct 1, 2026
Merged

security(teams): authorize /teams from the session, not the global users.role_id (#830) #844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee9a07d
fix(teams): authorize /teams from the session, not the global users.r…
alex-dembele Sep 29, 2026
4cbf2a4
test(teams): cover /teams success, cross-tenant 404 and non-admin 403…
alex-dembele Sep 29, 2026
57b3aa1
test(security): cite the /teams HTTP tests on the non-adjacent regist…
alex-dembele Sep 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 30 additions & 83 deletions backend/internal/handler/team_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,19 @@ import (

"github.com/gofiber/fiber/v2"
"github.com/google/uuid"
"gorm.io/gorm"

"github.com/opendefender/openrisk/internal/domain"
"github.com/opendefender/openrisk/internal/infrastructure/database"
"github.com/opendefender/openrisk/internal/middleware"
)

// Authorization for every /teams route is the RequireRole("admin") guard in
// the composition root, which reads the caller's role in the ACTIVE
// organization from the signed session. The handlers used to re-check the
// global users.role_id, and looked the caller up by the token id rather than
// the user id, so every call answered 404 (#830).

type CreateTeamInput struct {
Name string `json:"name" validate:"required"`
Description string `json:"description"`
Expand Down Expand Up @@ -57,16 +65,6 @@ func CreateTeam(c *fiber.Ctx) error {
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "Unauthorized"})
}

// Check if user is admin
var currentUser domain.User
if err := database.DB.Preload("Role").First(&currentUser, "id = ?", claims.ID).Error; err != nil {
return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "User not found"})
}

if currentUser.Role.Name != "admin" {
return c.Status(fiber.StatusForbidden).JSON(fiber.Map{"error": "Only admins can create teams"})
}

tenantID := safeGetUUID(c, "tenant_id")
if tenantID == uuid.Nil {
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "invalid tenant"})
Expand All @@ -78,6 +76,7 @@ func CreateTeam(c *fiber.Ctx) error {
}

team := domain.Team{
ID: uuid.New(),
TenantID: tenantID,
Name: input.Name,
Description: input.Description,
Expand Down Expand Up @@ -105,16 +104,6 @@ func GetTeams(c *fiber.Ctx) error {
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "Unauthorized"})
}

// Check if user is admin
var currentUser domain.User
if err := database.DB.Preload("Role").First(&currentUser, "id = ?", claims.ID).Error; err != nil {
return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "User not found"})
}

if currentUser.Role.Name != "admin" {
return c.Status(fiber.StatusForbidden).JSON(fiber.Map{"error": "Only admins can view teams"})
}

tenantID := safeGetUUID(c, "tenant_id")

var teams []domain.Team
Expand Down Expand Up @@ -147,16 +136,6 @@ func GetTeam(c *fiber.Ctx) error {
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "Unauthorized"})
}

// Check if user is admin
var currentUser domain.User
if err := database.DB.Preload("Role").First(&currentUser, "id = ?", claims.ID).Error; err != nil {
return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "User not found"})
}

if currentUser.Role.Name != "admin" {
return c.Status(fiber.StatusForbidden).JSON(fiber.Map{"error": "Only admins can view teams"})
}

tenantID := safeGetUUID(c, "tenant_id")
teamID := c.Params("id")
var team domain.Team
Expand Down Expand Up @@ -200,16 +179,6 @@ func UpdateTeam(c *fiber.Ctx) error {
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "Unauthorized"})
}

// Check if user is admin
var currentUser domain.User
if err := database.DB.Preload("Role").First(&currentUser, "id = ?", claims.ID).Error; err != nil {
return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "User not found"})
}

if currentUser.Role.Name != "admin" {
return c.Status(fiber.StatusForbidden).JSON(fiber.Map{"error": "Only admins can update teams"})
}

tenantID := safeGetUUID(c, "tenant_id")
teamID := c.Params("id")
var team domain.Team
Expand Down Expand Up @@ -255,30 +224,20 @@ func DeleteTeam(c *fiber.Ctx) error {
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "Unauthorized"})
}

// Check if user is admin
var currentUser domain.User
if err := database.DB.Preload("Role").First(&currentUser, "id = ?", claims.ID).Error; err != nil {
return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "User not found"})
}

if currentUser.Role.Name != "admin" {
return c.Status(fiber.StatusForbidden).JSON(fiber.Map{"error": "Only admins can delete teams"})
}

tenantID := safeGetUUID(c, "tenant_id")
teamID := c.Params("id")
var team domain.Team
if err := database.DB.First(&team, "id = ? AND tenant_id = ?", teamID, tenantID).Error; err != nil {
return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "Team not found"})
}

// Delete team members first
if err := database.DB.Where("team_id = ?", team.ID).Delete(&domain.TeamMember{}).Error; err != nil {
return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"error": "Failed to delete team members"})
}

// Delete team
if err := database.DB.Delete(&team).Error; err != nil {
// Members and team go together or not at all (RULE #7).
if err := database.DB.Transaction(func(tx *gorm.DB) error {
if err := tx.Where("team_id = ?", team.ID).Delete(&domain.TeamMember{}).Error; err != nil {
return err
}
return tx.Delete(&team).Error
}); err != nil {
return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"error": "Failed to delete team"})
}

Expand All @@ -292,20 +251,16 @@ func AddTeamMember(c *fiber.Ctx) error {
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "Unauthorized"})
}

// Check if user is admin
var currentUser domain.User
if err := database.DB.Preload("Role").First(&currentUser, "id = ?", claims.ID).Error; err != nil {
return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "User not found"})
tenantID := safeGetUUID(c, "tenant_id")
teamID, errTeam := uuid.Parse(c.Params("id"))
userID, errUser := uuid.Parse(c.Params("userId"))
if errTeam != nil {
return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "Team not found"})
}

if currentUser.Role.Name != "admin" {
return c.Status(fiber.StatusForbidden).JSON(fiber.Map{"error": "Only admins can add team members"})
if errUser != nil {
return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "User not found"})
}

tenantID := safeGetUUID(c, "tenant_id")
teamID := c.Params("id")
userID := c.Params("userId")

// Verify team exists in this tenant
var team domain.Team
if err := database.DB.First(&team, "id = ? AND tenant_id = ?", teamID, tenantID).Error; err != nil {
Expand All @@ -318,11 +273,12 @@ func AddTeamMember(c *fiber.Ctx) error {
return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "User not found"})
}

// The target user must belong to this tenant — never let an admin pull a
// member from another organization into their team.
// The target user must be an ACTIVE member of this tenant — never let an
// admin pull a member from another organization, or one whose access was
// withdrawn, into their team. Both answer exactly like an unknown id.
var orgMemberCount int64
if err := database.DB.Model(&domain.OrganizationMember{}).
Where("organization_id = ? AND user_id = ?", tenantID, userID).
Where("organization_id = ? AND user_id = ? AND is_active = ?", tenantID, userID, true).
Count(&orgMemberCount).Error; err != nil {
return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"error": "Failed to verify membership"})
}
Expand All @@ -338,8 +294,9 @@ func AddTeamMember(c *fiber.Ctx) error {

// Add member
member := domain.TeamMember{
TeamID: uuid.MustParse(teamID),
UserID: uuid.MustParse(userID),
ID: uuid.New(),
TeamID: teamID,
UserID: userID,
Role: "member",
JoinedAt: time.Now(),
}
Expand All @@ -358,16 +315,6 @@ func RemoveTeamMember(c *fiber.Ctx) error {
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "Unauthorized"})
}

// Check if user is admin
var currentUser domain.User
if err := database.DB.Preload("Role").First(&currentUser, "id = ?", claims.ID).Error; err != nil {
return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "User not found"})
}

if currentUser.Role.Name != "admin" {
return c.Status(fiber.StatusForbidden).JSON(fiber.Map{"error": "Only admins can remove team members"})
}

tenantID := safeGetUUID(c, "tenant_id")
teamID := c.Params("id")
userID := c.Params("userId")
Expand Down
Loading
Loading