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

Use username as clientId #1961

Merged
merged 4 commits into from Nov 21, 2018

Conversation

@tigercl
Copy link
Collaborator

tigercl commented Nov 16, 2018

@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 16, 2018

Pull Request Test Coverage Report for Build 3827

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 60.966%

Totals Coverage Status
Change from base Build 3818: 0.04%
Covered Lines: 2727
Relevant Lines: 4473

💛 - Coveralls
Copy link
Contributor

spring2maz left a comment

Questings after reading mosquitto config document

Set use_username_as_clientid to true to replace the clientid that a client connected with with its username. This allows authentication to be tied to the clientid, which means that it is possible to prevent one client disconnecting another by using the same clientid. Defaults to false.

If it is only to prevent one client disconnecting another, can we append username to client id?
This should allow one username to be used in different clients.
I'm not quite familiar with the use case, but presumably,
it is not unusual for group subscriber members to share credentials.

If a client connects with no username it will be disconnected as not authorised when this option is set to true. Do not use in conjunction with clientid_prefixes.

This doesn't seem to be implemented in this PR.
I'm guessing this is to avoid a malicious (or buggy) client being interfere others by using other's username as client.
I'm not sure how much an anonymous client could do (or cause broker to do) though.

test/emqx_protocol_SUITE.erl Outdated Show resolved Hide resolved
priv/emqx.schema Outdated Show resolved Hide resolved
@spring2maz

This comment has been minimized.

Copy link
Contributor

spring2maz commented Nov 18, 2018

client id has a limit of 23 bytes maximum, does username has such limit ?

tigercl added 2 commits Nov 19, 2018
@tigercl

This comment has been minimized.

Copy link
Collaborator Author

tigercl commented Nov 20, 2018

client id has a limit of 23 bytes maximum, does username has such limit ?

We will check length of the client id after using username replace client id, and the server allow ClientID's that contain more than 23 encoded bytes actually, we limit it by 'max_clientid_len' in emqx.conf now.

@turtleDeng turtleDeng merged commit 14b8036 into emqx30 Nov 21, 2018
3 checks passed
3 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.04%) to 60.966%
Details
@terry-xiaoyu terry-xiaoyu changed the title Use username replace id(issue#1737) Use username as clientId Nov 22, 2018
@tigercl

This comment has been minimized.

Copy link
Collaborator Author

tigercl commented Nov 22, 2018

Questings after reading mosquitto config document
@spring2maz Sorry for not replying in time, the plan you said is indeed more reasonable. I will discuss with @turtleDeng and decide how to achieve it later.

tigercl added a commit that referenced this pull request Nov 24, 2018
* Use username replace id(issue#1737)

* Add test case for issue#1737

* Make with_connection/2 support batch connect
@turtleDeng turtleDeng deleted the use_username_as_clientid branch Nov 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.