Feature/system info

[chrispotter]

Initial Solution to #1948
Impact: major
Type: feature

Issue

This is the start to a system-info plugin that will offer information related to system

Solution

This is step one from Issue #1948 to add a new graphql schema to be referenced from the dashboard to show appVersion.

Testing

http://localhost:3000/graphql-alpha

{
  systemInformation(shopId: "cmVhY3Rpb24vc2hvcDpKOEJocTN1VHRkZ3daeDNyeg==") {
    apiVersion
    mongoVersion {
      version
    }
    plugins {
      name
      version
    }
  }
}

Use admin auth token and your correct primary shop ID.

[focusaurus]

Looking good!

[mikemurray]

Maybe you're already thinking about this, but is this API going to remain public, or will it be behind auth of some kind? Since this is system info, it feels like it should be secured somehow.

[chrispotter]

@mikemurray I was considering throwing in
if (!context.userHasPermission(["owner", "admin"], id)) throw new ReactionError("access-denied", "User does not have permission"); but while I was testing my graphql-fu wasn't high enough to pass in an acceptable user through graphql-alpha and I forgot to add back in after.

@aldeed do you agree this should be behind authentication?

[chrispotter]

I think there is enough here for a PR with final review.

This can then be used when refactoring the admin page issue in #1948 when that is reconfigured to use updated API techniques. what do you think @focusaurus @aldeed ?

[aldeed]

Another round of comments

[ticean]

This should definitely be protected. We should not expose information like installed plugins to the public API. That would expose business secrets (ie which payment provider is used) as well as provide valuable information to attackers.

[aldeed]

@ticean In most cases it's going to be pretty obvious what payment provider is used. Frontend code will have stripe library, or you'll be redirected to paypal payment page, etc. But I'm fine with hiding plugin information from public API to be safe. What do you think about apiVersion field? Can we keep that public?

databaseInformation field should be admin-only.

[ticean]

I just used payment provider as an example. It doesn't matter if the users see which payment is used on the frontend. That doesn't expose which plugin and plugin version are providing the functionality. A plugin list does. Such a list provides mineable data that can be analyzed to describe all the functionalities the site has, the vulnerabilities available (via cross reference), and generally how up to date the site is keeping their dependencies.

I don't think any part of this endpoint should be public.

[chrispotter]

I think this will effect this PR

nevermind, looks like shopId is passed rather than as a context variable.

[chrispotter]

@aldeed i think this is ready for your review again!

[aldeed]

@chrispotter A few new suggestions

[aldeed]

I've started thinking about how we're soon wanting to move plugins to be npm packages. An import from outside the plugin like this one won't work in that case. At the moment, we only need the version. Instead, there are two files where new ReactionNodeApp() is done. You can import packageJson in those two files, and add the version to context in the ReactionNodeApp constructor options: appVersion: packageJson.version.

Then here you can use context.appVersion.

[aldeed]

Since initially reviewing this, we've been talking more about trying to stop using the Packages collection. The problem is that the collection can get out of sync with what plugins are truly in the codebase. So it would be better to access the actual list of plugins that were registered when the app started up.

On context, context.app is the ReactionNodeApp instance. On that you can find the plugin config for all plugins on app.registeredPlugins object. So I think it would be something like this:

const plugins = Object.values(context.app.registeredPlugins);

The rest should be the same.

[aldeed]

Note that unlike the Packages collection, registeredPlugins is NOT different per shopId. But I think you should keep the shopId param in GraphQL for the permission check and potential future use.

[aldeed]

All field descriptions should begin with a capital letter by convention. I guess our GraphQL linter does not check this.

[aldeed]

Merged in latest develop to fix the startup bug, and then tested. Looks good!