Skip to content

Commit 3d8629b

Browse files
MarcialRosalesmergify[bot]
authored andcommitted
Prevent change of username on token refresh
(cherry picked from commit 3718fe3) # Conflicts: # deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl
1 parent 06c34c7 commit 3d8629b

File tree

3 files changed

+108
-5
lines changed

3 files changed

+108
-5
lines changed

deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl

+52
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ check_topic_access(#auth_user{impl = DecodedTokenFun},
9898
end).
9999

100100
update_state(AuthUser, NewToken) ->
101+
<<<<<<< HEAD
101102
case check_token(NewToken) of
102103
%% avoid logging the token
103104
{error, _} = E -> E;
@@ -111,6 +112,32 @@ update_state(AuthUser, NewToken) ->
111112
{ok, AuthUser#auth_user{tags = Tags,
112113
impl = fun() -> DecodedToken end}}
113114
end.
115+
=======
116+
case resolve_resource_server(NewToken) of
117+
{error, _} = Err0 -> Err0;
118+
{ResourceServer, _} = Tuple ->
119+
case check_token(NewToken, Tuple) of
120+
%% avoid logging the token
121+
{refused, {error, {invalid_token, error, _Err, _Stacktrace}}} ->
122+
{refused, "Authentication using an OAuth 2/JWT token failed: provided token is invalid"};
123+
{refused, Err} ->
124+
{refused, rabbit_misc:format("Authentication using an OAuth 2/JWT token failed: ~tp", [Err])};
125+
{ok, DecodedToken} ->
126+
CurToken = AuthUser#auth_user.impl,
127+
case ensure_same_username(
128+
ResourceServer#resource_server.preferred_username_claims,
129+
CurToken(), DecodedToken) of
130+
ok ->
131+
Tags = tags_from(DecodedToken),
132+
{ok, AuthUser#auth_user{tags = Tags,
133+
impl = fun() -> DecodedToken end}};
134+
{error, mismatch_username_after_token_refresh} ->
135+
{refused,
136+
"Not allowed to change username on refreshed token"}
137+
end
138+
end
139+
end.
140+
>>>>>>> 3718fe3289 (Prevent change of username on token refresh)
114141

115142
expiry_timestamp(#auth_user{impl = DecodedTokenFun}) ->
116143
case DecodedTokenFun() of
@@ -145,8 +172,27 @@ authenticate(_, AuthProps0) ->
145172
case with_decoded_token(DecodedToken, Func) of
146173
{error, Err} ->
147174
{refused, "Authentication using an OAuth 2/JWT token failed: ~tp", [Err]};
175+
<<<<<<< HEAD
148176
Else ->
149177
Else
178+
=======
179+
{ok, DecodedToken} ->
180+
Func = fun(Token0) ->
181+
Username = username_from(
182+
ResourceServer#resource_server.preferred_username_claims,
183+
Token0),
184+
Tags = tags_from(Token0),
185+
{ok, #auth_user{username = Username,
186+
tags = Tags,
187+
impl = fun() -> Token0 end}}
188+
end,
189+
case with_decoded_token(DecodedToken, Func) of
190+
{error, Err} ->
191+
{refused, "Authentication using an OAuth 2/JWT token failed: ~tp", [Err]};
192+
Else ->
193+
Else
194+
end
195+
>>>>>>> 3718fe3289 (Prevent change of username on token refresh)
150196
end
151197
end.
152198

@@ -157,6 +203,12 @@ with_decoded_token(DecodedToken, Fun) ->
157203
rabbit_log:error(Msg),
158204
Err
159205
end.
206+
ensure_same_username(PreferredUsernameClaims, CurrentDecodedToken, NewDecodedToken) ->
207+
CurUsername = username_from(PreferredUsernameClaims, CurrentDecodedToken),
208+
case {CurUsername, username_from(PreferredUsernameClaims, NewDecodedToken)} of
209+
{CurUsername, CurUsername} -> ok;
210+
_ -> {error, mismatch_username_after_token_refresh}
211+
end.
160212

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

deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl

