Merge pull request #3863 from esl/api-modules/token

Improve error handling in token

big_tests/tests/graphql_muc_SUITE.erl

@@ -1630,7 +1630,7 @@ user_owner_set_user_affiliation_muc_not_configured(Config) ->
         fun user_owner_set_user_affiliation_muc_not_configured_story/2).
 
 user_owner_set_user_affiliation_muc_not_configured_story(Config, Alice) ->
-    Res = user_set_user_affiliation(Alice, get_room_name(), <<"ali">>, member, Config),
+    Res = user_set_user_affiliation(Alice, get_room_name(), <<"eddie@localhost">>, member, Config),
     get_not_loaded(Res).
 
 user_moderator_set_user_role_muc_not_configured(Config) ->

big_tests/tests/graphql_private_SUITE.erl

@@ -201,11 +201,11 @@ admin_get_private(Config, Alice) ->
 
 no_user_error_set(Config) ->
     ElemStr = exml:to_binary(private_input()),
-    Result = admin_set_private(<<"AAAAA">>, ElemStr, Config),
+    Result = admin_set_private(<<"eddie@otherhost">>, ElemStr, Config),
     ?assertEqual(<<"not_found">>, get_err_code(Result)).
 
 no_user_error_get(Config) ->
-    Result = admin_get_private(<<"AAAAA">>, <<"my_element">>, <<"alice:private:ns">>, Config),
+    Result = admin_get_private(<<"eddie@otherhost">>, <<"my_element">>, <<"alice:private:ns">>, Config),
     ?assertEqual(<<"not_found">>, get_err_code(Result)).
 
 private_input() ->
@@ -241,7 +241,7 @@ domain_admin_user_set_private_no_permission(Config, AliceBis) ->
     ElemStr = exml:to_binary(private_input()),
     Result = admin_set_private(user_to_bin(AliceBis), ElemStr, Config),
     get_unauthorized(Result),
-    Result2 = admin_set_private(<<"AAAAA">>, ElemStr, Config),
+    Result2 = admin_set_private(<<"eddie@otherhost">>, ElemStr, Config),
     get_unauthorized(Result2).
 
 domain_admin_user_get_private_no_permission(Config) ->
@@ -252,7 +252,7 @@ domain_admin_user_get_private_no_permission(Config, AliceBis) ->
     AliceBisBin = user_to_bin(AliceBis),
     Result = admin_get_private(AliceBisBin, <<"my_element">>, <<"alice:private:ns">>, Config),
     get_unauthorized(Result),
-    Result2 = admin_get_private(<<"AAAAA">>, <<"my_element">>, <<"alice:private:ns">>, Config),
+    Result2 = admin_get_private(<<"eddie@otherhost">>, <<"my_element">>, <<"alice:private:ns">>, Config),
     get_unauthorized(Result2).
 
 %% Commands

big_tests/tests/graphql_token_SUITE.erl

@@ -2,9 +2,10 @@
 
 -compile([export_all, nowarn_export_all]).
 
+-import(common_helper, [unprep/1]).
 -import(distributed_helper, [require_rpc_nodes/1, mim/0]).
 -import(graphql_helper, [execute_command/4, execute_user_command/5, user_to_bin/1,
-                         get_ok_value/2, get_err_code/1, get_unauthorized/1, get_not_loaded/1]).
+                         get_ok_value/2, get_err_code/1, get_err_msg/1, get_unauthorized/1, get_not_loaded/1]).
 
 -include_lib("eunit/include/eunit.hrl").
 
