Skip to content

Commit

Permalink
Workspace access check removed (#33)
Browse files Browse the repository at this point in the history
* Workspace access check removed

* List workspaces with user access rights

* Fix e2e tests
  • Loading branch information
b1es authored Oct 27, 2022
1 parent 9439403 commit c95b403
Show file tree
Hide file tree
Showing 15 changed files with 343 additions and 296 deletions.
39 changes: 22 additions & 17 deletions auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,19 +70,24 @@ const (
Volumes Subject = "volumes"
)

func (ra RoleAuthorizer) GetWorkspacePermissions(wsp string, usr user.User) (bool, bool, error) {
wss, err := ra.Workspaces.ListWorkspaces(context.TODO(), usr)
if err != nil {
return false, false, errors.Wrap(err, "could not get workspace permissions")
}
type AccessLevel struct {
User bool
Admin bool
}

func (ra RoleAuthorizer) GetWorkspacePermissions(wsp string, usr user.User) (AccessLevel, error) {
wss := ra.Workspaces.ListWorkspaces()

for _, ws := range wss {
var al AccessLevel
if ws.Name == wsp {
return ws.UserHasAccess(usr), ws.UserHasAdminAccess(usr), nil
al.User = ws.UserHasAccess(usr)
al.Admin = ws.UserHasAdminAccess(usr)
return al, nil
}
}

return false, false, nil
return AccessLevel{}, nil
}

func (ra RoleAuthorizer) GetSecretPermissions(usr user.User, data any) (map[Action]bool, error) {
Expand All @@ -93,16 +98,16 @@ func (ra RoleAuthorizer) GetSecretPermissions(usr user.User, data any) (map[Acti
return map[Action]bool{}, errors.Errorf("could not decode the workspace variable")
}

userAccess, adminAccess, err := ra.GetWorkspacePermissions(workspace, usr)
al, err := ra.GetWorkspacePermissions(workspace, usr)
if err != nil {
return map[Action]bool{}, errors.Wrap(err, "could not get secret permissions")
}

// this is where access levels map to actions.
p[Read] = userAccess || adminAccess
p[List] = userAccess || adminAccess
p[Write] = adminAccess
p[Delete] = adminAccess
p[Read] = al.User || al.Admin
p[List] = al.User || al.Admin
p[Write] = al.Admin
p[Delete] = al.Admin

return p, nil
}
Expand All @@ -115,16 +120,16 @@ func (ra RoleAuthorizer) GetVolumePermissions(usr user.User, data any) (map[Acti
return map[Action]bool{}, errors.Errorf("could not decode the workspace variable")
}

userAccess, adminAccess, err := ra.GetWorkspacePermissions(workspace, usr)
al, err := ra.GetWorkspacePermissions(workspace, usr)
if err != nil {
return map[Action]bool{}, errors.Wrap(err, "could not get secret permissions")
}

// this is where access levels map to actions.
p[Read] = userAccess || adminAccess
p[List] = userAccess || adminAccess
p[Write] = adminAccess
p[Delete] = adminAccess
p[Read] = al.User || al.Admin
p[List] = al.Admin || al.User
p[Write] = al.Admin
p[Delete] = al.Admin

return p, nil
}
Expand Down
53 changes: 27 additions & 26 deletions e2etest/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,32 +46,33 @@ var mockUser fuser.User = fuser.MockUser{
Uid: "0",
Name: "Auth Disabled",
Email: "auth@disabled",
Roles: []fuser.Role{"tester", "dummy"},
Roles: []fuser.Role{"tester", "dummy", "sandbox-developer"},
}

var testWorkspace string = `
---
# Namespace 'sandbox-project-a'
apiVersion: v1
kind: Namespace
metadata:
labels:
app.kubernetes.io/part-of: "flowify"
name: "test"
---
# Developer workspace environment
apiVersion: v1
kind: ConfigMap
metadata:
labels:
app.kubernetes.io/component: "workspace-config"
app.kubernetes.io/part-of: "flowify"
name: "test"
namespace: "test"
data:
roles: "[[\"tester\"]]"
`
// TODO: not in use, add clean workspace for e2e tests in k8s
// var testWorkspace string = `
// ---
// # Namespace 'sandbox-project-a'
// apiVersion: v1
// kind: Namespace
// metadata:
// labels:
// app.kubernetes.io/part-of: "flowify"
// name: "test"

// ---
// # Developer workspace environment
// apiVersion: v1
// kind: ConfigMap
// metadata:
// labels:
// app.kubernetes.io/component: "workspace-config"
// app.kubernetes.io/part-of: "flowify"
// name: "test"
// namespace: "test"
// data:
// roles: "[[\"tester\"]]"
// `

var configString = []byte(`
db:
Expand Down Expand Up @@ -380,7 +381,7 @@ func (s *e2eTestSuite) Test_Roundtrip_Workflow() {

resp, err := requestor(server_addr+"/api/v1/workflows/", http.MethodPost, wfReq)
s.NoError(err)
require.Equal(s.T(), http.StatusCreated, resp.StatusCode)
require.Equal(s.T(), http.StatusCreated, resp.StatusCode, BodyStringer{resp.Body})

var wfResp models.Workflow
err = marshalResponse(ResponseBodyBytes(resp), &wfResp)
Expand Down Expand Up @@ -445,7 +446,7 @@ func (s *e2eTestSuite) Test_Roundtrip_Job() {
resp2, err := requestor(fmt.Sprintf(server_addr+"/api/v1/jobs/%s", wfResp.Metadata.Uid.String()), http.MethodGet, wfReq)
s.NoError(err)

var wfResp2 models.Workflow
var wfResp2 models.Job
err = marshalResponse(ResponseBodyBytes(resp2), &wfResp2)
s.NoError(err)
s.Equal(wfResp, wfResp2, "expect roundtrip equality")
Expand Down
2 changes: 1 addition & 1 deletion e2etest/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func (s *e2eTestSuite) Test_Workspaces() {
require.Equal(s.T(), http.StatusOK, resp.StatusCode, BodyStringer{resp.Body})

type WorkspaceList struct {
Items []workspace.Workspace `json:"items"`
Items []workspace.WorkspaceGetRequest `json:"items"`
}
var list WorkspaceList
err = marshalResponse(ResponseBodyBytes(resp), &list)
Expand Down
2 changes: 1 addition & 1 deletion models/examples/minimal-any-workflow.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
"type": "any"
}
},
"workspace": "test"
"workspace": "sandbox-project-a"
}
27 changes: 8 additions & 19 deletions models/spec/workspace.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,21 @@
"description": "The name of the workspace",
"type": "string"
},
"hasAccess": {
"description": "Is access granted?",
"type": "boolean"
"description": {
"description": "The description of the workspace",
"type": "string"
},
"missingRoles": {
"description": "The roles a user needs in order to get access.",
"roles": {
"description": "The access roles user has for the workspace (user or admin).",
"type": "array",
"minItems": 0,
"uniqueItems": true,
"items": {
"schema": {
"type": "object",
"properties": {
"name": {
"description": "The name of the missing role",
"type": "string"
},
"description": {
"description": "The description of the missing role",
"type": "string"
}
}
}
"type": "string",
"pattern": "^(user|admin)$"
}
}
},
"additionalProperties": false,
"required": ["name", "hasAccess", "missingRoles"]
"required": ["name", "description", "roles"]
}
53 changes: 8 additions & 45 deletions pkg/workspace/workspace.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package workspace

import (
"context"
"encoding/json"
"strings"
"sync"
Expand Down Expand Up @@ -36,14 +35,11 @@ type Workspace struct {

// the list of required roles for access
Roles [][]userpkg.Role `json:"roles,omitempty"`

HasAccess bool `json:"hasAccess"`
MissingRoles [][]MissingRole `json:"missingRoles,omitempty"`
}

type WorkspaceClient interface {
// list the workspaces visible to a specific user
ListWorkspaces(ctx context.Context, user userpkg.User) ([]Workspace, error)
ListWorkspaces() []Workspace
GetNamespace() string
}

Expand Down Expand Up @@ -122,17 +118,6 @@ type MissingRole struct {
Description string `json:"description"`
}

// queries the access list for a matching access and returns true if found, else false
func HasAccess(ws []Workspace, ns string) bool {
for _, w := range ws {
if w.Name == ns && w.HasAccess {
return true
}
}

return false
}

func listWorkspaceConfigMaps(namespace string, cmInformer v1.ConfigMapInformer) ([]Workspace, error) {
// the lister finds all configmaps in the given namespace with the 'workspace-config' label

Expand Down Expand Up @@ -271,34 +256,12 @@ func getAccessTokens(cm *core.ConfigMap) ([][]userpkg.Role, error) {
}
}

func (wimpl *workspaceImpl) ListWorkspaces(ctx context.Context, user userpkg.User) ([]Workspace, error) {
// make a copy of the workspaces
// append only those that have read rights or are not hidden
aws := make([]Workspace, 0, len(wimpl.ws))
for _, w := range wimpl.ws {
aw := w
// set hasaccess for this particular user
aw.HasAccess = w.UserHasAccess(user)
if aw.HasAccess || !aw.HideForUnauthorized {

if !aw.HasAccess {
// append the missing roles
missingRoles := [][]MissingRole{}
for _, group := range aw.Roles {
missing := []MissingRole{}
for _, r := range group {
if !userpkg.UserHasRole(user, r) {
missing = append(missing, MissingRole{Name: r, Description: wimpl.roleDescriptions[string(r)]})
}
}
missingRoles = append(missingRoles, missing)
}
aw.MissingRoles = missingRoles
}
aws = append(aws, aw)
}
}
func (wimpl *workspaceImpl) ListWorkspaces() []Workspace {
return wimpl.ws
}

// return authorized list
return aws, nil
type WorkspaceGetRequest struct {
Name string `json:"name"`
Description string `json:"description"`
Roles []string `json:"roles"`
}
41 changes: 7 additions & 34 deletions pkg/workspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ import (
"encoding/json"
"testing"

"github.com/equinor/flowify-workflows-server/auth"
"github.com/equinor/flowify-workflows-server/pkg/workspace"
"github.com/equinor/flowify-workflows-server/user"
"github.com/stretchr/testify/require"
core "k8s.io/api/core/v1"
"k8s.io/client-go/kubernetes/fake"
Expand Down Expand Up @@ -117,32 +115,12 @@ func getClient() workspace.WorkspaceClient {

func Test_WorkspaceClientListWorkspaces(t *testing.T) {
client := getClient()
ws, err := client.ListWorkspaces(ctx, auth.AzureTokenUser{Roles: []user.Role{"token1", "token4", "token3", "token2"}})
require.NoError(t, err)
ws := client.ListWorkspaces()

// Should return both workspaces
require.Len(t, ws, 2)

ws, err = client.ListWorkspaces(ctx, auth.AzureTokenUser{Roles: []user.Role{"token1", "token2"}})
require.NoError(t, err)

// Should return one workspace with no access
require.Len(t, ws, 1)
require.Equal(t, "workspace-xyz", ws[0].Name)
require.Equal(t, false, ws[0].HasAccess)
require.Len(t, ws[0].MissingRoles, 1)
require.Equal(t, user.Role("token4"), ws[0].MissingRoles[0][0].Name)
require.Equal(t, "Only given to the bravest", ws[0].MissingRoles[0][0].Description)

ws, err = client.ListWorkspaces(ctx, auth.AzureTokenUser{Roles: []user.Role{"token1", "token4"}})
require.NoError(t, err)

// Should return one workspace that can be accessed
require.Len(t, ws, 2)

for _, w := range ws {
require.Contains(t, []string{"workspace-xyz", "workspace-abc"}, w.Name)
require.Equal(t, true, w.HasAccess)
}
}

Expand All @@ -154,16 +132,11 @@ func Test_WorkspaceNoRoleConfigMap(t *testing.T) {

client := workspace.NewWorkspaceClient(fake.NewSimpleClientset(&cm1, &cm2), namespace)

ws, err := client.ListWorkspaces(ctx, auth.AzureTokenUser{Roles: []user.Role{"token1", "token2"}})
require.NoError(t, err)
ws := client.ListWorkspaces()

// Should return one workspace with no access
require.Len(t, ws, 1)
require.Equal(t, "workspace-xyz", ws[0].Name)
require.Equal(t, false, ws[0].HasAccess)
require.Len(t, ws[0].MissingRoles, 1)
require.Equal(t, user.Role("token4"), ws[0].MissingRoles[0][0].Name)
require.Len(t, ws[0].MissingRoles[0][0].Description, 0, "no descriptions for any roles")

require.Nil(t, err)
// Should return both workspaces
require.Len(t, ws, 2)
for _, w := range ws {
require.Contains(t, []string{"workspace-xyz", "workspace-abc"}, w.Name)
}
}
3 changes: 2 additions & 1 deletion rest/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,8 +356,9 @@ func InitializeMetadata(ctx context.Context, meta *models.Metadata) error {
id := models.NewComponentReference()
meta.Uid = id
TouchMetadata(ctx, meta)
err := meta.Version.InitializeNew()

return nil
return err
}

func TouchMetadata(ctx context.Context, meta *models.Metadata) {
Expand Down
Loading

1 comment on commit c95b403

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage for this commit

62.00%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
apiserver
   apiserver.go60.34%100%100%60.34%100–113, 153–155, 160–163, 165, 174–177, 210–214, 216, 56–58, 60–62, 64–76, 79–85, 87–90, 92–98
   config.go0%100%100%0%101–106, 108–111, 113, 117–124, 126–131, 42–48, 51–62, 64, 67–76, 78–95
auth
   auth.go0%100%100%0%101–104, 107–112, 115–121, 123–126, 129–134, 137–158, 162–166, 168, 22–24, 37–44, 78–87, 90, 93–99
   azure_token.go58.14%100%100%58.14%104–105, 127–128, 136–137, 146–147, 67–72, 74–80, 82–86, 88–93, 96–99
   config.go0%100%100%0%26–35, 37–51, 53–56, 58, 61–72, 74–77
models
   models.go71.39%100%100%71.39%1004–1009, 1011, 1018–1019, 1029–1030, 1038–1040, 1049–1054, 1056, 1080–1081, 1091, 1097–1101, 1108, 1116–1117, 1130–1131, 1143–1144, 1156, 1162–1166, 117, 1175, 118, 1183–1184, 127–138, 140–143, 148–150, 154–155, 169–171, 185–186, 208–209, 217–220, 241–243, 247–249, 256–258, 273–276, 279–281, 283–285, 287–290, 292–293, 360–362, 373–374, 385–386, 394–398, 408–409, 418–419, 430–431, 439–443, 456–458, 470–471, 496, 503–504, 51, 513–514, 52–54, 540, 55, 57, 573–575, 582–584, 590–592, 598–600, 606–608, 614–616, 618–619, 671–672, 682–683, 693–694, 704–705, 715–716, 719–720, 777–778, 79, 799, 80, 800–808, 815–816, 824–825, 845–849, 851, 883–884, 89–91, 912, 919–920, 928–929, 949, 984–985, 993–995
   job.go88%100%100%88%22–23, 40
   validate.go60%100%100%60%18–20, 23–24, 33–35, 49–54, 61–62, 66–67
pkg/secret
   config.go0%100%100%0%22–27, 29–31, 33–37, 39–42, 44–51, 54–59, 61–65, 67
   secret.go82.78%100%100%82.78%123–124, 134–135, 149–150, 155–156, 161–162, 167–168, 49–51, 58–59, 72–73, 83–89
   mock.go0%100%100%0%13–20, 22–25, 27–30, 32–35
pkg/workspace
   workspace.go58.39%100%100%58.39%128–129, 136–137, 161–163, 170–172, 181–183, 185–193, 197–199, 202, 204, 207–210, 212–220, 224–226, 229, 231, 242–243, 252–253, 61–65, 67–69
   mock.go0%100%100%0%13–18, 20–23, 25–28, 30–33
rest
   userinfo.go0%100%100%0%10–25
   rest.go65.31%100%100%65.31%110–129, 131–135, 156–157, 164, 175–177, 247, 47–48, 53–54, 56–57, 63–64, 71–73, 76–80, 82–84
   components.go57.93%100%100%57.93%108–109, 113–115, 125–127, 131–133, 135–136, 140–143, 153–155, 158–160, 165–167, 169–170, 180–186, 188, 191–198, 207–208, 211–212, 220–222, 230–231, 235–237, 24–25, 251–253, 257–260, 265–268, 279, 28, 280–281, 285–287, 29, 290–293, 297–299, 310–312, 316–318, 326–331, 341–342, 345–346, 353–354, 370–373, 378–379, 381–382, 386–388, 395–396, 400–402, 414–417, 42–43, 92–94, 97–99
   jobs.go56.86%100%100%56.86%106–108, 112–114, 119–121, 125–128, 134–136, 141–143, 155–161, 164–165, 178–180, 184–186, 190–192, 194–196, 210–214, 216–220, 222–226, 228, 238–241, 244–246, 249–252, 258–261, 287–288, 293–294, 309, 31, 310, 32, 327–329, 33, 334–337, 346–347, 35, 351–353, 36, 360–361, 376–377, 38–39, 394–395, 40, 401–410, 73–78, 80–84, 86–90, 92, 98–99
   volumes.go73.68%100%100%73.68%106–108, 122–124, 128–130, 138–140, 144–146, 160–162, 177–179, 45–47, 53–55, 69–71, 78–79, 85–87
   workspaces.go100%100%100%100%
   workflows.go55.43%100%100%55.43%101–102, 112–114, 118–120, 123–126, 130–133, 144–146, 150–152, 154–156, 160–164, 175–177, 181–188, 194–198, 206–207, 214–215, 219–221, 228–229, 234–236, 247–250, 57–59, 62–64, 67–68, 73–75, 85–87, 90–92, 97–99
   secrets.go96.94%100%100%96.94%110–112
storage
   mongovolumestorage.go74.69%100%100%74.69%110–111, 118–119, 125–126, 136–137, 145–146, 160–161, 172–173, 181–182, 199–200, 213–214, 220–221, 226–227, 27–33, 37–39, 56–57, 67–68, 70–72
   local.go0%100%100%0%100–101, 103, 106–112, 114–124, 127, 18–21, 25–31, 33–38, 40, 43–53, 56, 61–67, 69–74, 76, 79–89, 92, 96–99
   mongo.go62.76%100%100%62.76%100, 1004–1005, 101–102, 1042–1043, 1048–1049, 1059–1060, 1068–1069, 1074–1079, 108, 1081–1089, 109, 1090–1095, 1098–1099, 110, 1100–1127, 1129–1143, 1148–1149, 1167–1168, 1176–1177, 1192–1193, 1212–1213, 1218–1219, 1245–1246, 1259–1260, 1262–1264, 1303–1304, 1309–1310, 1320–1321, 1329–1330, 139–145, 149–151, 157–163, 166–195, 217–218, 235–236, 249–250, 256–257, 262–263, 267–268, 271–272, 293–301, 305–306, 319–320, 323–324, 328–329, 346–347, 359–360, 396–397, 405–406, 409–410, 443–444, 449–469, 471–477, 479, 503–504, 516–517, 525–526, 530–531, 537–538, 543–544, 551–552, 560–561, 569–570, 581–582, 594–595, 599–600, 608–609, 61, 619, 62, 621–622, 63–65, 651–652, 66, 661–662, 665–666, 67, 671–672, 68, 684–685, 697–698, 70–73, 738–739, 745–746, 75, 756–757, 765–766, 771–784, 787–816, 818–832, 846–847, 853–854, 859–860, 865–869, 87, 870–879, 88, 880–892, 900–901, 91, 912–913, 92–99, 990–991
   references.go72.50%100%100%72.50%100–101, 23–24, 35–36, 42–43, 46–47, 59–60, 65–66, 76–77, 80–81, 87–88, 96–97
   parsequery.go82.91%100%100%82.91%106–107, 138–139, 147–148, 47–48, 62–63, 74–77, 82–83, 87–88, 95–96
transpiler
   argo.go100%100%100%100%
   transpiler.go62.93%100%100%62.93%118–119, 124–128, 143–155, 159–160, 201–229, 23, 230–232, 236–239, 24, 240–252, 254–257, 261–269, 275–277, 282–284, 287–288, 291–292, 296–297, 311–313, 316–317, 322–326, 332–333, 368–374, 403–404, 421–426, 434–435, 459–460, 463–464, 470–477, 483–484, 489–490, 493–494, 496–517, 521–524, 528–529, 550–551, 557–558, 575–578, 591–592, 612–613, 638–639, 642–644, 65, 651–652, 66, 663–670, 96–98
   helpers.go77.73%100%100%77.73%102–103, 108–109, 113–114, 124–125, 131, 144, 154, 161–162, 164–165, 172–173, 183, 192, 201, 218–219, 222–223, 228–229, 232–233, 247–248, 255–256, 26, 293–294, 296–298, 312–313, 331–332, 339–340, 344–345, 351–355, 86–87

Please sign in to comment.