Skip to content

Commit

Permalink
Service Accounts - fix privilege for get credentials (#72368)
Browse files Browse the repository at this point in the history
The action of "get service account credentials" should be covered by the
manage_service_account privilege. But a bug makes it require read access for
the security index. This PR addresses it by executing the request with the
security origin.
  • Loading branch information
ywangd authored Apr 28, 2021
1 parent bf4d88c commit 03459b0
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 33 deletions.
2 changes: 2 additions & 0 deletions x-pack/plugin/security/qa/service-account/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ testClusters.javaRestTest {
testDistribution = 'DEFAULT'
numberOfNodes = 2

extraConfigFile 'roles.yml', file('src/javaRestTest/resources/roles.yml')
extraConfigFile 'node.key', file('src/javaRestTest/resources/ssl/node.key')
extraConfigFile 'node.crt', file('src/javaRestTest/resources/ssl/node.crt')
extraConfigFile 'ca.crt', file('src/javaRestTest/resources/ssl/ca.crt')
Expand Down Expand Up @@ -41,4 +42,5 @@ testClusters.javaRestTest {

user username: "test_admin", password: 'x-pack-test-password', role: "superuser"
user username: "elastic/fleet-server", password: 'x-pack-test-password', role: "superuser"
user username: "service_account_manager", password: 'x-pack-test-password', role: "service_account_manager"
}
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,17 @@ protected String getProtocol() {
return "https";
}

@Override
protected Settings restAdminSettings() {
final String token = basicAuthHeaderValue("test_admin", new SecureString("x-pack-test-password".toCharArray()));
return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token)
.put(CERTIFICATE_AUTHORITIES, caPath)
.build();
}

@Override
protected Settings restClientSettings() {
String token = basicAuthHeaderValue("test_admin", new SecureString("x-pack-test-password".toCharArray()));
final String token = basicAuthHeaderValue("service_account_manager", new SecureString("x-pack-test-password".toCharArray()));
return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", token)
.put(CERTIFICATE_AUTHORITIES, caPath)
.build();
Expand Down Expand Up @@ -176,7 +184,7 @@ public void testAuthenticateShouldNotFallThroughInCaseOfFailure() throws IOExcep
if (securityIndexExists) {
final Request createRoleRequest = new Request("POST", "_security/role/dummy_role");
createRoleRequest.setJsonEntity("{\"cluster\":[]}");
assertOK(client().performRequest(createRoleRequest));
assertOK(adminClient().performRequest(createRoleRequest));
}
final Request request = new Request("GET", "_security/_authenticate");
request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "Bearer " + INVALID_SERVICE_TOKEN));
Expand All @@ -193,14 +201,14 @@ public void testAuthenticateShouldNotFallThroughInCaseOfFailure() throws IOExcep
public void testAuthenticateShouldWorkWithOAuthBearerToken() throws IOException {
final Request oauthTokenRequest = new Request("POST", "_security/oauth2/token");
oauthTokenRequest.setJsonEntity("{\"grant_type\":\"password\",\"username\":\"test_admin\",\"password\":\"x-pack-test-password\"}");
final Response oauthTokenResponse = client().performRequest(oauthTokenRequest);
final Response oauthTokenResponse = adminClient().performRequest(oauthTokenRequest);
assertOK(oauthTokenResponse);
final Map<String, Object> oauthTokenResponseMap = responseAsMap(oauthTokenResponse);
final String accessToken = (String) oauthTokenResponseMap.get("access_token");

final Request request = new Request("GET", "_security/_authenticate");
request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "Bearer " + accessToken));
final Response response = client().performRequest(request);
final Response response = adminClient().performRequest(request);
assertOK(response);
final Map<String, Object> responseMap = responseAsMap(response);
assertThat(responseMap.get("username"), equalTo("test_admin"));
Expand All @@ -209,7 +217,7 @@ public void testAuthenticateShouldWorkWithOAuthBearerToken() throws IOException
final String refreshToken = (String) oauthTokenResponseMap.get("refresh_token");
final Request refreshTokenRequest = new Request("POST", "_security/oauth2/token");
refreshTokenRequest.setJsonEntity("{\"grant_type\":\"refresh_token\",\"refresh_token\":\"" + refreshToken + "\"}");
final Response refreshTokenResponse = client().performRequest(refreshTokenRequest);
final Response refreshTokenResponse = adminClient().performRequest(refreshTokenRequest);
assertOK(refreshTokenResponse);
}

