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

[http] api_integration tests handle internal route restriction #192407

Conversation

TinaHeiligers
Copy link
Contributor

@TinaHeiligers TinaHeiligers commented Sep 9, 2024

fix #192052

Summary

Internal APIs will be restricted from public access as of 9.0.0. In non-serverless environments, this breaking change will result in a 400 error if an external request is made to an internal Kibana API (route access option as "internal" or "public").
This PR allows API owners of non-xpack plugins to run their ftr API integration tests against the restriction and adds examples of how to handle it.

Checklist

Note to reviewers: The header needed to allow access to internal apis shouldn't change your test output, with or without the restriction enabled.

How to test the changes work:

Non x-pack:

  1. Set server.restrictInternalApis: true in test/common/config.js
  2. Ensure your tests pass

x-pack:

  1. Set server.restrictInternalApis: true in x-pack/test/api_integration/apis/security/config.ts
  2. Ensure the spaces tests pass

@TinaHeiligers TinaHeiligers added Feature:http release_note:skip Skip the PR/issue when compiling release notes test-api-integration v8.16.0 labels Sep 9, 2024
@TinaHeiligers TinaHeiligers marked this pull request as ready for review September 9, 2024 21:18
@TinaHeiligers TinaHeiligers requested review from a team as code owners September 9, 2024 21:18
@@ -36,6 +36,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
serverArgs: [
...functionalConfig.get('kbnTestServer.serverArgs'),
'--telemetry.optIn=true',
'--server.restrictInternalApis=false',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set to true to test changes

@@ -30,6 +30,8 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
serverArgs: [
...apiIntegrationConfig.get('kbnTestServer.serverArgs'),
'--server.xsrf.disableProtection=true',
// disable internal API restriction. See https://github.com/elastic/kibana/issues/163654
'--server.restrictInternalApis=false',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set to true to enable restriction and run spaces api_integration tests. The tests should still pass.

@@ -51,6 +51,8 @@ export default async function ({ readConfigFile }) {
'--xpack.discoverEnhanced.actions.exploreDataInContextMenu.enabled=true',
'--savedObjects.maxImportPayloadBytes=10485760', // for OSS test management/_import_objects,
'--savedObjects.allowHttpApiAccess=false', // override default to not allow hiddenFromHttpApis saved objects access to the http APIs see https://github.com/elastic/dev/issues/2200
// explicitly disable internal API restriction. See https://github.com/elastic/kibana/issues/163654
'--server.restrictInternalApis=false',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting this to true will cause tests that haven't been updated to fail.

Copy link
Contributor Author

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notes for reviewers

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Overall prepration for restricting internal APIs LGTM. Left a q about some lines that are commented out, but once that's addressed looks good!

test/api_integration/apis/console/proxy_route.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data Discovery changes LGTM, thanks!

@jeramysoucy
Copy link
Contributor

jeramysoucy commented Sep 11, 2024

Just leaving a note, after modifying x-pack/test/ftr_apis/security_and_spaces/config.ts with server.restrictInternalApis set to true, running the FTR from x-pack/test/ftr_apis/security_and_spaces/config.ts will have failures, but running from x-pack/test/api_integration/apis/security/config.ts will not. Maybe running from x-pack/test/ftr_apis/security_and_spaces/config.ts is not intended, from main either config runs in the FTR without failures.

I don't think this should hold up this PR, but we (@elastic/kibana-security ) will want to make sure that all intended execution paths will pass with the restrictions enabled.

@TinaHeiligers
Copy link
Contributor Author

@elasticmachine merge upstream

@TinaHeiligers
Copy link
Contributor Author

TinaHeiligers commented Sep 11, 2024

Just leaving a note, after modifying x-pack/test/ftr_apis/security_and_spaces/config.ts with server.restrictInternalApis set to true, running the FTR from x-pack/test/ftr_apis/security_and_spaces/config.ts will have failures, but running from x-pack/test/api_integration/apis/security/config.ts will not. Maybe running from x-pack/test/ftr_apis/security_and_spaces/config.ts is not intended, from main either config runs in the FTR without failures.

I don't think this should hold up this PR, but we (@elastic/kibana-security ) will want to make sure that all intended execution paths will pass with the restrictions enabled.

@jeramysoucy Thanks for mentioning this. x-pack/test/ftr_apis/security_and_spaces were added in #149188, to support the FTR Saved Object Client we had to introduce for accessing and handling all saved objects types.

The config is for the testing client and shouldn't be used to run the API integration tests. I fixed the typo in the PR description. There's nothing for the platform security team to do here, besides be aware of it.

@TinaHeiligers TinaHeiligers enabled auto-merge (squash) September 11, 2024 19:10
@TinaHeiligers
Copy link
Contributor Author

@elasticmachine merge upstream

@TinaHeiligers TinaHeiligers requested review from markov00 and a team September 11, 2024 19:11
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@TinaHeiligers TinaHeiligers merged commit 3a68f8b into elastic:main Sep 12, 2024
19 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Sep 12, 2024
@TinaHeiligers TinaHeiligers deleted the kbn-192052-update-ftr-test-for-restriction branch September 12, 2024 19:23
TinaHeiligers added a commit that referenced this pull request Oct 2, 2024
fix #163654

This PR enforces internal API restrictions in our standard offering.
Internal APIs are subject to rapid change and are intentionally not
public. By restricting their access, we protect consumers from these
rapid changes.

This PR does not change any public APIs and they remain available for
external consumption.

## Note to reviewers:
I chose the most practical way of resolving the failures (add the header
or disable the restriction).

## Details
Requests to internal Kibana APIs will be restricted globally. This
allows more flexibility in making breaking changes to internal APIs,
without a risk to external consumers.

## Why are we doing this?
The restriction is there to help mitigate the risk of breaking external
integrations consuming APIs. Internal APIs are subject to frequent
changes, necessary for feature development.

## What this means to plugin authors :
Kibana core applies the restriction when enabled through HTTP config.

## What this means to Kibana consumers:
Explicitly restricting access to internal APIs has advantages for
external consumers:
- Consumers can safely integrate with Kibana's stable, public APIs
- Consumers are protected from Internal route development, which may
involve breaking changes
- Relevant information in Kibana's external documentation that is
user-friendly and complete.

KB article explaining the change (tracked as part of
elastic/kibana-team#1044)

## Release note
Starting with this release, requests to internal Kibana APIs are
globally restricted by default. This change is designed to provide more
flexibility in making breaking changes to internal APIs while protecting
external consumers from unexpected disruptions.
**Key Changes**:
• _Internal API Access_: External consumers no longer have access to
Kibana’s internal APIs, which are now strictly reserved for internal
development and subject to frequent changes. This helps ensure that
external integrations only interact with stable, public APIs.
• _Error Handling_: When a request is made to an internal API without
the proper internal identifier (header or query parameter), Kibana will
respond with a 400 Bad Request error, indicating that the route exists
but is not allowed under the current Kibana configuration.

## How to test this
### Running kibana
1. Set `server.restrictInternalApis: true` in `kibana.yml`
2. Start es with any license
3. Start kibana
4. Make an external request to an internal API:
<details>
  <summary>curl request to get the config for 9.0:</summary>
  
  ```unset
curl --location
'localhost:5601/abc/api/kibana/management/saved_objects/_bulk_get' \
--header 'Content-Type: application/json' \
--header 'kbn-xsrf: kibana' \
--header 'x-elastic-internal-origin: kibana' \
--header 'Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==' \
--data '[
    {
        "type": "config",
        "id": "9.0.0"
    }
]'
  ```
</details>

The request should be successful.

5. Make the same curl request without the internal origin header
<details>
  <summary>curl:</summary>
  
  ```unset
curl --location
'localhost:5601/abc/api/kibana/management/saved_objects/_bulk_get' \
--header 'Content-Type: application/json' \
--header 'kbn-xsrf: kibana' \
--header 'Authorization: Basic ZWxhc3RpYzpjaGFuZ2VtZQ==' \
--data '[
    {
        "type": "config",
        "id": "9.0.0"
    }
]'
  ```
</details>

The response should be an error similar to:
`{"statusCode":400,"error":"Bad Request","message":"uri
[/api/kibana/management/saved_objects/_bulk_get] with method [post]
exists but is not available with the current configuration"}`

6. Remove `server.restrictInternalApis` from `kibana.yml` or set it to
`false`.

7. Repeat both curl requests above. Both should respond without an
error.


### Checklist
Delete any items that are not applicable to this PR.
- [X] [Documentation] was added for features that require explanation or
tutorials (In PR #191943)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios (and PR
#192407)
- [x] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
(docker list updated in #156935,
cloud stack packs for 9.0 kibana to follow)


### Risk Matrix

| Risk | Probability | Severity | Mitigation/Notes |

|---------------------------|-------------|----------|-------------------------|
| The restriction is knowingly bypassed by end-users adding the required
header to use `internal` APIs | Low | High | Kibana's internal APIs are
not documented in the OAS specifications. External consumption will be
prevented unless explicitly bypassed. |
| Upstream services don't include the header and requests to `internal`
APIs fail | Medium | Medium | The Core team needs to ensure intra-stack
components are updated too |


### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:http release_note:skip Skip the PR/issue when compiling release notes test-api-integration v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Core] Restrict access to internal APIs ftr test updates prep
9 participants