From 9fc15b3d109a8a0495de00d402835b31f907d469 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Wed, 13 Mar 2024 17:35:13 +0500 Subject: [PATCH] Resolved user access management issue --- handlers/bounty.go | 26 +++++++++------ handlers/bounty_test.go | 63 ++++++++++++++++++----------------- handlers/organization_test.go | 16 +++++++-- handlers/organizations.go | 28 ++++++++++------ 4 files changed, 79 insertions(+), 54 deletions(-) diff --git a/handlers/bounty.go b/handlers/bounty.go index e7a24b68f..8261ac1a4 100644 --- a/handlers/bounty.go +++ b/handlers/bounty.go @@ -16,21 +16,27 @@ import ( "github.com/stakwork/sphinx-tribes/config" "github.com/stakwork/sphinx-tribes/db" "github.com/stakwork/sphinx-tribes/utils" + "gorm.io/gorm" ) type bountyHandler struct { - httpClient HttpClient - db db.Database - getSocketConnections func(host string) (db.Client, error) - generateBountyResponse func(bounties []db.Bounty) []db.BountyResponse + httpClient HttpClient + db db.Database + getSocketConnections func(host string) (db.Client, error) + generateBountyResponse func(bounties []db.Bounty) []db.BountyResponse + userHasAccess func(pubKeyFromAuth string, uuid string, role string) bool + userHasManageBountyRoles func(pubKeyFromAuth string, uuid string) bool } func NewBountyHandler(httpClient HttpClient, database db.Database) *bountyHandler { + dbConf := db.NewDatabaseConfig(&gorm.DB{}) return &bountyHandler{ - httpClient: httpClient, - db: database, - getSocketConnections: db.Store.GetSocketConnections, + httpClient: httpClient, + db: database, + getSocketConnections: db.Store.GetSocketConnections, + userHasAccess: dbConf.UserHasAccess, + userHasManageBountyRoles: dbConf.UserHasManageBountyRoles, } } @@ -239,7 +245,7 @@ func (h *bountyHandler) CreateOrEditBounty(w http.ResponseWriter, r *http.Reques // check if bounty belongs to user if pubKeyFromAuth != dbBounty.OwnerID { if bounty.OrgUuid != "" { - hasBountyRoles := h.db.UserHasManageBountyRoles(pubKeyFromAuth, bounty.OrgUuid) + hasBountyRoles := h.userHasManageBountyRoles(pubKeyFromAuth, bounty.OrgUuid) if !hasBountyRoles { msg := "You don't have a=the right permission ton update bounty" fmt.Println(msg) @@ -448,7 +454,7 @@ func (h *bountyHandler) MakeBountyPayment(w http.ResponseWriter, r *http.Request // check if user is the admin of the organization // or has a pay bounty role - hasRole := h.db.UserHasAccess(pubKeyFromAuth, bounty.OrgUuid, db.PayBounty) + hasRole := h.userHasAccess(pubKeyFromAuth, bounty.OrgUuid, db.PayBounty) if !hasRole { w.WriteHeader(http.StatusUnauthorized) json.NewEncoder(w).Encode("You don't have appropriate permissions to pay bounties") @@ -567,7 +573,7 @@ func (h *bountyHandler) BountyBudgetWithdraw(w http.ResponseWriter, r *http.Requ // check if user is the admin of the organization // or has a withdraw bounty budget role - hasRole := h.db.UserHasAccess(pubKeyFromAuth, request.OrgUuid, db.WithdrawBudget) + hasRole := h.userHasAccess(pubKeyFromAuth, request.OrgUuid, db.WithdrawBudget) if !hasRole { w.WriteHeader(http.StatusUnauthorized) errMsg := formatPayError("You don't have appropriate permissions to withdraw bounty budget") diff --git a/handlers/bounty_test.go b/handlers/bounty_test.go index 98ea511d9..4eed4179e 100644 --- a/handlers/bounty_test.go +++ b/handlers/bounty_test.go @@ -34,6 +34,12 @@ func TestCreateOrEditBounty(t *testing.T) { ctx := context.WithValue(context.Background(), auth.ContextKey, "test-key") mockDb := dbMocks.NewDatabase(t) mockClient := mocks.NewHttpClient(t) + mockUserHasManageBountyRolesTrue := func(pubKeyFromAuth string, uuid string) bool { + return true + } + mockUserHasManageBountyRolesFalse := func(pubKeyFromAuth string, uuid string) bool { + return false + } bHandler := NewBountyHandler(mockClient, mockDb) t.Run("should return error if body is not a valid json", func(t *testing.T) { @@ -127,13 +133,8 @@ func TestCreateOrEditBounty(t *testing.T) { t.Run("return error if user does not have required roles", func(t *testing.T) { rr := httptest.NewRecorder() handler := http.HandlerFunc(bHandler.CreateOrEditBounty) + bHandler.userHasManageBountyRoles = mockUserHasManageBountyRolesFalse - mockOrg := db.Organization{ - ID: 1, - Uuid: "org-1", - Name: "custom org", - OwnerPubKey: "org-key", - } existingBounty := db.Bounty{ ID: 1, Type: "coding", @@ -147,7 +148,6 @@ func TestCreateOrEditBounty(t *testing.T) { mockDb.On("UpdateBountyBoolColumn", mock.AnythingOfType("db.Bounty"), "show").Return(existingBounty) mockDb.On("UpdateBountyNullColumn", mock.AnythingOfType("db.Bounty"), "assignee").Return(existingBounty) mockDb.On("GetBounty", uint(1)).Return(existingBounty).Once() - mockDb.On("UserHasManageBountyRoles", "test-key", mockOrg.Uuid).Return(false).Once() body, _ := json.Marshal(updatedBounty) req, err := http.NewRequestWithContext(ctx, http.MethodPost, "/", bytes.NewReader(body)) @@ -163,13 +163,8 @@ func TestCreateOrEditBounty(t *testing.T) { t.Run("should allow to add or edit bounty if user has role", func(t *testing.T) { rr := httptest.NewRecorder() handler := http.HandlerFunc(bHandler.CreateOrEditBounty) + bHandler.userHasManageBountyRoles = mockUserHasManageBountyRolesTrue - mockOrg := db.Organization{ - ID: 1, - Uuid: "org-1", - Name: "custom org", - OwnerPubKey: "org-key", - } existingBounty := db.Bounty{ ID: 1, Type: "coding", @@ -183,7 +178,6 @@ func TestCreateOrEditBounty(t *testing.T) { mockDb.On("UpdateBountyBoolColumn", mock.AnythingOfType("db.Bounty"), "show").Return(existingBounty) mockDb.On("UpdateBountyNullColumn", mock.AnythingOfType("db.Bounty"), "assignee").Return(existingBounty) mockDb.On("GetBounty", uint(1)).Return(existingBounty).Once() - mockDb.On("UserHasManageBountyRoles", "test-key", mockOrg.Uuid).Return(true).Once() mockDb.On("CreateOrEditBounty", mock.AnythingOfType("db.Bounty")).Return(updatedBounty, nil).Once() body, _ := json.Marshal(updatedBounty) @@ -201,13 +195,9 @@ func TestCreateOrEditBounty(t *testing.T) { t.Run("should not update created at when bounty is updated", func(t *testing.T) { rr := httptest.NewRecorder() handler := http.HandlerFunc(bHandler.CreateOrEditBounty) + bHandler.userHasManageBountyRoles = mockUserHasManageBountyRolesTrue + now := time.Now().UnixMilli() - mockOrg := db.Organization{ - ID: 1, - Uuid: "org-1", - Name: "custom org", - OwnerPubKey: "org-key", - } existingBounty := db.Bounty{ ID: 1, Type: "coding", @@ -222,7 +212,6 @@ func TestCreateOrEditBounty(t *testing.T) { mockDb.On("UpdateBountyBoolColumn", mock.AnythingOfType("db.Bounty"), "show").Return(existingBounty) mockDb.On("UpdateBountyNullColumn", mock.AnythingOfType("db.Bounty"), "assignee").Return(existingBounty) mockDb.On("GetBounty", uint(1)).Return(existingBounty).Once() - mockDb.On("UserHasManageBountyRoles", "test-key", mockOrg.Uuid).Return(true).Once() mockDb.On("CreateOrEditBounty", mock.MatchedBy(func(b db.Bounty) bool { return b.Created == now })).Return(updatedBounty, nil).Once() @@ -1151,6 +1140,12 @@ func TestMakeBountyPayment(t *testing.T) { ctx := context.Background() mockDb := &dbMocks.Database{} mockHttpClient := &mocks.HttpClient{} + mockUserHasAccessTrue := func(pubKeyFromAuth string, uuid string, role string) bool { + return true + } + mockUserHasAccessFalse := func(pubKeyFromAuth string, uuid string, role string) bool { + return false + } mockGetSocketConnections := func(host string) (db.Client, error) { s, ws := MockNewWSServer(t) defer s.Close() @@ -1253,6 +1248,8 @@ func TestMakeBountyPayment(t *testing.T) { }) t.Run("401 error if user not organization admin or does not have PAY BOUNTY role", func(t *testing.T) { + bHandler.userHasAccess = mockUserHasAccessFalse + mockDb.On("GetBounty", mock.AnythingOfType("uint")).Return(db.Bounty{ ID: 1, Price: 1000, @@ -1260,7 +1257,6 @@ func TestMakeBountyPayment(t *testing.T) { Assignee: "assignee-1", Paid: false, }, nil) - mockDb.On("UserHasAccess", "valid-key", "org-1", db.PayBounty).Return(false) r := chi.NewRouter() r.Post("/gobounties/pay/{id}", bHandler.MakeBountyPayment) @@ -1283,6 +1279,7 @@ func TestMakeBountyPayment(t *testing.T) { mockDb := dbMocks.NewDatabase(t) mockHttpClient := mocks.NewHttpClient(t) bHandler := NewBountyHandler(mockHttpClient, mockDb) + bHandler.userHasAccess = mockUserHasAccessTrue mockDb.On("GetBounty", mock.AnythingOfType("uint")).Return(db.Bounty{ ID: 1, Price: 1000, @@ -1290,7 +1287,6 @@ func TestMakeBountyPayment(t *testing.T) { Assignee: "assignee-1", Paid: false, }, nil) - mockDb.On("UserHasAccess", "valid-key", "org-1", db.PayBounty).Return(true) mockDb.On("GetOrganizationBudget", "org-1").Return(db.BountyBudget{ TotalBudget: 500, }, nil) @@ -1313,6 +1309,7 @@ func TestMakeBountyPayment(t *testing.T) { t.Run("Should test that a successful WebSocket message is sent if the payment is successful", func(t *testing.T) { mockDb.ExpectedCalls = nil bHandler.getSocketConnections = mockGetSocketConnections + bHandler.userHasAccess = mockUserHasAccessTrue now := time.Now() expectedBounty := db.Bounty{ @@ -1326,7 +1323,6 @@ func TestMakeBountyPayment(t *testing.T) { } mockDb.On("GetBounty", bountyID).Return(bounty, nil) - mockDb.On("UserHasAccess", "valid-key", bounty.OrgUuid, db.PayBounty).Return(true) mockDb.On("GetOrganizationBudget", bounty.OrgUuid).Return(db.BountyBudget{TotalBudget: 2000}, nil) mockDb.On("GetPersonByPubkey", bounty.Assignee).Return(db.Person{OwnerPubKey: "assignee-1", OwnerRouteHint: "OwnerRouteHint"}, nil) mockDb.On("AddPaymentHistory", mock.AnythingOfType("db.PaymentHistory")).Return(db.PaymentHistory{ID: 1}) @@ -1373,9 +1369,9 @@ func TestMakeBountyPayment(t *testing.T) { bHandler2 := NewBountyHandler(mockHttpClient2, mockDb2) bHandler2.getSocketConnections = mockGetSocketConnections + bHandler2.userHasAccess = mockUserHasAccessTrue mockDb2.On("GetBounty", bountyID).Return(bounty, nil) - mockDb2.On("UserHasAccess", "valid-key", bounty.OrgUuid, db.PayBounty).Return(true) mockDb2.On("GetOrganizationBudget", bounty.OrgUuid).Return(db.BountyBudget{TotalBudget: 2000}, nil) mockDb2.On("GetPersonByPubkey", bounty.Assignee).Return(db.Person{OwnerPubKey: "assignee-1", OwnerRouteHint: "OwnerRouteHint"}, nil) @@ -1413,6 +1409,12 @@ func TestBountyBudgetWithdraw(t *testing.T) { ctx := context.Background() mockDb := dbMocks.NewDatabase(t) mockHttpClient := mocks.NewHttpClient(t) + mockUserHasAccessTrue := func(pubKeyFromAuth string, uuid string, role string) bool { + return true + } + mockUserHasAccessFalse := func(pubKeyFromAuth string, uuid string, role string) bool { + return false + } bHandler := NewBountyHandler(mockHttpClient, mockDb) unauthorizedCtx := context.WithValue(context.Background(), auth.ContextKey, "") authorizedCtx := context.WithValue(ctx, auth.ContextKey, "valid-key") @@ -1446,9 +1448,10 @@ func TestBountyBudgetWithdraw(t *testing.T) { }) t.Run("401 error if user is not the organization admin or does not have WithdrawBudget role", func(t *testing.T) { + bHandler.userHasAccess = mockUserHasAccessFalse + rr := httptest.NewRecorder() handler := http.HandlerFunc(bHandler.BountyBudgetWithdraw) - mockDb.On("UserHasAccess", "valid-key", mock.AnythingOfType("string"), db.WithdrawBudget).Return(false) validData := []byte(`{"orgUuid": "org-1", "paymentRequest": "invoice"}`) req, err := http.NewRequestWithContext(authorizedCtx, http.MethodPost, "/budget/withdraw", bytes.NewReader(validData)) @@ -1467,8 +1470,8 @@ func TestBountyBudgetWithdraw(t *testing.T) { mockDb := dbMocks.NewDatabase(t) mockHttpClient := mocks.NewHttpClient(t) bHandler := NewBountyHandler(mockHttpClient, mockDb) + bHandler.userHasAccess = mockUserHasAccessTrue - mockDb.On("UserHasAccess", "valid-key", "org-1", db.WithdrawBudget).Return(true) mockDb.On("GetOrganizationBudget", "org-1").Return(db.BountyBudget{ TotalBudget: 500, }, nil) @@ -1497,10 +1500,10 @@ func TestBountyBudgetWithdraw(t *testing.T) { mockDb := dbMocks.NewDatabase(t) mockHttpClient := mocks.NewHttpClient(t) bHandler := NewBountyHandler(mockHttpClient, mockDb) + bHandler.userHasAccess = mockUserHasAccessTrue paymentAmount := uint(1500) - mockDb.On("UserHasAccess", "valid-key", "org-1", db.WithdrawBudget).Return(true) mockDb.On("GetOrganizationBudget", "org-1").Return(db.BountyBudget{ TotalBudget: 5000, }, nil) @@ -1536,8 +1539,8 @@ func TestBountyBudgetWithdraw(t *testing.T) { mockDb := dbMocks.NewDatabase(t) mockHttpClient := mocks.NewHttpClient(t) bHandler := NewBountyHandler(mockHttpClient, mockDb) + bHandler.userHasAccess = mockUserHasAccessTrue - mockDb.On("UserHasAccess", "valid-key", "org-1", db.WithdrawBudget).Return(true) mockDb.On("GetOrganizationBudget", "org-1").Return(db.BountyBudget{ TotalBudget: 5000, }, nil) @@ -1573,6 +1576,7 @@ func TestBountyBudgetWithdraw(t *testing.T) { mockDb := dbMocks.NewDatabase(t) mockHttpClient := mocks.NewHttpClient(t) bHandler := NewBountyHandler(mockHttpClient, mockDb) + bHandler.userHasAccess = mockUserHasAccessTrue paymentAmount := uint(1500) initialBudget := uint(5000) @@ -1586,7 +1590,6 @@ func TestBountyBudgetWithdraw(t *testing.T) { mockHttpClient.ExpectedCalls = nil mockHttpClient.Calls = nil - mockDb.On("UserHasAccess", "valid-key", "org-1", db.WithdrawBudget).Return(true) mockDb.On("GetOrganizationBudget", "org-1").Return(db.BountyBudget{ TotalBudget: expectedFinalBudget, }, nil) diff --git a/handlers/organization_test.go b/handlers/organization_test.go index 933fd7fa0..6be18b905 100644 --- a/handlers/organization_test.go +++ b/handlers/organization_test.go @@ -409,6 +409,9 @@ func TestGetOrganizationBounties(t *testing.T) { func TestGetOrganizationBudget(t *testing.T) { ctx := context.WithValue(context.Background(), auth.ContextKey, "test-key") mockDb := mocks.NewDatabase(t) + mockUserHasAccess := func(pubKeyFromAuth string, uuid string, role string) bool { + return true + } oHandler := NewOrganizationHandler(mockDb) t.Run("Should test that a 401 is returned when trying to view an organization's budget without a token", func(t *testing.T) { @@ -437,7 +440,7 @@ func TestGetOrganizationBudget(t *testing.T) { Updated: nil, } - mockDb.On("UserHasAccess", "test-key", orgUUID, "VIEW REPORT").Return(true).Once() + oHandler.userHasAccess = mockUserHasAccess mockDb.On("GetOrganizationBudget", orgUUID).Return(expectedBudget).Once() rctx := chi.NewRouteContext() @@ -470,7 +473,10 @@ func TestGetOrganizationBudgetHistory(t *testing.T) { t.Run("Should test that a 401 is returned when trying to view an organization's budget history without a token", func(t *testing.T) { orgUUID := "valid-uuid" - mockDb.On("UserHasAccess", "", orgUUID, "VIEW REPORT").Return(false).Once() + mockUserHasAccess := func(pubKeyFromAuth string, uuid string, role string) bool { + return false + } + oHandler.userHasAccess = mockUserHasAccess rctx := chi.NewRouteContext() rctx.URLParams.Add("uuid", orgUUID) @@ -492,7 +498,11 @@ func TestGetOrganizationBudgetHistory(t *testing.T) { {BudgetHistory: db.BudgetHistory{ID: 2, OrgUuid: orgUUID, Created: nil, Updated: nil}, SenderName: "Sender2"}, } - mockDb.On("UserHasAccess", "test-key", orgUUID, "VIEW REPORT").Return(true).Once() + mockUserHasAccess := func(pubKeyFromAuth string, uuid string, role string) bool { + return true + } + oHandler.userHasAccess = mockUserHasAccess + mockDb.On("GetOrganizationBudgetHistory", orgUUID).Return(expectedBudgetHistory).Once() rctx := chi.NewRouteContext() diff --git a/handlers/organizations.go b/handlers/organizations.go index 7022c7eb5..2f0544f6c 100644 --- a/handlers/organizations.go +++ b/handlers/organizations.go @@ -13,20 +13,26 @@ import ( "github.com/stakwork/sphinx-tribes/auth" "github.com/stakwork/sphinx-tribes/db" "github.com/stakwork/sphinx-tribes/utils" + "gorm.io/gorm" ) type organizationHandler struct { - db db.Database - generateBountyHandler func(bounties []db.Bounty) []db.BountyResponse - getLightningInvoice func(payment_request string) (db.InvoiceResult, db.InvoiceError) + db db.Database + generateBountyHandler func(bounties []db.Bounty) []db.BountyResponse + getLightningInvoice func(payment_request string) (db.InvoiceResult, db.InvoiceError) + userHasAccess func(pubKeyFromAuth string, uuid string, role string) bool + userHasManageBountyRoles func(pubKeyFromAuth string, uuid string) bool } -func NewOrganizationHandler(db db.Database) *organizationHandler { - bHandler := NewBountyHandler(http.DefaultClient, db) +func NewOrganizationHandler(database db.Database) *organizationHandler { + bHandler := NewBountyHandler(http.DefaultClient, database) + dbConf := db.NewDatabaseConfig(&gorm.DB{}) return &organizationHandler{ - db: db, - generateBountyHandler: bHandler.GenerateBountyResponse, - getLightningInvoice: bHandler.GetLightningInvoice, + db: database, + generateBountyHandler: bHandler.GenerateBountyResponse, + getLightningInvoice: bHandler.GetLightningInvoice, + userHasAccess: dbConf.UserHasAccess, + userHasManageBountyRoles: dbConf.UserHasManageBountyRoles, } } @@ -487,7 +493,7 @@ func (oh *organizationHandler) GetUserDropdownOrganizations(w http.ResponseWrite organization := db.DB.GetOrganizationByUuid(uuid) bountyCount := db.DB.GetOrganizationBountyCount(uuid) hasRole := db.UserHasAccess(user.OwnerPubKey, uuid, db.ViewReport) - hasBountyRoles := oh.db.UserHasManageBountyRoles(user.OwnerPubKey, uuid) + hasBountyRoles := oh.userHasManageBountyRoles(user.OwnerPubKey, uuid) // don't add deleted organizations to the list if !organization.Deleted && hasBountyRoles { @@ -558,7 +564,7 @@ func (oh *organizationHandler) GetOrganizationBudget(w http.ResponseWriter, r *h } // if not the organization admin - hasRole := oh.db.UserHasAccess(pubKeyFromAuth, uuid, db.ViewReport) + hasRole := oh.userHasAccess(pubKeyFromAuth, uuid, db.ViewReport) if !hasRole { w.WriteHeader(http.StatusUnauthorized) json.NewEncoder(w).Encode("Don't have access to view budget") @@ -578,7 +584,7 @@ func (oh *organizationHandler) GetOrganizationBudgetHistory(w http.ResponseWrite uuid := chi.URLParam(r, "uuid") // if not the organization admin - hasRole := oh.db.UserHasAccess(pubKeyFromAuth, uuid, db.ViewReport) + hasRole := oh.userHasAccess(pubKeyFromAuth, uuid, db.ViewReport) if !hasRole { w.WriteHeader(http.StatusUnauthorized) json.NewEncoder(w).Encode("Don't have access to view budget history")