Skip to content

Commit 7da1c8a

Browse files
michaelklishinmichaelklishin
authored
Merge pull request #12818 from rabbitmq/token-refresh-cannot-change-username
OAuth 2: JWT token refresh should preserve user identity displayed in the UI
2 parents 1679137 + 3718fe3 commit 7da1c8a

File tree

3 files changed

+76
-10
lines changed

3 files changed

+76
-10
lines changed

‎deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,17 +103,26 @@ check_topic_access(#auth_user{impl = DecodedTokenFun},
103103
update_state(AuthUser, NewToken) ->
104104
case resolve_resource_server(NewToken) of
105105
{error, _} = Err0 -> Err0;
106-
{_, _} = Tuple ->
106+
{ResourceServer, _} = Tuple ->
107107
case check_token(NewToken, Tuple) of
108108
%% avoid logging the token
109109
{refused, {error, {invalid_token, error, _Err, _Stacktrace}}} ->
110110
{refused, "Authentication using an OAuth 2/JWT token failed: provided token is invalid"};
111111
{refused, Err} ->
112112
{refused, rabbit_misc:format("Authentication using an OAuth 2/JWT token failed: ~tp", [Err])};
113113
{ok, DecodedToken} ->
114-
Tags = tags_from(DecodedToken),
115-
{ok, AuthUser#auth_user{tags = Tags,
116-
impl = fun() -> DecodedToken end}}
114+
CurToken = AuthUser#auth_user.impl,
115+
case ensure_same_username(
116+
ResourceServer#resource_server.preferred_username_claims,
117+
CurToken(), DecodedToken) of
118+
ok ->
119+
Tags = tags_from(DecodedToken),
120+
{ok, AuthUser#auth_user{tags = Tags,
121+
impl = fun() -> DecodedToken end}};
122+
{error, mismatch_username_after_token_refresh} ->
123+
{refused,
124+
"Not allowed to change username on refreshed token"}
125+
end
117126
end
118127
end.
119128

