-
Notifications
You must be signed in to change notification settings - Fork 25.3k
[TEST] Remove token methods from HLRC SecurityClient #85515
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
Conversation
Pinging @elastic/es-security (Team:Security) |
Pinging @elastic/clients-team (Team:Clients) |
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
// This API returns 404 (with the same body as a 200 response) if there's nothing to delete. | ||
// RestClient will throw an exception on 404, but we don't want that, we want to parse the body and return it | ||
request.addParameter("ignore", "404"); |
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.
👍
...src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java
Outdated
Show resolved
Hide resolved
public record OAuth2Token(String accessToken, Optional<String> refreshToken, String principal) { | ||
|
||
@Nullable | ||
public String getRefreshToken() { | ||
return refreshToken.orElse(null); | ||
} | ||
} | ||
|
||
public record TokenInvalidation(int invalidated, int previouslyInvalidated, List<ElasticsearchException> errors) {} |
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 suspect we eventually want to turn these into production code. But it can wait.
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.
We have TokensInvalidationResult
but it contains a list of token ids, not a count of them.
I'm not sure whether we need a production record for the rest response, but if we do we'll move it where it's needed.
...src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java
Outdated
Show resolved
Hide resolved
...src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java
Outdated
Show resolved
Hide resolved
...src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java
Outdated
Show resolved
Hide resolved
...src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java
Outdated
Show resolved
Hide resolved
...src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java
Outdated
Show resolved
Hide resolved
final PlainActionFuture<org.elasticsearch.xpack.core.security.action.token.CreateTokenResponse> future1 = new PlainActionFuture<>(); | ||
final PlainActionFuture<CreateTokenResponse> future1 = new PlainActionFuture<>(); | ||
runAsClient.execute(CreateTokenAction.INSTANCE, createTokenRequest1, future1); |
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.
This method should be rewritten to use the new convenient methods from TestSecurityClient. I can follow up with a separate PR (since I wrote the original version).
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.
PR is #85819
@elasticmachine update branch |
|
This is a follow up PR for elastic#85515 to rewrite one more test to leverage the new TestSecurityClient and its convenient token methods. Relates: elastic#85515
* upstream/master: (40 commits) Fix BuildTests serialization (elastic#85827) Use urgent priority for node shutdown cluster state update (elastic#85838) Remove Task classes from HLRC (elastic#85835) Remove unused migration classes (elastic#85834) Remove uses of Charset name parsing (elastic#85795) Remove legacy versioned logic for DefaultSystemMemoryInfo (elastic#85761) Expose proxy settings for GCS repositories (elastic#85785) Remove SLM classes from HLRC (elastic#85825) TSDB: fix the time_series in order collect priority (elastic#85526) Remove ILM classes from HLRC (elastic#85822) FastVectorHighlighter should use ValueFetchers to load source data (elastic#85815) Iteratively execute synchronous ingest processors (elastic#84250) Remove TransformClient from HLRC (elastic#85787) Mute XPackRestIT deprecation/10_basic/Test Deprecations (elastic#85807) Unmute Lintian packaging test (elastic#85778) Add a highlighter unit test base class (elastic#85719) Remove NIO Transport Plugin (elastic#82085) [TEST] Remove token methods from HLRC SecurityClient (elastic#85515) [Test] Use thread-safe hashSet for result collection (elastic#85653) [TEST] Mute BuildTests.testSerialization (elastic#85801) ... # Conflicts: # server/src/test/java/org/elasticsearch/search/aggregations/timeseries/TimeSeriesIndexSearcherTests.java
The High Level Rest Client (HLRC) is deprecated and is no longer
shipped as a standalone artifact.
It can now be safely removed from the ES codebase.
This commit removes all token methods from the HLRC
SecurityClient, along with their associated Request/Response
classes.
Test code that depended on these methods has been migrated
to use
TestSecurityClient
instead.Relates: #83423