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

[Fleet] Handler api key creation errors when Fleet Admin is invalid #84576

Merged
merged 4 commits into from
Dec 1, 2020

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Nov 30, 2020

Summary

Resolve #84412

We use an user fleet_enroll stored in a saved object to create API keys in Fleet. Currently if this user is updated (like a password change) outside of Fleet, our APIs that used that user will return a 401 and this will cause the current User to be logged out from Kibana.

This PR fix this by handling that error and returning a 400 with the message Fleet Admin user is invalid also we clear the cache we use to store the fleet_enroll user.

How to test this?

  1. Setup Fleet and Agents
  2. Modify the fleet_enroll user /app/management/security/users/edit/fleet_enroll

Screen Shot 2020-11-30 at 2 43 01 PM

  1. Try to create a new enrollment API key
  • Before this caused the user to logged out
  • Now we should have this error message from the API

Screen Shot 2020-11-30 at 2 32 20 PM

@nchaulet nchaulet added bug Fixes for quality problems that affect the customer experience v8.0.0 v7.10.0 Team:Fleet Team label for Observability Data Collection Fleet team v7.11.0 labels Nov 30, 2020
@nchaulet nchaulet self-assigned this Nov 30, 2020
@nchaulet nchaulet requested a review from a team November 30, 2020 19:52
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@nchaulet nchaulet added the release_note:skip Skip the PR/issue when compiling release notes label Nov 30, 2020
Copy link
Contributor

@jfsiii jfsiii left a comment

Choose a reason for hiding this comment

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

The code and screenshots LGTM overall. WDYT about changing the request as KibanaRequest to KibanaRequest.from as mentioned in https://github.com/elastic/sdh-kibana/issues/1009#issuecomment-734192331 ?

@nchaulet
Copy link
Member Author

nchaulet commented Dec 1, 2020

@jfsiii Just updated from request as KibanaRequest to KibanaRequest.from

Co-authored-by: John Schulz <john.schulz@elastic.co>
Copy link
Contributor

@jfsiii jfsiii left a comment

Choose a reason for hiding this comment

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

:shipit: Haven't run locally, but the tests & code are clear.

I think we can remove the as cast now that we're using the factory from core.

@nchaulet
Copy link
Member Author

nchaulet commented Dec 1, 2020

I think we can remove the as cast now that we're using the factory from core.

We still need a as cast for the argument we pass to the factory, but yes it's cleaner

@nchaulet nchaulet merged commit 9b74fe6 into elastic:master Dec 1, 2020
@nchaulet nchaulet deleted the feature-handler-fleet-admin-error branch December 1, 2020 17:55
nchaulet added a commit to nchaulet/kibana that referenced this pull request Dec 1, 2020
…lastic#84576)

# Conflicts:
#	x-pack/plugins/fleet/public/applications/fleet/sections/agents/enrollment_token_list_page/components/new_enrollment_key_flyout.tsx
#	x-pack/test/fleet_api_integration/apis/enrollment_api_keys/crud.ts
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 2, 2020
* master:
  [Lens] Show color in flyout instead of auto (elastic#84532)
  [Lens] Use index pattern through service instead of reading saved object (elastic#84432)
  Make it possible to use Kibana anonymous authentication provider with ES anonymous access. (elastic#84074)
  TelemetryCollectionManager: Use X-Pack strategy as an OSS overwrite (elastic#84477)
  migrate away from rest_total_hits_as_int (elastic#84508)
  [Input Control] Custom renderer (elastic#84423)
  Attempt to more granularly separate App Search vs Workplace Search vs shared GitHub notifications (elastic#84713)
  [Security Solutino][Case] Case connector alert UI (elastic#82405)
  [Maps] Support runtime fields in tooltips (elastic#84377)
  [CCR] Fix row actions in follower index and auto-follow pattern tables (elastic#84433)
  [Enterprise Search] Migrate shared Indexing Status component (elastic#84571)
  [maps] remove fields from index-pattern test artifacts (elastic#84379)
  Add routes for use in Sources Schema (elastic#84579)
  Changes UI links for drilldowns (elastic#83971)
  endpoint telemetry cloned endpoint tests (elastic#81498)
  [Fleet] Handler api key creation errors when Fleet Admin is invalid (elastic#84576)
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
fleet 1.1MB 1.1MB +60.0B

History

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

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 3, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 4, 2020
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 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.10.0 v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet] Handle 401 errors when creating API keys
4 participants