From 0ded36364ef7dcf1c5f28e242a87216d29535da4 Mon Sep 17 00:00:00 2001 From: Asaf Shen Date: Sun, 16 Jun 2024 12:49:04 +0300 Subject: [PATCH 1/2] fix get teannts with descope tenants claim --- descope/internal/auth/auth.go | 8 ++++---- descope/internal/auth/auth_test.go | 12 +++++++++--- descope/internal/auth/utils.go | 7 +++---- descope/types.go | 7 +++++++ descope/types_test.go | 16 +++++++++++++++- 5 files changed, 38 insertions(+), 12 deletions(-) diff --git a/descope/internal/auth/auth.go b/descope/internal/auth/auth.go index b42e8a3b..1b2f986a 100644 --- a/descope/internal/auth/auth.go +++ b/descope/internal/auth/auth.go @@ -821,13 +821,13 @@ func getAuthorizationClaimItems(token *descope.Token, tenant string, claim strin } } else { var claimValue []interface{} - if token.Claims[claimDescopeCurrentTenant] == tenant && len(token.GetTenants()) == 0 { + if v, ok := token.GetTenantValue(tenant, claim).([]interface{}); ok { + claimValue = v + } else if token.Claims[descope.ClaimDescopeCurrentTenant] == tenant { // The token may have the current tenant in the "dct" claim and without the "tenants" claim if v, ok := token.Claims[claim].([]interface{}); ok { claimValue = v } - } else if v, ok := token.GetTenantValue(tenant, claim).([]interface{}); ok { - claimValue = v } for i := range claimValue { @@ -846,7 +846,7 @@ func getAuthorizationClaimItems(token *descope.Token, tenant string, claim strin } func isAssociatedWithTenant(token *descope.Token, tenant string) bool { - return slices.Contains(token.GetTenants(), tenant) || (token.Claims != nil && token.Claims[claimDescopeCurrentTenant] == tenant) + return slices.Contains(token.GetTenants(), tenant) || (token.Claims != nil && token.Claims[descope.ClaimDescopeCurrentTenant] == tenant) } func getPendingRefFromResponse(httpResponse *api.HTTPResponse) (*descope.EnchantedLinkResponse, error) { diff --git a/descope/internal/auth/auth_test.go b/descope/internal/auth/auth_test.go index b2f3ce11..195fe3dc 100644 --- a/descope/internal/auth/auth_test.go +++ b/descope/internal/auth/auth_test.go @@ -79,9 +79,9 @@ var ( } mockAuthorizationCurrentTenantToken = &descope.Token{ Claims: map[string]any{ - claimPermissions: permissions, - claimRoles: roles, - claimDescopeCurrentTenant: "t1", + claimPermissions: permissions, + claimRoles: roles, + descope.ClaimDescopeCurrentTenant: "t1", }, } ) @@ -1045,6 +1045,12 @@ func TestValidatePermissions(t *testing.T) { )) } +func TestGetTenants(t *testing.T) { + require.Equal(t, []string{}, mockAuthorizationToken.GetTenants()) + require.Equal(t, []string{"t1"}, mockAuthorizationCurrentTenantToken.GetTenants()) + require.ElementsMatch(t, []string{"kuku", "t1"}, mockAuthorizationTenantToken.GetTenants()) +} + func TestGetMatchedPermissions(t *testing.T) { a, err := newTestAuth(nil, DoOkWithBody(nil, "")) require.NoError(t, err) diff --git a/descope/internal/auth/utils.go b/descope/internal/auth/utils.go index b66404d5..7a71e9df 100644 --- a/descope/internal/auth/utils.go +++ b/descope/internal/auth/utils.go @@ -231,10 +231,9 @@ func newExchangeAccessKeyBody(loginOptions *descope.AccessKeyLoginOptions) *exch } const ( - claimAttributeName = "drn" - claimPermissions = "permissions" - claimRoles = "roles" - claimDescopeCurrentTenant = "dct" + claimAttributeName = "drn" + claimPermissions = "permissions" + claimRoles = "roles" ) var ( diff --git a/descope/types.go b/descope/types.go index ae581bdf..fc821830 100644 --- a/descope/types.go +++ b/descope/types.go @@ -187,6 +187,9 @@ type Token struct { func (to *Token) GetTenants() []string { tenants := to.getTenants() + if len(tenants) == 0 && to.Claims != nil && to.Claims[ClaimDescopeCurrentTenant] != nil { + return []string{to.Claims[ClaimDescopeCurrentTenant].(string)} + } return maps.Keys(tenants) } @@ -218,6 +221,9 @@ func (to *Token) IsPermitted(permission string) bool { func (to *Token) IsPermittedPerTenant(tenant string, permission string) bool { permitted := false tenants := to.getTenants() + if to.Claims[ClaimDescopeCurrentTenant] == tenant && len(tenants) == 0 { + return to.IsPermitted(permission) + } tPermissions, ok := tenants[tenant] if ok { if tPermissionsMap, ok := tPermissions.(map[string]any); ok { @@ -917,6 +923,7 @@ const ( ContextUserIDPropertyKey ContextKey = ContextUserIDProperty ClaimAuthorizedTenants = "tenants" ClaimAuthorizedGlobalPermissions = "permissions" + ClaimDescopeCurrentTenant = "dct" EnvironmentVariableProjectID = "DESCOPE_PROJECT_ID" EnvironmentVariablePublicKey = "DESCOPE_PUBLIC_KEY" diff --git a/descope/types_test.go b/descope/types_test.go index 1bbfb509..2cc38b80 100644 --- a/descope/types_test.go +++ b/descope/types_test.go @@ -92,7 +92,7 @@ func TestGetCreatedTime(t *testing.T) { assert.True(t, r.GetCreatedTime().Equal(now)) } -func TestIsPermittedPerTenant(t *testing.T) { +func TestIsPermittedPerTenantFromTenantsClaim(t *testing.T) { tenantID := "somestring" dt := &Token{ Claims: map[string]any{ @@ -111,6 +111,20 @@ func TestIsPermittedPerTenant(t *testing.T) { assert.False(t, p) } +func TestIsPermittedPerTenantWithCurrentTenant(t *testing.T) { + tenantID := "t1" + dt := &Token{ + Claims: map[string]any{ + ClaimDescopeCurrentTenant: tenantID, + ClaimAuthorizedGlobalPermissions: []any{"a", "b", "c"}, + }, + } + p := dt.IsPermittedPerTenant(tenantID, "a") + assert.True(t, p) + p = dt.IsPermittedPerTenant(tenantID, "d") + assert.False(t, p) +} + func TestIsPermitted(t *testing.T) { dt := &Token{ Claims: map[string]any{ From 7e50eadd878422746f80ce88f696f3e75eb9b832 Mon Sep 17 00:00:00 2001 From: Asaf Shen Date: Sun, 16 Jun 2024 13:12:26 +0300 Subject: [PATCH 2/2] CR fix --- descope/internal/auth/auth.go | 4 +- descope/internal/auth/auth_test.go | 60 +++++++++++++++++++++--------- 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/descope/internal/auth/auth.go b/descope/internal/auth/auth.go index 1b2f986a..53cb8280 100644 --- a/descope/internal/auth/auth.go +++ b/descope/internal/auth/auth.go @@ -823,8 +823,10 @@ func getAuthorizationClaimItems(token *descope.Token, tenant string, claim strin var claimValue []interface{} if v, ok := token.GetTenantValue(tenant, claim).([]interface{}); ok { claimValue = v - } else if token.Claims[descope.ClaimDescopeCurrentTenant] == tenant { + } else if token.Claims[descope.ClaimDescopeCurrentTenant] == tenant && token.Claims[descope.ClaimAuthorizedTenants] == nil { // The token may have the current tenant in the "dct" claim and without the "tenants" claim + // Note: We also must ensure that the tenants claim is not present because in the if "tenants" claim exists, + // the top level claim represents for the project level roles/permissions if v, ok := token.Claims[claim].([]interface{}); ok { claimValue = v } diff --git a/descope/internal/auth/auth_test.go b/descope/internal/auth/auth_test.go index 195fe3dc..f7e18097 100644 --- a/descope/internal/auth/auth_test.go +++ b/descope/internal/auth/auth_test.go @@ -77,13 +77,26 @@ var ( }, }, } - mockAuthorizationCurrentTenantToken = &descope.Token{ + mockAuthorizationCurrentTenantTokenNoTenants = &descope.Token{ Claims: map[string]any{ claimPermissions: permissions, claimRoles: roles, descope.ClaimDescopeCurrentTenant: "t1", }, } + mockAuthorizationCurrentTenantTokenWithTenants = &descope.Token{ + Claims: map[string]any{ + claimPermissions: permissions, + claimRoles: roles, + descope.ClaimAuthorizedTenants: map[string]any{ + "t1": map[string]any{ + claimPermissions: []interface{}{"t1-perm1", "t1-perm2"}, + claimRoles: []interface{}{"t1-role1", "t1-role2"}, + }, + "t2": map[string]any{}, + }, + }, + } ) func readBodyMap(r *http.Request) (m map[string]interface{}, err error) { @@ -1030,9 +1043,12 @@ func TestValidatePermissions(t *testing.T) { require.True(t, a.ValidateTenantPermissions(context.Background(), mockAuthorizationTenantToken, "t1", []string{})) require.False(t, a.ValidateTenantPermissions(context.Background(), mockAuthorizationTenantToken, "t2", []string{})) - require.True(t, a.ValidateTenantPermissions(context.Background(), mockAuthorizationCurrentTenantToken, "t1", []string{"foo"})) - require.False(t, a.ValidateTenantPermissions(context.Background(), mockAuthorizationCurrentTenantToken, "t1", []string{"qux"})) - require.False(t, a.ValidateTenantPermissions(context.Background(), mockAuthorizationCurrentTenantToken, "t2", []string{"foo"})) + require.True(t, a.ValidateTenantPermissions(context.Background(), mockAuthorizationCurrentTenantTokenNoTenants, "t1", []string{"foo"})) + require.False(t, a.ValidateTenantPermissions(context.Background(), mockAuthorizationCurrentTenantTokenNoTenants, "t1", []string{"qux"})) + require.False(t, a.ValidateTenantPermissions(context.Background(), mockAuthorizationCurrentTenantTokenNoTenants, "t2", []string{"foo"})) + + require.True(t, a.ValidateTenantPermissions(context.Background(), mockAuthorizationCurrentTenantTokenWithTenants, "t1", []string{"t1-perm1"})) + require.False(t, a.ValidateTenantPermissions(context.Background(), mockAuthorizationCurrentTenantTokenWithTenants, "t1", []string{"foo"})) // check when the value of the claim is not a map require.False(t, a.ValidateTenantPermissions( @@ -1045,12 +1061,6 @@ func TestValidatePermissions(t *testing.T) { )) } -func TestGetTenants(t *testing.T) { - require.Equal(t, []string{}, mockAuthorizationToken.GetTenants()) - require.Equal(t, []string{"t1"}, mockAuthorizationCurrentTenantToken.GetTenants()) - require.ElementsMatch(t, []string{"kuku", "t1"}, mockAuthorizationTenantToken.GetTenants()) -} - func TestGetMatchedPermissions(t *testing.T) { a, err := newTestAuth(nil, DoOkWithBody(nil, "")) require.NoError(t, err) @@ -1068,8 +1078,11 @@ func TestGetMatchedPermissions(t *testing.T) { require.Equal(t, []string{"foo", "bar"}, a.GetMatchedTenantPermissions(context.Background(), mockAuthorizationTenantToken, "kuku", []string{"foo", "bar"})) require.Equal(t, []string{"foo", "bar"}, a.GetMatchedTenantPermissions(context.Background(), mockAuthorizationTenantToken, "kuku", []string{"foo", "bar", "qux"})) - require.Equal(t, []string{"foo", "bar"}, a.GetMatchedTenantPermissions(context.Background(), mockAuthorizationCurrentTenantToken, "t1", []string{"foo", "bar", "qux"})) - require.Equal(t, []string{}, a.GetMatchedTenantPermissions(context.Background(), mockAuthorizationCurrentTenantToken, "t2", []string{"foo", "bar", "qux"})) + require.Equal(t, []string{"foo", "bar"}, a.GetMatchedTenantPermissions(context.Background(), mockAuthorizationCurrentTenantTokenNoTenants, "t1", []string{"foo", "bar", "qux"})) + require.Equal(t, []string{}, a.GetMatchedTenantPermissions(context.Background(), mockAuthorizationCurrentTenantTokenNoTenants, "t2", []string{"foo", "bar", "qux"})) + + require.Equal(t, []string{"t1-perm1"}, a.GetMatchedTenantPermissions(context.Background(), mockAuthorizationCurrentTenantTokenWithTenants, "t1", []string{"t1-perm1"})) + require.Equal(t, []string{}, a.GetMatchedTenantPermissions(context.Background(), mockAuthorizationCurrentTenantTokenWithTenants, "t1", []string{"foo"})) } func TestValidateRoles(t *testing.T) { @@ -1102,9 +1115,12 @@ func TestValidateRoles(t *testing.T) { require.True(t, a.ValidateTenantRoles(context.Background(), mockAuthorizationTenantToken, "t1", []string{})) require.False(t, a.ValidateTenantRoles(context.Background(), mockAuthorizationTenantToken, "t2", []string{})) - require.True(t, a.ValidateTenantRoles(context.Background(), mockAuthorizationCurrentTenantToken, "t1", []string{"abc"})) - require.False(t, a.ValidateTenantRoles(context.Background(), mockAuthorizationCurrentTenantToken, "t1", []string{"tuv"})) - require.False(t, a.ValidateTenantRoles(context.Background(), mockAuthorizationCurrentTenantToken, "t2", []string{"abc"})) + require.True(t, a.ValidateTenantRoles(context.Background(), mockAuthorizationCurrentTenantTokenNoTenants, "t1", []string{"abc"})) + require.False(t, a.ValidateTenantRoles(context.Background(), mockAuthorizationCurrentTenantTokenNoTenants, "t1", []string{"tuv"})) + require.False(t, a.ValidateTenantRoles(context.Background(), mockAuthorizationCurrentTenantTokenNoTenants, "t2", []string{"abc"})) + + require.True(t, a.ValidateTenantRoles(context.Background(), mockAuthorizationCurrentTenantTokenWithTenants, "t1", []string{"t1-role1"})) + require.False(t, a.ValidateTenantRoles(context.Background(), mockAuthorizationCurrentTenantTokenWithTenants, "t1", []string{"abc"})) } func TestGetMatchedRoles(t *testing.T) { @@ -1124,8 +1140,18 @@ func TestGetMatchedRoles(t *testing.T) { require.Equal(t, []string{"abc", "xyz"}, a.GetMatchedTenantRoles(context.Background(), mockAuthorizationTenantToken, "kuku", []string{"abc", "xyz"})) require.Equal(t, []string{"abc", "xyz"}, a.GetMatchedTenantRoles(context.Background(), mockAuthorizationTenantToken, "kuku", []string{"abc", "xyz", "tuv"})) - require.Equal(t, []string{"abc", "xyz"}, a.GetMatchedTenantRoles(context.Background(), mockAuthorizationCurrentTenantToken, "t1", []string{"abc", "xyz", "tuv"})) - require.Equal(t, []string{}, a.GetMatchedTenantRoles(context.Background(), mockAuthorizationCurrentTenantToken, "t2", []string{"abc", "xyz", "tuv"})) + require.Equal(t, []string{"abc", "xyz"}, a.GetMatchedTenantRoles(context.Background(), mockAuthorizationCurrentTenantTokenNoTenants, "t1", []string{"abc", "xyz", "tuv"})) + require.Equal(t, []string{}, a.GetMatchedTenantRoles(context.Background(), mockAuthorizationCurrentTenantTokenNoTenants, "t2", []string{"abc", "xyz", "tuv"})) + + require.Equal(t, []string{"t1-role1"}, a.GetMatchedTenantRoles(context.Background(), mockAuthorizationCurrentTenantTokenWithTenants, "t1", []string{"t1-role1"})) + require.Equal(t, []string{}, a.GetMatchedTenantRoles(context.Background(), mockAuthorizationCurrentTenantTokenWithTenants, "t1", []string{"abc"})) +} + +func TestGetTenants(t *testing.T) { + require.Equal(t, []string{}, mockAuthorizationToken.GetTenants()) + require.Equal(t, []string{"t1"}, mockAuthorizationCurrentTenantTokenNoTenants.GetTenants()) + require.ElementsMatch(t, []string{"kuku", "t1"}, mockAuthorizationTenantToken.GetTenants()) + require.ElementsMatch(t, []string{"t1", "t2"}, mockAuthorizationCurrentTenantTokenWithTenants.GetTenants()) } func TestMe(t *testing.T) {