From 3718fe32895e3f549d6442b5866f52084c295519 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Wed, 27 Nov 2024 10:41:28 +0100 Subject: [PATCH] Prevent change of username on token refresh --- .../src/rabbit_auth_backend_oauth2.erl | 25 ++++++++++++--- .../test/jwks_SUITE.erl | 31 ++++++++++++++++++- .../test/system_SUITE.erl | 30 +++++++++++++++--- 3 files changed, 76 insertions(+), 10 deletions(-) diff --git a/deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl b/deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl index 27874000b00a..849349f04780 100644 --- a/deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl +++ b/deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl @@ -103,7 +103,7 @@ check_topic_access(#auth_user{impl = DecodedTokenFun}, update_state(AuthUser, NewToken) -> case resolve_resource_server(NewToken) of {error, _} = Err0 -> Err0; - {_, _} = Tuple -> + {ResourceServer, _} = Tuple -> case check_token(NewToken, Tuple) of %% avoid logging the token {refused, {error, {invalid_token, error, _Err, _Stacktrace}}} -> @@ -111,9 +111,18 @@ update_state(AuthUser, NewToken) -> {refused, Err} -> {refused, rabbit_misc:format("Authentication using an OAuth 2/JWT token failed: ~tp", [Err])}; {ok, DecodedToken} -> - Tags = tags_from(DecodedToken), - {ok, AuthUser#auth_user{tags = Tags, - impl = fun() -> DecodedToken end}} + CurToken = AuthUser#auth_user.impl, + case ensure_same_username( + ResourceServer#resource_server.preferred_username_claims, + CurToken(), DecodedToken) of + ok -> + Tags = tags_from(DecodedToken), + {ok, AuthUser#auth_user{tags = Tags, + impl = fun() -> DecodedToken end}}; + {error, mismatch_username_after_token_refresh} -> + {refused, + "Not allowed to change username on refreshed token"} + end end end. @@ -139,7 +148,7 @@ authenticate(_, AuthProps0) -> {refused, "Authentication using an OAuth 2/JWT token failed: provided token is invalid", []}; {refused, Err} -> {refused, "Authentication using an OAuth 2/JWT token failed: ~tp", [Err]}; - {ok, DecodedToken} -> + {ok, DecodedToken} -> Func = fun(Token0) -> Username = username_from( ResourceServer#resource_server.preferred_username_claims, @@ -164,6 +173,12 @@ with_decoded_token(DecodedToken, Fun) -> rabbit_log:error(Msg), Err end. +ensure_same_username(PreferredUsernameClaims, CurrentDecodedToken, NewDecodedToken) -> + CurUsername = username_from(PreferredUsernameClaims, CurrentDecodedToken), + case {CurUsername, username_from(PreferredUsernameClaims, NewDecodedToken)} of + {CurUsername, CurUsername} -> ok; + _ -> {error, mismatch_username_after_token_refresh} + end. validate_token_expiry(#{<<"exp">> := Exp}) when is_integer(Exp) -> Now = os:system_time(seconds), diff --git a/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl index 0a0be86ba833..67874f83e424 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl @@ -58,7 +58,8 @@ groups() -> test_failed_connection_with_a_token_with_insufficient_resource_permission, test_failed_connection_with_algorithm_restriction, test_failed_token_refresh_case1, - test_failed_token_refresh_case2 + test_failed_token_refresh_case2, + cannot_change_username_on_refreshed_token ]}, {no_peer_verification, [], [ {group, happy_path}, @@ -521,6 +522,11 @@ generate_valid_token(Config, Jwk, Scopes, Audience) -> IncludeKid = rabbit_ct_helpers:get_config(Config, include_kid, true), ?UTIL_MOD:sign_token_hs(Token, Jwk, IncludeKid). +generate_valid_token_with_sub(Config, Jwk, Scopes, Sub) -> + Token = ?UTIL_MOD:token_with_sub(?UTIL_MOD:fixture_token_with_scopes(Scopes), Sub), + IncludeKid = rabbit_ct_helpers:get_config(Config, include_kid, true), + ?UTIL_MOD:sign_token_hs(Token, Jwk, IncludeKid). + generate_valid_token_with_extra_fields(Config, ExtraFields) -> Jwk = case rabbit_ct_helpers:get_config(Config, fixture_jwk) of @@ -937,6 +943,29 @@ test_failed_token_refresh_case2(Config) -> close_connection(Conn). +cannot_change_username_on_refreshed_token(Config) -> + Jwk = + case get_config(Config, fixture_jwk) of + undefined -> ?UTIL_MOD:fixture_jwk(); + Value -> Value + end, + {_, CurToken} = generate_valid_token(Config, Jwk, <<"oldUsername">>, [ + <<"rabbitmq.configure:vhost4/*">>, + <<"rabbitmq.write:vhost4/*">>, + <<"rabbitmq.read:vhost4/*">>]), + Conn = open_unmanaged_connection(Config, 0, <<"vhost4">>, + <<"oldUsername">>, CurToken), + + {_, RefreshToken} = generate_valid_token_with_sub(Config, Jwk, <<"newUsername">>, + [<<"rabbitmq.configure:vhost4/*">>, + <<"rabbitmq.write:vhost4/*">>, + <<"rabbitmq.read:vhost4/*">>]), + + %% the error is communicated asynchronously via a connection-level error + ?assertException(exit, _, amqp_connection:update_secret(Conn, RefreshToken, + <<"token refresh">>)). + + test_failed_connection_with_algorithm_restriction(Config) -> {_Algo, Token} = get_config(Config, fixture_jwt), ?assertMatch({error, {auth_failure, _}}, diff --git a/deps/rabbitmq_auth_backend_oauth2/test/system_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/system_SUITE.erl index 8ba8eb33575a..45a2f2046a14 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/system_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/system_SUITE.erl @@ -52,7 +52,8 @@ groups() -> {token_refresh, [], [ test_failed_token_refresh_case1, - test_failed_token_refresh_case2 + test_failed_token_refresh_case2, + refreshed_token_cannot_change_username ]}, {extra_scopes_source, [], [ @@ -323,21 +324,33 @@ preconfigure_node(Config) -> rabbit_ct_helpers:set_config(Config, {fixture_jwk, Jwk}). +generate_valid_token_with_sub(Config, Sub) -> + generate_valid_token(Config, + ?UTIL_MOD:full_permission_scopes(), undefined, Sub). + generate_valid_token(Config) -> generate_valid_token(Config, ?UTIL_MOD:full_permission_scopes()). generate_valid_token(Config, Scopes) -> - generate_valid_token(Config, Scopes, undefined). + generate_valid_token(Config, Scopes, undefined, undefined). generate_valid_token(Config, Scopes, Audience) -> + generate_valid_token(Config, Scopes, Audience, undefined). + +generate_valid_token(Config, Scopes, Audience, Sub) -> Jwk = case rabbit_ct_helpers:get_config(Config, fixture_jwk) of undefined -> ?UTIL_MOD:fixture_jwk(); Value -> Value end, - Token = case Audience of + Token0 = case Audience of undefined -> ?UTIL_MOD:fixture_token_with_scopes(Scopes); - DefinedAudience -> maps:put(<<"aud">>, DefinedAudience, ?UTIL_MOD:fixture_token_with_scopes(Scopes)) + DefinedAudience -> maps:put(<<"aud">>, DefinedAudience, + ?UTIL_MOD:fixture_token_with_scopes(Scopes)) end, + Token = case Sub of + undefined -> Token0; + _ -> maps:put(<<"sub">>, Sub, Token0) + end, ?UTIL_MOD:sign_token_hs(Token, Jwk). generate_valid_token_with_extra_fields(Config, ExtraFields) -> @@ -913,6 +926,15 @@ test_failed_token_refresh_case1(Config) -> close_connection(Conn). +refreshed_token_cannot_change_username(Config) -> + {_, Token} = generate_valid_token_with_sub(Config, <<"username">>), + Conn = open_unmanaged_connection(Config, 0, <<"vhost4">>, <<"username">>, Token), + {_, RefreshedToken} = generate_valid_token_with_sub(Config, <<"username2">>), + + %% the error is communicated asynchronously via a connection-level error + ?assertException(exit, {{nodedown,not_allowed},_}, amqp_connection:update_secret(Conn, RefreshedToken, <<"token refresh">>)). + + test_failed_token_refresh_case2(Config) -> {_Algo, Token} = generate_valid_token(Config, [<<"rabbitmq.configure:vhost4/*">>, <<"rabbitmq.write:vhost4/*">>,