Expand Down Expand Up @@ -334,7 +342,7 @@ public void testGetServiceAccountCredentials() throws IOException {
public void testClearCache() throws IOException {
final Request clearCacheRequest = new Request("POST", "_security/service/elastic/fleet-server/credential/token/"
+ randomFrom("", "*", "api-token-1", "api-token-1,api-token2") + "/_clear_cache");
final Response clearCacheResponse = client().performRequest(clearCacheRequest);
final Response clearCacheResponse = adminClient().performRequest(clearCacheRequest);
assertOK(clearCacheResponse);
final Map<String, Object> clearCacheResponseMap = responseAsMap(clearCacheResponse);
@SuppressWarnings("unchecked")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
service_account_manager:
cluster:
- "manage_service_account"
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,37 @@

import java.util.Map;

import static org.elasticsearch.test.SecuritySettingsSource.TEST_PASSWORD_HASHED;
import static org.elasticsearch.test.SecuritySettingsSource.addSSLSettingsForNodePEMFiles;
import static org.elasticsearch.test.SecuritySettingsSourceField.TEST_PASSWORD;
import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;

public class ServiceAccountSingleNodeTests extends SecuritySingleNodeTestCase {

private static final String BEARER_TOKEN = "AAEAAWVsYXN0aWMvZmxlZXQtc2VydmVyL3Rva2VuMTpyNXdkYmRib1FTZTl2R09Ld2FKR0F3";
private static final String SERVICE_ACCOUNT_MANAGER_NAME = "service_account_manager";

@Override
protected String configUsers() {
return super.configUsers()
+ SERVICE_ACCOUNT_MANAGER_NAME + ":" + TEST_PASSWORD_HASHED + "\n";
}

@Override
protected String configRoles() {
return super.configRoles()
+ SERVICE_ACCOUNT_MANAGER_NAME + ":\n"
+ " cluster:\n"
+ " - 'manage_service_account'\n";
}

@Override
protected String configUsersRoles() {
return super.configUsersRoles()
+ SERVICE_ACCOUNT_MANAGER_NAME + ":" + SERVICE_ACCOUNT_MANAGER_NAME + "\n";
}

@Override
protected Settings nodeSettings() {
Expand Down Expand Up @@ -80,25 +104,20 @@ public void testAuthenticateWithServiceFileToken() {
public void testApiServiceAccountToken() {
final IndexServiceAccountTokenStore store = node().injector().getInstance(IndexServiceAccountTokenStore.class);
final Cache<String, ListenableFuture<CachingServiceAccountTokenStore.CachedResult>> cache = store.getCache();
final CreateServiceAccountTokenRequest createServiceAccountTokenRequest =
new CreateServiceAccountTokenRequest("elastic", "fleet-server", "api-token-1");
final CreateServiceAccountTokenResponse createServiceAccountTokenResponse =
client().execute(CreateServiceAccountTokenAction.INSTANCE, createServiceAccountTokenRequest).actionGet();
assertThat(createServiceAccountTokenResponse.getName(), equalTo("api-token-1"));
final SecureString secretValue1 = createApiServiceToken("api-token-1");
assertThat(cache.count(), equalTo(0));

final AuthenticateRequest authenticateRequest = new AuthenticateRequest("elastic/fleet-server");
final AuthenticateResponse authenticateResponse =
createServiceAccountClient(createServiceAccountTokenResponse.getValue().toString())
.execute(AuthenticateAction.INSTANCE, authenticateRequest).actionGet();
final AuthenticateResponse authenticateResponse = createServiceAccountClient(secretValue1.toString())
.execute(AuthenticateAction.INSTANCE, authenticateRequest).actionGet();
assertThat(authenticateResponse.authentication(), equalTo(getExpectedAuthentication("api-token-1")));
// cache is populated after authenticate
assertThat(cache.count(), equalTo(1));

final DeleteServiceAccountTokenRequest deleteServiceAccountTokenRequest =
new DeleteServiceAccountTokenRequest("elastic", "fleet-server", "api-token-1");
final DeleteServiceAccountTokenResponse deleteServiceAccountTokenResponse =
client().execute(DeleteServiceAccountTokenAction.INSTANCE, deleteServiceAccountTokenRequest).actionGet();
final DeleteServiceAccountTokenResponse deleteServiceAccountTokenResponse = createServiceAccountManagerClient()
.execute(DeleteServiceAccountTokenAction.INSTANCE, deleteServiceAccountTokenRequest).actionGet();
assertThat(deleteServiceAccountTokenResponse.found(), is(true));
// cache is cleared after token deletion
assertThat(cache.count(), equalTo(0));
Expand Down Expand Up @@ -138,6 +157,11 @@ public void testClearCache() {
assertThat(cache.count(), equalTo(1));
}

private Client createServiceAccountManagerClient() {
return client().filterWithHeader(Map.of("Authorization",
basicAuthHeaderValue(SERVICE_ACCOUNT_MANAGER_NAME, new SecureString(TEST_PASSWORD.toCharArray()))));
}

private Client createServiceAccountClient() {
return createServiceAccountClient(BEARER_TOKEN);
}
Expand All @@ -160,7 +184,8 @@ private SecureString createApiServiceToken(String tokenName) {
final CreateServiceAccountTokenRequest createServiceAccountTokenRequest =
new CreateServiceAccountTokenRequest("elastic", "fleet-server", tokenName);
final CreateServiceAccountTokenResponse createServiceAccountTokenResponse =
client().execute(CreateServiceAccountTokenAction.INSTANCE, createServiceAccountTokenRequest).actionGet();
createServiceAccountManagerClient().execute(
CreateServiceAccountTokenAction.INSTANCE, createServiceAccountTokenRequest).actionGet();
assertThat(createServiceAccountTokenResponse.getName(), equalTo(tokenName));
return createServiceAccountTokenResponse.getValue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.elasticsearch.common.CharArrays;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.query.BoolQueryBuilder;
Expand All @@ -57,6 +58,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.function.Supplier;

import static org.elasticsearch.action.bulk.TransportSingleItemBulkWriteAction.toSingleItemBulkRequest;
import static org.elasticsearch.search.SearchService.DEFAULT_KEEPALIVE_SETTING;
Expand Down Expand Up @@ -145,22 +147,28 @@ public void findTokensFor(ServiceAccountId accountId, ActionListener<Collection<
} else if (false == frozenSecurityIndex.isAvailable()) {
listener.onFailure(frozenSecurityIndex.getUnavailableReason());
} else {
// TODO: wildcard support?
final BoolQueryBuilder query = QueryBuilders.boolQuery()
.filter(QueryBuilders.termQuery("doc_type", SERVICE_ACCOUNT_TOKEN_DOC_TYPE))
.must(QueryBuilders.termQuery("username", accountId.asPrincipal()));
final SearchRequest request = client.prepareSearch(SECURITY_MAIN_ALIAS)
.setScroll(DEFAULT_KEEPALIVE_SETTING.get(getSettings()))
.setQuery(query)
.setSize(1000)
.setFetchSource(false)
.request();
request.indicesOptions().ignoreUnavailable();

logger.trace("Searching tokens for service account [{}]", accountId);
ScrollHelper.fetchAllByEntity(client, request,
new ContextPreservingActionListener<>(client.threadPool().getThreadContext().newRestorableContext(false), listener),
hit -> extractTokenInfo(hit.getId(), accountId));
securityIndex.checkIndexVersionThenExecute(listener::onFailure, () -> {
final Supplier<ThreadContext.StoredContext> contextSupplier =
client.threadPool().getThreadContext().newRestorableContext(false);
try (ThreadContext.StoredContext ignore = client.threadPool().getThreadContext().stashWithOrigin(SECURITY_ORIGIN)) {
// TODO: wildcard support?
final BoolQueryBuilder query = QueryBuilders.boolQuery()
.filter(QueryBuilders.termQuery("doc_type", SERVICE_ACCOUNT_TOKEN_DOC_TYPE))
.must(QueryBuilders.termQuery("username", accountId.asPrincipal()));
final SearchRequest request = client.prepareSearch(SECURITY_MAIN_ALIAS)
.setScroll(DEFAULT_KEEPALIVE_SETTING.get(getSettings()))
.setQuery(query)
.setSize(1000)
.setFetchSource(false)
.request();
request.indicesOptions().ignoreUnavailable();

logger.trace("Searching tokens for service account [{}]", accountId);
ScrollHelper.fetchAllByEntity(client, request,
new ContextPreservingActionListener<>(contextSupplier, listener),
hit -> extractTokenInfo(hit.getId(), accountId));
}
});
}
}

Expand Down

0 comments on commit 03459b0

Please sign in to comment.