Skip to content

Commit

Permalink
Prevent change of username on token refresh
Browse files Browse the repository at this point in the history
  • Loading branch information
@MarcialRosales
MarcialRosales committed Nov 27, 2024
1 parent 1679137 commit 3718fe3
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 10 deletions.
25 changes: 20 additions & 5 deletions deps/rabbitmq_auth_backend_oauth2/src/rabbit_auth_backend_oauth2.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,26 @@ 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}}} ->
{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}}
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.

Expand All @@ -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,
Expand All @@ -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),
Expand Down
31 changes: 30 additions & 1 deletion deps/rabbitmq_auth_backend_oauth2/test/jwks_SUITE.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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, _}},
Expand Down
30 changes: 26 additions & 4 deletions deps/rabbitmq_auth_backend_oauth2/test/system_SUITE.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, [], [
Expand Down Expand Up @@ -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) ->
Expand Down Expand Up @@ -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/*">>,
Expand Down

0 comments on commit 3718fe3

Please sign in to comment.