Skip to content
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

Fix harcoded password rds rule #79

Merged
merged 3 commits into from
Nov 25, 2019
Merged

Conversation

oscarbc96
Copy link
Contributor

Changed

  • HardcodedRDSPasswordRule now reports two different messages when there is a missing echo or a readable password.

Fixes

  • HardcodedRDSPasswordRule was wrongly adding an error when a value is provided.

jsoucheiron
jsoucheiron previously approved these changes Nov 25, 2019
from cfripper.model.result import Result
from cfripper.model.utils import convert_json_or_yaml_to_dict
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these imports have been imported in but don't seem to be used in the test file

if master_user_password == Parameter.NO_ECHO_WITH_DEFAULT:
self.add_failure(type(self).__name__, self.REASON_DEFAULT.format(resource_type, logical_id))
return True
elif master_user_password not in (Parameter.NO_ECHO_NO_DEFAULT, Parameter.NO_ECHO_WITH_VALUE,):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comma at end of line here

@@ -19,29 +19,22 @@

class HardcodedRDSPasswordRule(Rule):

REASON = "Default RDS {} password parameter or missing NoEcho for {}."
REASON_DEFAULT = "Default RDS {} password parameter (readable in plain-text) {}."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change to (readable in plain-text) for {}. :)

Copy link
Contributor

@ocrawford555 ocrawford555 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice stuff Oscar :)

@jsoucheiron jsoucheiron merged commit db0bcde into master Nov 25, 2019
@oscarbc96 oscarbc96 deleted the update_harcoded_password_rds branch December 11, 2019 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants