elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.77k stars 8.17k forks source link

Log a warning when detecting requests from integrations with internal APIs #194772

Open TinaHeiligers opened 4 days ago

TinaHeiligers commented 4 days ago

Kibana should log a warning when detecting requests from integrations with internal APIs before enforcing the restriction. Consumers can use these logs to audit their integrations and make changes before the restriction becomes enforced.

elasticmachine commented 4 days ago

Pinging @elastic/kibana-core (Team:Core)

rudolf commented 4 days ago

Consider whether we should add this for all routes marked as deprecated: true if that's not a lot of additional effort since this would benefit all deprecated APIs.

rudolf commented 4 days ago

In @Bamieh 's PR we're registering a callback to coreSetup.http.registerOnPostValidation so I think we could use the same approach for the logger. Once his PR is merged we could potentially move the logging logic into createRouteDeprecationHandler

https://github.com/elastic/kibana/blob/5de5824ffd13f092877b79c885b5506a7ac1831b/packages/core/root/core-root-server-internal/src/server.ts#L538-L546

Actually I see he's already passing in a logger and based on the context I think he was planning to add logging there. So we could maybe wait till he's back and/or contribute to his branch https://github.com/elastic/kibana/blob/5de5824ffd13f092877b79c885b5506a7ac1831b/packages/core/root/core-root-server-internal/src/server.ts#L542

TinaHeiligers commented 4 days ago

Actually I see he's already passing in a logger and based on the context I think he was planning to add logging there. So we could maybe wait till he's back and/or contribute to his branch

It's a bit complicated with restricting internal APIs because the change isn't a route-deprecation itself. We're deprecating access to internal routes when config prevents it.

I've gone with a similar strategy as version checking in createBuildNrMismatchLoggerPreResonseHandler.

/**
 * This should remain part of the logger prefix so that we can notify/track
 * when we see this logged for observability purposes.
 */
const INTERNAL_API_RESTRICTED_LOGGER_NAME = 'kbn-internal-api-restricted';
export const createRestrictInternalRoutesPostAuthHandler = (
  config: HttpConfig,
  log: Logger
): OnPostAuthHandler => {
  const isRestrictionEnabled = config.restrictInternalApis;
  log = log.get(INTERNAL_API_RESTRICTED_LOGGER_NAME);

  return (request, response, toolkit) => {
    const isInternalRoute = request.route.options.access === 'internal';
    if (isInternalRoute && !request.isInternalApiRequest) {
      if (isRestrictionEnabled) {
        // throw 400
        return response.badRequest({
          body: `uri [${request.url.pathname}] with method [${request.route.method}] exists but is not available with the current configuration`,
        });
      } else {
        // warn if the restriction is not enforced
        log.warn(
          `Access to uri [${request.url.pathname}] with method [${request.route.method}] is deprecated`
        );
      }
    }
    return toolkit.next();
  };
};

@rudolf I've moved the issue out of in progress to clarifying for now.