Skip to content

Commit

Permalink
Use Authorizer interface instead of rbac directly
Browse files Browse the repository at this point in the history 
This makes rbac use the new Authorizer interface. All previous calls to
rbac specific functions are now handled via the new interface.

Signed-off-by: Thomas Hipp <[email protected]>
  • Loading branch information
@monstermunchkin
monstermunchkin committed Jul 20, 2023
1 parent 570d80e commit 284731b
Show file tree
Hide file tree
Showing 18 changed files with 124 additions and 151 deletions.
14 changes: 6 additions & 8 deletions lxd/api_1.0.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import (
"strings"

"github.com/canonical/lxd/client"
"github.com/canonical/lxd/lxd/auth"
"github.com/canonical/lxd/lxd/auth/candid"
"github.com/canonical/lxd/lxd/auth/oidc"
"github.com/canonical/lxd/lxd/auth/rbac"
"github.com/canonical/lxd/lxd/cluster"
clusterConfig "github.com/canonical/lxd/lxd/cluster/config"
"github.com/canonical/lxd/lxd/config"
Expand Down Expand Up @@ -375,7 +375,7 @@ func api10Get(d *Daemon, r *http.Request) response.Response {
fullSrv.AuthUserName = requestor.Username
fullSrv.AuthUserMethod = requestor.Protocol

if rbac.UserIsAdmin(r) {
if d.authorizer.UserIsAdmin(r) {
fullSrv.Config, err = daemonConfigRender(s)
if err != nil {
return response.InternalError(err)
Expand Down Expand Up @@ -911,12 +911,10 @@ func doApi10UpdateTriggers(d *Daemon, nodeChanged, clusterChanged map[string]str
if rbacChanged {
apiURL, apiKey, apiExpiry, agentURL, agentUsername, agentPrivateKey, agentPublicKey := clusterConfig.RBACServer()

// Since RBAC seems to have been set up already, we need to disable it temporarily
if d.rbac != nil {
d.candidVerifier = nil
d.rbac.StopStatusCheck()
d.rbac = nil
}
d.candidVerifier = nil
d.authorizer.StopStatusCheck()
// Reset to default authorizer in case rbac fails.
d.authorizer = auth.DefaultAuthorizer()

err := d.setupRBACServer(apiURL, apiKey, apiExpiry, agentURL, agentUsername, agentPrivateKey, agentPublicKey)
if err != nil {
Expand Down
35 changes: 14 additions & 21 deletions lxd/api_project.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

"github.com/gorilla/mux"

"github.com/canonical/lxd/lxd/auth/rbac"
"github.com/canonical/lxd/lxd/db"
"github.com/canonical/lxd/lxd/db/cluster"
"github.com/canonical/lxd/lxd/db/operationtype"
Expand Down Expand Up @@ -147,7 +146,7 @@ func projectsGet(d *Daemon, r *http.Request) response.Response {

filtered := []api.Project{}
for _, project := range projects {
if !rbac.UserHasPermission(r, project.Name, "view") {
if !d.State().Authorizer.UserHasPermission(r, project.Name, "view") {
continue
}

Expand Down Expand Up @@ -334,11 +333,9 @@ func projectsPost(d *Daemon, r *http.Request) response.Response {
return response.SmartError(fmt.Errorf("Failed creating project %q: %w", project.Name, err))
}

if d.rbac != nil {
err = d.rbac.AddProject(id, project.Name)
if err != nil {
return response.SmartError(err)
}
err = d.authorizer.AddProject(id, project.Name)
if err != nil {
return response.SmartError(err)
}

requestor := request.CreateRequestor(r)
Expand Down Expand Up @@ -405,7 +402,7 @@ func projectGet(d *Daemon, r *http.Request) response.Response {
}

// Check user permissions
if !rbac.UserHasPermission(r, name, "view") {
if !d.State().Authorizer.UserHasPermission(r, name, "view") {
return response.Forbidden(nil)
}

Expand Down Expand Up @@ -475,7 +472,7 @@ func projectPut(d *Daemon, r *http.Request) response.Response {
}

// Check user permissions
if !rbac.UserHasPermission(r, name, "manage-projects") {
if !s.Authorizer.UserHasPermission(r, name, "manage-projects") {
return response.Forbidden(nil)
}

Expand Down Expand Up @@ -566,7 +563,7 @@ func projectPatch(d *Daemon, r *http.Request) response.Response {
}

// Check user permissions
if !rbac.UserHasPermission(r, name, "manage-projects") {
if !s.Authorizer.UserHasPermission(r, name, "manage-projects") {
return response.Forbidden(nil)
}

Expand Down Expand Up @@ -843,11 +840,9 @@ func projectPost(d *Daemon, r *http.Request) response.Response {
return err
}

if d.rbac != nil {
err = d.rbac.RenameProject(id, req.Name)
if err != nil {
return err
}
err = d.authorizer.RenameProject(id, req.Name)
if err != nil {
return err
}

requestor := request.CreateRequestor(r)
Expand Down Expand Up @@ -923,11 +918,9 @@ func projectDelete(d *Daemon, r *http.Request) response.Response {
return response.SmartError(err)
}

if d.rbac != nil {
err = d.rbac.DeleteProject(id)
if err != nil {
return response.SmartError(err)
}
err = d.authorizer.DeleteProject(id)
if err != nil {
return response.SmartError(err)
}

requestor := request.CreateRequestor(r)
Expand Down Expand Up @@ -977,7 +970,7 @@ func projectStateGet(d *Daemon, r *http.Request) response.Response {
}

// Check user permissions.
if !rbac.UserHasPermission(r, name, "view") {
if !d.State().Authorizer.UserHasPermission(r, name, "view") {
return response.Forbidden(nil)
}

Expand Down
81 changes: 59 additions & 22 deletions lxd/auth/rbac/server.go → lxd/auth/rbac/rbac.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"github.com/go-macaroon-bakery/macaroon-bakery/v3/httpbakery"
"github.com/go-macaroon-bakery/macaroon-bakery/v3/httpbakery/agent"

"github.com/canonical/lxd/lxd/auth"
"github.com/canonical/lxd/lxd/request"
"github.com/canonical/lxd/shared"
"github.com/canonical/lxd/shared/logger"
)
Expand All @@ -45,14 +47,8 @@ type rbacStatus struct {
// Errors.
var errUnknownUser = fmt.Errorf("Unknown RBAC user")

// UserAccess struct for permission checks.
type UserAccess struct {
Admin bool
Projects map[string][]string
}

// Server represents an RBAC server.
type Server struct {
// server represents an RBAC server.
type server struct {
apiURL string
apiKey string

Expand All @@ -74,8 +70,8 @@ type Server struct {
}

// NewServer returns a new RBAC server instance.
func NewServer(apiURL string, apiKey string, agentAuthURL string, agentUsername string, agentPrivateKey string, agentPublicKey string) (*Server, error) {
r := Server{
func NewServer(apiURL string, apiKey string, agentAuthURL string, agentUsername string, agentPrivateKey string, agentPublicKey string, getProjects func() (map[int64]string, error)) (auth.Authorizer, error) {
r := server{
apiURL: apiURL,
apiKey: apiKey,
lastSyncID: "",
Expand Down Expand Up @@ -121,11 +117,52 @@ func NewServer(apiURL string, apiKey string, agentAuthURL string, agentUsername
return nil, err
}

// Perform full sync when online
go func() {
for {
err = r.SyncProjects()
if err != nil {
time.Sleep(time.Minute)
continue
}

break
}
}()

r.StartStatusCheck()

return &r, nil
}

// UserIsAdmin checks whether the requestor is a global admin.
func (r *server) UserIsAdmin(req *http.Request) bool {
val := req.Context().Value(request.CtxAccess)
if val == nil {
return false
}

ua := val.(*auth.UserAccess)
return ua.Admin
}

// UserHasPermission checks whether the requestor has a specific permission on a project.
func (r *server) UserHasPermission(req *http.Request, projectName string, permission string) bool {
val := req.Context().Value(request.CtxAccess)
if val == nil {
return false
}

ua := val.(*auth.UserAccess)
if ua.Admin {
return true
}

return shared.StringInSlice(permission, ua.Projects[projectName])
}

// StartStatusCheck runs a status checking loop.
func (r *Server) StartStatusCheck() {
func (r *server) StartStatusCheck() {
var status rbacStatus

// Figure out the new URL.
Expand Down Expand Up @@ -201,12 +238,12 @@ func (r *Server) StartStatusCheck() {
}

// StopStatusCheck stops the periodic status checker.
func (r *Server) StopStatusCheck() {
func (r *server) StopStatusCheck() {
r.ctxCancel()
}

// SyncProjects updates the list of projects in RBAC.
func (r *Server) SyncProjects() error {
func (r *server) SyncProjects() error {
if r.projectsFunc == nil {
return fmt.Errorf("ProjectsFunc isn't configured yet, cannot sync")
}
Expand Down Expand Up @@ -245,7 +282,7 @@ func (r *Server) SyncProjects() error {
}

// AddProject adds a new project resource to RBAC.
func (r *Server) AddProject(id int64, name string) error {
func (r *server) AddProject(id int64, name string) error {
resource := rbacResource{
Name: name,
Identifier: strconv.FormatInt(id, 10),
Expand All @@ -266,7 +303,7 @@ func (r *Server) AddProject(id int64, name string) error {
}

// DeleteProject adds a new project resource to RBAC.
func (r *Server) DeleteProject(id int64) error {
func (r *server) DeleteProject(id int64) error {
// Update RBAC
err := r.postResources(nil, []string{strconv.FormatInt(id, 10)}, false)
if err != nil {
Expand All @@ -287,12 +324,12 @@ func (r *Server) DeleteProject(id int64) error {
}

// RenameProject renames an existing project resource in RBAC.
func (r *Server) RenameProject(id int64, name string) error {
func (r *server) RenameProject(id int64, name string) error {
return r.AddProject(id, name)
}

// UserAccess returns a UserAccess struct for the user.
func (r *Server) UserAccess(username string) (*UserAccess, error) {
func (r *server) UserAccess(username string) (*auth.UserAccess, error) {
r.permissionsLock.Lock()
defer r.permissionsLock.Unlock()

Expand All @@ -310,7 +347,7 @@ func (r *Server) UserAccess(username string) (*UserAccess, error) {
}

// Prepare the response.
access := UserAccess{
access := auth.UserAccess{
Admin: shared.StringInSlice("admin", permissions[""]),
Projects: map[string][]string{},
}
Expand All @@ -337,7 +374,7 @@ func (r *Server) UserAccess(username string) (*UserAccess, error) {
return &access, nil
}

func (r *Server) flushCache() {
func (r *server) flushCache() {
r.permissionsLock.Lock()
defer r.permissionsLock.Unlock()

Expand All @@ -354,7 +391,7 @@ func (r *Server) flushCache() {
logger.Info("Flushed RBAC permissions cache")
}

func (r *Server) syncAdmin(username string) bool {
func (r *server) syncAdmin(username string) bool {
u, err := url.Parse(r.apiURL)
if err != nil {
return false
Expand Down Expand Up @@ -387,7 +424,7 @@ func (r *Server) syncAdmin(username string) bool {
return shared.StringInSlice("admin", permissions[""])
}

func (r *Server) syncPermissions(username string) error {
func (r *server) syncPermissions(username string) error {
u, err := url.Parse(r.apiURL)
if err != nil {
return err
Expand Down Expand Up @@ -427,7 +464,7 @@ func (r *Server) syncPermissions(username string) error {
return nil
}

func (r *Server) postResources(updates []rbacResource, removals []string, force bool) error {
func (r *server) postResources(updates []rbacResource, removals []string, force bool) error {
// Make sure that we have a baseline sync in place
if !force && r.lastSyncID == "" {
return r.SyncProjects()
Expand Down
34 changes: 0 additions & 34 deletions lxd/auth/rbac/requests.go
View file

This file was deleted.

7 changes: 3 additions & 4 deletions lxd/certificates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/gorilla/mux"

"github.com/canonical/lxd/client"
"github.com/canonical/lxd/lxd/auth/rbac"
"github.com/canonical/lxd/lxd/cluster"
clusterConfig "github.com/canonical/lxd/lxd/cluster/config"
clusterRequest "github.com/canonical/lxd/lxd/cluster/request"
Expand Down Expand Up @@ -541,7 +540,7 @@ func certificatesPost(d *Daemon, r *http.Request) response.Response {
}

// Handle requests by non-admin users.
if !rbac.UserIsAdmin(r) {
if !d.authorizer.UserIsAdmin(r) {
// Non-admin cannot issue tokens.
if req.Token {
return response.Forbidden(nil)
Expand Down Expand Up @@ -983,7 +982,7 @@ func doCertificateUpdate(d *Daemon, dbInfo api.Certificate, req api.CertificateP
// In order to prevent possible future security issues, the certificate information is
// reset in case a non-admin user is performing the update.
certProjects := req.Projects
if !rbac.UserIsAdmin(r) {
if !d.authorizer.UserIsAdmin(r) {
if r.TLS == nil {
response.Forbidden(fmt.Errorf("Cannot update certificate information"))
}
Expand Down Expand Up @@ -1128,7 +1127,7 @@ func certificateDelete(d *Daemon, r *http.Request) response.Response {
}

// Non-admins are able to delete only their own certificate.
if !rbac.UserIsAdmin(r) {
if !d.authorizer.UserIsAdmin(r) {
if r.TLS == nil {
response.Forbidden(fmt.Errorf("Cannot delete certificate"))
}
Expand Down
Loading

0 comments on commit 284731b

Please sign in to comment.