@@ -139,7 +148,7 @@ authenticate(_, AuthProps0) ->
139148
{refused, "Authentication using an OAuth 2/JWT token failed: provided token is invalid", []};
140149
{refused, Err} ->
141150
{refused, "Authentication using an OAuth 2/JWT token failed: ~tp", [Err]};
142-
{ok, DecodedToken} ->
151+
{ok, DecodedToken} ->
143152
Func = fun(Token0) ->
144153
Username = username_from(
145154
ResourceServer#resource_server.preferred_username_claims,
@@ -164,6 +173,12 @@ with_decoded_token(DecodedToken, Fun) ->
164173
rabbit_log:error(Msg),
165174
Err
166175
end.
176+
ensure_same_username(PreferredUsernameClaims, CurrentDecodedToken, NewDecodedToken) ->
177+
CurUsername = username_from(PreferredUsernameClaims, CurrentDecodedToken),
178+
case {CurUsername, username_from(PreferredUsernameClaims, NewDecodedToken)} of
179+
{CurUsername, CurUsername} -> ok;
180+
_ -> {error, mismatch_username_after_token_refresh}
181+
end.
167182

168183
validate_token_expiry(#{<<"exp">> := Exp}) when is_integer(Exp) ->
169184
Now = os:system_time(seconds),

‎deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ groups() ->
5858
test_failed_connection_with_a_token_with_insufficient_resource_permission,
5959
test_failed_connection_with_algorithm_restriction,
6060
test_failed_token_refresh_case1,
61-
test_failed_token_refresh_case2
61+
test_failed_token_refresh_case2,
62+
cannot_change_username_on_refreshed_token
6263
]},
6364
{no_peer_verification, [], [
6465
{group, happy_path},
@@ -521,6 +522,11 @@ generate_valid_token(Config, Jwk, Scopes, Audience) ->
521522
IncludeKid = rabbit_ct_helpers:get_config(Config, include_kid, true),
522523
?UTIL_MOD:sign_token_hs(Token, Jwk, IncludeKid).
523524

525+
generate_valid_token_with_sub(Config, Jwk, Scopes, Sub) ->
526+
Token = ?UTIL_MOD:token_with_sub(?UTIL_MOD:fixture_token_with_scopes(Scopes), Sub),
527+
IncludeKid = rabbit_ct_helpers:get_config(Config, include_kid, true),
528+
?UTIL_MOD:sign_token_hs(Token, Jwk, IncludeKid).
529+
524530
generate_valid_token_with_extra_fields(Config, ExtraFields) ->
525531
Jwk =
526532
case rabbit_ct_helpers:get_config(Config, fixture_jwk) of
@@ -937,6 +943,29 @@ test_failed_token_refresh_case2(Config) ->
937943

938944
close_connection(Conn).
939945

946+
cannot_change_username_on_refreshed_token(Config) ->
947+
Jwk =
948+
case get_config(Config, fixture_jwk) of
949+
undefined -> ?UTIL_MOD:fixture_jwk();
950+
Value -> Value
951+
end,
952+
{_, CurToken} = generate_valid_token(Config, Jwk, <<"oldUsername">>, [
953+
<<"rabbitmq.configure:vhost4/*">>,
954+
<<"rabbitmq.write:vhost4/*">>,
955+
<<"rabbitmq.read:vhost4/*">>]),
956+
Conn = open_unmanaged_connection(Config, 0, <<"vhost4">>,
957+
<<"oldUsername">>, CurToken),
958+
959+
{_, RefreshToken} = generate_valid_token_with_sub(Config, Jwk, <<"newUsername">>,
960+
[<<"rabbitmq.configure:vhost4/*">>,
961+
<<"rabbitmq.write:vhost4/*">>,
962+
<<"rabbitmq.read:vhost4/*">>]),
963+
964+
%% the error is communicated asynchronously via a connection-level error
965+
?assertException(exit, _, amqp_connection:update_secret(Conn, RefreshToken,
966+
<<"token refresh">>)).
967+
968+
940969
test_failed_connection_with_algorithm_restriction(Config) ->
941970
{_Algo, Token} = get_config(Config, fixture_jwt),
942971
?assertMatch({error, {auth_failure, _}},

‎deps/rabbitmq_auth_backend_oauth2/test/system_SUITE.erl‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ groups() ->
5252

5353
{token_refresh, [], [
5454
test_failed_token_refresh_case1,
55-
test_failed_token_refresh_case2
55+
test_failed_token_refresh_case2,
56+
refreshed_token_cannot_change_username
5657
]},
5758

5859
{extra_scopes_source, [], [
@@ -323,21 +324,33 @@ preconfigure_node(Config) ->
323324

324325
rabbit_ct_helpers:set_config(Config, {fixture_jwk, Jwk}).
325326

327+
generate_valid_token_with_sub(Config, Sub) ->
328+
generate_valid_token(Config,
329+
?UTIL_MOD:full_permission_scopes(), undefined, Sub).
330+
326331
generate_valid_token(Config) ->
327332
generate_valid_token(Config, ?UTIL_MOD:full_permission_scopes()).
328333

329334
generate_valid_token(Config, Scopes) ->
330-
generate_valid_token(Config, Scopes, undefined).
335+
generate_valid_token(Config, Scopes, undefined, undefined).
331336

332337
generate_valid_token(Config, Scopes, Audience) ->
338+
generate_valid_token(Config, Scopes, Audience, undefined).
339+
340+
generate_valid_token(Config, Scopes, Audience, Sub) ->
333341
Jwk = case rabbit_ct_helpers:get_config(Config, fixture_jwk) of
334342
undefined -> ?UTIL_MOD:fixture_jwk();
335343
Value -> Value
336344
end,
337-
Token = case Audience of
345+
Token0 = case Audience of
338346
undefined -> ?UTIL_MOD:fixture_token_with_scopes(Scopes);
339-
DefinedAudience -> maps:put(<<"aud">>, DefinedAudience, ?UTIL_MOD:fixture_token_with_scopes(Scopes))
347+
DefinedAudience -> maps:put(<<"aud">>, DefinedAudience,
348+
?UTIL_MOD:fixture_token_with_scopes(Scopes))
340349
end,
350+
Token = case Sub of
351+
undefined -> Token0;
352+
_ -> maps:put(<<"sub">>, Sub, Token0)
353+
end,
341354
?UTIL_MOD:sign_token_hs(Token, Jwk).
342355

343356
generate_valid_token_with_extra_fields(Config, ExtraFields) ->
@@ -913,6 +926,15 @@ test_failed_token_refresh_case1(Config) ->
913926

914927
close_connection(Conn).
915928

929+
refreshed_token_cannot_change_username(Config) ->
930+
{_, Token} = generate_valid_token_with_sub(Config, <<"username">>),
931+
Conn = open_unmanaged_connection(Config, 0, <<"vhost4">>, <<"username">>, Token),
932+
{_, RefreshedToken} = generate_valid_token_with_sub(Config, <<"username2">>),
933+
934+
%% the error is communicated asynchronously via a connection-level error
935+
?assertException(exit, {{nodedown,not_allowed},_}, amqp_connection:update_secret(Conn, RefreshedToken, <<"token refresh">>)).
936+
937+
916938
test_failed_token_refresh_case2(Config) ->
917939
{_Algo, Token} = generate_valid_token(Config, [<<"rabbitmq.configure:vhost4/*">>,
918940
<<"rabbitmq.write:vhost4/*">>,

0 commit comments

Comments
 (0)