Add createApiKey support to security plugin #42146
Conversation
mikecote
added
Team:Security
|
Pinging @elastic/kibana-security |
|
Will re-writing this code in new platform |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💚 Build Succeeded |
There was a problem hiding this comment.
Since @azasypkin was the one who migrated authentication to the NP and added the test coverage I'd like to get his opinion on this as well. This is a pedantic nitpick, but the way the tests are written right now we end up "duplicating" certain tests in x-pack/plugins/security/server/authentication/api_key.test.ts and x-pack/plugins/security/server/authentication/index.test.ts. I'm torn on what, if anything, we should change.
💚 Build Succeeded |
| if (isSecurityFeatureDisabled()) { | ||
| return null; | ||
| } | ||
| return await callCluster('shield.createApiKey', { body }); |
There was a problem hiding this comment.
nit: it'd be great if we can at least debug-log here whenever new API key is requested. Later, it feels, we'll need to issue an audit log event here as well.... (ping @kobelb).
All in all it seems we'll end up with a APIKeys.create/invalidate/retrieve/* class here that can accept logger/auditLogger/clusterClient/isSecurityFeatureDisabled at the initialization (constructor) stage :) Having said that I'm totally fine to keep it as is right now if you're not up to this kind of generalization at this stage.
There was a problem hiding this comment.
For the time being, I think we can ignore the audit logging here... We only have to write entries to the Kibana audit log when we're performing authentication/authorization ourselves in Kibana and can't defer to the Elasticsearch audit log. At least, that's been the current thinking, which is entirely up for debate once we start to focus more on audit logging.
There was a problem hiding this comment.
We only have to write entries to the Kibana audit log when we're performing authentication/authorization ourselves in Kibana and can't defer to the Elasticsearch audit log.
Right, good point, thanks!
There was a problem hiding this comment.
Should the comment stay for now or be removed?
This is just a temporary restriction, right? Otherwise, aren't we going to end up restricting which users are able to create alerts? |
This comment has been minimized.
This comment has been minimized.
|
retest |
💚 Build Succeeded |
* Add createApiKey support to security plugin * Expiration is optional * Start moving code to new platform * Add unit tests * Fix jest test * Apply PR feedback * Apply PR feedback * Apply PR feedback pt2
💚 Build Succeeded |
In this PR, I'm adding a function to the security plugin that allows to create API Keys in Elasticsearch.
Creating API keys is required for alerting in order to execute requests on behalf of the user at a future time.
Resolves #39412