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
fix: mongodb authz crash #10098
fix: mongodb authz crash #10098
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️
Just changelogs are missing?
073f95e
to
9a1fff4
Compare
Fixed |
Pull Request Test Coverage Report for Build 4375831073
💛 - Coveralls |
%% For some reason the collection name is turned into an atom by the emqx_authz | ||
%% application which causes the mongo query to break. TODO: This should probably | ||
%% be fixed at another level. | ||
make_collection_bin(Atom) when is_atom(Atom) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the field is of type binary()
if it ended up as an atom, we should investigate (because it could be a potential atom leak here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will try to investigate this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last night I thought we were in a hurry to get this into 5.1 but now when I know that we have several weeks left until that release I will spend some more time and try to fix the problem at the root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed by changing the authz schema (see new commit below). I don't understand why the authz schema don't piggyback on the mongodb connector schema but maybe there is a good reason for that?
changes/v5.0.20-en.md
Outdated
|
||
## Bug Fixes | ||
|
||
- [#10098](https://github.com/emqx/emqx/pull/10098) When configuring mongodb authorization the mongodb connector crashed with an error in the log file. The reason was that the collection name had the wrong type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- [#10098](https://github.com/emqx/emqx/pull/10098) When configuring mongodb authorization the mongodb connector crashed with an error in the log file. The reason was that the collection name had the wrong type. | |
- Fix mongodb authorization crash when `collection` is configured. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The crash does not happen when the collection is configured. It happens when the authorization system tries to query the mongodb database.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the changelog text. Please check the Chinese version that is auto translated.
This fixes a crash with an error in the log file (see below) that happened when the MongoDB authorization module queried the database. The reason is that the collection name that was sent to the mongodb connection was an atom. This is fixed by making sure it is not an atom. 2023-03-08T17:16:34.215523+01:00 [error] msg: query_mongo_error, mfa: emqx_authz_mongodb:authorize/4, line: 95, peername: 127.0.0.1:53212, clientid: client123, collection: mqtt_acl, filter: #{username => <<"emqx_u">>}, reason: {resource_error,#{msg => #{error => {error,{error_cannot_parse_response,{op_msg_response,#{<<"code">> => 73,<<"codeName">> => <<"InvalidNamespace">>,<<"errmsg">> => <<"Failed to parse namespace element">>,<<"ok">> => 0.0}}}},id => <<"emqx_authz_mongodb:3">>,name => call_query,request => {find,mqtt_acl,#{username => <<"emqx_u">>},#{}},stacktrace => [{mc_connection_man,reply,1,[{file,"mc_connection_man.erl"},{line,123}], ...]}, reason => exception}}, resource_id: <<"emqx_authz_mongodb:3">> Fixes: emqx#9783
88a01d4
to
aa57ea9
Compare
Squashed the commits after latest changes. |
When configuring mongodb authorization the mongodb connector crashed with the following error in the log file. The reason is that the collection name that was sent to the mongodb connection was an atom. This is fixed by making sure it is not an atom. An even better fix would probably be to to figure out why the configuration data for the connection has the collection stored as an atom and fix the issue at the source.
Fixes: #9783