Skip to content
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

feat: Enhance RemoteServer and Console create-user to assign group #1405

Merged
merged 6 commits into from
Dec 28, 2023

Conversation

gramian
Copy link
Collaborator

@gramian gramian commented Dec 27, 2023

What does this PR do?

  1. The RemoteServe class method createUser is overloaded with a variant with the signature
public void createUser(final String userName, final String password, final HashMap<String,String> databases)

As for the HTTP API server command the databases HashMap should have keys being database names and values being a group name.
The former function with a List<String> databases argument is still available, but its body changed so that the new method is called by it.

  1. The Console class method executeCreateUser is updated so the new RemoteServer createUser method is used. However not too robustly, and error handling is defered to the RemoteServer or eventually to the ServerSecurityUser class.

So now in console the following can be done:

CREATE USER user IDENTIFIED BY password GRANT CONNECT TO db:group

Motivation

Missing capability to assign group during user creation in console, even though possibl via HTTP API.

Additional Notes

It is possible to pass an empty group, but AFAIK this is also allowed in the HTTP server command.

Checklist

  • I have run the build using mvn clean package command
  • My unit tests cover both failure and success scenarios

@@ -467,16 +467,25 @@ private void executeCreateUser(final String params) {
checkHasSpaces("User name", userName);

final String password;
final List<String> databases;
HashMap<String,String> databases = new HashMap<String,String>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

my suggestion:
final Map<String,String>...

@@ -74,7 +74,7 @@ public String toString() {
return protocol + "://" + currentServer + ":" + currentPort;
}

public void createUser(final String userName, final String password, final List<String> databases) {
public void createUser(final String userName, final String password, final HashMap<String,String> databases) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

again, prefer interface:
final Map<String,String

@@ -101,6 +101,16 @@ public void createUser(final String userName, final String password, final List<
}
}

public void createUser(final String userName, final String password, final List<String> databases) {

HashMap<String,String> databasesWithGroups = new HashMap<String, String>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

again:
Map<String,String> databasesWithGroups = new HashMap<String, String>();

@lvca lvca added the enhancement New feature or request label Dec 28, 2023
@lvca lvca added this to the 23.12.1 milestone Dec 28, 2023
@lvca
Copy link
Contributor

lvca commented Dec 28, 2023

Good for me ;-)

Copy link
Collaborator

@robfrank robfrank left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify mergify bot merged commit 6313d89 into ArcadeData:main Dec 28, 2023
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants