Fixes 2450: remove org admin skip setting (#444)

* Fixes 2450: remove org admin skip setting * update org admin script for mac
content-services · Oct 30, 2023 · b0094d2 · b0094d2
1 parent 148384a
commit b0094d2
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 24 deletions.
diff --git a/deployments/deployment.yaml b/deployments/deployment.yaml
@@ -215,8 +215,6 @@ objects:
                     name: cs-pulp-admin-password
                     key: password
                     optional: true
-              - name: RBAC_ORG_ADMIN_SKIP
-                value: ${RBAC_ORG_ADMIN_SKIP}
               - name: LOGGING_LEVEL
                 value: ${{LOGGING_LEVEL}}
               - name: CLIENTS_RBAC_BASE_URL
@@ -413,9 +411,6 @@ parameters:
     description: Whether the Admin Tasks feature should be turned on
   - name: FEATURES_ADMIN_TASKS_ACCOUNTS
     description: Comma separated list of account number that can access the feature
-  - name: RBAC_ORG_ADMIN_SKIP
-    description: skip rbac check if the user is an org admin
-    default: 'false'
   - name: OPTIONS_ALWAYS_RUN_CRON_TASKS
     description: Option to make testing nightly snapshotting & introspection easier
     default: 'false'

diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -36,7 +36,6 @@ type Configuration struct {
 	NotificationsClient cloudevents.Client `mapstructure:"notification_client"`
 	Tasking             Tasking            `mapstructure:"tasking"`
 	Features            FeatureSet         `mapstructure:"features"`
-	RbacOrgAdminSkip    bool               `mapstructure:"rbac_org_admin_skip"`
 }
 
 type Clients struct {
@@ -204,7 +203,6 @@ func setDefaults(v *viper.Viper) {
 	v.SetDefault("Loaded", true)
 	// In viper you have to set defaults, otherwise loading from ENV doesn't work
 	//   without a config file present
-	v.SetDefault("rbac_org_admin_skip", false)
 	v.SetDefault("database.host", "")
 	v.SetDefault("database.port", "")
 	v.SetDefault("database.user", "")

diff --git a/pkg/rbac/client_wrapper.go b/pkg/rbac/client_wrapper.go
@@ -27,7 +27,6 @@ import (
 
 	"github.com/RedHatInsights/rbac-client-go"
 	"github.com/content-services/content-sources-backend/pkg/cache"
-	"github.com/content-services/content-sources-backend/pkg/config"
 	"github.com/redhatinsights/platform-go-middlewares/identity"
 	"github.com/rs/zerolog"
 )
@@ -101,9 +100,5 @@ func (r *ClientWrapperImpl) Allowed(ctx context.Context, resource Resource, verb
 }
 
 func skipRbacCheck(ctx context.Context) bool {
-	if config.Get().RbacOrgAdminSkip {
-		return identity.Get(ctx).Identity.User.OrgAdmin
-	} else {
-		return false
-	}
+	return identity.Get(ctx).Identity.User.OrgAdmin
 }
diff --git a/pkg/rbac/client_wrapper_test.go b/pkg/rbac/client_wrapper_test.go
@@ -8,8 +8,10 @@ import (
 	"github.com/RedHatInsights/rbac-client-go"
 	"github.com/content-services/content-sources-backend/pkg/cache"
 	"github.com/content-services/content-sources-backend/pkg/config"
+	test_handler "github.com/content-services/content-sources-backend/pkg/test/handler"
 	mocks_rbac "github.com/content-services/content-sources-backend/pkg/test/mocks/rbac"
 	"github.com/labstack/echo/v4"
+	"github.com/redhatinsights/platform-go-middlewares/identity"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
 )
@@ -56,6 +58,7 @@ func TestRbacSuite(t *testing.T) {
 
 func (s *RbacTestSuite) TestCachesWhenNotFound() {
 	ctx := context.TODO()
+	ctx = context.WithValue(ctx, identity.Key, test_handler.MockIdentity)
 	var emptyList rbac.AccessList
 	s.mockCache.On("GetAccessList", ctx).Return(nil, cache.NotFound)
 	s.mockCache.On("SetAccessList", ctx, emptyList).Return(nil, cache.NotFound)
@@ -66,10 +69,22 @@ func (s *RbacTestSuite) TestCachesWhenNotFound() {
 
 func (s *RbacTestSuite) TestCachesWhenNotFoundAgain() {
 	ctx := context.TODO()
+	ctx = context.WithValue(ctx, identity.Key, test_handler.MockIdentity)
 	var emptyList rbac.AccessList
 
 	s.mockCache.On("GetAccessList", ctx).Return(emptyList, nil)
 	allowed, err := s.rbac.Allowed(ctx, "repositories", "read")
 	assert.NoError(s.T(), err)
 	assert.False(s.T(), allowed)
 }
+
+func (s *RbacTestSuite) TestOrgAdminSkip() {
+	ctx := context.TODO()
+	mockIdentity := test_handler.MockIdentity
+	mockIdentity.Identity.User.OrgAdmin = true
+	ctx = context.WithValue(ctx, identity.Key, mockIdentity)
+
+	allowed, err := s.rbac.Allowed(ctx, "repositories", "read")
+	assert.NoError(s.T(), err)
+	assert.True(s.T(), allowed)
+}
diff --git a/pkg/test/handler/utils.go b/pkg/test/handler/utils.go
@@ -11,18 +11,18 @@ import (
 
 var MockAccountNumber = seeds.RandomAccountId()
 var MockOrgId = seeds.RandomOrgId()
+var MockIdentity = identity.XRHID{
+	Identity: identity.Identity{
+		AccountNumber: MockAccountNumber,
+		Internal: identity.Internal{
+			OrgID: MockOrgId,
+		},
+		Type: "Associate",
+	},
+}
 
 func EncodedIdentity(t *testing.T) string {
-	mockIdentity := identity.XRHID{
-		Identity: identity.Identity{
-			AccountNumber: MockAccountNumber,
-			Internal: identity.Internal{
-				OrgID: MockOrgId,
-			},
-			Type: "Associate",
-		},
-	}
-	jsonIdentity, err := json.Marshal(mockIdentity)
+	jsonIdentity, err := json.Marshal(MockIdentity)
 	if err != nil {
 		t.Error("Could not marshal JSON")
 	}

diff --git a/scripts/header_org_admin.sh b/scripts/header_org_admin.sh
@@ -30,7 +30,7 @@ fi
 
 case "$( uname -s )" in
 "Darwin" )
-  ENC="$(echo "{\"identity\":{\"type\":\"User\",\"user\":{\"username\":\"${USER_NAME}\"},\"account_number\":\"${ACCOUNT_ID}\",\"internal\":{\"org_id\":\"${ORG_ID}\"}}}" | base64 -b 0)"
+  ENC="$(echo "{\"identity\":{\"type\":\"User\",\"user\":{\"is_org_admin\":true, \"username\":\"${USER_NAME}\"},\"account_number\":\"${ACCOUNT_ID}\",\"internal\":{\"org_id\":\"${ORG_ID}\"}}}" | base64 -b 0)"
 ;;
 
 "Linux" | *)