Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions backend/cmd/server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2049,9 +2049,11 @@ func main() {
adminRole := middleware.RequireRole("admin")
protected.Get("/users", adminRole, handlers.GetUsers)
protected.Post("/users", adminRole, handlers.CreateUser)
protected.Patch("/users/:id/status", adminRole, handlers.UpdateUserStatus)
protected.Patch("/users/:id/role", adminRole, handlers.UpdateUserRole)
protected.Delete("/users/:id", adminRole, handlers.DeleteUser)
// PATCH /users/:id/status, PATCH /users/:id/role and DELETE /users/:id were
// removed (#807). They wrote the global users row, so an admin of one
// organization could lock a person out of every other one, or delete them
// everywhere. A member's role and status are per organization and live on
// /organization/members/:memberId/{role,status}.
// Self-service profile, preferences and avatar (#719, replaces the
// misleading PATCH /users/:id of #574). Every verb acts on the session's
// own user, so the session is the authorization. The avatar read is gated
Expand Down
44 changes: 44 additions & 0 deletions backend/internal/application/auth/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,17 @@ func (uc *LoginUseCase) Execute(ctx context.Context, input LoginInput) (*LoginOu
if err != nil {
return nil, fmt.Errorf("failed to get organization membership: %w", err)
}
if member != nil && !member.IsActive {
// The default organization withdrew this person's access. That is its
// decision about itself only: if another organization still grants
// access, sign them in there. Otherwise one organization could lock a
// person out of every other one just by being their default (#807).
if alt, altErr := uc.fallbackMembership(ctx, user.ID, org.ID); altErr != nil {
return nil, altErr
} else if alt != nil {
org, member = alt.Organization, alt
}
}
if member != nil {
// A revoked (deactivated) membership must not yield a session, even though
// the user account itself is active: being removed from an organization is
Expand Down Expand Up @@ -378,6 +389,39 @@ func (uc *LoginUseCase) decideMFA(ctx context.Context, member *domain.Organizati
return domain.DecideMFA(in)
}

// activeMembershipLister is the optional read login uses to find another
// organization to sign a person into when their default one withdrew access.
// GormUserRepository implements it; a repository without it keeps the old
// behavior of refusing the sign-in.
type activeMembershipLister interface {
ListActiveMemberships(ctx context.Context, userID uuid.UUID) ([]*domain.OrganizationMember, error)
}

// fallbackMembership returns the user's earliest-joined active membership in
// an active organization other than exclude, or nil when there is none.
func (uc *LoginUseCase) fallbackMembership(ctx context.Context, userID, exclude uuid.UUID) (*domain.OrganizationMember, error) {
lister, ok := uc.userRepo.(activeMembershipLister)
if !ok {
return nil, nil
}
memberships, err := lister.ListActiveMemberships(ctx, userID)
if err != nil {
return nil, fmt.Errorf("failed to list organization memberships: %w", err)
}
var best *domain.OrganizationMember
for _, m := range memberships {
if m == nil || !m.IsActive || m.OrganizationID == exclude ||
m.Organization == nil || !m.Organization.IsActive {
continue
}
if best == nil || m.JoinedAt.Before(best.JoinedAt) ||
(m.JoinedAt.Equal(best.JoinedAt) && m.OrganizationID.String() < best.OrganizationID.String()) {
best = m
}
}
return best, nil
}

// UserRepository interface for user operations
type UserRepository interface {
GetByEmail(ctx context.Context, email string) (*domain.User, error)
Expand Down
96 changes: 96 additions & 0 deletions backend/internal/application/auth/login_fallback_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// Copyright (c) 2026 OpenDefender Contributors
// SPDX-License-Identifier: AGPL-3.0-only
// This program is free software: you can redistribute it and/or modify it under
// the terms of the GNU Affero General Public License v3.0 (see LICENSE).

package auth

import (
"context"
"errors"
"testing"
"time"

"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/opendefender/openrisk/internal/domain"
)

// #807 — the organization login falls back to when the default one withdrew
// access. The end-to-end proof (a real membership write in A, a real sign-in
// landing in B) is handler.TestCrossTenant_AdminOfAWithdrawingAccessLeavesBUntouched.

// listingUsers is loginUsers plus the optional membership listing.
type listingUsers struct {
loginUsers
active []*domain.OrganizationMember
err error
}

func (l *listingUsers) ListActiveMemberships(context.Context, uuid.UUID) ([]*domain.OrganizationMember, error) {
return l.active, l.err
}

func activeIn(org *domain.Organization, joined time.Time) *domain.OrganizationMember {
return &domain.OrganizationMember{
ID: uuid.New(), OrganizationID: org.ID, Organization: org,
Role: domain.RoleUser, IsActive: true, JoinedAt: joined,
}
}

func TestLoginFallback_Success_PicksEarliestActiveOtherOrganization(t *testing.T) {
def := &domain.Organization{ID: uuid.New(), IsActive: true}
older := &domain.Organization{ID: uuid.New(), IsActive: true}
newer := &domain.Organization{ID: uuid.New(), IsActive: true}
closed := &domain.Organization{ID: uuid.New(), IsActive: false}

repo := &listingUsers{active: []*domain.OrganizationMember{
activeIn(def, loginNow.Add(-72*time.Hour)), // the default itself is never chosen
activeIn(newer, loginNow.Add(-1*time.Hour)),
activeIn(closed, loginNow.Add(-96*time.Hour)), // an inactive organization is skipped
activeIn(older, loginNow.Add(-48*time.Hour)),
}}
uc := &LoginUseCase{userRepo: repo}

got, err := uc.fallbackMembership(context.Background(), uuid.New(), def.ID)
require.NoError(t, err)
require.NotNil(t, got)
assert.Equal(t, older.ID, got.OrganizationID)
}

func TestLoginFallback_NotFound_NoOtherActiveMembership(t *testing.T) {
def := &domain.Organization{ID: uuid.New(), IsActive: true}
uc := &LoginUseCase{userRepo: &listingUsers{active: []*domain.OrganizationMember{activeIn(def, loginNow)}}}

got, err := uc.fallbackMembership(context.Background(), uuid.New(), def.ID)
require.NoError(t, err)
assert.Nil(t, got)

// A repository that cannot list memberships keeps the old refusal.
uc = &LoginUseCase{userRepo: &loginUsers{}}
got, err = uc.fallbackMembership(context.Background(), uuid.New(), def.ID)
require.NoError(t, err)
assert.Nil(t, got)
}

func TestLoginFallback_Unauthorized_WithdrawnDefaultWithNowhereElseIsRefused(t *testing.T) {
uc, users, _, _ := newLoginHarness(t, domain.RoleUser)
users.member.IsActive = false
users.member.Status = domain.MembershipRevoked

out, err := uc.Execute(context.Background(), LoginInput{
Email: "admin@opendefender.io", Password: "Ancre-Vitrail7-Cobalt",
})
require.Error(t, err)
assert.Nil(t, out)
assert.Contains(t, err.Error(), "revoked")
}

func TestLoginFallback_ListingErrorIsNotASession(t *testing.T) {
uc := &LoginUseCase{userRepo: &listingUsers{err: errors.New("db down")}}
got, err := uc.fallbackMembership(context.Background(), uuid.New(), uuid.New())
require.Error(t, err)
assert.Nil(t, got)
}
23 changes: 23 additions & 0 deletions backend/internal/handler/authz_route_coverage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,3 +337,26 @@ func TestProtectedRoutes_MostRoutesCarryAPermissionGuard(t *testing.T) {
assert.Greater(t, guarded*100/len(routes), 60,
"fewer than 60%% of protected routes carry a permission guard — something was dropped wholesale")
}

// #807 — these routes wrote the global users row, so an administrator of one
// organization could deactivate or delete a person in every organization they
// belong to. They were removed, not guarded; per-organization role and status
// live on /organization/members/:memberId. Mounting any of them again, under
// any guard, fails here.
var removedGlobalAccountRoutes = []string{
"PATCH /users/:id/status",
"PATCH /users/:id/role",
"DELETE /users/:id",
}

func TestProtectedRoutes_GlobalAccountWritesStayRemoved(t *testing.T) {
mounted := map[string]bool{}
for _, r := range parseProtectedRoutes(t) {
mounted[routeKey(r)] = true
}
for _, k := range removedGlobalAccountRoutes {
assert.False(t, mounted[k], "%s acts on the global account, not the caller's membership (#807)", k)
}
// Guard against a parser that silently matches nothing.
assert.True(t, mounted["GET /users"], "the parser no longer sees the /users block")
}
Loading
Loading