Skip to content

Add tracing for authorization#80815

Merged
ywangd merged 10 commits intoelastic:feature/apm-integrationfrom
ywangd:apm/authz-tracing
Nov 19, 2021
Merged

Add tracing for authorization#80815
ywangd merged 10 commits intoelastic:feature/apm-integrationfrom
ywangd:apm/authz-tracing

Conversation

@ywangd
Copy link
Copy Markdown
Member

@ywangd ywangd commented Nov 18, 2021

This PR adds tracing report for any authorizations except those for the _system user.

  • Can be turned off with xpack.security.authz.tracing: false
  • AuthZ is tied to relevant task by traceparent (it however does not
    cover the first authZ)
  • x-opaque-id is auto configured at rest layer if not already exists.
    This helps chain all relevant actions together.
  • ApmIT now has security enabled.

* Can be turned off with xpack.security.authz.tracing: false
* AuthZ is tied to relevant task by traceparent (it however does not
  cover the first authZ)
* x-opaque-id is auto configured at rest layer if not already exists.
  This helps chain all relevant actions together.
* ApmIT now has security enabled.
Comment on lines +117 to +122
// Populate x-opaque-id if not already exists to chain all related actions together
if (authentication != null && false == SystemUser.is(authentication.getUser())) {
if (threadContext.getHeader(Task.X_OPAQUE_ID) == null) {
threadContext.putHeader(Task.X_OPAQUE_ID, UUIDs.base64UUID());
}
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

X-Opaque-Id automatically gets populate across related tasks and also cross nodes. This could be an alternative to chain all things together. It does not differentiate between parent and child. But it may not matter since the timestamp should tell most of the story?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like capturing this, if we have it we might as well use it - if nothing else, it provides an easy way to go find the trace for a request you just made if you're running this on a live system.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes it is attached to every span in ApmTracer#onTraceStarted

APMTracer.APM_TOKEN_SETTING.getKey(),
System.getProperty("tests.apm.token", "")
);
builder.put("xpack.security.authz.tracing", true);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

A new setting to enable/disable authZ tracing.

Comment on lines -163 to +195
client().prepareSearch()
.setQuery(new RangeQueryBuilder("@timestamp").gt("2021-11-01"))
.setSearchType(SearchType.QUERY_THEN_FETCH)
.setPreFilterShardSize(1)
.execute()
.actionGet(10, TimeUnit.SECONDS);
final Request searchRequest = new Request("GET", "_search");
searchRequest.addParameter("search_type", "query_then_fetch");
searchRequest.addParameter("pre_filter_shard_size", "1");
searchRequest.setJsonEntity("{\"query\":{\"range\":{\"@timestamp\":{\"gt\":\"2021-11-01\"}}}}");
searchRequest.setOptions(
searchRequest.getOptions()
.toBuilder()
.addHeader(
"Authorization",
UsernamePasswordToken.basicAuthHeaderValue(
SecuritySettingsSource.TEST_USER_NAME,
new SecureString(SecuritySettingsSourceField.TEST_PASSWORD.toCharArray())
)
)
);

final Response searchResponse = getRestClient().performRequest(searchRequest);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Issue the request at REST layer so it goes through all authentication/authorization layer. Otherwise the first task is created without auth being triggered.

@ywangd ywangd added :Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. >enhancement labels Nov 18, 2021
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Nov 18, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@ywangd
Copy link
Copy Markdown
Member Author

ywangd commented Nov 18, 2021

@elasticmachine run elasticsearch-ci/part-2

Copy link
Copy Markdown
Contributor

@AthenaEryma AthenaEryma left a comment

Choose a reason for hiding this comment

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

LGTM! The concept of having separate flags for different parts of tracing is interesting, that might be a solution for something I've been trying to figure out.

import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;

public class AuthorizationTracer {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should consolidate and/or standardize these ThingTracer classes at some point. Not for this PR thought.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I agree. We could provide a common (base) bridging class between xpack module and APMTracer.

Comment on lines +117 to +122
// Populate x-opaque-id if not already exists to chain all related actions together
if (authentication != null && false == SystemUser.is(authentication.getUser())) {
if (threadContext.getHeader(Task.X_OPAQUE_ID) == null) {
threadContext.putHeader(Task.X_OPAQUE_ID, UUIDs.base64UUID());
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like capturing this, if we have it we might as well use it - if nothing else, it provides an easy way to go find the trace for a request you just made if you're running this on a live system.

@ywangd
Copy link
Copy Markdown
Member Author

ywangd commented Nov 19, 2021

Thanks for the review @gwbrown

@ywangd
Copy link
Copy Markdown
Member Author

ywangd commented Nov 19, 2021

@elasticmachine run elasticsearch-ci/bwc

@ywangd
Copy link
Copy Markdown
Member Author

ywangd commented Nov 19, 2021

@elasticmachine run elasticsearch-ci/part-2

@ywangd ywangd merged commit f3f9835 into elastic:feature/apm-integration Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. >enhancement Team:Distributed Meta label for distributed team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants