From 21c37a3f1406735ea133eb5a6e0cbd92a079da96 Mon Sep 17 00:00:00 2001 From: firest Date: Wed, 20 Sep 2023 17:41:51 +0800 Subject: [PATCH] fix(sso): adjust the schema of LDAP API && add more logs --- .../emqx_dashboard/include/emqx_dashboard.hrl | 2 +- .../src/emqx_dashboard_sso_api.erl | 2 + .../src/emqx_dashboard_sso_ldap.erl | 57 +++++++++++++------ .../src/emqx_dashboard_sso_manager.erl | 51 +++++++++++------ .../test/emqx_dashboard_sso_ldap_SUITE.erl | 4 +- 5 files changed, 80 insertions(+), 36 deletions(-) diff --git a/apps/emqx_dashboard/include/emqx_dashboard.hrl b/apps/emqx_dashboard/include/emqx_dashboard.hrl index 8cb1853c9..3e089ccbf 100644 --- a/apps/emqx_dashboard/include/emqx_dashboard.hrl +++ b/apps/emqx_dashboard/include/emqx_dashboard.hrl @@ -21,7 +21,7 @@ %% In full RBAC feature, the role may be customised created and deleted, %% a predefined configuration would replace these macros. -define(ROLE_VIEWER, <<"viewer">>). --define(ROLE_SUPERUSER, <<"superuser">>). +-define(ROLE_SUPERUSER, <<"administrator">>). -define(ROLE_DEFAULT, ?ROLE_SUPERUSER). -define(SSO_USERNAME(Backend, Name), {Backend, Name}). diff --git a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_api.erl b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_api.erl index 93a5d7be7..e95e623ea 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_api.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_api.erl @@ -156,8 +156,10 @@ backend(get, #{bindings := #{backend := Type}}) -> {200, to_json(Backend)} end; backend(put, #{bindings := #{backend := Backend}, body := Config}) -> + ?SLOG(info, #{msg => "Update SSO backend", backend => Backend, config => Config}), on_backend_update(Backend, Config, fun emqx_dashboard_sso_manager:update/2); backend(delete, #{bindings := #{backend := Backend}}) -> + ?SLOG(info, #{msg => "Delete SSO backend", backend => Backend}), handle_backend_update_result(emqx_dashboard_sso_manager:delete(Backend), undefined). sso_parameters(Params) -> diff --git a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl index df9b2935c..395deea7d 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl @@ -40,7 +40,7 @@ fields(ldap) -> [ {query_timeout, fun query_timeout/1} ] ++ - emqx_ldap:fields(config) ++ emqx_ldap:fields(bind_opts); + adjust_ldap_fields(emqx_ldap:fields(config)); fields(login) -> [ emqx_dashboard_sso_schema:backend_schema([ldap]) @@ -84,6 +84,46 @@ destroy(#{resource_id := ResourceId}) -> _ = emqx_resource:remove_local(ResourceId), ok. +parse_config(Config0) -> + Config = ensure_bind_password(Config0), + State = lists:foldl( + fun(Key, Acc) -> + case maps:find(Key, Config) of + {ok, Value} when is_binary(Value) -> + Acc#{Key := erlang:binary_to_list(Value)}; + _ -> + Acc + end + end, + Config, + [query_timeout] + ), + {Config, State}. + +%% In this feature, the `bind_password` is fixed, so it should conceal from the swagger, +%% but the connector still needs it, hence we should add it back here +ensure_bind_password(Config) -> + Config#{bind_password => <<"${password}">>}. + +adjust_ldap_fields(Fields) -> + adjust_ldap_fields(Fields, []). + +adjust_ldap_fields([{filter, Meta} | T], Acc) -> + adjust_ldap_fields( + T, + [ + {filter, Meta#{ + default => <<"(objectClass=user)">>, + example => <<"(objectClass=user)">> + }} + | Acc + ] + ); +adjust_ldap_fields([Any | T], Acc) -> + adjust_ldap_fields(T, [Any | Acc]); +adjust_ldap_fields([], Acc) -> + lists:reverse(Acc). + login( #{<<"username">> := Username} = Req, #{ @@ -115,21 +155,6 @@ login( Error end. -parse_config(Config) -> - State = lists:foldl( - fun(Key, Acc) -> - case maps:find(Key, Config) of - {ok, Value} when is_binary(Value) -> - Acc#{Key := erlang:binary_to_list(Value)}; - _ -> - Acc - end - end, - Config, - [query_timeout] - ), - {Config, State}. - ensure_user_exists(Username) -> case emqx_dashboard_admin:lookup_user(ldap, Username) of [User] -> diff --git a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_manager.erl b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_manager.erl index 6097a39ff..afa27cb47 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_manager.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_manager.erl @@ -6,6 +6,8 @@ -behaviour(gen_server). +-include_lib("emqx/include/logger.hrl"). + %% API -export([start_link/0]). @@ -122,11 +124,8 @@ init([]) -> start_backend_services(), {ok, #{}}. -handle_call({update_config, Req, NewConf, OldConf}, _From, State) -> - Result = on_config_update(Req, NewConf, OldConf), - io:format(">>> on_config_update:~p~n,Req:~p~n NewConf:~p~n OldConf:~p~n", [ - Result, Req, NewConf, OldConf - ]), +handle_call({update_config, Req, NewConf}, _From, State) -> + Result = on_config_update(Req, NewConf), {reply, Result, State}; handle_call(_Request, _From, State) -> Reply = ok, @@ -156,22 +155,40 @@ start_backend_services() -> lists:foreach( fun({Backend, Config}) -> Provider = provider(Backend), - on_backend_updated( - emqx_dashboard_sso:create(Provider, Config), - fun(State) -> - ets:insert(dashboard_sso, #dashboard_sso{backend = Backend, state = State}) - end - ) + case emqx_dashboard_sso:create(Provider, Config) of + {ok, State} -> + ?SLOG(info, #{ + msg => "Start SSO backend successfully", + backend => Backend + }), + ets:insert(dashboard_sso, #dashboard_sso{backend = Backend, state = State}); + {error, Reason} -> + ?SLOG(error, #{ + msg => "Start SSO backend failed", + backend => Backend, + reason => Reason + }) + end end, maps:to_list(Backends) ). -update_config(_Backend, UpdateReq) -> +update_config(Backend, UpdateReq) -> case emqx_conf:update([dashboard_sso], UpdateReq, #{override_to => cluster}) of {ok, UpdateResult} -> #{post_config_update := #{?MODULE := Result}} = UpdateResult, + ?SLOG(info, #{ + msg => "Update SSO configuration successfully", + backend => Backend, + result => Result + }), Result; - Error -> + {error, Reason} = Error -> + ?SLOG(error, #{ + msg => "Update SSO configuration failed", + backend => Backend, + reason => Reason + }), Error end. @@ -187,11 +204,11 @@ pre_config_update(_Path, {delete, Backend}, OldConf) -> {ok, maps:remove(BackendBin, OldConf)} end. -post_config_update(_Path, UpdateReq, NewConf, OldConf, _AppEnvs) -> - Result = call({update_config, UpdateReq, NewConf, OldConf}), +post_config_update(_Path, UpdateReq, NewConf, _OldConf, _AppEnvs) -> + Result = call({update_config, UpdateReq, NewConf}), {ok, Result}. -on_config_update({update, Backend, _Config}, NewConf, _OldConf) -> +on_config_update({update, Backend, _Config}, NewConf) -> Provider = provider(Backend), Config = maps:get(Backend, NewConf), case lookup(Backend) of @@ -210,7 +227,7 @@ on_config_update({update, Backend, _Config}, NewConf, _OldConf) -> end ) end; -on_config_update({delete, Backend}, _NewConf, _OldConf) -> +on_config_update({delete, Backend}, _NewConf) -> case lookup(Backend) of undefined -> {error, not_exists}; diff --git a/apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl b/apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl index b0144feaa..5166aa4c5 100644 --- a/apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl +++ b/apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl @@ -136,10 +136,10 @@ ldap_config(Override) -> <<"enable">> => <<"false">>, <<"server">> => ldap_server(), <<"base_dn">> => <<"uid=${username},ou=testdevice,dc=emqx,dc=io">>, + <<"filter">> => <<"(objectClass=mqttUser)">>, <<"username">> => <<"cn=root,dc=emqx,dc=io">>, <<"password">> => <<"public">>, - <<"pool_size">> => 8, - <<"bind_password">> => <<"${password}">> + <<"pool_size">> => 8 }, Override ).