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.

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

Improve misleading update() actions from db_manager #11222

Merged
merged 5 commits into from Jan 10, 2019

Conversation

Projects
None yet
2 participants
@ebleiweiss-r7
Copy link
Contributor

ebleiweiss-r7 commented Jan 9, 2019

Find all instances where Mdm::Xxx::update() is called and replace them with an update!() call on an instance of the object, instead of calling directly from the Mdm class.

From the original bug report (MS-3082):

Calling Mdm::<Model>.update(<id>, <changes>) on an ActiveRecord model does not raise an exception if validations fail for the updated attributes. To the user it looks like the update goes through successfully, but the new data is never stored in the database. We need to change this to better display to the user why their data is not being saved.

This happens locally, as well as through the API (MS-3675):

Calling the update_* method on a given model is not correctly returning error information when the update fails. Worse, it actually returns what looks to be a useable object back, which creates even more confusion.

Verification

  • ./msfdb start
  • Go to https://localhost:8080/api/v1/api-docs
  • Authenticate with api key

Test host

  • Add a host [POST]
  • Update a value for the host [PUT]
  • Get the host again [GET]
  • Verify that the value was set properly

Test loot

  • Repeat the above process for loot

Test note

  • Repeat the above process for note

Test service

  • Repeat the above process for service

Test session

  • Repeat the above process for session

Test user

  • Repeat the above process for user

Test workspace

  • Repeat the above process for workspace

Things not included in these testing instructions

  • Updating user token (lib/msf/core/db_manager/user.rb create_new_user_token)
  • Updating vuln detail (lib/msf/core/db_manager/vuln_detail.rb update_vuln_details)

ebleiweiss-r7 added some commits Jan 7, 2019

@jbarnett-r7 jbarnett-r7 self-assigned this Jan 10, 2019

ebleiweiss-r7 added some commits Jan 10, 2019

@jbarnett-r7 jbarnett-r7 merged commit 0435d7e into rapid7:master Jan 10, 2019

3 checks passed

Metasploit Automation - Sanity Test Execution Successfully completed all tests.
Details
Metasploit Automation - Test Execution Successfully completed all tests.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

jbarnett-r7 added a commit that referenced this pull request Jan 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment