diff --git a/management/server/http/handlers/accounts/accounts_handler.go b/management/server/http/handlers/accounts/accounts_handler.go index 27a57c43425..c05105abca4 100644 --- a/management/server/http/handlers/accounts/accounts_handler.go +++ b/management/server/http/handlers/accounts/accounts_handler.go @@ -237,15 +237,20 @@ func (h *handler) updateAccount(w http.ResponseWriter, r *http.Request) { return } - _, userID := userAuth.AccountId, userAuth.UserId + accountID, userID := userAuth.AccountId, userAuth.UserId vars := mux.Vars(r) - accountID := vars["accountId"] - if len(accountID) == 0 { + reqAccountID := vars["accountId"] + if len(reqAccountID) == 0 { util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "invalid accountID ID"), w) return } + if reqAccountID != accountID { + util.WriteError(r.Context(), status.Errorf(status.PermissionDenied, "requested account ID does not match authenticated account"), w) + return + } + var req api.PutApiAccountsAccountIdJSONRequestBody err = json.NewDecoder(r.Body).Decode(&req) if err != nil { @@ -310,6 +315,8 @@ func (h *handler) deleteAccount(w http.ResponseWriter, r *http.Request) { return } + accountID := userAuth.AccountId + vars := mux.Vars(r) targetAccountID := vars["accountId"] if len(targetAccountID) == 0 { @@ -317,7 +324,12 @@ func (h *handler) deleteAccount(w http.ResponseWriter, r *http.Request) { return } - err = h.accountManager.DeleteAccount(r.Context(), targetAccountID, userAuth.UserId) + if targetAccountID != accountID { + util.WriteError(r.Context(), status.Errorf(status.PermissionDenied, "requested account ID does not match authenticated account"), w) + return + } + + err = h.accountManager.DeleteAccount(r.Context(), accountID, userAuth.UserId) if err != nil { util.WriteError(r.Context(), err, w) return diff --git a/management/server/http/handlers/accounts/accounts_handler_test.go b/management/server/http/handlers/accounts/accounts_handler_test.go index 6cbd5908d92..ac5ef3011f3 100644 --- a/management/server/http/handlers/accounts/accounts_handler_test.go +++ b/management/server/http/handlers/accounts/accounts_handler_test.go @@ -271,6 +271,15 @@ func TestAccounts_AccountsHandler(t *testing.T) { expectedStatus: http.StatusUnprocessableEntity, expectedArray: false, }, + { + name: "PutAccount with mismatched accountId returns forbidden", + expectedBody: false, + requestType: http.MethodPut, + requestPath: "/api/accounts/different_account_id", + requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 15552000,\"peer_login_expiration_enabled\": true},\"onboarding\": {\"onboarding_flow_pending\": true,\"signup_form_pending\": true}}"), + expectedStatus: http.StatusForbidden, + expectedArray: false, + }, } for _, tc := range tt { @@ -330,3 +339,32 @@ func TestAccounts_AccountsHandler(t *testing.T) { }) } } + +func TestDeleteAccount_CrossAccountForbidden(t *testing.T) { + accountID := "test_account" + adminUser := types.NewAdminUser("test_user") + + handler := initAccountsTestData(t, &types.Account{ + Id: accountID, + Domain: "hotmail.com", + Network: types.NewNetwork(), + Users: map[string]*types.User{ + adminUser.Id: adminUser, + }, + Settings: &types.Settings{}, + }) + + recorder := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodDelete, "/api/accounts/different_account_id", nil) + req = nbcontext.SetUserAuthInRequest(req, auth.UserAuth{ + UserId: adminUser.Id, + AccountId: accountID, + Domain: "hotmail.com", + }) + + router := mux.NewRouter() + router.HandleFunc("/api/accounts/{accountId}", handler.deleteAccount).Methods("DELETE") + router.ServeHTTP(recorder, req) + + assert.Equal(t, http.StatusForbidden, recorder.Code, "cross-account delete should be forbidden") +}