@@ -46,17 +47,21 @@ user_token_not_configured_tests() ->
 
 domain_admin_tests() ->
     [admin_request_token_test,
+     admin_request_token_test_unprep,
      domain_admin_request_token_no_permission_test,
      domain_admin_revoke_token_no_permission_test,
      admin_revoke_token_no_token_test,
-     admin_revoke_token_test].
+     admin_revoke_token_test,
+     admin_revoke_token_test_unprep].
 
 admin_tests() ->
     [admin_request_token_test,
-     admin_request_token_no_user_test,
-     admin_revoke_token_no_user_test,
+     admin_request_token_test_unprep,
+     admin_request_token_no_host_test,
+     admin_revoke_token_no_host_test,
      admin_revoke_token_no_token_test,
-     admin_revoke_token_test].
+     admin_revoke_token_test,
+     admin_revoke_token_test_unprep].
 
 admin_token_not_configured_tests() ->
     [admin_request_token_test_not_configured,
@@ -155,7 +160,7 @@ user_revoke_token_test(Config, Alice) ->
     user_request_token(Alice, Config),
     Res2 = user_revoke_token(Alice, Config),
     ParsedRes = get_ok_value([data, token, revokeToken], Res2),
-    ?assertEqual(<<"Revoked.">>, ParsedRes).
+    ?assertEqual(<<"Revoked">>, ParsedRes).
 
 % User test cases mod_token not configured
 
@@ -185,8 +190,8 @@ domain_admin_request_token_no_permission_test(Config, AliceBis) ->
     % External domain user
     Res = admin_request_token(user_to_bin(AliceBis), Config),
     get_unauthorized(Res),
-    % Non-existing user
-    Res2 = admin_request_token(<<"AAAAA">>, Config),
+    % Non-existing domain
+    Res2 = admin_request_token(<<"eddie@otherhost">>, Config),
     get_unauthorized(Res2).
 
 domain_admin_revoke_token_no_permission_test(Config) ->
@@ -197,8 +202,8 @@ domain_admin_revoke_token_no_permission_test(Config, AliceBis) ->
     % External domain user
     Res = admin_revoke_token(user_to_bin(AliceBis), Config),
     get_unauthorized(Res),
-    % Non-existing user
-    Res2 = admin_revoke_token(<<"AAAAA">>, Config),
+    % Non-existing domain
+    Res2 = admin_revoke_token(<<"eddie@otherhost">>, Config),
     get_unauthorized(Res2).
 
 % Admin tests
@@ -213,13 +218,23 @@ admin_request_token_test(Config, Alice) ->
     ?assert(is_binary(Refresh)),
     ?assert(is_binary(Access)).
 
-admin_request_token_no_user_test(Config) ->
-    Res = admin_request_token(<<"AAAAA">>, Config),
-    ?assertEqual(<<"not_found">>, get_err_code(Res)).
+admin_request_token_test_unprep(Config) ->
+    escalus:fresh_story_with_config(Config, [{alice, 1}], fun admin_request_token_test_unprep/2).
 
-admin_revoke_token_no_user_test(Config) ->
-    Res = admin_revoke_token(<<"AAAAA">>, Config),
-    ?assertEqual(<<"not_found">>, get_err_code(Res)).
+admin_request_token_test_unprep(Config, Alice) ->
+    Res = admin_request_token(unprep(user_to_bin(Alice)), Config),
+    #{<<"refresh">> := Refresh, <<"access">> := Access} =
+        get_ok_value([data, token, requestToken], Res),
+    ?assert(is_binary(Refresh)),
+    ?assert(is_binary(Access)).
+
+admin_request_token_no_host_test(Config) ->
+    Res = admin_request_token(<<"eddie@otherhost">>, Config),
+    ?assertEqual(<<"Unknown domain">>, get_err_msg(Res)).
+
+admin_revoke_token_no_host_test(Config) ->
+    Res = admin_revoke_token(<<"eddie@otherhost">>, Config),
+    ?assertEqual(<<"Unknown domain">>, get_err_msg(Res)).
 
 admin_revoke_token_no_token_test(Config) ->
     escalus:fresh_story_with_config(Config, [{alice, 1}], fun admin_revoke_token_no_token_test/2).
@@ -235,7 +250,16 @@ admin_revoke_token_test(Config, Alice) ->
     admin_request_token(user_to_bin(Alice), Config),
     Res2 = admin_revoke_token(user_to_bin(Alice), Config),
     ParsedRes = get_ok_value([data, token, revokeToken], Res2),
-    ?assertEqual(<<"Revoked.">>, ParsedRes).
+    ?assertEqual(<<"Revoked">>, ParsedRes).
+
+admin_revoke_token_test_unprep(Config) ->
+    escalus:fresh_story_with_config(Config, [{alice, 1}], fun admin_revoke_token_test_unprep/2).
+
+admin_revoke_token_test_unprep(Config, Alice) ->
+    admin_request_token(unprep(user_to_bin(Alice)), Config),
+    Res2 = admin_revoke_token(unprep(user_to_bin(Alice)), Config),
+    ParsedRes = get_ok_value([data, token, revokeToken], Res2),
+    ?assertEqual(<<"Revoked">>, ParsedRes).
 
 % Admin test cases token not configured
 

big_tests/tests/graphql_vcard_SUITE.erl

@@ -53,20 +53,20 @@ user_vcard_not_configured_tests() ->
 domain_admin_vcard_tests() ->
     [admin_set_vcard,
      admin_set_vcard_incomplete_fields,
+     domain_admin_set_vcard_no_host,
      domain_admin_set_vcard_no_permission,
-     domain_admin_set_vcard_no_user,
      admin_get_vcard,
      admin_get_vcard_no_vcard,
-     domain_admin_get_vcard_no_user,
+     domain_admin_get_vcard_no_host,
      domain_admin_get_vcard_no_permission].
 
 admin_vcard_tests() ->
     [admin_set_vcard,
      admin_set_vcard_incomplete_fields,
-     admin_set_vcard_no_user,
+     admin_set_vcard_no_host,
      admin_get_vcard,
      admin_get_vcard_no_vcard,
-     admin_get_vcard_no_user].
+     admin_get_vcard_no_host].
 
 admin_vcard_not_configured_tests() ->
     [admin_set_vcard_not_configured,
@@ -199,8 +199,8 @@ user_get_others_vcard_no_user(Config) ->
                                     fun user_get_others_vcard_no_user/2).
 
 user_get_others_vcard_no_user(Config, Alice) ->
