From 3d8629b2a303ff2789335875da70aa43eede3c7f Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Wed, 27 Nov 2024 10:41:28 +0100 Subject: [PATCH 1/5] Prevent change of username on token refresh (cherry picked from commit 3718fe32895e3f549d6442b5866f52084c295519) # Conflicts: # deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl --- .../src/rabbit_auth_backend_oauth2.erl | 52 +++++++++++++++++++ .../test/jwks_SUITE.erl | 31 ++++++++++- .../test/system_SUITE.erl | 30 +++++++++-- 3 files changed, 108 insertions(+), 5 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 a43212655b87..ab30459252a8 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 @@ -98,6 +98,7 @@ check_topic_access(#auth_user{impl = DecodedTokenFun}, end). update_state(AuthUser, NewToken) -> +<<<<<<< HEAD case check_token(NewToken) of %% avoid logging the token {error, _} = E -> E; @@ -111,6 +112,32 @@ update_state(AuthUser, NewToken) -> {ok, AuthUser#auth_user{tags = Tags, impl = fun() -> DecodedToken end}} end. +======= + case resolve_resource_server(NewToken) of + {error, _} = Err0 -> Err0; + {ResourceServer, _} = Tuple -> + case check_token(NewToken, Tuple) of + %% avoid logging the token + {refused, {error, {invalid_token, error, _Err, _Stacktrace}}} -> + {refused, "Authentication using an OAuth 2/JWT token failed: provided token is invalid"}; + {refused, Err} -> + {refused, rabbit_misc:format("Authentication using an OAuth 2/JWT token failed: ~tp", [Err])}; + {ok, DecodedToken} -> + 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. +>>>>>>> 3718fe3289 (Prevent change of username on token refresh) expiry_timestamp(#auth_user{impl = DecodedTokenFun}) -> case DecodedTokenFun() of @@ -145,8 +172,27 @@ authenticate(_, AuthProps0) -> case with_decoded_token(DecodedToken, Func) of {error, Err} -> {refused, "Authentication using an OAuth 2/JWT token failed: ~tp", [Err]}; +<<<<<<< HEAD Else -> Else +======= + {ok, DecodedToken} -> + Func = fun(Token0) -> + Username = username_from( + ResourceServer#resource_server.preferred_username_claims, + Token0), + Tags = tags_from(Token0), + {ok, #auth_user{username = Username, + tags = Tags, + impl = fun() -> Token0 end}} + end, + case with_decoded_token(DecodedToken, Func) of + {error, Err} -> + {refused, "Authentication using an OAuth 2/JWT token failed: ~tp", [Err]}; + Else -> + Else + end +>>>>>>> 3718fe3289 (Prevent change of username on token refresh) end end. @@ -157,6 +203,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 db4de4d8a677..93fa28eb0885 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl @@ -45,7 +45,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}, @@ -531,6 +532,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 undefined -> ?UTIL_MOD:fixture_jwk(); @@ -912,6 +918,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} = rabbit_ct_helpers: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 e17a76281411..692a9e2ab15d 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, [], [ @@ -312,21 +313,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) -> @@ -693,6 +706,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/*">>, From 8ce0bb154bfae97d0b315d3cfe1a3c25cf2cc986 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Thu, 28 Nov 2024 08:39:31 +0100 Subject: [PATCH 2/5] Resolve conflicts --- .../src/rabbit_auth_backend_oauth2.erl | 96 ++++++------------- .../src/uaa_jwt.erl | 2 + 2 files changed, 33 insertions(+), 65 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 ab30459252a8..cdfe6e15056e 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 @@ -20,8 +20,9 @@ % for testing -export([post_process_payload/2, get_expanded_scopes/2]). - +-import(uaa_jwt, [resolve_resource_server_id/1]). -import(rabbit_data_coercion, [to_map/1]). +-import(rabbit_oauth2_config, [get_preferred_username_claims/1]). -ifdef(TEST). -compile(export_all). @@ -98,46 +99,28 @@ check_topic_access(#auth_user{impl = DecodedTokenFun}, end). update_state(AuthUser, NewToken) -> -<<<<<<< HEAD - case check_token(NewToken) of - %% avoid logging the token - {error, _} = E -> E; - {refused, {error, {invalid_token, error, _Err, _Stacktrace}}} -> - {refused, "Authentication using an OAuth 2/JWT token failed: provided token is invalid"}; - {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}} - end. -======= - case resolve_resource_server(NewToken) of - {error, _} = Err0 -> Err0; - {ResourceServer, _} = Tuple -> - case check_token(NewToken, Tuple) of - %% avoid logging the token - {refused, {error, {invalid_token, error, _Err, _Stacktrace}}} -> - {refused, "Authentication using an OAuth 2/JWT token failed: provided token is invalid"}; - {refused, Err} -> - {refused, rabbit_misc:format("Authentication using an OAuth 2/JWT token failed: ~tp", [Err])}; - {ok, DecodedToken} -> - 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 + case check_token(NewToken) of + %% avoid logging the token + {error, _} = E -> E; + {refused, {error, {invalid_token, error, _Err, _Stacktrace}}} -> + {refused, "Authentication using an OAuth 2/JWT token failed: provided token is invalid"}; + {refused, Err} -> + {refused, rabbit_misc:format("Authentication using an OAuth 2/JWT token failed: ~tp", [Err])}; + {ok, DecodedToken} -> + ResourceServerId = resolve_resource_server_id(DecodedToken), + CurToken = AuthUser#auth_user.impl, + case ensure_same_username( + get_preferred_username_claims(ResourceServerId), + 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. ->>>>>>> 3718fe3289 (Prevent change of username on token refresh) expiry_timestamp(#auth_user{impl = DecodedTokenFun}) -> case DecodedTokenFun() of @@ -162,37 +145,20 @@ authenticate(_, AuthProps0) -> {refused, "Authentication using an OAuth 2/JWT token failed: ~tp", [Err]}; {ok, DecodedToken} -> Func = fun(Token0) -> - Username = username_from(rabbit_oauth2_config:get_preferred_username_claims(), Token0), - Tags = tags_from(Token0), - - {ok, #auth_user{username = Username, - tags = Tags, - impl = fun() -> Token0 end}} - end, + ResourceServerId = resolve_resource_server_id(Token0), + Username = username_from( + get_preferred_username_claims(ResourceServerId), + Token0), + Tags = tags_from(Token0), + {ok, #auth_user{username = Username, + tags = Tags, + impl = fun() -> Token0 end}} + end, case with_decoded_token(DecodedToken, Func) of {error, Err} -> {refused, "Authentication using an OAuth 2/JWT token failed: ~tp", [Err]}; -<<<<<<< HEAD Else -> Else -======= - {ok, DecodedToken} -> - Func = fun(Token0) -> - Username = username_from( - ResourceServer#resource_server.preferred_username_claims, - Token0), - Tags = tags_from(Token0), - {ok, #auth_user{username = Username, - tags = Tags, - impl = fun() -> Token0 end}} - end, - case with_decoded_token(DecodedToken, Func) of - {error, Err} -> - {refused, "Authentication using an OAuth 2/JWT token failed: ~tp", [Err]}; - Else -> - Else - end ->>>>>>> 3718fe3289 (Prevent change of username on token refresh) end end. diff --git a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl index eafaa2122c74..8416fdde8f18 100644 --- a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl +++ b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl @@ -9,6 +9,7 @@ -export([add_signing_key/3, decode_and_verify/1, get_jwk/2, + resolve_resource_server_id/1, verify_signing_key/2]). -export([client_id/1, sub/1, client_id/2, sub/2]). @@ -79,6 +80,7 @@ decode_and_verify(Token) -> end end. +-spec resolve_resource_server_id(binary()) -> binary() | {error, term()}. resolve_resource_server_id(Token) -> case uaa_jwt_jwt:get_aud(Token) of {error, _} = Error -> From ad84dd0dca67fab16582e7e9e32f9de8f23c031a Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Thu, 28 Nov 2024 08:43:41 +0100 Subject: [PATCH 3/5] Import missing function --- deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl index 93fa28eb0885..bc1256da8b9d 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl @@ -17,6 +17,10 @@ open_unmanaged_connection/4, open_unmanaged_connection/5, close_connection_and_channel/2]). -import(rabbit_mgmt_test_util, [amqp_port/1]). +-import(rabbit_ct_helpers, [ + set_config/2, + get_config/2, get_config/3 +]). all() -> [ From 7c37546858dfd60e3ca1111fa15fd98f80862a55 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Fri, 29 Nov 2024 09:30:30 +0100 Subject: [PATCH 4/5] Fix dialyzer errors --- deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl | 11 +++++++++-- selenium/.gitignore | 7 +++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl index 8416fdde8f18..c7288bef4e81 100644 --- a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl +++ b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl @@ -80,13 +80,20 @@ decode_and_verify(Token) -> end end. --spec resolve_resource_server_id(binary()) -> binary() | {error, term()}. -resolve_resource_server_id(Token) -> +-spec resolve_resource_server_id(binary()|map()) -> binary() | {error, term()}. +resolve_resource_server_id(Token) when is_binary(Token) -> case uaa_jwt_jwt:get_aud(Token) of {error, _} = Error -> Error; {ok, Audience} -> rabbit_oauth2_config:get_resource_server_id_for_audience(Audience) + end; +resolve_resource_server_id(Token) when is_map(Token) -> + case maps:get(<<"aud">>, Token, undefined) of + undefined -> + {error, audience_not_found_in_token}; + Audience -> + rabbit_oauth2_config:get_resource_server_id_for_audience(Audience) end. -spec get_jwk(binary(), oauth_provider_id()) -> {ok, map()} | {error, term()}. diff --git a/selenium/.gitignore b/selenium/.gitignore index 63c36b351eb4..ee78120fba98 100644 --- a/selenium/.gitignore +++ b/selenium/.gitignore @@ -7,3 +7,10 @@ suites/screens/* test/oauth/*/h2/*.trace.db test/oauth/*/h2/*.lock.db */target/* +tls-gen +test/*/certs/*.pem +test/*/certs/*.p12 +test/*/certs/*.jks +test/*/*/*.pem +test/*/*/*.p12 +test/*/*/*.jks \ No newline at end of file From c46a22cb8ec8cc1dd3574fe85e60045c4fa889d8 Mon Sep 17 00:00:00 2001 From: Marcial Rosales Date: Fri, 29 Nov 2024 10:03:48 +0100 Subject: [PATCH 5/5] Fix test failure --- deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl index c7288bef4e81..cf14486c6ead 100644 --- a/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl +++ b/deps/rabbitmq_auth_backend_oauth2/src/uaa_jwt.erl @@ -81,21 +81,22 @@ decode_and_verify(Token) -> end. -spec resolve_resource_server_id(binary()|map()) -> binary() | {error, term()}. -resolve_resource_server_id(Token) when is_binary(Token) -> - case uaa_jwt_jwt:get_aud(Token) of - {error, _} = Error -> - Error; - {ok, Audience} -> - rabbit_oauth2_config:get_resource_server_id_for_audience(Audience) - end; resolve_resource_server_id(Token) when is_map(Token) -> case maps:get(<<"aud">>, Token, undefined) of undefined -> {error, audience_not_found_in_token}; Audience -> rabbit_oauth2_config:get_resource_server_id_for_audience(Audience) + end; +resolve_resource_server_id(Token) -> + case uaa_jwt_jwt:get_aud(Token) of + {error, _} = Error -> + Error; + {ok, Audience} -> + rabbit_oauth2_config:get_resource_server_id_for_audience(Audience) end. + -spec get_jwk(binary(), oauth_provider_id()) -> {ok, map()} | {error, term()}. get_jwk(KeyId, OAuthProviderId) -> get_jwk(KeyId, OAuthProviderId, true).