Skip to content

Conversation

@iGxnon
Copy link
Contributor

@iGxnon iGxnon commented Dec 29, 2023

Base on #5

Please briefly answer these questions:

  • what problem are you trying to solve? (or if there's no problem, what's the motivation for this change?)
    Add auth client
  • what changes does this pull request make?
    Add auth client
  • are there any non-obvious implications of these changes? (does it break compatibility with previous versions, etc)
    No

Comment on lines +39 to +44
public AuthenticateResponse authenticate(AuthenticateRequest req) throws InvalidProtocolBufferException, CommandExecutionException {
RequestWithToken reqToken = RequestWithToken.newBuilder().setToken(this.token).setAuthenticateRequest(req).build();
Command cmd = Command.newBuilder().setRequest(reqToken).build();
CommandResponse resp = this.curpClient.propose(cmd, false);
return resp.getAuthenticateResponse();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The authenticate operation does need to set token.

public AuthUserGetResponse userGet(AuthUserGetRequest req) throws InvalidProtocolBufferException, CommandExecutionException {
RequestWithToken reqToken = RequestWithToken.newBuilder().setToken(this.token).setAuthUserGetRequest(req).build();
Command cmd = Command.newBuilder().setRequest(reqToken).build();
CommandResponse resp = this.curpClient.propose(cmd, false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

userGet and roleGet could use fast path.

return resp.getAuthenticateResponse();
}

public AuthUserAddResponse userAdd(AuthUserAddRequest req) throws InvalidProtocolBufferException, CommandExecutionException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please hash the password in userAdd and userChangePassword

@iGxnon
Copy link
Contributor Author

iGxnon commented Feb 7, 2024

Close because unused, see #7

@iGxnon iGxnon closed this Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants