Skip to content

Commit

Permalink
Merge pull request #122 from kjaskiewiczz/no-rbac
Browse files Browse the repository at this point in the history
remove RBAC support; RBAC for this service is handled by the useradm-enterprise service
  • Loading branch information
lluiscampos committed Jul 9, 2021
2 parents 4185f38 + 3847544 commit 28b61ae
Show file tree
Hide file tree
Showing 8 changed files with 2 additions and 481 deletions.
20 changes: 0 additions & 20 deletions api/http/management.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"io/ioutil"
"net/http"
"strconv"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -140,25 +139,6 @@ func (h ManagementController) Connect(c *gin.Context) {
userID := idata.Subject
deviceID := c.Param("deviceId")

if len(c.Request.Header.Get(model.RBACHeaderRemoteTerminalGroups)) > 1 {
groups := strings.Split(
c.Request.Header.Get(model.RBACHeaderRemoteTerminalGroups), ",")

allowed, err := h.app.RemoteTerminalAllowed(ctx, tenantID, deviceID, groups)
if err != nil {
l.Error(err)
c.JSON(http.StatusInternalServerError, gin.H{
"error": "internal error",
})
return
} else if !allowed {
c.JSON(http.StatusForbidden, gin.H{
"error": "Access denied (RBAC).",
})
return
}
}

session := &model.Session{
TenantID: tenantID,
UserID: userID,
Expand Down
42 changes: 0 additions & 42 deletions api/http/management_filetransfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -476,22 +476,6 @@ func (h ManagementController) DownloadFile(c *gin.Context) {
return
}

allowed, err := h.fileTransferAllowed(c, params.TenantID, params.Device.ID)
if err != nil {
l.Error(err)
c.JSON(http.StatusInternalServerError, gin.H{
"error": errors.Wrap(err, "failed to check RBAC"),
})
return
} else if !allowed {
msg := "Access denied (RBAC)."
l.Warn(msg)
c.JSON(http.StatusForbidden, gin.H{
"error": msg,
})
return
}

ctx := c.Request.Context()
if err := h.app.DownloadFile(ctx, params.UserID, params.Device.ID,
*request.Path); err != nil {
Expand Down Expand Up @@ -890,22 +874,6 @@ func (h ManagementController) UploadFile(c *gin.Context) {

defer request.File.Close()

allowed, err := h.fileTransferAllowed(c, params.TenantID, params.Device.ID)
if err != nil {
l.Error(err.Error())
c.JSON(http.StatusInternalServerError, gin.H{
"error": errors.Wrap(err, "failed to check RBAC"),
})
return
} else if !allowed {
msg := "Access denied (RBAC)."
l.Warn(msg)
c.JSON(http.StatusForbidden, gin.H{
"error": msg,
})
return
}

ctx := c.Request.Context()
if err := h.app.UploadFile(ctx, params.UserID, params.Device.ID,
*request.Path); err != nil {
Expand All @@ -918,13 +886,3 @@ func (h ManagementController) UploadFile(c *gin.Context) {

h.uploadFileResponse(c, params, request)
}

func (h ManagementController) fileTransferAllowed(c *gin.Context, tenantID string,
deviceID string) (bool, error) {
if len(c.Request.Header.Get(model.RBACHeaderRemoteTerminalGroups)) == 0 {
return true, nil
}
groups := strings.Split(
c.Request.Header.Get(model.RBACHeaderRemoteTerminalGroups), ",")
return h.app.RemoteTerminalAllowed(c, tenantID, deviceID, groups)
}
193 changes: 0 additions & 193 deletions api/http/management_filetransfer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"testing"
"time"

"github.com/gin-gonic/gin"
"github.com/google/uuid"
"github.com/mendersoftware/go-lib-micro/identity"
"github.com/mendersoftware/go-lib-micro/ws"
Expand Down Expand Up @@ -83,10 +82,6 @@ func TestManagementDownloadFile(t *testing.T) {
Path string
Identity *identity.Identity

RBACGroups string
RBACAllowed bool
RBACError error

GetDevice *model.Device
GetDeviceError error
DeviceFunc func(*nats_mocks.Client)
Expand Down Expand Up @@ -1049,46 +1044,6 @@ func TestManagementDownloadFile(t *testing.T) {

HTTPStatus: http.StatusInternalServerError,
},
{
Name: "ko, RBAC check failure",
DeviceID: "1234567890",
Identity: &identity.Identity{
Subject: "00000000-0000-0000-0000-000000000000",
Tenant: "000000000000000000000000",
IsUser: true,
},
Path: "/absolute/path",

RBACGroups: "group1,group",
RBACError: errors.New("error"),

GetDevice: &model.Device{
ID: "1234567890",
Status: model.DeviceStatusConnected,
},

HTTPStatus: http.StatusInternalServerError,
},
{
Name: "ko, RBAC forbidden",
DeviceID: "1234567890",
Identity: &identity.Identity{
Subject: "00000000-0000-0000-0000-000000000000",
Tenant: "000000000000000000000000",
IsUser: true,
},
Path: "/absolute/path",

RBACGroups: "group1,group",
RBACAllowed: false,

GetDevice: &model.Device{
ID: "1234567890",
Status: model.DeviceStatusConnected,
},

HTTPStatus: http.StatusForbidden,
},
{
Name: "ko, bad request, relative path",
DeviceID: "1234567890",
Expand Down Expand Up @@ -1234,18 +1189,6 @@ func TestManagementDownloadFile(t *testing.T) {
t.FailNow()
}

if tc.RBACGroups != "" {
req.Header.Set(model.RBACHeaderRemoteTerminalGroups, tc.RBACGroups)
app.On("RemoteTerminalAllowed",
mock.MatchedBy(func(ctx context.Context) bool {
return true
}),
tc.Identity.Tenant,
tc.DeviceID,
strings.Split(tc.RBACGroups, ","),
).Return(tc.RBACAllowed, tc.RBACError)
}

if tc.Identity != nil {
jwt := GenerateJWT(*tc.Identity)
req.Header.Set(headerAuthorization, "Bearer "+jwt)
Expand Down Expand Up @@ -1296,10 +1239,6 @@ func TestManagementUploadFile(t *testing.T) {
File []byte
Identity *identity.Identity

RBACGroups string
RBACAllowed bool
RBACError error

GetDevice *model.Device
GetDeviceError error
DeviceFunc func(*nats_mocks.Client)
Expand Down Expand Up @@ -1971,58 +1910,6 @@ func TestManagementUploadFile(t *testing.T) {

HTTPStatus: http.StatusInternalServerError,
},
{
Name: "ko, RBAC check failure",
DeviceID: "1234567890",
Identity: &identity.Identity{
Subject: "00000000-0000-0000-0000-000000000000",
Tenant: "000000000000000000000000",
IsUser: true,
},
Body: map[string][]string{
fieldUploadPath: {"/absolute/path"},
fieldUploadUID: {"0"},
fieldUploadGID: {"0"},
fieldUploadMode: {"0644"},
},
File: []byte("1234567890"),

RBACGroups: "group1,group",
RBACError: errors.New("error"),

GetDevice: &model.Device{
ID: "1234567890",
Status: model.DeviceStatusConnected,
},

HTTPStatus: http.StatusInternalServerError,
},
{
Name: "ko, RBAC forbidden",
DeviceID: "1234567890",
Identity: &identity.Identity{
Subject: "00000000-0000-0000-0000-000000000000",
Tenant: "000000000000000000000000",
IsUser: true,
},
Body: map[string][]string{
fieldUploadPath: {"/absolute/path"},
fieldUploadUID: {"0"},
fieldUploadGID: {"0"},
fieldUploadMode: {"0644"},
},
File: []byte("1234567890"),

RBACGroups: "group1,group",
RBACAllowed: false,

GetDevice: &model.Device{
ID: "1234567890",
Status: model.DeviceStatusConnected,
},

HTTPStatus: http.StatusForbidden,
},
{
Name: "ko, failed to submit audit log",
DeviceID: "1234567890",
Expand Down Expand Up @@ -2266,18 +2153,6 @@ func TestManagementUploadFile(t *testing.T) {
req.Header.Add("Content-Type", "multipart/form-data; boundary=\"boundary\"")
}

if tc.RBACGroups != "" {
req.Header.Set(model.RBACHeaderRemoteTerminalGroups, tc.RBACGroups)
app.On("RemoteTerminalAllowed",
mock.MatchedBy(func(ctx context.Context) bool {
return true
}),
tc.Identity.Tenant,
tc.DeviceID,
strings.Split(tc.RBACGroups, ","),
).Return(tc.RBACAllowed, tc.RBACError)
}

if tc.Identity != nil {
jwt := GenerateJWT(*tc.Identity)
req.Header.Set(headerAuthorization, "Bearer "+jwt)
Expand All @@ -2299,71 +2174,3 @@ func TestManagementUploadFile(t *testing.T) {
})
}
}

func TestFileTransferAllowed(t *testing.T) {
testCases := []struct {
Name string

Groups string
TenantID string
DeviceID string

Allowed bool
Error error

ReturnAllowed bool
ReturnError error
}{
{
Name: "no group restrictions",

ReturnAllowed: true,
},
{
Name: "group restrictions",
Groups: "group1,group2",

Allowed: true,

ReturnAllowed: true,
},
{
Name: "group restrictions",
Groups: "group1,group2",

Allowed: false,
Error: errors.New("generic error"),

ReturnAllowed: false,
ReturnError: errors.New("generic error"),
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
ctx := &gin.Context{}
ctx.Request, _ = http.NewRequest("POST", "/", nil)
ctx.Request.Header.Set(model.RBACHeaderRemoteTerminalGroups, tc.Groups)

app := &app_mocks.App{}
defer app.AssertExpectations(t)

if tc.Groups != "" {
app.On("RemoteTerminalAllowed",
ctx,
tc.TenantID,
tc.DeviceID,
strings.Split(tc.Groups, ","),
).Return(tc.Allowed, tc.Error)
}

ctrl := NewManagementController(app, nil)
allowed, err := ctrl.fileTransferAllowed(ctx, tc.TenantID, tc.DeviceID)
assert.Equal(t, tc.ReturnAllowed, allowed)
if tc.ReturnError == nil {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, tc.ReturnError.Error())
}
})
}
}
Loading

0 comments on commit 28b61ae

Please sign in to comment.