0
\$\begingroup\$

To bootstrap our application we have a configuration stored in a JSON file. This config holds different settings, like definition of endpoints (e. g. an API or SMS gateway etc.) or some default values for login.

endpoint.ts

export const isEndpoint = (candidate: any): candidate is Endpoint => {
  return (
    candidate && typeof candidate === 'object' &&
    candidate.hasOwnProperty('protocol') && (candidate.protocol === null || ['http', 'https'].includes(candidate.protocol)) &&
    candidate.hasOwnProperty('address') && (candidate.address === null || typeof candidate.address === 'string') &&
    candidate.hasOwnProperty('port') && (candidate.port === null || typeof candidate.port === 'number') &&
    candidate.hasOwnProperty('path') && (candidate.path === null || typeof candidate.path === 'string')
  );
}

export interface Endpoint {
  protocol: 'http'|'https'|null,
  address: string|null,
  port: number|null,
  path: string|null,
}

configutation.ts

import { Endpoint, isEndpoint } from './endpoint';

export const isConfiguration = (candidate: any): candidate is Configuration => {
  return (
    candidate && typeof candidate === 'object' &&
    candidate.hasOwnProperty('endpoints') &&
      candidate.endpoints.hasOwnProperty('api') && isEndpoint(candidate.endpoints.api) &&
      candidate.endpoints.hasOwnProperty('smsGateway') && isEndpoint(candidate.endpoints.smsGateway) &&
    candidate.hasOwnProperty('auth') &&
      candidate.auth.hasOwnProperty('company') && (candidate.auth.company === null || typeof candidate.auth.company === 'string')
  );
}

export interface Configuration {
  endpoints: {
    api: Endpoint,
    smsGateway: Endpoint
  },

  auth: {
    company: string|null
  }
}

In the end the code is used in an Angular service like this:

get configuration$(): Observable<Configuration> {
  if (this.configutation$) {
    return this.configutation$;
  }

  const path = environment.configuration;

  this.configutation$ = this.http.get<any>(path).pipe(
    map(configutation => {   
      if (isConfigutation(configutation)) {
        return configutation;
      }

      throw new Error('Configutation is broken.');
    }),
    shareReplay(1),
    catchError(error => {
      console.log(error);
      return of(null);
    })
  );

  return this.configutation$;
}

Now the isEndpoint and isConfiguration methos seems very cumbersome, especially when the configuration grows. I was thinking of using JSON schema and validate the input against the schema file. But if somebody replaces that file for example during build, then it still could go wrong.

Can this be improved?

Is this the TypeScrpt way or how could this be improved?

\$\endgroup\$
1
\$\begingroup\$

If you're worried that someone may replace your schema during build, you should also be worried that someone might replace any other file as well. That's pure paranoia...

I would say the typescript way is to write the config in typescript as well. If that's not an option, then sure go for schema validation.

Anyway for your current implementation, there is a quite a lot that can be simplified in those conditions.

This:

candidate && typeof candidate === 'object'

can be replaced with just

typeof candidate === 'object'

As there is no falsy value of type object.

Further those hasOwnProperty checks are not necesary. Deserialized json will have all those properties belong to their own. And if somebody passes an object that was not created by parsing json, but let's say instantiating a class, do you really care if those properties belong to the instance, the class or its parent?

export const isConfiguration = (candidate: any): candidate is Configuration => {
  return (
    typeof candidate === 'object' &&
    typeof candidate.endpoints === 'object' &&
    isEndpoint(candidate.endpoints.api) &&
    isEndpoint(candidate.endpoints.smsGateway) &&
    typeof candidate.auth === 'object' &&
    (candidate.auth.company === null || typeof candidate.auth.company === 'string')
  )
}
\$\endgroup\$
1
  • \$\begingroup\$ Thanks again for your answer. One minor thing: candidate && typeof candidate === 'object' can't really be simplified, because typeof null === "object", right? \$\endgroup\$
    – lampshade
    Jun 4 '21 at 8:22
0
\$\begingroup\$

I am sorry, my answer is not a classical "code review" with the goal to improve the code. In this case i am focusing on the whole approach. So if you wanted a classical code review, you could skip reading here.

First, what is the problem you want to address?
That someone maliciously changes the configuration? Then a schema validation will not help, because the "new" endpoints will still be a valid endpoint. Or you would have to hardcode some constraints like "only https, root domain has to be ACME.com, has to end with ...Endpoint, etc etc " in your validation.
If security is your pain point, then i would focus more on securing the build process. Because if the attacker could replace the configuration file, then she could also very likely change the file which contains the schema validation ...

If its about making a regular developer mistake and you want to identify not valid entries, there are multiple approachs.
But first i would verify that you want to provide the configuration at build time and not at runtime.

At Buildtime

You could make use of existing formats like http://json-schema.org/ with the already existient validation implementations.
You could switch the file format to one which allows IDE validation support. For example you could switch to a TS-File. You could write your own validation. ...

Personaly, i use TS files for configurations i know at build time. All possible configurations have to fullfill the central configuration interface.

At Runtime

But most of my configurations (in special the endpoints) i do not create at build time. The reason is, that i want to create one artefact and use it in my DEV environment, in my integration environment, in my test environment and then finaly in the production environment. If i have to create a new artefact for each environment, then its quite possible that something changes last second and the artefact in the production is different the the one tested before.... :-(
Therefore most configurations are provided as a regular endpoint by the backend. And the backend server gets it from the database, a central environment configurator, from server properties,...
Because those values could be changed at runtime, the operations team should always use a 4 eye principle. You could still add some kind of validation (in the storage itself, like in the database, or in the backend before the values are provided to the client, or then in the client itself), but i would in most cases doubt that the benefits outweigh the cost.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.