From b67fca391127ad6aa3345aa67a7460a10a856c52 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Wed, 3 Feb 2021 18:20:24 +0300 Subject: [PATCH] MSPF-608: Drop implicit access claim decoding on verify (#23) --- src/uac.erl | 32 +++++++++++------------------ src/uac_authorizer_jwt.erl | 42 ++++++++++++++++++++------------------ test/uac_tests_SUITE.erl | 24 +++------------------- 3 files changed, 37 insertions(+), 61 deletions(-) diff --git a/src/uac.erl b/src/uac.erl index 9bbc894..9d31e2e 100644 --- a/src/uac.erl +++ b/src/uac.erl @@ -15,7 +15,6 @@ %%API -export([configure/1]). --export([get_acl/1]). -export([authorize_api_key/2]). -export([authorize_operation/2]). -export([authorize_operation/3]). @@ -32,8 +31,7 @@ -type configuration() :: configuration(any()). -type verification_opts() :: #{ - check_expired_as_of => genlib_time:ts(), - domains_to_decode => [domain_name()] + check_expired_as_of => genlib_time:ts() }. -type api_key() :: binary(). @@ -83,17 +81,21 @@ authorize_api_key(bearer, Token, VerificationOpts) -> %% --spec authorize_operation(uac_conf:operation_access_scopes(), context()) -> ok | {error, unauthorized}. +-spec authorize_operation(uac_conf:operation_access_scopes(), context()) -> + ok | {error, unauthorized} | {error, {acl, _Reason}}. authorize_operation(AccessScope, Context) -> authorize_operation(AccessScope, Context, uac_conf:get_domain_name()). --spec authorize_operation(uac_conf:operation_access_scopes(), context(), domain_name()) -> ok | {error, unauthorized}. -authorize_operation(AccessScope, {_, _, Claims, _}, Domain) -> - ACL = get_acl(Claims, Domain), - authorize_operation_(AccessScope, ACL). +-spec authorize_operation(uac_conf:operation_access_scopes(), context(), domain_name()) -> + ok | {error, unauthorized} | {error, {acl, _Reason}}. +authorize_operation(AccessScope, Context, Domain) -> + case uac_authorizer_jwt:get_acl(Domain, Context) of + {ok, ACL} -> + authorize_operation_(AccessScope, ACL); + {error, Reason} -> + {error, {acl, Reason}} + end. -authorize_operation_(_, undefined) -> - {error, unauthorized}; authorize_operation_(AccessScope, ACL) -> case lists:all( @@ -109,16 +111,6 @@ authorize_operation_(AccessScope, ACL) -> {error, unauthorized} end. --spec get_acl(context()) -> undefined | uac_acl:t(). -get_acl({_, _, Claims, _}) -> - get_acl(Claims, uac_conf:get_domain_name()). - -get_acl(Claims, Domain) -> - case genlib_map:get(<<"resource_access">>, Claims) of - undefined -> undefined; - DomainRoles when is_map(DomainRoles) -> genlib_map:get(Domain, DomainRoles) - end. - %% % App %% diff --git a/src/uac_authorizer_jwt.erl b/src/uac_authorizer_jwt.erl index ea7d96a..9ee60be 100644 --- a/src/uac_authorizer_jwt.erl +++ b/src/uac_authorizer_jwt.erl @@ -17,6 +17,8 @@ -export([get_subject_email/1]). -export([set_subject_email/2]). -export([get_expires_at/1]). +-export([get_acl/1]). +-export([get_acl/2]). -export([get_subject_id/1]). -export([get_claims/1]). @@ -288,7 +290,7 @@ verify_with_key(JWK, ExpandedToken, VerificationOpts, Metadata) -> case jose_jwt:verify(JWK, ExpandedToken) of {true, #jose_jwt{fields = Claims}, _JWS} -> _ = validate_claims(Claims, VerificationOpts), - get_result(Claims, VerificationOpts, Metadata); + get_result(Claims, Metadata); {false, _JWT, _JWS} -> {error, invalid_signature} end. @@ -302,13 +304,13 @@ validate_claims(Claims, [{Name, Claim, Validator} | Rest], VerificationOpts) -> validate_claims(Claims, [], _) -> Claims. -get_result(Claims, VerificationOpts, Metadata) -> +get_result(Claims, Metadata) -> try #{ ?CLAIM_TOKEN_ID := TokenID, ?CLAIM_SUBJECT_ID := SubjectID } = Claims, - {ok, {TokenID, SubjectID, decode_roles(Claims, VerificationOpts), Metadata}} + {ok, {TokenID, SubjectID, Claims, Metadata}} catch error:{badarg, _} = Reason -> throw({invalid_token, {malformed_acl, Reason}}) @@ -408,6 +410,23 @@ set_subject_email(SubjectID, Claims) -> false = maps:is_key(?CLAIM_SUBJECT_EMAIL, Claims), Claims#{?CLAIM_SUBJECT_EMAIL => SubjectID}. +-spec get_acl(t()) -> {ok, uac_acl:t()} | {error, missing | {invalid, _Reason}}. +get_acl(Context) -> + get_acl(uac_conf:get_domain_name(), Context). + +-spec get_acl(domain_name(), t()) -> {ok, uac_acl:t()} | {error, missing | {invalid, _Reason}}. +get_acl(Domain, {_Id, _Subject, Claims, _Metadata}) -> + case genlib_map:get(?CLAIM_ACCESS, Claims) of + #{Domain := #{<<"roles">> := Roles}} -> + try + {ok, uac_acl:decode(Roles)} + catch + error:Reason -> {error, {invalid, Reason}} + end; + _ -> + {error, missing} + end. + %% encode_roles(DomainRoles) when is_map(DomainRoles) andalso map_size(DomainRoles) > 0 -> @@ -416,23 +435,6 @@ encode_roles(DomainRoles) when is_map(DomainRoles) andalso map_size(DomainRoles) encode_roles(_) -> #{}. -decode_roles(Claims, VerificationOpts) -> - case genlib_map:get(?CLAIM_ACCESS, Claims) of - undefined -> - Claims; - ResourceAcceess when is_map(ResourceAcceess) -> - % @FIXME This is a temporary solution - % rework interface the way this line won't be needed - Domains = maps:get(domains_to_decode, VerificationOpts, maps:keys(ResourceAcceess)), - DomainRoles = maps:map( - fun(_, #{<<"roles">> := Roles}) -> uac_acl:decode(Roles) end, - maps:with(Domains, ResourceAcceess) - ), - Claims#{?CLAIM_ACCESS => DomainRoles}; - _ -> - throw({invalid_token, {invalid, acl}}) - end. - %% insert_key(Keyname, KeyInfo = #{kid := KID}) -> diff --git a/test/uac_tests_SUITE.erl b/test/uac_tests_SUITE.erl index 517f430..5548184 100644 --- a/test/uac_tests_SUITE.erl +++ b/test/uac_tests_SUITE.erl @@ -20,8 +20,7 @@ unknown_resources_ok_test/1, unknown_resources_fail_encode_test/1, cant_authorize_without_resource_access/1, - no_expiration_claim_allowed/1, - configure_processed_domains_test/1 + no_expiration_claim_allowed/1 ]). -type test_case_name() :: atom(). @@ -49,8 +48,7 @@ all() -> unknown_resources_ok_test, unknown_resources_fail_encode_test, cant_authorize_without_resource_access, - no_expiration_claim_allowed, - configure_processed_domains_test + no_expiration_claim_allowed ]. -spec init_per_suite(config()) -> config(). @@ -176,7 +174,7 @@ unknown_resources_ok_test(_) -> cant_authorize_without_resource_access(_) -> {ok, Token} = issue_token(#{}, unlimited), {ok, AccessContext} = uac:authorize_api_key(<<"Bearer ", Token/binary>>, #{}), - {error, unauthorized} = uac:authorize_operation([], AccessContext). + {error, {acl, missing}} = uac:authorize_operation([], AccessContext). -spec unknown_resources_fail_encode_test(config()) -> _. unknown_resources_fail_encode_test(_) -> @@ -189,22 +187,6 @@ no_expiration_claim_allowed(_) -> {ok, Token} = uac_authorizer_jwt:issue(unique_id(), PartyID, #{}, test), {ok, _} = uac:authorize_api_key(<<"Bearer ", Token/binary>>, #{}). --spec configure_processed_domains_test(config()) -> _. -configure_processed_domains_test(_) -> - ACL = ?TEST_SERVICE_ACL(read), - Domain1 = <<"api-1">>, - Domain2 = <<"api-2">>, - {ok, Token} = issue_token( - #{ - Domain1 => uac_acl:from_list(ACL), - Domain2 => uac_acl:from_list(ACL) - }, - unlimited - ), - {ok, AccessContext} = uac:authorize_api_key(<<"Bearer ", Token/binary>>, #{domains_to_decode => [Domain1]}), - ok = uac:authorize_operation([], AccessContext, Domain1), - {error, unauthorized} = uac:authorize_operation([], AccessContext, Domain2). - %% issue_token(DomainRoles, Expiration) when is_map(DomainRoles) ->