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

License Key Value Stored in config/project/project.yaml #30

Closed
curtismorte opened this issue Apr 19, 2021 · 5 comments · Fixed by #32
Closed

License Key Value Stored in config/project/project.yaml #30

curtismorte opened this issue Apr 19, 2021 · 5 comments · Fixed by #32
Assignees
Labels
enhancement New feature or request

Comments

@curtismorte
Copy link

Hey Guys,
Great plugin!

I happened to notice that the sandboxLicenseKey and licenseKey value is getting stored into the project configuration file (/config/project/project.yaml). A solution to avoid committing sensitive values into version control is to use an environmental configuration.

Per the Craft documentation:

Plugin settings that may need to change per-environment, or contain sensitive information, should be implemented as environmental settings.

For example, the current Avatax definition is:

avatax:
    edition: standard
    enabled: true
    schemaVersion: 2.0.0
    settings:
      accountId: ''
      certCaptureClientId: null
      certCapturePassword: null
      certCaptureUsername: null
      companyCode: ''
      debug: ''
      defaultDiscountCode: OD010000
      defaultShippingCode: FR
      defaultTaxCode: P0000000
      enableAddressValidation: ''
      enableCommitting: ''
      enablePartialRefunds: ''
      enableTaxCalculation: ''
      environment: sandbox
      licenseKey: ''
      sandboxAccountId: ''
      sandboxCompanyCode: ''
      sandboxLicenseKey: ''
      shipFromCity: ''
      shipFromCountry: ''
      shipFromName: ''
      shipFromState: ''
      shipFromStreet1: ''
      shipFromStreet2: ''
      shipFromStreet3: ''
      shipFromZipCode: ''

With the environment settings specified above, that could turn into:

avatax:
    edition: standard
    enabled: true
    schemaVersion: 2.0.0
    settings:
      ...
      licenseKey: '$AVALARA_LICENSE_KEY'
      sandboxLicenseKey: '$AVALARA_SANDBOX_LICENSE_KEY'
      ...

Assuming you were to set the values AVALARA_LICENSE_KEY AVALARA_SANDBOX_LICENSE_KEY in your /.env file.

@knynkwl
Copy link

knynkwl commented Apr 20, 2021

+1

@siffring
Copy link
Member

Good idea @curtismorte! We'll add this.

@siffring siffring assigned siffring and imagehat and unassigned siffring Apr 20, 2021
@siffring siffring added the enhancement New feature or request label Apr 20, 2021
@curtismorte
Copy link
Author

curtismorte commented Apr 20, 2021

As a side note,
We're fighting two things here. The first is a quick way to shim up a database without the actual data (a la project configuration files /app/project/project.yaml) and having environment specific configurations. While environment specific configurations are really what I am after, we still don't want configs being committed with keys. So thanks for adding the enhancement label.

Per the documentation you can get around this by:

  1. Installing the plugin, not specifying settings through the control panel, and committing your project config file to version control with empty values.
  2. Specifying an environment configuration file.

When you have an environment configuration file, you can't adjust settings via the control panel so the project config can't get updated ;)

An example configuration file: (/app/avatax.php)

<?php
use craft\helpers\App;

/**
 * Avatax plugin for Craft CMS 3.x
 *
 * Calculate and add sales tax to an order's base tax using Avalara's AvaTax service.
 *
 * @link      http://surprisehighway.com
 * @copyright Copyright (c) 2019 Surprise Highway
 */

/**
 * Avatax config.php
 *
 * This file exists only as a template for the Avatax settings.
 * It does nothing on its own.
 *
 * Don't edit this file, instead copy it to 'craft/config' as 'avatax.php'
 * and make your changes there to override default settings.
 *
 * Once copied to 'craft/config', this file will be multi-environment aware as
 * well, so you can have different settings groups for each environment, just as
 * you do for 'general.php'
 */

return [
    '*' => [
        // Enable debugging - true or false
        'debug' => false,

        // The address you will be posting from.
        'shipFromName' => '',
        'shipFromStreet1' => '',
        'shipFromStreet2' => '',
        'shipFromStreet3' => '',
        'shipFromCity' => '',
        'shipFromState' => '',
        'shipFromZipCode' => '',
        'shipFromCountry' => '',

        // The default Avalara Tax Code to use for Products.
        'defaultTaxCode' => '',

        // The default Avalara Tax Code to use for Shipping.
        'defaultShippingCode' => 'FR',

        // The default Avalara Tax Code to use for Discounts.
        'defaultDiscountCode' => 'OD010000',

        // Production account information.
        'accountId' => App::env('AVALARA_ACCOUNT_ID'),
        'licenseKey' => App::env('AVALARA_LICENSE_KEY'),
        'companyCode' => App::env('AVALARA_COMPANY_CODE'),

        // Sandbox account information.
        'sandboxAccountId' => App::env('AVALARA_SANDBOX_ACCOUNT_ID'),,
        'sandboxLicenseKey' => App::env('AVALARA_SANDBOX_LICENSE_KEY'),
        'sandboxCompanyCode' => App::env('AVALARA_SANDBOX_COMPANY_CODE'),

        // AvaTax options - true or false
        'enableTaxCalculation' => true,
        'enableCommitting' => false,
        'enableAddressValidation' => true,
        'enablePartialRefunds' => true,
    ],

    'dev' => [
        // Enable debugging - true or false
        'debug' => true,

        // Environment - 'production' or 'sandbox'.
        'environment' => 'sandbox',
    ],

    'stage' => [
        // Environment - 'production' or 'sandbox'.
        'environment' => 'sandbox',
    ],

    'production' => [
        // Environment - 'production' or 'sandbox'.
        'environment' => 'production',

        // AvaTax options - true or false
        'enableCommitting' => true,
    ],
];

Remember Avalara sandbox accounts are actually different accounts that you must pay for (yeah don't worry, I'm not amazingly thrilled by this either) so if you want to use your production account for all environments that is fine, just make sure you have transaction committing turned off for any non-production environments. Note that you still get billed for your production usage because of this. Take a dive further into AvaTax sandbox environments to learn more. This is also not legal / tax advice ;)

@knynkwl
Copy link

knynkwl commented Apr 20, 2021

Should this have been closed? Ideally, this plugin would utilize https://craftcms.com/docs/3.x/extend/environmental-settings.html#autosuggest-inputs

@imagehat
Copy link
Collaborator

imagehat commented May 21, 2021

@knynkwl @curtismorte - Just pushed v2.1.4 which addresses these issues. In the plugin settings I've made the account info fields autosuggest fields that will suggest ENV variables.

On the project config side it turns out if you do something like 'accountId' => App::env('AVALARA_ACCOUNT_ID'), the actual value is parsed and exposed in the project.yaml file. So instead I added the parseEnv logic to the plugin settings and you can instead now do something like this in your project config once your ENV variables are set up:

// Production account information from ENV.
'accountId'          => '$AVATAX_ACCOUNT_ID',
'licenseKey'         => '$AVATAX_LICENSE_KEY',
'companyCode'        => '$AVATAX_COMPANY_CODE',

// Sandbox account information from ENV.
'sandboxAccountId'   => '$AVATAX_SANDBOX_ACCOUNT_ID',
'sandboxLicenseKey'  => '$AVATAX_SANDBOX_LICENSE_KEY',
'sandboxCompanyCode' => '$AVATAX_SANDBOX_COMPANY_CODE',

Updated in the readme here:
https://github.com/surprisehighway/craft-avatax#config-overrides

Great ideas all around, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants