-
Notifications
You must be signed in to change notification settings - Fork 283
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
[Extensions] Add a internal user provider interface #2713
Changes from all commits
483b9e2
6d58e08
42f460f
b6ab47a
e1422fb
882b652
8297b84
2ddfc06
3af1e8d
d689d23
5d4cc8f
13f3819
34b4e4b
10bd3bb
0f7f62f
0327f3b
2263129
9d2ec1d
b1ffa6d
9b59a76
6b52eeb
cd36a68
cd45201
7ab1862
518ef90
c8df4fc
4605b55
eb826d8
abecea7
1c3ac14
f9020b0
a1ab910
114f05d
7c4a464
5983d98
9639f87
cd5c4e3
80bb066
6d8cb97
5e2410f
1cec4e4
4ede132
cd5f3b9
1577af5
b47a802
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 |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package org.opensearch.security.user; | ||
|
||
import com.fasterxml.jackson.databind.node.ObjectNode; | ||
import java.io.IOException; | ||
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; | ||
|
||
/** | ||
* This interface will be defined in core following these changes. For now this is used for testing in the Security Plugin. | ||
* | ||
*/ | ||
public interface InternalUserProvider { | ||
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. What do you think about making this a 'ServiceAccountManager'? Then it would have only the use cases needed in OpenSearch core, during extensions registration for find or create with enable / disable? |
||
|
||
public void putInternalUser(String userInfo) throws java.io.IOException; | ||
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'd recommend against throwing IOException, can we try /catch where this is coming from or if you think its needed throw specific exceptions? |
||
|
||
public SecurityDynamicConfiguration<?> getInternalUser(String userInfo); | ||
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.
|
||
|
||
public void removeInternalUser(String userInfo); | ||
|
||
public String getInternalUserAuthToken(String userInfo) throws IOException; | ||
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 don't think this return type should be a string in OpenSearch, it should be some kind of interface around the auth token. @cwperks Did you have ideas about how we would pass refresh token in the java ecosystem? |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
package org.opensearch.security.user; | ||
|
||
import com.fasterxml.jackson.databind.JsonNode; | ||
import com.fasterxml.jackson.databind.node.ObjectNode; | ||
import java.io.IOException; | ||
import org.opensearch.common.inject.Inject; | ||
import org.opensearch.security.DefaultObjectMapper; | ||
import org.opensearch.security.securityconf.impl.CType; | ||
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; | ||
|
||
public class SecurityInternalUserProvider implements InternalUserProvider{ | ||
|
||
UserService userService; | ||
|
||
@Inject | ||
public SecurityInternalUserProvider(UserService userService) { | ||
this.userService = userService; | ||
} | ||
|
||
@Override | ||
public void putInternalUser(String userInfo) throws IOException { | ||
|
||
JsonNode content = DefaultObjectMapper.readTree(userInfo); | ||
final ObjectNode contentAsNode = (ObjectNode) content; | ||
|
||
SecurityDynamicConfiguration<?> internalUsersConfiguration = userService.createOrUpdateAccount(contentAsNode); | ||
userService.saveAndUpdateConfigs(userService.getUserConfigName().toString(), userService.client, CType.INTERNALUSERS, internalUsersConfiguration); | ||
} | ||
|
||
@Override | ||
public SecurityDynamicConfiguration<?> getInternalUser(String username) { | ||
|
||
final SecurityDynamicConfiguration<?> internalUsersConfiguration = userService.load(userService.getUserConfigName(), true); | ||
|
||
// no specific resource requested, return complete config | ||
if (username == null || username.length() == 0) { | ||
return internalUsersConfiguration; | ||
} | ||
|
||
final boolean userExisted = internalUsersConfiguration.exists(username); | ||
|
||
if (!userExisted) { | ||
throw new UserServiceException("Failed to retrieve requested internal user."); | ||
} | ||
|
||
internalUsersConfiguration.removeOthers(username); | ||
return internalUsersConfiguration; | ||
} | ||
|
||
@Override | ||
public void removeInternalUser(String username) { | ||
final SecurityDynamicConfiguration<?> internalUsersConfiguration = userService.load(userService.getUserConfigName(), true); | ||
internalUsersConfiguration.remove(username); | ||
} | ||
|
||
@Override | ||
public String getInternalUserAuthToken(String username) throws IOException { | ||
return userService.generateAuthToken(username); | ||
} | ||
} |
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.
Do you think we should create a separate handler for this API? It may get confusing that the same handler is used for multiple endpoints.
Can you also add more description to the PR about the API being introduced?
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.
The
InternalUsersApiAction
already includes several endpoints so I thought it was appropriate to add this here. It is just a special instance of theGET
functionality.I will add some extra documentation.