-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Require granted API Keys to have a name #71623
Conversation
Pinging @elastic/kibana-security (Team:Security) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alerting code LGTM
ACK: reviewing... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
@@ -697,4 +711,8 @@ export class AlertsClient { | |||
references, | |||
}; | |||
} | |||
|
|||
private generateAPIKeyName(alertTypeId: string, alertName: string) { | |||
return _.truncate(`Alerting: ${alertTypeId}/${alertName}`, { length: 256 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: is this 256
limit documented anywhere? Just curious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, I found this by testing API keys with very long names. Also tested a bunch of "special" characters, and there doesn't seem to be a restriction on what you're allowed to put here
@@ -188,7 +189,7 @@ export class APIKeys { | |||
* Tries to grant an API key for the current user. | |||
* @param request Request instance. | |||
*/ | |||
async grantAsInternalUser(request: KibanaRequest) { | |||
async grantAsInternalUser(request: KibanaRequest, createParams: CreateAPIKeyParams) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional nit: missing JSDoc for createParams
(I admit these JSDocs are useless most of the time, so feel free to ignore to not wait for CI run once again).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to merge w/o the JSDoc comment, just in the interest of time given our tight schedule today. I'll try to be more diligent about this going forward though!
💚 Build SucceededBuild metrics
To update your PR or re-run it, just comment with: |
* master: (21 commits) [Maps] 7.9 design improvements (elastic#71563) [ML] Changing all calls to ML endpoints to use internal user (elastic#70487) [eventLog] prevent log writing when initialization fails (elastic#71339) [Observability] landing page always being displayed (elastic#71494) [IM] Address data stream copy feedback (elastic#71615) [Logs UI] Anomalies page dataset filtering (elastic#71110) [data.search.aggs] Remove `use_field_mapping` from top hits agg (elastic#71168) [ML] Anomaly swim lane embeddable navigation and filter actions (elastic#71082) Fixes typo in siem_cloudtrail job description (elastic#71569) Require granted API Keys to have a name (elastic#71623) Update getUsageForCollection (elastic#71609) Only fetch saved elements once (elastic#71310) [SecuritySolution][Resolver] Adding siem index and guarding process ancestry (elastic#71570) [APM] Additional data telemetry changes (elastic#71112) [Visualize] Fix export table for table export links (elastic#71249) [Search] Server side search API (elastic#70446) use inclusive language (elastic#71607) [Security Solution] Hide timeline footer when Resolver is open (elastic#71516) [Index template wizard] Remove shadow and use border for components panels (elastic#71606) [ML] Kibana API endpoint for histogram chart data (elastic#70976) ...
Co-authored-by: Elastic Machine <[email protected]>
Summary
Resolves #71620