Skip to content
This repository has been archived by the owner on Apr 22, 2019. It is now read-only.

Sensitive and environment-specific settings #178

Closed
leoquijano opened this issue Sep 19, 2018 · 5 comments
Closed

Sensitive and environment-specific settings #178

leoquijano opened this issue Sep 19, 2018 · 5 comments
Assignees

Comments

@leoquijano
Copy link
Contributor

leoquijano commented Sep 19, 2018

Hello,

I'm using the ClassyLlama_AvaTax module v1.4.5 in Magento v2.2.6, and I noticed that some settings are not being marked as sensitive, for both production and development modes:

  • Account number.
  • License key.
  • Company code.

Also, it seems that the following setting should be environment-specific:

  • Live mode.

Not having these settings set as sensitive and environment-specific means that they will be exported by bin/magento app:config:dump into app/etc/config.php, a file which is intended to be version controlled. That risks exposing some private settings, and also makes it more difficult to use the same code base for both production and development sites.

Following Magento's recommendations, I created a custom module with a simple di.xml file for my projects, as follows:

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
  <type name="Magento\Config\Model\Config\TypePool">
    <arguments>
      <argument name="sensitive" xsi:type="array">
        <item name="tax/avatax/production_account_number" xsi:type="string">1</item>
        <item name="tax/avatax/production_license_key" xsi:type="string">1</item>
        <item name="tax/avatax/production_company_code" xsi:type="string">1</item>
        <item name="tax/avatax/development_account_number" xsi:type="string">1</item>
        <item name="tax/avatax/development_license_key" xsi:type="string">1</item>
        <item name="tax/avatax/development_company_code" xsi:type="string">1</item>
      </argument>
      <argument name="environment" xsi:type="array">
        <item name="tax/avatax/live_mode" xsi:type="string">1</item>
      </argument>
    </arguments>
  </type>
</config>

This works for my setup, but I figured it would be better if it's included in the official version of the module. Can you take a look and see if including it is feasible?

Thanks.

@erikhansen
Copy link
Contributor

@eoquijano Thanks for your suggestion. Would you be able to submit these changes as a PR to this extension? If not, we will add this to our backlog of tasks to address, but it will take longer to implement.

@leoquijano
Copy link
Contributor Author

Hi. I may be able to do it later after I finish with a project's deadline. The change itself is very simple but I think you have an set of integration tests to set up?

leoquijano added a commit to leoquijano/ClassyLlama_AvaTax that referenced this issue Oct 7, 2018
@leoquijano
Copy link
Contributor Author

Hi @erikhansen, I created PR #186 with the additions to di.xml. Testing was done manually and things worked as expected on my local dev.

erikhansen added a commit that referenced this issue Oct 11, 2018
…ment-specific-settings

[#178] Added sensitive and environment-specific entries to the configuration type pool
@erikhansen
Copy link
Contributor

@leoquijano Thanks for your contribution!

@rsisco I've reviewed and merged @leoquijano's PR associated with this issue. Can you include this in the next release of the extension (and ensure to credit @leoquijano for the contribution)?

@rsisco
Copy link

rsisco commented Oct 15, 2018

This has been included in version 1.4.8. Closing issue.

@rsisco rsisco closed this as completed Oct 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants