Skip to content

Conversation

@nchaulet
Copy link
Member

@nchaulet nchaulet commented Dec 31, 2024

Summary

Resolve #203170

Use Kibana authz instead of having custom logic for Fleet when it's possible. This add the required privileges to the generated open API doc.

Privileges are tested with API integration tests, so I do not think we will introduce a regression or a breaking change with that PR.

It was not possible to migrate all routes as some scenario are not yet supported like a route need (privilege1 AND privilege2) OR (privilege3 AND privilege 4) @legrego do you think it will be possible to support this kind of rules? do you want me to create an issue for that.

Screenshot 2025-01-03 at 8 57 21 AM

Do we want to backport this to 8.x?

nchaulet and others added 11 commits December 31, 2024 10:53
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --include-path /api/dashboards --update'
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --include-path /api/dashboards --update'
@nchaulet nchaulet marked this pull request as ready for review January 2, 2025 17:48
@nchaulet nchaulet requested a review from a team as a code owner January 2, 2025 17:48
@nchaulet nchaulet marked this pull request as draft January 2, 2025 17:48
@nchaulet nchaulet added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Jan 2, 2025
nchaulet and others added 7 commits January 2, 2025 14:01
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --include-path /api/dashboards --update'
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --include-path /api/dashboards --update'
@nchaulet nchaulet self-assigned this Jan 3, 2025
@nchaulet nchaulet added the Team:Fleet Team label for Observability Data Collection Fleet team label Jan 3, 2025
@nchaulet nchaulet marked this pull request as ready for review January 3, 2025 14:00
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@legrego
Copy link
Member

legrego commented Jan 3, 2025

It was not possible to migrate all routes as some scenario are not yet supported like a route need (privilege1 AND privilege2) OR (privilege3 AND privilege 4) @legrego do you think it will be possible to support this kind of rules? do you want me to create an issue for that.

@elena-shostak what do you think about this request?

…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --include-path /api/dashboards --update'
@nchaulet
Copy link
Member Author

nchaulet commented Jan 3, 2025

@elasticmachine merge upstream

@nchaulet nchaulet requested a review from juliaElastic January 3, 2025 15:41
Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

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

LGTM

nchaulet and others added 8 commits January 3, 2025 10:54
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --include-path /api/dashboards --update'
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --include-path /api/dashboards --update'
@nchaulet
Copy link
Member Author

nchaulet commented Jan 6, 2025

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💔 Build Failed

Failed CI Steps

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [211d4a6]

History

cc @nchaulet

@nchaulet
Copy link
Member Author

nchaulet commented Jan 6, 2025

@elasticmachine merge upstream

@nchaulet nchaulet merged commit 0b8ae36 into elastic:main Jan 6, 2025
9 checks passed
@nchaulet nchaulet deleted the feature-kibana-authz branch January 6, 2025 19:41
viduni94 pushed a commit to viduni94/kibana that referenced this pull request Jan 23, 2025
elena-shostak added a commit that referenced this pull request Apr 3, 2025
## Summary

Currently, our `requiredPrivileges` structure supports `allRequired` and
`anyRequired` for defining authorization logic. However, there is [a
need to
support](#205335 (comment))
more complex scenarios as `(privilege1 AND privilege2) OR (privilege3
AND privilege4)`

To achieve `anyRequired` has been extended to allow defining multiple
AND conditions evaluated with OR logic:
```ts
security: {
  authz: {
    requiredPrivileges: [{
       anyRequired: [
          { allOf: ['privilege1', 'privilege2'] }, 
          { allOf: ['privilege3', 'privilege4'] }
        ] 
      }
    ]
  }
}
```

`allRequired` now also supports scenarios `(privilege1 OR privilege2)
AND (privilege3 OR privilege4)`
```ts
security: {
  authz: {
    requiredPrivileges: [{
       allRequired: [
          { anyOf: ['privilege1', 'privilege2'] }, 
          { anyOf: ['privilege3', 'privilege4'] }
        ] 
      }
    ]
  }
}
```

> [!IMPORTANT]
> We expect to have unique privileges in `anyOf` or `allOf` conditions,
assuming that most complex conditions can be simplified by boolean
algebra laws (OR/AND distributive etc).


### Checklist

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [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
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

__Closes: https://github.com/elastic/kibana/issues/210977__

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
elena-shostak added a commit to elena-shostak/kibana that referenced this pull request Apr 3, 2025
## Summary

Currently, our `requiredPrivileges` structure supports `allRequired` and
`anyRequired` for defining authorization logic. However, there is [a
need to
support](elastic#205335 (comment))
more complex scenarios as `(privilege1 AND privilege2) OR (privilege3
AND privilege4)`

To achieve `anyRequired` has been extended to allow defining multiple
AND conditions evaluated with OR logic:
```ts
security: {
  authz: {
    requiredPrivileges: [{
       anyRequired: [
          { allOf: ['privilege1', 'privilege2'] },
          { allOf: ['privilege3', 'privilege4'] }
        ]
      }
    ]
  }
}
```

`allRequired` now also supports scenarios `(privilege1 OR privilege2)
AND (privilege3 OR privilege4)`
```ts
security: {
  authz: {
    requiredPrivileges: [{
       allRequired: [
          { anyOf: ['privilege1', 'privilege2'] },
          { anyOf: ['privilege3', 'privilege4'] }
        ]
      }
    ]
  }
}
```

> [!IMPORTANT]
> We expect to have unique privileges in `anyOf` or `allOf` conditions,
assuming that most complex conditions can be simplified by boolean
algebra laws (OR/AND distributive etc).

### Checklist

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [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
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

__Closes: https://github.com/elastic/kibana/issues/210977__

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit ed05808)

# Conflicts:
#	oas_docs/bundle.json
#	oas_docs/bundle.serverless.json
#	oas_docs/output/kibana.serverless.yaml
#	oas_docs/output/kibana.yaml
#	src/core/packages/http/router-server-internal/tsconfig.json
#	src/platform/packages/shared/kbn-router-to-openapispec/src/__snapshots__/generate_oas.test.ts.snap
#	src/platform/packages/shared/kbn-router-to-openapispec/src/generate_oas.test.fixture.ts
elena-shostak added a commit to elena-shostak/kibana that referenced this pull request Apr 3, 2025
## Summary

Currently, our `requiredPrivileges` structure supports `allRequired` and
`anyRequired` for defining authorization logic. However, there is [a
need to
support](elastic#205335 (comment))
more complex scenarios as `(privilege1 AND privilege2) OR (privilege3
AND privilege4)`

To achieve `anyRequired` has been extended to allow defining multiple
AND conditions evaluated with OR logic:
```ts
security: {
  authz: {
    requiredPrivileges: [{
       anyRequired: [
          { allOf: ['privilege1', 'privilege2'] },
          { allOf: ['privilege3', 'privilege4'] }
        ]
      }
    ]
  }
}
```

`allRequired` now also supports scenarios `(privilege1 OR privilege2)
AND (privilege3 OR privilege4)`
```ts
security: {
  authz: {
    requiredPrivileges: [{
       allRequired: [
          { anyOf: ['privilege1', 'privilege2'] },
          { anyOf: ['privilege3', 'privilege4'] }
        ]
      }
    ]
  }
}
```

> [!IMPORTANT]
> We expect to have unique privileges in `anyOf` or `allOf` conditions,
assuming that most complex conditions can be simplified by boolean
algebra laws (OR/AND distributive etc).

### Checklist

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [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
- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

__Closes: https://github.com/elastic/kibana/issues/210977__

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit ed05808)

# Conflicts:
#	oas_docs/bundle.json
#	oas_docs/bundle.serverless.json
#	oas_docs/output/kibana.serverless.yaml
#	oas_docs/output/kibana.yaml
#	src/platform/packages/shared/kbn-router-to-openapispec/src/__snapshots__/generate_oas.test.ts.snap
#	src/platform/packages/shared/kbn-router-to-openapispec/src/generate_oas.test.fixture.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Fleet] Convert Fleet roles/permissions to new Kibana authz framework

5 participants