-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Improve auditing of API key authentication #58928
Improve auditing of API key authentication #58928
Conversation
Pinging @elastic/es-security (:Security/Audit) |
...n/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java
Outdated
Show resolved
Hide resolved
Runnable action = () -> { | ||
logger.trace("Established authentication [{}] for request [{}]", authentication, request); | ||
listener.onResponse(authentication); | ||
}; | ||
try { | ||
authenticationSerializer.writeToContext(authentication, threadContext); | ||
request.authenticationSuccess(authentication); | ||
} catch (Exception e) { |
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.
Moving the auditing of authentication_success
down here avoids the unlikely, but confusing case of authentication_success
followed by authentication_failed
.
user = new User("hulk", new String[] { "superuser" }, new User("authenticated_user", new String[] { "other" })); | ||
} else { | ||
user = new User("hulk", new String[] { "superuser" }); | ||
} |
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.
mix-in creating an API Key while running-as.
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
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
1. Add the `apikey.id`, `apikey.name` and `authentication.type` fields to the `access_granted`, `access_denied`, `authentication_success`, and (some) `tampered_request` audit events. The `apikey.id` and `apikey.name` are present only when authn using an API Key. 2. When authn with an API Key, the `user.realm` field now contains the effective realm name of the user that created the key, instead of the synthetic value of `_es_api_key`.
This PR describes the new audit entry attributes api_key.id, api_key.name and authentication.type, as well as the meaning of existing attributes when authentication is performed using API keys. This should've been part of #58928
This PR describes the new audit entry attributes api_key.id, api_key.name and authentication.type, as well as the meaning of existing attributes when authentication is performed using API keys. This should've been part of #58928
This PR describes the new audit entry attributes api_key.id, api_key.name and authentication.type, as well as the meaning of existing attributes when authentication is performed using API keys. This should've been part of #58928
These changes roughly follow #52551 (comment) after some course correct.
apikey.id
,apikey.name
andauthentication.type
fields to theaccess_granted
,access_denied
,authentication_success
and (some)tampered_request
audit events (apikey.id
andapikey.name
are present only when authn with an API Key)user.realm
field now contains the realm name of the user that created the key, instead of the synthetic value of_es_api_key
. Note that in this circumstance, theuser.name
field reflects the name of the user that created the key (they are consistent)This is now handled separately in Ensure authz role for API key is named after owner role #59041 .user.roles
contains the role names (only of found/resolved roles) of the user that created the key. It previously wrongly had the role descriptor names from the API key definition.Relates #52551