Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Not needed?] Modify the permission system #7

Draft
wants to merge 1 commit into
base: soracom_main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/api/dashboard_permission.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ func (hs *HTTPServer) UpdateDashboardPermissions(c *models.ReqContext) response.
return response.Error(400, err.Error(), err)
}

if err == guardian.ErrGuardianOverrideDuplicate {
return response.Error(409, err.Error(), err)
}

return response.Error(500, "Error while checking dashboard permissions", err)
}

Expand Down
13 changes: 9 additions & 4 deletions pkg/services/dashboards/database/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,20 @@ func (d *DashboardStore) GetDashboardAclInfoList(ctx context.Context, query *mod
d.is_folder,
CASE WHEN (da.dashboard_id = -1 AND d.folder_id > 0) OR da.dashboard_id = d.folder_id THEN ` + d.dialect.BooleanStr(true) + ` ELSE ` + falseStr + ` END AS inherited
FROM dashboard as d
-- get folder dashboards for reference purposes
LEFT JOIN dashboard folder on folder.id = d.folder_id
-- get the acls under the following conditions
LEFT JOIN dashboard_acl AS da ON
-- if it is for this dashboard
da.dashboard_id = d.id OR
-- or if it is for the parent folder
da.dashboard_id = d.folder_id OR
(
-- include default permissions -->
da.org_id = -1 AND (
(folder.id IS NOT NULL AND folder.has_acl = ` + falseStr + `) OR
(folder.id IS NULL AND d.has_acl = ` + falseStr + `)
-- include default permissions if dashboard has no acl AND ((folder is set without acl)
-- OR ( folder is NOT set and dashboard has no acl set ))
da.org_id = -1 AND d.has_acl = ` + falseStr + ` AND (
(folder.id IS NOT NULL AND folder.has_acl = ` + falseStr + `) OR
(folder.id IS NULL)
)
)
LEFT JOIN ` + d.dialect.Quote("user") + ` AS u ON u.id = da.user_id
Expand Down
6 changes: 5 additions & 1 deletion pkg/services/dashboards/database/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ func (d *DashboardStore) UpdateDashboardACL(ctx context.Context, dashboardID int
return fmt.Errorf("deleting from dashboard_acl failed: %w", err)
}

hasAcl := false

for _, item := range items {
if item.UserID == 0 && item.TeamID == 0 && (item.Role == nil || !item.Role.IsValid()) {
return models.ErrDashboardAclInfoMissing
Expand All @@ -214,10 +216,12 @@ func (d *DashboardStore) UpdateDashboardACL(ctx context.Context, dashboardID int
if _, err := sess.Insert(item); err != nil {
return err
}

hasAcl = true
}

// Update dashboard HasAcl flag
dashboard := models.Dashboard{HasAcl: true}
dashboard := models.Dashboard{HasAcl: hasAcl}
_, err = sess.Cols("has_acl").Where("id=?", dashboardID).Update(&dashboard)
return err
})
Expand Down
15 changes: 11 additions & 4 deletions pkg/services/guardian/guardian.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import (
)

var (
ErrGuardianPermissionExists = errors.New("permission already exists")
ErrGuardianOverride = errors.New("you can only override a permission to be higher")
ErrGuardianPermissionExists = errors.New("permission already exists")
ErrGuardianOverride = errors.New("you can only override a permission to be higher")
ErrGuardianOverrideDuplicate = errors.New("you can not override with duplicate permissions")
)

// DashboardGuardian to be used for guard against operations without access on dashboard and acl
Expand Down Expand Up @@ -205,9 +206,15 @@ func (g *dashboardGuardianImpl) CheckPermissionBeforeUpdate(permission models.Pe
continue
}

if a.IsDuplicateOf(existingPerm) && a.Permission <= existingPerm.Permission {
return false, ErrGuardianOverride
if a.IsDuplicateOf(existingPerm) {
return false, ErrGuardianOverrideDuplicate
}
// Disabling the following check because it goes against the way we are using permissions https://soracom.slack.com/archives/C9PHDH1QB/p1555664739066900
// existing permissions returned by GetAcl above include default parent folder permissions if this dashboard has never had permissions set before
// and is inside a folder. That could possibly cause an error _before_ we are able to set the permissions catch 22.
// if a.Permission <= existingPerm.Permission {
// return false, ErrGuardianOverride
// }
}
}

Expand Down