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

[FEATURE] Add the ability to load site specific or custom configurations #1639

Open
vinhtrinh opened this issue Jan 15, 2024 · 3 comments
Open
Labels
Acknowledged Team has responded to issue

Comments

@vinhtrinh
Copy link

Is your feature request related to a problem? Please describe.

When implementing PWA apps, we need a secure location to store and retrieve site-specific and custom configurations for third-party integrations. These configurations should not be exposed to the client-side. Examples include: site preferences, third-party service API keys, etc.

Describe the solution you'd like

The custom configurations should align seamlessly with the existing application configurations functionalities.

Additional context

Some simple examples of usage:

// file: config/my-service.js
module.exports = {
    apiKey: process.env.MY_SERVICE_API_KEY,
    apiSecret: process.env.MY_SERVICE_API_SECRET
}

// file: config/site-1/default.js
module.exports = {
    someKey: 'some value',
    anotherKey: 'another value'
}

// Gets the configurations
import {getConfig, getSiteConfig} from '@salesforce/pwa-kit-runtime/utils/ssr-config'

// default application configurations
const appConfig = getConfig()

const siteConfig = getSiteConfig()
const myServiceConfig = getConfig({configFile: 'config/my-service'})
@bendvc
Copy link
Collaborator

bendvc commented Jan 18, 2024

Hi @vinhtrinh, thanks for your interest in the PWA-Kit. I have a few comments about your suggestion and ideas what we might be able to do to provide you some assistance.

First, with regards to this code :

// file: config/my-service.js
module.exports = {
    apiKey: process.env.MY_SERVICE_API_KEY,
    apiSecret: process.env.MY_SERVICE_API_SECRET
}

I want to say that, you've high lighted something that isn't currently supported, that is placing secrets in the config. The reason being is the react rendering pipeline on the server simple serializes the config without prejudice. Meaning that any secretes you have in environment variables will most likely end up on the client (bad). This shouldn't be too hard to add support for. For example we could treat any key in the config that ends with Secret as a secret and not serialize it. But that might be a breaking change, so we'd have to think of a way that isn't breaking.

We are open to contributions being an open source project, if that is something you want to look into. I'll bring it up to the team and see if its something we can get on our roadmap.

Moving on, looking at you other suggestion about site specific configurations being build in:

const siteConfig = getSiteConfig()

Unfortunately in javascript land we do not know what the current site is unless we were to look at the window.location, but then again that would only work on the browser. Fortunately this is pretty easy to solve in your application as the config files themselves are javascript. You can simply import you site specific config into the main config file and place it under the site property. So it's not going to be something that we'll be adding support for.

// config/default.js

const site1Config = require('./site-1/default.js')

export.default = {
    sites: [
         site1: sites1Config
    ]
}

Within you react app you can use the multisite hook to get you site information:

const {site} = useMultiSite()

I hope this helped.

@bendvc bendvc added the Acknowledged Team has responded to issue label Jan 18, 2024
@vinhtrinh
Copy link
Author

Hi @bendvc, thank you for the detailed response.

If we want to use naming convention to exclude secret or private configs, I would suggest that use the _ prefix as it's common convention in javascript world to define a private thing. However, this new convention might conflict with existing project config names. Thereforce, this solution only suitable for the next major release.

It would be great if we can supports both because the custom configuration has it own benefits in real projects.

  • Secret Confguration
    • Developers can store secret configuration in a separate file.
    • We can further protect sensitive data by restricting access to the /config folder from the client.
  • Organized Configuration
    • This feature allows developers to organize the configuration structures for specific project needs.
    • In SFRA, most of projects uses 30 ~ 50 cartridges. I mean, that is a lot of codes and configuration. Put all the configuration into a single entry point can become unwieldy.
  • Simplified Configuration Definition and Usage
    • While direct use of environment variables is possible, custom configuration will streamline the process, especially when handling different credentials for different sites.
    • Instead of process.env[siteId.toUpperCase() + '_MY_SERVICE_API_KEY']; with this feature, developers can get the configuration much easier myService.apiKey
  • Selective Configuration To Load
    • Custom configuration allow load specific configuration for a specific service, rather than the entire application configuration.
  • Environment Specific Configuration
    • By seamlessly aligning with existing application configuration functionalities, it allow developers to create different configuration files for different environments.

About the getSiteConfig() method, it's an optional helper method. If we don't know the configuration in the config file, we can change the siteId argument to required and remove the default site fallback behavior (or even remove this helper method from the package).

The main purpose of this method is provide a standard configuration structure as a reference to avoid unexpected behavior. Site configuration (or any other custom configuration files) should be placed in a sub-directory to avoid environment name conflicts. E.g., config/sites/RefArch/my-config.js, config/organization/my-config.js

For Example: the issue with the default Retail React App setup is: if developer create a new environment in the MRT named sites, the application will not work on that environment because getConfig is now resolve sites.js as application configuration instead of the default.js. This issue caused by configuration setup, and can be fixed by create a new environment with a different name. However, we can avoid this unexpected issue by move all custom configuration to sub-directories as a best practice.

@bendvc
Copy link
Collaborator

bendvc commented Feb 19, 2024

Hey @vinhtrinh

There are a lot of good ideas so I'll try to keep the message short. When it comes to supportive private secrets we most likely won't support using a configuration file as a valid place to house secrets. The reason being is that it opens up the number of attack vectors to have that sensitive data stolen, you would have your repository and the aws environment. For that reason we are currently looking into ways to support secret values where they would be only stored as environment variables.

With regards to requesting a stricter more organized configuration schema, we can take that on board. The beauty of use having javascript support for our configurations is that you can require files from any custom structure that you choose to create.

Continuing, we do already support environment specific configurations as shown in this doc.

Selective configuration.. I'm not sure I understand, can you give me an example/use case of this feature?

With respect to the example of having an environment named "sites", probably rare, but valid. I suggest that you create a bug issue for that so we can track and look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Acknowledged Team has responded to issue
Projects
None yet
Development

No branches or pull requests

2 participants