Skip to content

Commit ff03aae

Browse files
csg-pr-botDev Agentwanghh2000
authored and
Rader
committed
Refactor check accounting uuid by namespace uuid for user or org (#965)
* Refactor check accounting uuid by namespace uuid for user or org * update namespace type * update namespace component to return namespace type --------- Co-authored-by: Dev Agent <dev-agent@example.com> Co-authored-by: Haihui.Wang <wanghh2000@163.com>
1 parent 3a4d264 commit ff03aae

9 files changed

Lines changed: 56 additions & 24 deletions

File tree

‎accounting/router/api.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,9 @@ import (
1313
"opencsg.com/csghub-server/api/middleware"
1414
bldmq "opencsg.com/csghub-server/builder/mq"
1515
"opencsg.com/csghub-server/common/config"
16-
"opencsg.com/csghub-server/mq"
1716
)
1817

19-
func NewAccountRouter(config *config.Config, mqHandler mq.MessageQueue, mqFactory bldmq.MessageQueueFactory) (*gin.Engine, error) {
18+
func NewAccountRouter(config *config.Config, mqFactory bldmq.MessageQueueFactory) (*gin.Engine, error) {
2019
r := gin.New()
2120
middleware.SetInfraMiddleware(r, config, instrumentation.Account)
2221
needAPIKey := middleware.NeedAPIKey(config)
@@ -36,7 +35,7 @@ func NewAccountRouter(config *config.Config, mqHandler mq.MessageQueue, mqFactor
3635
return nil, fmt.Errorf("error creating multi sync handler, error: %w", err)
3736
}
3837
createMeteringRoutes(apiGroup, meterHandler)
39-
err = createAdvancedRoutes(apiGroup, config, mqHandler, mqFactory)
38+
err = createAdvancedRoutes(apiGroup, config, mqFactory)
4039
if err != nil {
4140
return nil, fmt.Errorf("error creating accounting advanced routes, error:%w", err)
4241
}

‎accounting/router/api_ce.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@ import (
66
"github.com/gin-gonic/gin"
77
bldmq "opencsg.com/csghub-server/builder/mq"
88
"opencsg.com/csghub-server/common/config"
9-
"opencsg.com/csghub-server/mq"
109
)
1110

12-
func createAdvancedRoutes(apiGroup *gin.RouterGroup, config *config.Config, mqHandler mq.MessageQueue, mqFactory bldmq.MessageQueueFactory) error {
11+
func createAdvancedRoutes(apiGroup *gin.RouterGroup, config *config.Config, mqFactory bldmq.MessageQueueFactory) error {
1312
return nil
1413
}

‎builder/rpc/types.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ type Namespace struct {
55
Type string `json:"type"`
66
UUID string `json:"uuid,omitempty"`
77
Avatar string `json:"avatar,omitempty"`
8+
NSType string `json:"nstype,omitempty"`
89
}
910

1011
type User struct {

‎cmd/csghub-server/cmd/accounting/launch.go‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"opencsg.com/csghub-server/builder/store/database"
1616
"opencsg.com/csghub-server/common/config"
1717
"opencsg.com/csghub-server/common/i18n"
18-
"opencsg.com/csghub-server/mq"
1918
)
2019

2120
var launchCmd = &cobra.Command{
@@ -41,11 +40,6 @@ var launchCmd = &cobra.Command{
4140
return fmt.Errorf("database initialization failed: %w", err)
4241
}
4342

44-
mqHandler, err := mq.GetOrInit(cfg)
45-
if err != nil {
46-
return fmt.Errorf("fail to build message queue handler: %w", err)
47-
}
48-
4943
mqFactory, err := bldmq.GetOrInitMessageQueueFactory(cfg)
5044
if err != nil {
5145
return fmt.Errorf("failed to creating message queue factory: %w", err)
@@ -70,7 +64,7 @@ var launchCmd = &cobra.Command{
7064
panic(err)
7165
}
7266

73-
r, err := router.NewAccountRouter(cfg, mqHandler, mqFactory)
67+
r, err := router.NewAccountRouter(cfg, mqFactory)
7468
if err != nil {
7569
return fmt.Errorf("failed to init router: %w", err)
7670
}

‎common/types/namespace.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,5 @@ type Namespace struct {
1111
Type string
1212
Avatar string
1313
UUID string
14+
NSType string
1415
}

‎component/accounting.go‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@ package component
22

33
import (
44
"context"
5-
"errors"
65
"fmt"
76

87
"opencsg.com/csghub-server/builder/accounting"
8+
"opencsg.com/csghub-server/builder/git/membership"
99
"opencsg.com/csghub-server/builder/rpc"
1010
"opencsg.com/csghub-server/builder/store/database"
1111
"opencsg.com/csghub-server/common/config"
12+
"opencsg.com/csghub-server/common/errorx"
1213
"opencsg.com/csghub-server/common/types"
1314
)
1415

@@ -69,12 +70,43 @@ func NewAccountingComponent(config *config.Config) (AccountingComponent, error)
6970
}
7071

7172
func (ac *accountingComponentImpl) ListMeteringsByUserIDAndTime(ctx context.Context, req types.ActStatementsReq) (interface{}, error) {
72-
user, err := ac.userStore.FindByUsername(ctx, req.CurrentUser)
73+
if err := ac.allowQueryData(ctx, req.CurrentUser, req.UserUUID); err != nil {
74+
return nil, errorx.Forbidden(err, map[string]any{
75+
"user": req.CurrentUser,
76+
})
77+
}
78+
return ac.accountingClient.ListMeteringsByUserIDAndTime(req)
79+
}
80+
81+
// CanQueryUserData checks if the current user has permission to query the target user's data.
82+
// Permission is granted if:
83+
// 1. Current user is the same as the target user (querying own data)
84+
// 2. Current user is an admin
85+
// 3. Current user is a member of the organization that owns the target user's namespace
86+
func (ac *accountingComponentImpl) allowQueryData(ctx context.Context, currentUser, targetUUID string) error {
87+
user, err := ac.userSvcClient.GetUserByName(ctx, currentUser)
7388
if err != nil {
74-
return nil, fmt.Errorf("user does not exist, %w", err)
89+
return fmt.Errorf("current user not found: %w", err)
7590
}
76-
if user.UUID != req.UserUUID {
77-
return nil, errors.New("invalid user")
91+
92+
if user.IsAdmin() || user.UUID == targetUUID {
93+
return nil
7894
}
79-
return ac.accountingClient.ListMeteringsByUserIDAndTime(req)
95+
96+
ns, err := ac.userSvcClient.GetNameSpaceInfoByUUID(ctx, targetUUID)
97+
if err != nil {
98+
return fmt.Errorf("target namespace not found: %w", err)
99+
}
100+
101+
if ns.NSType != string(database.OrgNamespace) {
102+
return fmt.Errorf("do not have permission to query the target org's data: %w", err)
103+
}
104+
105+
// Check if current user is member of org that owns target user's namespace
106+
role, err := ac.userSvcClient.GetMemberRoleByUUID(ctx, ns.UUID, currentUser)
107+
if err != nil || role == membership.RoleUnknown {
108+
return fmt.Errorf("do not have permission to query the target org's data: %w", err)
109+
}
110+
111+
return nil
80112
}

‎component/accounting_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,21 @@ import (
55
"testing"
66

77
"github.com/stretchr/testify/require"
8-
"opencsg.com/csghub-server/builder/store/database"
98
"opencsg.com/csghub-server/common/types"
109
)
1110

1211
func TestAccountingComponent_ListMeteringsByUserIDAndTime(t *testing.T) {
1312
ctx := context.TODO()
1413
ac := initializeTestAccountingComponent(ctx, t)
14+
ac.userSvcClient = ac.mocks.userSvcClient
1515

1616
req := types.ActStatementsReq{
1717
CurrentUser: "user",
1818
UserUUID: "uuid",
1919
}
20-
ac.mocks.stores.UserMock().EXPECT().FindByUsername(ctx, "user").Return(database.User{
21-
UUID: "uuid",
20+
ac.mocks.userSvcClient.EXPECT().GetUserByName(ctx, "user").Return(&types.User{
21+
UUID: "uuid",
22+
Roles: []string{},
2223
}, nil)
2324
ac.mocks.accountingClient.EXPECT().ListMeteringsByUserIDAndTime(req).Return(
2425
"", nil,

‎user/component/namespace.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,10 @@ func (c *namespaceComponentImpl) GetInfoByUUID(ctx context.Context, uuid string)
4444

4545
func (c *namespaceComponentImpl) buildNamespace(ctx context.Context, dbns *database.Namespace) (*types.Namespace, error) {
4646
ns := &types.Namespace{
47-
Path: dbns.Path,
48-
Type: string(dbns.NamespaceType),
49-
UUID: dbns.UUID,
47+
Path: dbns.Path,
48+
Type: string(dbns.NamespaceType),
49+
NSType: string(dbns.NamespaceType),
50+
UUID: dbns.UUID,
5051
}
5152
switch dbns.NamespaceType {
5253
case database.UserNamespace:

‎user/component/namespace_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ func TestNamespaceComponent_GetInfo(t *testing.T) {
3939
Path: user.Username,
4040
Type: "user",
4141
Avatar: user.Avatar,
42+
NSType: "user",
4243
}
4344
require.EqualValues(t, expected, actual)
4445
})
@@ -80,6 +81,7 @@ func TestNamespaceComponent_GetInfo(t *testing.T) {
8081
Path: org.Name,
8182
Type: org.OrgType,
8283
Avatar: org.Logo,
84+
NSType: "organization",
8385
}
8486
require.EqualValues(t, expected, actual)
8587

@@ -117,6 +119,7 @@ func TestNamespaceComponent_GetInfoByUUID(t *testing.T) {
117119
Type: "user",
118120
UUID: namespaceUUID,
119121
Avatar: user.Avatar,
122+
NSType: "user",
120123
}
121124
require.EqualValues(t, expected, actual)
122125
})
@@ -161,6 +164,7 @@ func TestNamespaceComponent_GetInfoByUUID(t *testing.T) {
161164
Type: org.OrgType,
162165
UUID: namespaceUUID,
163166
Avatar: org.Logo,
167+
NSType: "organization",
164168
}
165169
require.EqualValues(t, expected, actual)
166170
})

0 commit comments

Comments
 (0)