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

[FTR] allow to call roleScopedSupertest service with Cookie header #192727

Merged
merged 39 commits into from
Sep 19, 2024

Conversation

dmlemeshko
Copy link
Member

@dmlemeshko dmlemeshko commented Sep 12, 2024

Summary

During the sync with kibana-security team we agreed on how Kibana APIs should be tested:

API Authentication in Kibana: Public vs. Internal APIs

Kibana provides both public and internal APIs, each requiring authentication with the correct privileges. However, the method of testing these APIs varies, depending on how they are untilized by end users.

  • Public APIs: When testing HTTP requests to public APIs, API key-based authentication should be used. It reflect how end user call these APIs. Due to existing restrictions, we utilize Admin user credentials to generate API keys for various roles. While the API key permissions are correctly scoped according to the assigned role, the user will internally be recognized as Admin during authentication.

  • Internal APIs: Direct HTTP requests to internal APIs are generally not expected. However, for testing purposes, authentication should be performed using the Cookie header. This approach simulates client-side behavior during browser interactions, mirroring how internal APIs are indirectly invoked.

To simplify the process of creating/updating the tests, this PR makes few changes to roleScopedSupertest service

  1. testing public APIs (nothing changed)
      const supertestAdminWithApiKey = await roleScopedSupertest.getSupertestWithRoleScope('admin', {
        withCustomHeaders: { 'accept-encoding': 'gzip' },
      });
      const response = await supertestAdminWithApiKey.get('/app/kibana');
  1. testing internal APIs
      const  supertestAdminWithCookieCredentials = await roleScopedSupertest.getSupertestWithRoleScope(
        'admin',
        {
          useCookieHeader: true, // will use Cookie header instead of API key
          withInternalHeaders: true,
        }
      );
      await supertestAdminWithCookieCredentials
        .post(`/internal/kibana/settings/${TEST_SETTING}`)
        .send({ value: 100 })
        .expect(200);

I updated some of the existing tests according to the new approach.

Docs for serverless and deployment-agnostic api integration tests were updated accordingly

@dmlemeshko dmlemeshko added v9.0.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Sep 13, 2024
@dmlemeshko dmlemeshko changed the title [FTR] allow to call supertestWithRoleScope service with Cookie header [FTR] allow to call roleScopedSupertest service with Cookie header Sep 13, 2024
Comment on lines +26 to +32
supertestAdminWithCookieCredentials = await roleScopedSupertest.getSupertestWithRoleScope(
'admin',
{
useCookieHeader: true,
withInternalHeaders: true,
}
);
Copy link
Member Author

Choose a reason for hiding this comment

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

Since all the APIs in this suite are internal, we use supertest scoped to Admin role and cookie header for auth

Comment on lines +27 to +33
supertestAdminWithCookieCredentials = await roleScopedSupertest.getSupertestWithRoleScope(
'admin',
{
useCookieHeader: true,
withInternalHeaders: true,
}
);
Copy link
Member Author

Choose a reason for hiding this comment

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

Since all the APIs in this suite are internal, we use supertest scoped to Admin role and cookie header for auth

Comment on lines +24 to +34
supertestAdminWithCookieCredentials = await roleScopedSupertest.getSupertestWithRoleScope(
'admin',
{
useCookieHeader: true,
withInternalHeaders: true,
withCustomHeaders: {
[ELASTIC_HTTP_VERSION_HEADER]: KQL_TELEMETRY_ROUTE_LATEST_VERSION,
'content-type': 'application/json',
},
}
);
Copy link
Member Author

Choose a reason for hiding this comment

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

Since all the APIs in this suite are internal, we use supertest scoped to Admin role and cookie header for auth. Repeated headers moved directly to scoped supertest creation.

@dmlemeshko dmlemeshko enabled auto-merge (squash) September 18, 2024 08:49
Copy link
Contributor

@jeramysoucy jeramysoucy left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. Mostly nits and some typos. Thank you for putting this together!

