Skip to content

ARTEMIS-3176 don't log JDBC datasource 'password' property#3488

Closed
jbertram wants to merge 1 commit intoapache:masterfrom
jbertram:ARTEMIS-3176
Closed

ARTEMIS-3176 don't log JDBC datasource 'password' property#3488
jbertram wants to merge 1 commit intoapache:masterfrom
jbertram:ARTEMIS-3176

Conversation

@jbertram
Copy link
Contributor

No description provided.

.keySet()
.stream()
.map(key -> key + "=" + dataSourceProperties.get(key))
.map(key -> key + "=" + (key.equals("password") ? "****" : dataSourceProperties.get(key)))
Copy link
Member

Choose a reason for hiding this comment

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

Could the key be uppercase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would only be uppercase if org.apache.activemq.artemis.core.config.storage.DatabaseStorageConfiguration#getDataSource changes. It's hard-coded to use password.

I thought about making it a constant, but to be able to use it from org.apache.activemq.artemis.jdbc.store.drivers.JDBCDataSourceUtils I'd have to move it (and org.apache.activemq.artemis.core.config.StoreConfiguration) out of the artemis-server module and into the artemis-commons module.

Copy link
Member

@brusdev brusdev Mar 12, 2021

Choose a reason for hiding this comment

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

Could a user set the password using a data-source-property with an uppercase key?
<data-source-property key="PASSWORD" value="artemis" />

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brusdev, yes, I would assume that's theoretically possible. I will update the PR to use equalsIgnoreCase instead of equals.

Copy link
Member

@brusdev brusdev left a comment

Choose a reason for hiding this comment

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

LGTM +1

@asfgit asfgit closed this in 5514bca Mar 17, 2021
@jbertram jbertram deleted the ARTEMIS-3176 branch June 10, 2021 19:04
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.

2 participants