Merge pull request #14094 from lafirest/fix/invalid_oidc

fix(OIDC): return error when login with an OIDC provider which does not work

apps/emqx_dashboard_sso/src/emqx_dashboard_sso_api.erl

@@ -177,13 +177,18 @@ login(post, #{bindings := #{backend := Backend}, body := Body} = Request) ->
                         request => emqx_utils:redact(Request)
                     }),
                     Redirect;
-                {error, Reason} ->
+                {error, Reason0} ->
+                    Reason = emqx_utils:redact(Reason0),
                     ?SLOG(info, #{
                         msg => "dashboard_sso_login_failed",
                         request => emqx_utils:redact(Request),
-                        reason => emqx_utils:redact(Reason)
+                        reason => Reason
                     }),
-                    {401, #{code => ?BAD_USERNAME_OR_PWD, message => <<"Auth failed">>}}
+                    {401, #{
+                        code => ?BAD_USERNAME_OR_PWD,
+                        message => <<"Auth failed">>,
+                        reason => Reason
+                    }}
             end
     end.
 

apps/emqx_dashboard_sso/src/emqx_dashboard_sso_oidc.erl

@@ -214,25 +214,28 @@ login(
     }),
 
     Data = maps:with([nonce, require_pkce, pkce_verifier], Opts),
-    State = emqx_dashboard_sso_oidc_session:new(Data),
-
-    case
-        oidcc:create_redirect_url(
-            ?PROVIDER_SVR_NAME,
-            ClientId,
-            emqx_secret:unwrap(Secret),
-            Opts#{
-                state => State,
-                client_jwks => ClientJwks,
-                preferred_auth_methods => AuthMethods
-            }
-        )
-    of
-        {ok, [Base, Delimiter, Params]} ->
-            RedirectUri = <<Base/binary, Delimiter/binary, Params/binary>>,
-            Redirect = {302, ?RESPHEADERS#{<<"location">> => RedirectUri}, ?REDIRECT_BODY},
-            {redirect, Redirect};
-        {error, _Reason} = Error ->
+    case emqx_dashboard_sso_oidc_session:new(Data) of
+        {ok, State} ->
+            case
+                oidcc:create_redirect_url(
+                    ?PROVIDER_SVR_NAME,
+                    ClientId,
+                    emqx_secret:unwrap(Secret),
+                    Opts#{
+                        state => State,
+                        client_jwks => ClientJwks,
+                        preferred_auth_methods => AuthMethods
+                    }
+                )
+            of
+                {ok, [Base, Delimiter, Params]} ->
+                    RedirectUri = <<Base/binary, Delimiter/binary, Params/binary>>,
+                    Redirect = {302, ?RESPHEADERS#{<<"location">> => RedirectUri}, ?REDIRECT_BODY},
+                    {redirect, Redirect};
+                {error, _Reason} = Error ->
+                    Error
+            end;
+        Error ->
             Error
     end.
 

apps/emqx_dashboard_sso/src/emqx_dashboard_sso_oidc_session.erl

@@ -72,16 +72,23 @@ stop() ->
     ok.
 
 new(Data) ->
-    State = new_state(),
-    ets:insert(
-        ?TAB,
-        #?TAB{
-            state = State,
-            created_at = ?NOW,
-            data = Data
-        }
-    ),
-    State.
+    case ets:whereis(?TAB) of
+        undefined ->
+            %% The OIDCC may crash for some reason, even if we have some monitor to observe it
+            %% users also may open an OIDC login before the monitor finds it has crashed
+            {error, <<"No valid OIDC provider">>};
+        _ ->
+            State = new_state(),
+            ets:insert(
+                ?TAB,
+                #?TAB{
+                    state = State,
+                    created_at = ?NOW,
+                    data = Data
+                }
+            ),
+            {ok, State}
+    end.
 
 delete(State) ->
     ets:delete(?TAB, State).

changes/ee/14094.en.md

@@ -0,0 +1,2 @@
+Return an error if login with an invalid OIDC provider.
+