-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ export interface CreateAPIKeyParams { | |
} | ||
|
||
interface GrantAPIKeyParams { | ||
api_key: CreateAPIKeyParams; | ||
grant_type: 'password' | 'access_token'; | ||
username?: string; | ||
password?: string; | ||
|
@@ -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 commentThe reason will be displayed to describe this comment to others. Learn more. optional nit: missing JSDoc for There was a problem hiding this comment. Choose a reason for hiding this commentThe 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! |
||
if (!this.license.isEnabled()) { | ||
return null; | ||
} | ||
|
@@ -200,7 +201,7 @@ export class APIKeys { | |
`Unable to grant an API Key, request does not contain an authorization header` | ||
); | ||
} | ||
const params = this.getGrantParams(authorizationHeader); | ||
const params = this.getGrantParams(createParams, authorizationHeader); | ||
|
||
// User needs `manage_api_key` or `grant_api_key` privilege to use this API | ||
let result: GrantAPIKeyResult; | ||
|
@@ -281,9 +282,13 @@ export class APIKeys { | |
return disabledFeature === 'api_keys'; | ||
} | ||
|
||
private getGrantParams(authorizationHeader: HTTPAuthorizationHeader): GrantAPIKeyParams { | ||
private getGrantParams( | ||
createParams: CreateAPIKeyParams, | ||
authorizationHeader: HTTPAuthorizationHeader | ||
): GrantAPIKeyParams { | ||
if (authorizationHeader.scheme.toLowerCase() === 'bearer') { | ||
return { | ||
api_key: createParams, | ||
grant_type: 'access_token', | ||
access_token: authorizationHeader.credentials, | ||
}; | ||
|
@@ -294,6 +299,7 @@ export class APIKeys { | |
authorizationHeader.credentials | ||
); | ||
return { | ||
api_key: createParams, | ||
grant_type: 'password', | ||
username: basicCredentials.username, | ||
password: basicCredentials.password, | ||
|
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