+30-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ groups() ->
4545
test_failed_connection_with_a_token_with_insufficient_resource_permission,
4646
test_failed_connection_with_algorithm_restriction,
4747
test_failed_token_refresh_case1,
48-
test_failed_token_refresh_case2
48+
test_failed_token_refresh_case2,
49+
cannot_change_username_on_refreshed_token
4950
]},
5051
{no_peer_verification, [], [
5152
{group, happy_path},
@@ -531,6 +532,11 @@ generate_valid_token(Config, Jwk, Scopes, Audience) ->
531532
IncludeKid = rabbit_ct_helpers:get_config(Config, include_kid, true),
532533
?UTIL_MOD:sign_token_hs(Token, Jwk, IncludeKid).
533534

535+
generate_valid_token_with_sub(Config, Jwk, Scopes, Sub) ->
536+
Token = ?UTIL_MOD:token_with_sub(?UTIL_MOD:fixture_token_with_scopes(Scopes), Sub),
537+
IncludeKid = rabbit_ct_helpers:get_config(Config, include_kid, true),
538+
?UTIL_MOD:sign_token_hs(Token, Jwk, IncludeKid).
539+
534540
generate_valid_token_with_extra_fields(Config, ExtraFields) ->
535541
Jwk = case rabbit_ct_helpers:get_config(Config, fixture_jwk) of
536542
undefined -> ?UTIL_MOD:fixture_jwk();
@@ -912,6 +918,29 @@ test_failed_token_refresh_case2(Config) ->
912918

913919
close_connection(Conn).
914920

921+
cannot_change_username_on_refreshed_token(Config) ->
922+
Jwk =
923+
case get_config(Config, fixture_jwk) of
924+
undefined -> ?UTIL_MOD:fixture_jwk();
925+
Value -> Value
926+
end,
927+
{_, CurToken} = generate_valid_token(Config, Jwk, <<"oldUsername">>, [
928+
<<"rabbitmq.configure:vhost4/*">>,
929+
<<"rabbitmq.write:vhost4/*">>,
930+
<<"rabbitmq.read:vhost4/*">>]),
931+
Conn = open_unmanaged_connection(Config, 0, <<"vhost4">>,
932+
<<"oldUsername">>, CurToken),
933+
934+
{_, RefreshToken} = generate_valid_token_with_sub(Config, Jwk, <<"newUsername">>,
935+
[<<"rabbitmq.configure:vhost4/*">>,
936+
<<"rabbitmq.write:vhost4/*">>,
937+
<<"rabbitmq.read:vhost4/*">>]),
938+
939+
%% the error is communicated asynchronously via a connection-level error
940+
?assertException(exit, _, amqp_connection:update_secret(Conn, RefreshToken,
941+
<<"token refresh">>)).
942+
943+
915944
test_failed_connection_with_algorithm_restriction(Config) ->
916945
{_Algo, Token} = rabbit_ct_helpers:get_config(Config, fixture_jwt),
917946
?assertMatch({error, {auth_failure, _}},

deps/rabbitmq_auth_backend_oauth2/test/system_SUITE.erl

+26-4
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, [], [
@@ -312,21 +313,33 @@ preconfigure_node(Config) ->
312313

313314
rabbit_ct_helpers:set_config(Config, {fixture_jwk, Jwk}).
314315

316+
generate_valid_token_with_sub(Config, Sub) ->
317+
generate_valid_token(Config,
318+
?UTIL_MOD:full_permission_scopes(), undefined, Sub).
319+
315320
generate_valid_token(Config) ->
316321
generate_valid_token(Config, ?UTIL_MOD:full_permission_scopes()).
317322

318323
generate_valid_token(Config, Scopes) ->
319-
generate_valid_token(Config, Scopes, undefined).
324+
generate_valid_token(Config, Scopes, undefined, undefined).
320325

321326
generate_valid_token(Config, Scopes, Audience) ->
327+
generate_valid_token(Config, Scopes, Audience, undefined).
328+
329+
generate_valid_token(Config, Scopes, Audience, Sub) ->
322330
Jwk = case rabbit_ct_helpers:get_config(Config, fixture_jwk) of
323331
undefined -> ?UTIL_MOD:fixture_jwk();
324332
Value -> Value
325333
end,
326-
Token = case Audience of
334+
Token0 = case Audience of
327335
undefined -> ?UTIL_MOD:fixture_token_with_scopes(Scopes);
328-
DefinedAudience -> maps:put(<<"aud">>, DefinedAudience, ?UTIL_MOD:fixture_token_with_scopes(Scopes))
336+
DefinedAudience -> maps:put(<<"aud">>, DefinedAudience,
337+
?UTIL_MOD:fixture_token_with_scopes(Scopes))
329338
end,
339+
Token = case Sub of
340+
undefined -> Token0;
341+
_ -> maps:put(<<"sub">>, Sub, Token0)
342+
end,
330343
?UTIL_MOD:sign_token_hs(Token, Jwk).
331344

332345
generate_valid_token_with_extra_fields(Config, ExtraFields) ->
@@ -693,6 +706,15 @@ test_failed_token_refresh_case1(Config) ->
693706

694707
close_connection(Conn).
695708

709+
refreshed_token_cannot_change_username(Config) ->
710+
{_, Token} = generate_valid_token_with_sub(Config, <<"username">>),
711+
Conn = open_unmanaged_connection(Config, 0, <<"vhost4">>, <<"username">>, Token),
712+
{_, RefreshedToken} = generate_valid_token_with_sub(Config, <<"username2">>),
713+
714+
%% the error is communicated asynchronously via a connection-level error
715+
?assertException(exit, {{nodedown,not_allowed},_}, amqp_connection:update_secret(Conn, RefreshedToken, <<"token refresh">>)).
716+
717+
696718
test_failed_token_refresh_case2(Config) ->
697719
{_Algo, Token} = generate_valid_token(Config, [<<"rabbitmq.configure:vhost4/*">>,
698720
<<"rabbitmq.write:vhost4/*">>,

0 commit comments

Comments
 (0)