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

Wrong field name in login() method #47

Open
martinjuiz opened this issue Dec 17, 2019 · 10 comments
Open

Wrong field name in login() method #47

martinjuiz opened this issue Dec 17, 2019 · 10 comments

Comments

@martinjuiz
Copy link

The method RocketChatClientCallBuilder.login in master branch is using username field to set the username value but the API is expecting user instead.
By using username the response is always 401 Unauthorized (other endpoints are already accepting username so my understanding is this field name should be standardised).

@waiwaic waiwaic mentioned this issue Dec 17, 2019
@col-panic
Copy link
Collaborator

Could you please provide a sample case / test-case showing a scenario where the current login fails and subsequently works (using your modification)? I do currently use the tool as is for login, but it works with RocketChat 2.3!

@markusstoll
Copy link

It does not work with RocketChat 3.3
I could verify by changing paramer username to user to make it work

@col-panic
Copy link
Collaborator

I just tested against 3.3 and it seems to work. What is your usage scenario? RocketChatClientCallBuilder.login is a private method you should not be required to directly use it?

@markusstoll
Copy link

I tried with rocket-chat-rest-client obtained via Maven and failed against Rocket.Chat V3.3.0 (using new RocketChatClient(<params>))

To make it work, I checked out the project and made the named change and again used new RocketChatClient(<params>).

I guess you agree, that the Rocket.Chat API https://docs.rocket.chat/api/rest-api/methods/authentication/login does require the parameter user rather than username?

@col-panic
Copy link
Collaborator

so you tried the maven version, not the current master in this repository? The version distributed via maven is quite old.

It does not matter whether we agree on the documentation - first a test must fail - to this end I updated the build to test against 3.3.3 which does build correctly.

Please test your code using the current master as served in this repo, you could also share a snipped of what is not working for you, to include as additional test.

@markusstoll
Copy link

markusstoll commented Jun 16, 2020

Well, I did not write clear enough. After failing with maven version, I cloned current master and debugged and verified the failure. Then I changed the parameter name to user and got things working.

my test code is a 2 liner:

rc = new RocketChatClient(<params>)
rc.getGroupsApi()

@col-panic
Copy link
Collaborator

I just tested this current code against my running productive 3.3.3 installation with

RocketChatClient rc = new RocketChatClient("https://rocketchat.mydomain.com", "user", "password");
Group[] list = rc.getGroupsApi().list();
for (Group group : list) {
	System.out.println(group);
}

and it works without a problem.

@markusstoll
Copy link

So the point might very well be on the other end. My Rocket.Chat installation is a new installation of Version 33.0 from Scratch using the official docker image.
I am going to update it to 3.3.3 for verification

@markusstoll
Copy link

After update to Rocket.Chat V3.3.3 I still observer the same.

I reduced the client part to a setup in PostMan, still the same. Using username does not work, using user does.

@Falco20019
Copy link

Falco20019 commented Nov 10, 2020

Both username and user work. I had the same problem, but it was due to using LDAP. Try using the email address or the display name. These work. Sadly not the LDAP username... That's a problem of Rocket.Chat and not of this library.

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

No branches or pull requests

4 participants