-    Result = user_get_vcard(Alice, <<"AAAAA">>, Config),
-    ?assertEqual(<<"User does not exist">>, get_err_msg(Result)).
+    Result = user_get_vcard(Alice, <<"eddie@otherhost">>, Config),
+    ?assertEqual(<<"Host does not exist">>, get_err_msg(Result)).
 
 % User VCard not configured test cases
 
@@ -231,12 +231,12 @@ user_get_their_vcard_not_configured(Config, Alice) ->
 
 %% Domain admin test cases
 
-domain_admin_set_vcard_no_user(Config) ->
+domain_admin_set_vcard_no_host(Config) ->
     Vcard = complete_vcard_input(),
-    get_unauthorized(set_vcard(Vcard, <<"AAAAA">>, Config)).
+    get_unauthorized(set_vcard(Vcard, <<"eddie@otherhost">>, Config)).
 
-domain_admin_get_vcard_no_user(Config) ->
-    get_unauthorized(get_vcard(<<"AAAAA">>, Config)).
+domain_admin_get_vcard_no_host(Config) ->
+    get_unauthorized(get_vcard(<<"eddie@otherhost">>, Config)).
 
 domain_admin_get_vcard_no_permission(Config) ->
     escalus:fresh_story_with_config(Config, [{alice_bis, 1}],
@@ -276,10 +276,10 @@ admin_set_vcard(Config, Alice, _Bob) ->
     ParsedResultGet = get_ok_value([data, vcard, getVcard], ResultGet),
     ?assertEqual(Vcard, skip_null_fields(ParsedResultGet)).
 
-admin_set_vcard_no_user(Config) ->
+admin_set_vcard_no_host(Config) ->
     Vcard = complete_vcard_input(),
-    Result = set_vcard(Vcard, <<"AAAAA">>, Config),
-    ?assertEqual(<<"User does not exist">>, get_err_msg(Result)).
+    Result = set_vcard(Vcard, <<"eddie@otherhost">>, Config),
+    ?assertEqual(<<"Host does not exist">>, get_err_msg(Result)).
 
 admin_get_vcard(Config) ->
     escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}],
@@ -308,9 +308,9 @@ admin_get_vcard_no_vcard(Config, Alice) ->
     Result = get_vcard(user_to_bin(Alice), Config),
     ?assertEqual(<<"Vcard for user not found">>, get_err_msg(Result)).
 
-admin_get_vcard_no_user(Config) ->
-    Result = get_vcard(<<"AAAAA">>, Config),
-    ?assertEqual(<<"User does not exist">>, get_err_msg(Result)).
+admin_get_vcard_no_host(Config) ->
+    Result = get_vcard(<<"eddie@otherhost">>, Config),
+    ?assertEqual(<<"Host does not exist">>, get_err_msg(Result)).
 
 %% Admin VCard not configured test cases
 

big_tests/tests/oauth_SUITE.erl

@@ -291,15 +291,15 @@ revoke_token_cmd_when_no_token(Config) ->
     %% when revoking token
     R = mimctl(Config, ["revoke_token", escalus_users:get_jid(Config, bob)]),
     %% then no token was found
-    "Error: \"User or token not found.\"\n" = R.
+    "Error: \"User or token not found\"\n" = R.
 
 revoke_token_cmd(Config) ->
     %% given existing user and token present in the database
     _Tokens = request_tokens_once_logged_in_impl(Config, bob),
     %% when
     R = mimctl(Config, ["revoke_token", escalus_users:get_jid(Config, bob)]),
     %% then
-    "Revoked.\n" = R.
+    "Revoked\n" = R.
 
 token_removed_on_user_removal(Config) ->
     %% given existing user with token and XMPP (de)registration available

src/graphql/admin/mongoose_graphql_token_admin_mutation.erl

@@ -20,12 +20,12 @@
  request_token(JID) ->
     case mod_auth_token_api:create_token(JID) of
         {ok, _} = Result -> Result;
-        {error, Error} -> make_error(Error, #{user => JID})
+        Error -> make_error(Error, #{user => JID})
     end.
 
  -spec revoke_token(jid:jid()) -> {ok, string()} | {error, resolver_error()}.
  revoke_token(JID) ->
     case mod_auth_token_api:revoke_token_command(JID) of
         {ok, _} = Result -> Result;
-        {error, Error} -> make_error(Error, #{user => JID})
+        Error -> make_error(Error, #{user => JID})
     end.

src/graphql/mongoose_graphql_scalar.erl

@@ -47,6 +47,8 @@ jid_from_binary(Value) ->
     case jid:from_binary(Value) of
         error ->
             {error, failed_to_parse_jid};
+        #jid{luser = <<>>} ->
+            {error, jid_without_user};
         Jid ->
             {ok, Jid}
     end.

src/graphql/user/mongoose_graphql_last_user_mutation.erl

@@ -11,7 +11,7 @@
 
 -type last_info() :: map().
 -type args() :: mongoose_graphql:args().
--type ctx() :: mongoose_graphql:ctx().
+-type ctx() :: mongoose_graphql:context().
 
 execute(Ctx, last, <<"setLast">>, Args) ->
    set_last(Ctx, Args).

src/graphql/user/mongoose_graphql_last_user_query.erl

@@ -11,7 +11,7 @@
 
 -type last_info() :: mongoose_graphql_last_helper:last_info().
 -type args() :: mongoose_graphql:args().
--type ctx() :: mongoose_graphql:ctx().
+-type ctx() :: mongoose_graphql:context().
 
 execute(Ctx, last, <<"getLast">>, Args) ->
     get_last(Ctx, Args).

src/graphql/user/mongoose_graphql_token_user_mutation.erl

@@ -11,7 +11,7 @@
 
  -type token_info() :: map().
  -type args() :: mongoose_graphql:args().
- -type ctx() :: mongoose_graphql:ctx().
+ -type ctx() :: mongoose_graphql:context().
 
  execute(Ctx, token, <<"requestToken">>, Args) ->
     request_token(Ctx, Args);
@@ -22,12 +22,12 @@
  request_token(#{user := JID}, #{}) ->
     case mod_auth_token_api:create_token(JID) of
         {ok, _} = Result -> Result;
-        {error, Error} -> make_error(Error, #{user => JID})
+        Error -> make_error(Error, #{user => JID})
     end.
 
  -spec revoke_token(ctx(), args()) -> {ok, string()} | {error, resolver_error()}.
  revoke_token(#{user := JID}, #{}) ->
     case mod_auth_token_api:revoke_token_command(JID) of
         {ok, _} = Result -> Result;
-        {error, Error} -> make_error(Error, #{user => JID})
+        Error -> make_error(Error, #{user => JID})
     end.

src/mod_auth_token.erl

@@ -416,13 +416,14 @@ key_name(refresh)   -> token_secret;
 key_name(provision) -> provision_pre_shared.
 
 -spec revoke_token_command(Owner) -> ResTuple when
-      Owner :: jid:jid(),
+      Owner :: binary(),
       ResCode :: ok | not_found | error,
       ResTuple :: {ResCode, string()}.
 revoke_token_command(Owner) ->
-    case mod_auth_token_api:revoke_token_command(Owner) of
+    JID = jid:from_binary(Owner),
+    case mod_auth_token_api:revoke_token_command(JID) of
         {ok, _} = Result -> Result;
-        {error, Error} -> Error
+        Error -> Error
     end.
 
 -spec clean_tokens(Acc, Params, Extra) -> {ok, Acc} when