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

[ResponseOps][Rules] Remove unintended internal Find routes API with public access #192957

Closed
cnasikas opened this issue Sep 15, 2024 · 4 comments · Fixed by #193757
Closed

[ResponseOps][Rules] Remove unintended internal Find routes API with public access #192957

cnasikas opened this issue Sep 15, 2024 · 4 comments · Fixed by #193757
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@cnasikas
Copy link
Member

There is a bug in our code that registers the following API GET [/internal/alerting/rules/_find] with public access. This happens because the same code is being used to register a public and internal API. The function to register is being called twice, one for the public and one for internal usage. There are no checks in place that guards the registration of the public route when is being called to register the internal usage.

@cnasikas cnasikas added bug Fixes for quality problems that affect the customer experience Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Sep 15, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@Zacqary
Copy link
Contributor

Zacqary commented Sep 18, 2024

There is a bug in our code that registers the following API GET [/internal/alerting/rules/_find] with public access.

Do you mean excludeFromPublicApi: false, in this block?

export const findInternalRulesRoute = (
  router: IRouter<AlertingRequestHandlerContext>,
  licenseState: ILicenseState,
  usageCounter?: UsageCounter
) => {
  buildFindRulesRoute({
    excludeFromPublicApi: false,
    licenseState,
    path: INTERNAL_ALERTING_API_FIND_RULES_PATH,
    router,
    usageCounter,
  });
};

I think that's just a confusingly named variable. Note how the public route uses excludeFromPublicApi: true. This variable, if true, excludes all fields that are in fieldsToExcludeFromPublicApi from the response:

  if (excludeFromPublicApi) {
    try {
      validateOperationOnAttributes(
        filterKueryNode,
        sortField,
        restOptions.searchFields,
        context.fieldsToExcludeFromPublicApi
      );
    } 

@Zacqary
Copy link
Contributor

Zacqary commented Sep 18, 2024

Oh never mind it's access: 'public' in the options

@cnasikas
Copy link
Member Author

cnasikas commented Sep 19, 2024

Yes, that's it, access: 'public'! findInternalRulesRoute calls buildFindRulesRoute which in turn on L41 calls router.get with access: 'public' and path: INTERNAL_ALERTING_API_FIND_RULES_PATH.

kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Oct 23, 2024
…public access (elastic#193757)

## Summary

Fixes elastic#192957

Removes the `internal/_find` route from public access by moving the
hard-coded `options` into the route builder functions.

---------

Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit 196caba)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Oct 23, 2024
…public access (elastic#193757)

## Summary

Fixes elastic#192957

Removes the `internal/_find` route from public access by moving the
hard-coded `options` into the route builder functions.

---------

Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit 196caba)
kibanamachine added a commit that referenced this issue Oct 23, 2024
…I with public access (#193757) (#197358)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[ResponseOps][Rules] Remove unintended internal Find routes API with
public access (#193757)](#193757)

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

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

<!--BACKPORT [{"author":{"name":"Zacqary Adam
Xeper","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-23T04:33:31Z","message":"[ResponseOps][Rules]
Remove unintended internal Find routes API with public access
(#193757)\n\n## Summary\r\n\r\nFixes #192957 \r\n\r\nRemoves the
`internal/_find` route from public access by moving the\r\nhard-coded
`options` into the route builder
functions.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"196cabad9bbd0511219fc7833a62cb8a0bb61514","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:ResponseOps","v9.0.0","Feature:Alerting/RulesFramework","backport:prev-minor","ci:project-deploy-observability","v8.16.0"],"title":"[ResponseOps][Rules]
Remove unintended internal Find routes API with public
access","number":193757,"url":"https://github.com/elastic/kibana/pull/193757","mergeCommit":{"message":"[ResponseOps][Rules]
Remove unintended internal Find routes API with public access
(#193757)\n\n## Summary\r\n\r\nFixes #192957 \r\n\r\nRemoves the
`internal/_find` route from public access by moving the\r\nhard-coded
`options` into the route builder
functions.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"196cabad9bbd0511219fc7833a62cb8a0bb61514"}},"sourceBranch":"main","suggestedTargetBranches":["8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193757","number":193757,"mergeCommit":{"message":"[ResponseOps][Rules]
Remove unintended internal Find routes API with public access
(#193757)\n\n## Summary\r\n\r\nFixes #192957 \r\n\r\nRemoves the
`internal/_find` route from public access by moving the\r\nhard-coded
`options` into the route builder
functions.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"196cabad9bbd0511219fc7833a62cb8a0bb61514"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Zacqary Adam Xeper <[email protected]>
kibanamachine added a commit that referenced this issue Oct 23, 2024
… with public access (#193757) (#197359)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ResponseOps][Rules] Remove unintended internal Find routes API with
public access (#193757)](#193757)

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

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

<!--BACKPORT [{"author":{"name":"Zacqary Adam
Xeper","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-23T04:33:31Z","message":"[ResponseOps][Rules]
Remove unintended internal Find routes API with public access
(#193757)\n\n## Summary\r\n\r\nFixes #192957 \r\n\r\nRemoves the
`internal/_find` route from public access by moving the\r\nhard-coded
`options` into the route builder
functions.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"196cabad9bbd0511219fc7833a62cb8a0bb61514","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:ResponseOps","v9.0.0","Feature:Alerting/RulesFramework","backport:prev-minor","ci:project-deploy-observability","v8.16.0"],"title":"[ResponseOps][Rules]
Remove unintended internal Find routes API with public
access","number":193757,"url":"https://github.com/elastic/kibana/pull/193757","mergeCommit":{"message":"[ResponseOps][Rules]
Remove unintended internal Find routes API with public access
(#193757)\n\n## Summary\r\n\r\nFixes #192957 \r\n\r\nRemoves the
`internal/_find` route from public access by moving the\r\nhard-coded
`options` into the route builder
functions.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"196cabad9bbd0511219fc7833a62cb8a0bb61514"}},"sourceBranch":"main","suggestedTargetBranches":["8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193757","number":193757,"mergeCommit":{"message":"[ResponseOps][Rules]
Remove unintended internal Find routes API with public access
(#193757)\n\n## Summary\r\n\r\nFixes #192957 \r\n\r\nRemoves the
`internal/_find` route from public access by moving the\r\nhard-coded
`options` into the route builder
functions.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"196cabad9bbd0511219fc7833a62cb8a0bb61514"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Zacqary Adam Xeper <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants