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

Cookie based authentication for websocket connections #231

Closed
phanimahesh opened this issue Aug 7, 2015 · 16 comments
Closed

Cookie based authentication for websocket connections #231

phanimahesh opened this issue Aug 7, 2015 · 16 comments
Assignees
Milestone

Comments

@phanimahesh
Copy link
Contributor

Is it possible to authenticate websocket connections from the cookie in initial HTTP Upgrade request? I haven't found any simple way to do it.

@phanimahesh phanimahesh changed the title Websocket authentication Cookie based authentication for websocket connections Aug 7, 2015
@emqplus
Copy link
Contributor

emqplus commented Aug 7, 2015

@phanimahesh, this is a good idea:) Do your write Erlang? Please read emqttd_ws_client.erl that the module could get cookie from HTTP Upgrade request

@phanimahesh
Copy link
Contributor Author

I am working in elixir but can read erlang. I have read emqttd_ws_client.erl and can patch it to extract the cookies. However, I am not entirely sure of how to pass this on to the auth plugins.

Should the mqtt_client record be updated? What would be the cleanest way to do this?

@phanimahesh
Copy link
Contributor Author

@Erylee I need some suggestions:

  • Should a new user defined function be allowed to extract username and password using any arbitrary roles? Or
  • Should new configuration values be added to allow mapping cookie keys to username and password?

What about whitelisting allowed origins for websocket connections, either by using a CORS header or otherwise?

@emqplus
Copy link
Contributor

emqplus commented Aug 10, 2015

@phanimahesh, If I add a 'cookie' field to 'mqtt_user' record, will it work for your project?

@phanimahesh
Copy link
Contributor Author

Do you mean mqtt_client?

If the cookie is available in the auth plugin hook in any way, it works for our project.

If you are planning on modifying any of the records, it may be a better idea to pass on all headers as a map or proplist. If someone uses custom headers or let's say http basic auth, this will cover all such scenarios.

We need this asap, and are planning to do it ourselves tomorrow. We can send a pull request for this tomorrow if it is okay with you.

emqplus pushed a commit that referenced this issue Aug 10, 2015
@emqplus
Copy link
Contributor

emqplus commented Aug 10, 2015

@phanimahesh, I create a branch for this issue:) and submit a quick fix. Please check it now.

Branch: https://github.com/emqtt/emqttd/tree/issue%23231

Commit: bb02ced

@phanimahesh
Copy link
Contributor Author

Looks great!

However, I think including a full list of headers is a good idea. What if someone later wants to authorize using bearer tokens or basic auth?

I was thinking along the lines of

Headers = mochiweb_request:get(headers,Req),
Header_list = mochiweb_headers:to_list(Headers),
ProtoState = emqttd_protocol:init(Peername, SendFun, [{ws_initial_headers, Header_list}|PktOpts]),

in mqtt_ws_client.erl and corresponding changes elsewhere. What do you think?

@phanimahesh
Copy link
Contributor Author

@Erylee: Can you have a look at #240 ?

@emqplus
Copy link
Contributor

emqplus commented Aug 13, 2015

@phanimahesh, It's awesome:) I merge the pull request to 0.10.0 release. Thanks.

@phanimahesh
Copy link
Contributor Author

Thanks! :)

@emqplus emqplus added this to the 0.10.0 milestone Aug 13, 2015
@emqplus emqplus self-assigned this Aug 13, 2015
@emqplus emqplus closed this as completed Aug 13, 2015
@phanimahesh
Copy link
Contributor Author

@emqplus This is no longer possible with emqx3.0. Was it removed intentionally or accidentally? Will you accept a PR adding it back?

@gilbertwong96
Copy link
Contributor

gilbertwong96 commented Nov 5, 2018

@phanimahesh Sorry, because we have migrated webserver from mochiweb to cowboy, so it was removed accidentally. Welcome for your PR !

@gilbertwong96 gilbertwong96 reopened this Nov 5, 2018
@gilbertwong96 gilbertwong96 modified the milestones: 0.10.0, 3.1-beta.1 Feb 11, 2019
@gilbertwong96 gilbertwong96 modified the milestones: 3.1-beta.1, 3.1-beta.2 Mar 7, 2019
@gilbertwong96 gilbertwong96 modified the milestones: 3.1-beta.2, 3.1-beta.3 Mar 18, 2019
@gilbertwong96 gilbertwong96 self-assigned this Apr 8, 2019
@gilbertwong96 gilbertwong96 removed this from the 3.1-rc.1 milestone Apr 8, 2019
@gilbertwong96 gilbertwong96 added this to the 3.1-rc.2 milestone Apr 8, 2019
@gilbertwong96
Copy link
Contributor

This feature has been implemented in emqx 3.1-rc.2.

@dufourc
Copy link

dufourc commented Sep 17, 2020

Hey I read @phanimahesh PR and from what I understood, the headers has been added into the ClientInfo therefore the cookies are in the ClientInfo as well. But where is it used concretely? How can I use it with the different auth plugins?

If I read the emqx-auth-http docs, I can't see the headers, neither cookies as params. I went further and I tried to read the erlang code with my really basic understanding of it. I think it the params could be added here https://github.com/emqx/emqx-auth-http/blob/master/src/emqx_auth_http_cli.erl#L78 adding a line in the map to pass the sessionId (the name could be specified in the config). Or maybe forward the cookies?

I saw as well a feature request here: #2676. @gilbertwong96 What where you thinking to do for emqx_auth_cookie? Would it check if the session_id is present in a certain table (SQL request) or make an HTTP call?

In this page(https://docs.emqx.io/broker/latest/en/introduction/checklist.html) I can see 'Browser cookie authentication'.

I'm sorry but I'm a bit confused if this is possible to achieve or not and how.

I'm happy to contribute but I might need some guidance to decide where and how to implement it.

@HJianBo
Copy link
Member

HJianBo commented Sep 28, 2020

Hi @dufourc These discussions may be out of date. I have transfer this comment to a New issue : #3747

@dufourc
Copy link

dufourc commented Sep 28, 2020

Hey @HJianBo thanks! I think, you're right, it might be better to create another issue
When I wrote the comment I was really confused. After it, I realised that this issue here was to have access to the cookies/headers from the ClientInfo to be able to use it in a plugin. And that @phanimahesh has probably developed his own plugin to use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants