-
Notifications
You must be signed in to change notification settings - Fork 25.3k
HLRC: Add delete user action #35294
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
HLRC: Add delete user action #35294
Changes from all commits
396e65c
876f4a0
ae9059f
2d4aa98
321935a
e4a0904
058f6c4
86786e2
ff6f055
041aeb6
ee6d4c6
a2f2d52
7465462
b83312d
e89ea8b
7e90b83
f3e76a5
0351a26
5f10520
1f7fb31
c697fc0
a55981a
4368f2e
d4d66af
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 |
---|---|---|
|
@@ -35,6 +35,8 @@ | |
import org.elasticsearch.client.security.DeleteRoleMappingResponse; | ||
import org.elasticsearch.client.security.DeleteRoleRequest; | ||
import org.elasticsearch.client.security.DeleteRoleResponse; | ||
import org.elasticsearch.client.security.DeleteUserRequest; | ||
import org.elasticsearch.client.security.DeleteUserResponse; | ||
import org.elasticsearch.client.security.DisableUserRequest; | ||
import org.elasticsearch.client.security.EmptyResponse; | ||
import org.elasticsearch.client.security.EnableUserRequest; | ||
|
@@ -102,6 +104,33 @@ public void putUserAsync(PutUserRequest request, RequestOptions options, ActionL | |
PutUserResponse::fromXContent, listener, emptySet()); | ||
} | ||
|
||
/** | ||
* Removes user from the native realm synchronously. | ||
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-delete-user.html"> | ||
* the docs</a> for more. | ||
* @param request the request with the user to delete | ||
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized | ||
* @return the response from the delete user call | ||
* @throws IOException in case there is a problem sending the request or parsing back the response | ||
*/ | ||
public DeleteUserResponse deleteUser(DeleteUserRequest request, RequestOptions options) throws IOException { | ||
return restHighLevelClient.performRequestAndParseEntity(request, SecurityRequestConverters::deleteUser, options, | ||
DeleteUserResponse::fromXContent, singleton(404)); | ||
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. Hey I saw some updates on this PR and I just wanted to throw a reminder out that we are not going to do a singleton(404) here, because we want a delete that is not found to throw an exception. Also, last time we spoke you were going to change this to AcknowledgedResponse. <3 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 have tried those approaches but they seem not to work. If I use
I think I need to override the class to be able to parse the response. Then I tried not to catch the exception and then I got the following error:
Not sure if I can implement it in any other way without changing the underlaying implementation. 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. It might also depend on which implementation of 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.
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. heh, duh... Sorry, ive been on vacation and full of turkey since last week. 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. so the only thing here now is to remove 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. No problem, happy you enjoyed your time off! Yes, the issue we have is the
Not sure if we can avoid without changing the current implementation. 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. Apologies, I misread the comment before. Since server side is not throwing exceptions, lets keep the |
||
} | ||
|
||
/** | ||
* Asynchronously deletes a user in the native realm. | ||
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-delete-user.html"> | ||
* the docs</a> for more. | ||
* @param request the request with the user to delete | ||
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized | ||
* @param listener the listener to be notified upon request completion | ||
*/ | ||
public void deleteUserAsync(DeleteUserRequest request, RequestOptions options, ActionListener<DeleteUserResponse> listener) { | ||
restHighLevelClient.performRequestAsyncAndParseEntity(request, SecurityRequestConverters::deleteUser, options, | ||
DeleteUserResponse::fromXContent, listener, singleton(404)); | ||
} | ||
|
||
/** | ||
* Create/Update a role mapping. | ||
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-put-role-mapping.html"> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
/* | ||
* Licensed to Elasticsearch under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
package org.elasticsearch.client.security; | ||
|
||
import org.elasticsearch.client.Validatable; | ||
|
||
import java.util.Objects; | ||
|
||
/** | ||
* A request to delete a user from the native realm. | ||
*/ | ||
public final class DeleteUserRequest implements Validatable { | ||
|
||
private final String name; | ||
private final RefreshPolicy refreshPolicy; | ||
|
||
public DeleteUserRequest(String name) { | ||
this(name, RefreshPolicy.IMMEDIATE); | ||
} | ||
|
||
public DeleteUserRequest(String name, RefreshPolicy refreshPolicy) { | ||
this.name = Objects.requireNonNull(name, "user name is required"); | ||
this.refreshPolicy = Objects.requireNonNull(refreshPolicy, "refresh policy is required"); | ||
} | ||
|
||
public String getName() { | ||
return name; | ||
} | ||
|
||
public RefreshPolicy getRefreshPolicy() { | ||
return refreshPolicy; | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(name, refreshPolicy); | ||
} | ||
|
||
@Override | ||
public boolean equals(Object obj) { | ||
if (this == obj) { | ||
return true; | ||
} | ||
if (obj == null) { | ||
return false; | ||
} | ||
if (getClass() != obj.getClass()) { | ||
return false; | ||
} | ||
final DeleteUserRequest other = (DeleteUserRequest) obj; | ||
|
||
return (refreshPolicy == other.refreshPolicy) && Objects.equals(name, other.name); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/* | ||
* Licensed to Elasticsearch under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
package org.elasticsearch.client.security; | ||
|
||
import org.elasticsearch.client.core.AcknowledgedResponse; | ||
import org.elasticsearch.common.xcontent.ConstructingObjectParser; | ||
import org.elasticsearch.common.xcontent.XContentParser; | ||
|
||
import java.io.IOException; | ||
|
||
/** | ||
* Response for a user being deleted from the native realm | ||
*/ | ||
public final class DeleteUserResponse extends AcknowledgedResponse { | ||
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 think we can gut this altogether since optional has a "isPresent()" field. See #35470 and leave your thoughts there or here. 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. Im still on the fence for whether we even need a found true/false in the code since we return an optional. WDYT? |
||
|
||
private static final String PARSE_FIELD_NAME = "found"; | ||
|
||
private static final ConstructingObjectParser<DeleteUserResponse, Void> PARSER = AcknowledgedResponse | ||
.generateParser("delete_user_response", DeleteUserResponse::new, PARSE_FIELD_NAME); | ||
|
||
public DeleteUserResponse(boolean acknowledged) { | ||
super(acknowledged); | ||
} | ||
|
||
public static DeleteUserResponse fromXContent(final XContentParser parser) throws IOException { | ||
return PARSER.parse(parser, null); | ||
} | ||
|
||
@Override | ||
protected String getFieldName() { | ||
return PARSE_FIELD_NAME; | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.