x-pack/test_serverless/README.md Outdated Show resolved Hide resolved
x-pack/test_serverless/README.md Outdated Show resolved Hide resolved
x-pack/test_serverless/README.md Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a note - we'll want to revisit these when we make Spaces CRUD APIs public and enable multiple spaces in serverless.

@dmlemeshko dmlemeshko requested a review from a team as a code owner September 18, 2024 12:02
Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

LGTM !!

Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jeramysoucy jeramysoucy left a comment

Choose a reason for hiding this comment

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

LGTM! Just found one missed change. Thanks for making these updates, Dima!

@@ -68,7 +68,7 @@ export default function ({ getService }: FtrProviderContext) {

({ body, status } = await supertestWithoutAuth
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this one was missed

Suggested change
({ body, status } = await supertestWithoutAuth
({ body, status } = await supertestAdminWithCookieCredentials

Copy link
Member Author

Choose a reason for hiding this comment

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

But this scenario is testing the case without auth: we suppose to get 401.

On main we don't set API key header https://github.com/elastic/kibana/blob/main/x-pack/test_serverless/api_integration/test_suites/common/platform_security/api_keys.ts#L69-L79

Should I keep it as is?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see. I think that was maybe unintentional, looking at the comment

// expect a rejection because we're not using the internal header

I think the test was intended to fail for not utilizing the internal header, like with the other test cases.
But, this is on our team, not yours. I don't want to hold up your PR. I can fix this when I open a PR to adjust the access level for some of our other endopints.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/ftr-common-functional-services 81 83 +2
Unknown metric groups

API count

id before after diff
@kbn/ftr-common-functional-services 106 108 +2

History

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

cc @dmlemeshko

@dmlemeshko dmlemeshko merged commit d9148f1 into elastic:main Sep 19, 2024
21 checks passed
@dmlemeshko dmlemeshko added v8.16.0 backport:version Backport to applied version labels and removed backport:skip This commit does not require backporting labels Sep 23, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 23, 2024
…lastic#192727)

## Summary

During the sync with kibana-security team we agreed on how Kibana APIs
should be tested:

### API Authentication in Kibana: Public vs. Internal APIs

Kibana provides both public and internal APIs, each requiring
authentication with the correct privileges. However, the method of
testing these APIs varies, depending on how they are untilized by end
users.

- **Public APIs**: When testing HTTP requests to public APIs, **API
key-based authentication** should be used. It reflect how end user call
these APIs. Due to existing restrictions, we utilize `Admin` user
credentials to generate API keys for various roles. While the API key
permissions are correctly scoped according to the assigned role, the
user will internally be recognized as `Admin` during authentication.

- **Internal APIs**: Direct HTTP requests to internal APIs are generally
not expected. However, for testing purposes, authentication should be
performed **using the Cookie header**. This approach simulates
client-side behavior during browser interactions, mirroring how internal
APIs are indirectly invoked.

To simplify the process of creating/updating the tests, this PR makes
few changes to `roleScopedSupertest` service

1) testing public APIs (nothing changed)

```ts
      const supertestAdminWithApiKey = await roleScopedSupertest.getSupertestWithRoleScope('admin', {
        withCustomHeaders: { 'accept-encoding': 'gzip' },
      });
      const response = await supertestAdminWithApiKey.get('/app/kibana');
```

2) testing internal APIs
```ts
      const  supertestAdminWithCookieCredentials = await roleScopedSupertest.getSupertestWithRoleScope(
        'admin',
        {
          useCookieHeader: true, // will use Cookie header instead of API key
          withInternalHeaders: true,
        }
      );
      await supertestAdminWithCookieCredentials
        .post(`/internal/kibana/settings/${TEST_SETTING}`)
        .send({ value: 100 })
        .expect(200);
```

I updated some of the existing tests according to the new approach.

Docs for serverless and deployment-agnostic api integration tests were
updated accordingly

(cherry picked from commit d9148f1)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Sep 23, 2024
…der (#192727) (#193679)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[FTR] allow to call roleScopedSupertest service with Cookie header
(#192727)](#192727)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Dzmitry
Lemechko","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-19T23:10:18Z","message":"[FTR]
allow to call roleScopedSupertest service with Cookie header
(#192727)\n\n## Summary\r\n\r\nDuring the sync with kibana-security team
we agreed on how Kibana APIs\r\nshould be tested:\r\n\r\n### API
Authentication in Kibana: Public vs. Internal APIs\r\n\r\nKibana
provides both public and internal APIs, each requiring\r\nauthentication
with the correct privileges. However, the method of\r\ntesting these
APIs varies, depending on how they are untilized by
end\r\nusers.\r\n\r\n- **Public APIs**: When testing HTTP requests to
public APIs, **API\r\nkey-based authentication** should be used. It
reflect how end user call\r\nthese APIs. Due to existing restrictions,
we utilize `Admin` user\r\ncredentials to generate API keys for various
roles. While the API key\r\npermissions are correctly scoped according
to the assigned role, the\r\nuser will internally be recognized as
`Admin` during authentication.\r\n\r\n- **Internal APIs**: Direct HTTP
requests to internal APIs are generally\r\nnot expected. However, for
testing purposes, authentication should be\r\nperformed **using the
Cookie header**. This approach simulates\r\nclient-side behavior during
browser interactions, mirroring how internal\r\nAPIs are indirectly
invoked.\r\n\r\nTo simplify the process of creating/updating the tests,
this PR makes\r\nfew changes to `roleScopedSupertest` service\r\n\r\n1)
testing public APIs (nothing changed)\r\n\r\n```ts\r\n const
supertestAdminWithApiKey = await
roleScopedSupertest.getSupertestWithRoleScope('admin', {\r\n
withCustomHeaders: { 'accept-encoding': 'gzip' },\r\n });\r\n const
response = await
supertestAdminWithApiKey.get('/app/kibana');\r\n```\r\n\r\n2) testing
internal APIs\r\n```ts\r\n const supertestAdminWithCookieCredentials =
await roleScopedSupertest.getSupertestWithRoleScope(\r\n 'admin',\r\n
{\r\n useCookieHeader: true, // will use Cookie header instead of API
key\r\n withInternalHeaders: true,\r\n }\r\n );\r\n await
supertestAdminWithCookieCredentials\r\n
.post(`/internal/kibana/settings/${TEST_SETTING}`)\r\n .send({ value:
100 })\r\n .expect(200);\r\n```\r\n\r\nI updated some of the existing
tests according to the new approach.\r\n\r\nDocs for serverless and
deployment-agnostic api integration tests were\r\nupdated
accordingly","sha":"d9148f1d83e31f9f6a226c0c867e39d4aeb8d48f","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","v8.16.0","backport:version"],"title":"[FTR]
allow to call roleScopedSupertest service with Cookie
header","number":192727,"url":"https://github.com/elastic/kibana/pull/192727","mergeCommit":{"message":"[FTR]
allow to call roleScopedSupertest service with Cookie header
(#192727)\n\n## Summary\r\n\r\nDuring the sync with kibana-security team
we agreed on how Kibana APIs\r\nshould be tested:\r\n\r\n### API
Authentication in Kibana: Public vs. Internal APIs\r\n\r\nKibana
provides both public and internal APIs, each requiring\r\nauthentication
with the correct privileges. However, the method of\r\ntesting these
APIs varies, depending on how they are untilized by
end\r\nusers.\r\n\r\n- **Public APIs**: When testing HTTP requests to
public APIs, **API\r\nkey-based authentication** should be used. It
reflect how end user call\r\nthese APIs. Due to existing restrictions,
we utilize `Admin` user\r\ncredentials to generate API keys for various
roles. While the API key\r\npermissions are correctly scoped according
to the assigned role, the\r\nuser will internally be recognized as
`Admin` during authentication.\r\n\r\n- **Internal APIs**: Direct HTTP
requests to internal APIs are generally\r\nnot expected. However, for
testing purposes, authentication should be\r\nperformed **using the
Cookie header**. This approach simulates\r\nclient-side behavior during
browser interactions, mirroring how internal\r\nAPIs are indirectly
invoked.\r\n\r\nTo simplify the process of creating/updating the tests,
this PR makes\r\nfew changes to `roleScopedSupertest` service\r\n\r\n1)
testing public APIs (nothing changed)\r\n\r\n```ts\r\n const
supertestAdminWithApiKey = await
roleScopedSupertest.getSupertestWithRoleScope('admin', {\r\n
withCustomHeaders: { 'accept-encoding': 'gzip' },\r\n });\r\n const
response = await
supertestAdminWithApiKey.get('/app/kibana');\r\n```\r\n\r\n2) testing
internal APIs\r\n```ts\r\n const supertestAdminWithCookieCredentials =
await roleScopedSupertest.getSupertestWithRoleScope(\r\n 'admin',\r\n
{\r\n useCookieHeader: true, // will use Cookie header instead of API
key\r\n withInternalHeaders: true,\r\n }\r\n );\r\n await
supertestAdminWithCookieCredentials\r\n
.post(`/internal/kibana/settings/${TEST_SETTING}`)\r\n .send({ value:
100 })\r\n .expect(200);\r\n```\r\n\r\nI updated some of the existing
tests according to the new approach.\r\n\r\nDocs for serverless and
deployment-agnostic api integration tests were\r\nupdated
accordingly","sha":"d9148f1d83e31f9f6a226c0c867e39d4aeb8d48f"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192727","number":192727,"mergeCommit":{"message":"[FTR]
allow to call roleScopedSupertest service with Cookie header
(#192727)\n\n## Summary\r\n\r\nDuring the sync with kibana-security team
we agreed on how Kibana APIs\r\nshould be tested:\r\n\r\n### API
Authentication in Kibana: Public vs. Internal APIs\r\n\r\nKibana
provides both public and internal APIs, each requiring\r\nauthentication
with the correct privileges. However, the method of\r\ntesting these
APIs varies, depending on how they are untilized by
end\r\nusers.\r\n\r\n- **Public APIs**: When testing HTTP requests to
public APIs, **API\r\nkey-based authentication** should be used. It
reflect how end user call\r\nthese APIs. Due to existing restrictions,
we utilize `Admin` user\r\ncredentials to generate API keys for various
roles. While the API key\r\npermissions are correctly scoped according
to the assigned role, the\r\nuser will internally be recognized as
`Admin` during authentication.\r\n\r\n- **Internal APIs**: Direct HTTP
requests to internal APIs are generally\r\nnot expected. However, for
testing purposes, authentication should be\r\nperformed **using the
Cookie header**. This approach simulates\r\nclient-side behavior during
browser interactions, mirroring how internal\r\nAPIs are indirectly
invoked.\r\n\r\nTo simplify the process of creating/updating the tests,
this PR makes\r\nfew changes to `roleScopedSupertest` service\r\n\r\n1)
testing public APIs (nothing changed)\r\n\r\n```ts\r\n const
supertestAdminWithApiKey = await
roleScopedSupertest.getSupertestWithRoleScope('admin', {\r\n
withCustomHeaders: { 'accept-encoding': 'gzip' },\r\n });\r\n const
response = await
supertestAdminWithApiKey.get('/app/kibana');\r\n```\r\n\r\n2) testing
internal APIs\r\n```ts\r\n const supertestAdminWithCookieCredentials =
await roleScopedSupertest.getSupertestWithRoleScope(\r\n 'admin',\r\n
{\r\n useCookieHeader: true, // will use Cookie header instead of API
key\r\n withInternalHeaders: true,\r\n }\r\n );\r\n await
supertestAdminWithCookieCredentials\r\n
.post(`/internal/kibana/settings/${TEST_SETTING}`)\r\n .send({ value:
100 })\r\n .expect(200);\r\n```\r\n\r\nI updated some of the existing
tests according to the new approach.\r\n\r\nDocs for serverless and
deployment-agnostic api integration tests were\r\nupdated
accordingly","sha":"d9148f1d83e31f9f6a226c0c867e39d4aeb8d48f"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Dzmitry Lemechko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.