From f588ae44183188eff1b09b27518345e682cb2dae Mon Sep 17 00:00:00 2001 From: Sam Perrin Date: Wed, 15 Jul 2020 06:01:37 +0100 Subject: [PATCH] Delete user rbac attachments on deletion of users (#271) * Delete user rbac attachments on deletion of users * Add a DeleteUser method so we can delete rbac attachments for a user. * Make noop.go fully no-op (makes sure that a call to DeleteUser) even with RBAC off is OK. All other endpoints now fully no-op for consistency. * Introduce a UserHandler to contain dependencies for the user handlers * Fix tests post merge * Move handler deps to providers * Created a RBAC Provider * Created a User Provider * Fix linting issues --- gaia.go | 11 ++ handlers/auth_test.go | 32 ++--- handlers/handler.go | 37 +++--- handlers/service.go | 3 + providers/provider.go | 28 +++++ .../rbac.go => providers/rbac/provider.go | 31 +++-- .../rbac/provider_test.go | 74 ++++++------ .../user.go => providers/user/provider.go | 111 +++++++++--------- .../user/provider_test.go | 91 ++++++++------ security/rbac/noop.go | 20 ++-- security/rbac/noop_test.go | 12 +- security/rbac/service.go | 9 ++ server/server.go | 6 + 13 files changed, 273 insertions(+), 192 deletions(-) create mode 100644 providers/provider.go rename handlers/rbac.go => providers/rbac/provider.go (77%) rename handlers/rbac_test.go => providers/rbac/provider_test.go (90%) rename handlers/user.go => providers/user/provider.go (73%) rename handlers/user_test.go => providers/user/provider_test.go (85%) diff --git a/gaia.go b/gaia.go index 2cdc743f..e9a1055c 100644 --- a/gaia.go +++ b/gaia.go @@ -4,6 +4,7 @@ import ( "os" "time" + "github.com/dgrijalva/jwt-go" hclog "github.com/hashicorp/go-hclog" "github.com/robfig/cron" ) @@ -156,6 +157,16 @@ const ( StartReasonScheduled = "scheduled" ) +// JwtExpiry is the default JWT expiry. +const JwtExpiry = 12 * 60 * 60 + +// JwtCustomClaims is the custom JWT claims for a Gaia session. +type JwtCustomClaims struct { + Username string `json:"username"` + Roles []string `json:"roles"` + jwt.StandardClaims +} + // User is the user object type User struct { Username string `json:"username,omitempty"` diff --git a/handlers/auth_test.go b/handlers/auth_test.go index 5c1b1e42..c9e5ba6a 100644 --- a/handlers/auth_test.go +++ b/handlers/auth_test.go @@ -128,11 +128,11 @@ func TestAuthBarrierHMACTokenWithHMACKey(t *testing.T) { JWTKey: []byte("hmac-jwt-key"), } - claims := jwtCustomClaims{ + claims := gaia.JwtCustomClaims{ Username: "test-user", Roles: []string{}, StandardClaims: jwt.StandardClaims{ - ExpiresAt: time.Now().Unix() + jwtExpiry, + ExpiresAt: time.Now().Unix() + gaia.JwtExpiry, IssuedAt: time.Now().Unix(), Subject: "Gaia Session Token", }, @@ -164,11 +164,11 @@ func TestAuthBarrierRSATokenWithRSAKey(t *testing.T) { JWTKey: key, } - claims := jwtCustomClaims{ + claims := gaia.JwtCustomClaims{ Username: "test-user", Roles: []string{}, StandardClaims: jwt.StandardClaims{ - ExpiresAt: time.Now().Unix() + jwtExpiry, + ExpiresAt: time.Now().Unix() + gaia.JwtExpiry, IssuedAt: time.Now().Unix(), Subject: "Gaia Session Token", }, @@ -199,11 +199,11 @@ func TestAuthBarrierHMACTokenWithRSAKey(t *testing.T) { JWTKey: &rsa.PrivateKey{}, } - claims := jwtCustomClaims{ + claims := gaia.JwtCustomClaims{ Username: "test-user", Roles: []string{}, StandardClaims: jwt.StandardClaims{ - ExpiresAt: time.Now().Unix() + jwtExpiry, + ExpiresAt: time.Now().Unix() + gaia.JwtExpiry, IssuedAt: time.Now().Unix(), Subject: "Gaia Session Token", }, @@ -242,11 +242,11 @@ func TestAuthBarrierRSATokenWithHMACKey(t *testing.T) { JWTKey: []byte("hmac-jwt-key"), } - claims := jwtCustomClaims{ + claims := gaia.JwtCustomClaims{ Username: "test-user", Roles: []string{}, StandardClaims: jwt.StandardClaims{ - ExpiresAt: time.Now().Unix() + jwtExpiry, + ExpiresAt: time.Now().Unix() + gaia.JwtExpiry, IssuedAt: time.Now().Unix(), Subject: "Gaia Session Token", }, @@ -298,11 +298,11 @@ func TestAuthBarrierNoPerms(t *testing.T) { JWTKey: []byte("hmac-jwt-key"), } - claims := jwtCustomClaims{ + claims := gaia.JwtCustomClaims{ Username: "test-user", Roles: []string{}, StandardClaims: jwt.StandardClaims{ - ExpiresAt: time.Now().Unix() + jwtExpiry, + ExpiresAt: time.Now().Unix() + gaia.JwtExpiry, IssuedAt: time.Now().Unix(), Subject: "Gaia Session Token", }, @@ -333,11 +333,11 @@ func TestAuthBarrierAllPerms(t *testing.T) { JWTKey: []byte("hmac-jwt-key"), } - claims := jwtCustomClaims{ + claims := gaia.JwtCustomClaims{ Username: "test-user", Roles: []string{"CatOneGetSingle", "CatOnePostSingle", "CatTwoGetSingle", "CatTwoPostSingle"}, StandardClaims: jwt.StandardClaims{ - ExpiresAt: time.Now().Unix() + jwtExpiry, + ExpiresAt: time.Now().Unix() + gaia.JwtExpiry, IssuedAt: time.Now().Unix(), Subject: "Gaia Session Token", }, @@ -386,11 +386,11 @@ func Test_AuthMiddleware_Enforcer_PermissionDenied(t *testing.T) { JWTKey: []byte("hmac-jwt-key"), } - claims := jwtCustomClaims{ + claims := gaia.JwtCustomClaims{ Username: "enforcer-perms-err", Roles: []string{}, StandardClaims: jwt.StandardClaims{ - ExpiresAt: time.Now().Unix() + jwtExpiry, + ExpiresAt: time.Now().Unix() + gaia.JwtExpiry, IssuedAt: time.Now().Unix(), Subject: "Gaia Session Token", }, @@ -420,11 +420,11 @@ func Test_AuthMiddleware_Enforcer_UnknownError(t *testing.T) { } gaia.Cfg.Logger = hclog.NewNullLogger() - claims := jwtCustomClaims{ + claims := gaia.JwtCustomClaims{ Username: "enforcer-err", Roles: []string{}, StandardClaims: jwt.StandardClaims{ - ExpiresAt: time.Now().Unix() + jwtExpiry, + ExpiresAt: time.Now().Unix() + gaia.JwtExpiry, IssuedAt: time.Now().Unix(), Subject: "Gaia Session Token", }, diff --git a/handlers/handler.go b/handlers/handler.go index b88bc9b3..2540d92c 100644 --- a/handlers/handler.go +++ b/handlers/handler.go @@ -34,17 +34,14 @@ func (s *GaiaHandler) InitHandlers(e *echo.Echo) error { // Endpoints for Gaia primary instance if gaia.Cfg.Mode == gaia.ModeServer { - // Users - apiGrp.POST("login", UserLogin) - - apiAuthGrp.GET("users", UserGetAll) - apiAuthGrp.POST("user/password", UserChangePassword) - apiAuthGrp.DELETE("user/:username", UserDelete) - apiAuthGrp.GET("user/:username/permissions", UserGetPermissions) - apiAuthGrp.PUT("user/:username/permissions", UserPutPermissions) - apiAuthGrp.POST("user", UserAdd) - apiAuthGrp.PUT("user/:username/reset-trigger-token", UserResetTriggerToken) - + apiGrp.POST("login", s.deps.UserProvider.UserLogin) + apiAuthGrp.GET("users", s.deps.UserProvider.UserGetAll) + apiAuthGrp.POST("user/password", s.deps.UserProvider.UserChangePassword) + apiAuthGrp.DELETE("user/:username", s.deps.UserProvider.UserDelete) + apiAuthGrp.GET("user/:username/permissions", s.deps.UserProvider.UserGetPermissions) + apiAuthGrp.PUT("user/:username/permissions", s.deps.UserProvider.UserPutPermissions) + apiAuthGrp.POST("user", s.deps.UserProvider.UserAdd) + apiAuthGrp.PUT("user/:username/reset-trigger-token", s.deps.UserProvider.UserResetTriggerToken) apiAuthGrp.GET("permission", PermissionGetAll) // Pipelines @@ -86,19 +83,15 @@ func (s *GaiaHandler) InitHandlers(e *echo.Echo) error { apiAuthGrp.POST("secret", SetSecret) apiAuthGrp.PUT("secret/update", SetSecret) - // RBAC - rbacHandler := rbacHandler{ - svc: s.deps.RBACService, - } // RBAC - Management - apiAuthGrp.GET("rbac/roles", rbacHandler.getAllRoles) - apiAuthGrp.PUT("rbac/roles/:role", rbacHandler.addRole) - apiAuthGrp.DELETE("rbac/roles/:role", rbacHandler.deleteRole) - apiAuthGrp.PUT("rbac/roles/:role/attach/:username", rbacHandler.attachRole) - apiAuthGrp.DELETE("rbac/roles/:role/attach/:username", rbacHandler.detachRole) - apiAuthGrp.GET("rbac/roles/:role/attached", rbacHandler.getRolesAttachedUsers) + apiAuthGrp.GET("rbac/roles", s.deps.RBACProvider.GetAllRoles) + apiAuthGrp.PUT("rbac/roles/:role", s.deps.RBACProvider.AddRole) + apiAuthGrp.DELETE("rbac/roles/:role", s.deps.RBACProvider.DeleteRole) + apiAuthGrp.PUT("rbac/roles/:role/attach/:username", s.deps.RBACProvider.DetachRole) + apiAuthGrp.DELETE("rbac/roles/:role/attach/:username", s.deps.RBACProvider.DetachRole) + apiAuthGrp.GET("rbac/roles/:role/attached", s.deps.RBACProvider.GetRolesAttachedUsers) // RBAC - Users - apiAuthGrp.GET("users/:username/rbac/roles", rbacHandler.getUserAttachedRoles) + apiAuthGrp.GET("users/:username/rbac/roles", s.deps.RBACProvider.GetUserAttachedRoles) } // Worker diff --git a/handlers/service.go b/handlers/service.go index 05a97dd0..aeb78dbc 100644 --- a/handlers/service.go +++ b/handlers/service.go @@ -1,6 +1,7 @@ package handlers import ( + "github.com/gaia-pipeline/gaia/providers" "github.com/gaia-pipeline/gaia/providers/pipelines" "github.com/gaia-pipeline/gaia/providers/workers" "github.com/gaia-pipeline/gaia/security" @@ -15,6 +16,8 @@ type Dependencies struct { Scheduler service.GaiaScheduler PipelineService pipeline.Servicer PipelineProvider pipelines.PipelineProviderer + UserProvider providers.UserProvider + RBACProvider providers.RBACProvider WorkerProvider workers.WorkerProviderer Certificate security.CAAPI RBACService rbac.Service diff --git a/providers/provider.go b/providers/provider.go new file mode 100644 index 00000000..bf373cb2 --- /dev/null +++ b/providers/provider.go @@ -0,0 +1,28 @@ +package providers + +import ( + "github.com/labstack/echo" +) + +// RBACProvider provides all the handler endpoints for RBAC actions. +type RBACProvider interface { + AddRole(c echo.Context) error + DeleteRole(c echo.Context) error + GetAllRoles(c echo.Context) error + GetUserAttachedRoles(c echo.Context) error + GetRolesAttachedUsers(c echo.Context) error + AttachRole(c echo.Context) error + DetachRole(c echo.Context) error +} + +// UserProvider provides all the handler endpoints for User actions. +type UserProvider interface { + UserLogin(c echo.Context) error + UserGetAll(c echo.Context) error + UserChangePassword(c echo.Context) error + UserResetTriggerToken(c echo.Context) error + UserDelete(c echo.Context) error + UserAdd(c echo.Context) error + UserGetPermissions(c echo.Context) error + UserPutPermissions(c echo.Context) error +} diff --git a/handlers/rbac.go b/providers/rbac/provider.go similarity index 77% rename from handlers/rbac.go rename to providers/rbac/provider.go index 8bc8f5f2..6478f856 100644 --- a/handlers/rbac.go +++ b/providers/rbac/provider.go @@ -1,4 +1,4 @@ -package handlers +package rbac import ( "net/http" @@ -9,11 +9,18 @@ import ( "github.com/gaia-pipeline/gaia/security/rbac" ) -type rbacHandler struct { +// Provider represents the RBAC provider. +type Provider struct { svc rbac.Service } -func (h *rbacHandler) addRole(c echo.Context) error { +// NewProvider creates a new Provider. +func NewProvider(svc rbac.Service) *Provider { + return &Provider{svc: svc} +} + +// AddRole adds an RBAC role using the RBAC service. +func (h *Provider) AddRole(c echo.Context) error { role := c.Param("role") if role == "" { return c.String(http.StatusBadRequest, "Must provide role.") @@ -33,7 +40,8 @@ func (h *rbacHandler) addRole(c echo.Context) error { return c.String(http.StatusOK, "Role created successfully.") } -func (h *rbacHandler) deleteRole(c echo.Context) error { +// DeleteRole deletes an RBAC role using the RBAC service. +func (h *Provider) DeleteRole(c echo.Context) error { role := c.Param("role") if role == "" { return c.String(http.StatusBadRequest, "Must provide role.") @@ -47,11 +55,13 @@ func (h *rbacHandler) deleteRole(c echo.Context) error { return c.String(http.StatusOK, "Role deleted successfully.") } -func (h *rbacHandler) getAllRoles(c echo.Context) error { +// GetAllRoles gets all RBAC roles. +func (h *Provider) GetAllRoles(c echo.Context) error { return c.JSON(http.StatusOK, h.svc.GetAllRoles()) } -func (h *rbacHandler) getUserAttachedRoles(c echo.Context) error { +// GetUserAttachedRoles gets all roles attached to a specific user. +func (h *Provider) GetUserAttachedRoles(c echo.Context) error { username := c.Param("username") if username == "" { return c.String(http.StatusBadRequest, "Must provide username.") @@ -66,7 +76,8 @@ func (h *rbacHandler) getUserAttachedRoles(c echo.Context) error { return c.JSON(http.StatusOK, roles) } -func (h *rbacHandler) getRolesAttachedUsers(c echo.Context) error { +// GetRolesAttachedUsers gets all users attached to a role. +func (h *Provider) GetRolesAttachedUsers(c echo.Context) error { role := c.Param("role") if role == "" { return c.String(http.StatusBadRequest, "Must provide role.") @@ -81,7 +92,8 @@ func (h *rbacHandler) getRolesAttachedUsers(c echo.Context) error { return c.JSON(http.StatusOK, roles) } -func (h *rbacHandler) attachRole(c echo.Context) error { +// AttachRole attches a role to a user. +func (h *Provider) AttachRole(c echo.Context) error { role := c.Param("role") if role == "" { return c.String(http.StatusBadRequest, "Must provide role.") @@ -100,7 +112,8 @@ func (h *rbacHandler) attachRole(c echo.Context) error { return c.String(http.StatusOK, "Role attached successfully.") } -func (h *rbacHandler) detachRole(c echo.Context) error { +// DetachRole deteches a role from a user. +func (h *Provider) DetachRole(c echo.Context) error { role := c.Param("role") if role == "" { return c.String(http.StatusBadRequest, "Must provide role.") diff --git a/handlers/rbac_test.go b/providers/rbac/provider_test.go similarity index 90% rename from handlers/rbac_test.go rename to providers/rbac/provider_test.go index dd32f851..5e743799 100644 --- a/handlers/rbac_test.go +++ b/providers/rbac/provider_test.go @@ -1,4 +1,4 @@ -package handlers +package rbac import ( "bytes" @@ -65,8 +65,8 @@ func (e *mockRBACSvc) DetachRole(username string, role string) error { return errors.New("an error") } -func Test_rbacHandler_addRole(t *testing.T) { - handler := rbacHandler{ +func Test_rbacHandler_AddRole(t *testing.T) { + handler := Provider{ svc: &mockRBACSvc{}, } @@ -101,7 +101,7 @@ func Test_rbacHandler_addRole(t *testing.T) { c.SetParamNames("role") c.SetParamValues("success") - err := handler.addRole(c) + err := handler.AddRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusOK) assert.Equal(t, rec.Body.String(), "Role created successfully.") @@ -114,7 +114,7 @@ func Test_rbacHandler_addRole(t *testing.T) { c := e.NewContext(req, rec) c.SetPath("/api/v1/rbac/roles/:role") - err := handler.addRole(c) + err := handler.AddRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusBadRequest) assert.Equal(t, rec.Body.String(), "Must provide role.") @@ -130,7 +130,7 @@ func Test_rbacHandler_addRole(t *testing.T) { c.SetParamNames("role") c.SetParamValues("success") - err := handler.addRole(c) + err := handler.AddRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusBadRequest) assert.Equal(t, rec.Body.String(), "Invalid body provided.") @@ -146,15 +146,15 @@ func Test_rbacHandler_addRole(t *testing.T) { c.SetParamNames("role") c.SetParamValues("error") - err := handler.addRole(c) + err := handler.AddRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusInternalServerError) assert.Equal(t, rec.Body.String(), "An error occurred while adding the role.") }) } -func Test_rbacHandler_deleteRole(t *testing.T) { - handler := rbacHandler{ +func Test_Provider_DeleteRole(t *testing.T) { + handler := Provider{ svc: &mockRBACSvc{}, } @@ -174,7 +174,7 @@ func Test_rbacHandler_deleteRole(t *testing.T) { c.SetParamNames("role") c.SetParamValues("delme") - err := handler.deleteRole(c) + err := handler.DeleteRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusOK) assert.Equal(t, rec.Body.String(), "Role deleted successfully.") @@ -186,7 +186,7 @@ func Test_rbacHandler_deleteRole(t *testing.T) { c := e.NewContext(req, rec) c.SetPath("/api/v1/rbac/roles/:role") - err := handler.deleteRole(c) + err := handler.DeleteRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusBadRequest) assert.Equal(t, rec.Body.String(), "Must provide role.") @@ -200,15 +200,15 @@ func Test_rbacHandler_deleteRole(t *testing.T) { c.SetParamNames("role") c.SetParamValues("error") - err := handler.deleteRole(c) + err := handler.DeleteRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusInternalServerError) assert.Equal(t, rec.Body.String(), "An error occurred while deleting the role.") }) } -func Test_rbacHandler_getAllRoles(t *testing.T) { - handler := rbacHandler{ +func Test_Provider_getAllRoles(t *testing.T) { + handler := Provider{ svc: &mockRBACSvc{}, } @@ -225,14 +225,14 @@ func Test_rbacHandler_getAllRoles(t *testing.T) { c := e.NewContext(req, rec) c.SetPath("/api/v1/rbac/roles") - err := handler.getAllRoles(c) + err := handler.GetAllRoles(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusOK) assert.Equal(t, rec.Body.String(), "[\"role-a\",\"role-b\"]\n") } -func Test_rbacHandler_getUserAttachedRoles(t *testing.T) { - handler := rbacHandler{ +func Test_Provider_getUserAttachedRoles(t *testing.T) { + handler := Provider{ svc: &mockRBACSvc{}, } @@ -252,7 +252,7 @@ func Test_rbacHandler_getUserAttachedRoles(t *testing.T) { c.SetParamNames("username") c.SetParamValues("test") - err := handler.getUserAttachedRoles(c) + err := handler.GetUserAttachedRoles(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusOK) assert.Equal(t, rec.Body.String(), "[\"role-a\",\"role-b\"]\n") @@ -264,7 +264,7 @@ func Test_rbacHandler_getUserAttachedRoles(t *testing.T) { c := e.NewContext(req, rec) c.SetPath("/api/v1/users/:username/rbac/roles") - err := handler.getUserAttachedRoles(c) + err := handler.GetUserAttachedRoles(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusBadRequest) assert.Equal(t, rec.Body.String(), "Must provide username.") @@ -278,15 +278,15 @@ func Test_rbacHandler_getUserAttachedRoles(t *testing.T) { c.SetParamNames("username") c.SetParamValues("error") - err := handler.getUserAttachedRoles(c) + err := handler.GetUserAttachedRoles(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusInternalServerError) assert.Equal(t, rec.Body.String(), "An error occurred while getting the roles.") }) } -func Test_rbacHandler_getRolesAttachedUsers(t *testing.T) { - handler := rbacHandler{ +func Test_Provider_GetRolesAttachedUsers(t *testing.T) { + handler := Provider{ svc: &mockRBACSvc{}, } @@ -306,7 +306,7 @@ func Test_rbacHandler_getRolesAttachedUsers(t *testing.T) { c.SetParamNames("role") c.SetParamValues("test") - err := handler.getRolesAttachedUsers(c) + err := handler.GetRolesAttachedUsers(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusOK) assert.Equal(t, rec.Body.String(), "[\"user-a\",\"user-b\"]\n") @@ -318,7 +318,7 @@ func Test_rbacHandler_getRolesAttachedUsers(t *testing.T) { c := e.NewContext(req, rec) c.SetPath("/api/v1/rbac/roles/:role/attached") - err := handler.getRolesAttachedUsers(c) + err := handler.GetRolesAttachedUsers(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusBadRequest) assert.Equal(t, rec.Body.String(), "Must provide role.") @@ -332,15 +332,15 @@ func Test_rbacHandler_getRolesAttachedUsers(t *testing.T) { c.SetParamNames("role") c.SetParamValues("error") - err := handler.getRolesAttachedUsers(c) + err := handler.GetRolesAttachedUsers(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusInternalServerError) assert.Equal(t, rec.Body.String(), "An error occurred while getting the users.") }) } -func Test_rbacHandler_attachRole(t *testing.T) { - handler := rbacHandler{ +func Test_Provider_attachRole(t *testing.T) { + provider := Provider{ svc: &mockRBACSvc{}, } @@ -360,7 +360,7 @@ func Test_rbacHandler_attachRole(t *testing.T) { c.SetParamNames("role", "username") c.SetParamValues("test-role", "test-user") - err := handler.attachRole(c) + err := provider.AttachRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusOK) assert.Equal(t, rec.Body.String(), "Role attached successfully.") @@ -372,7 +372,7 @@ func Test_rbacHandler_attachRole(t *testing.T) { c := e.NewContext(req, rec) c.SetPath("/api/v1/rbac/roles/:role/attach/:username") - err := handler.attachRole(c) + err := provider.AttachRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusBadRequest) assert.Equal(t, rec.Body.String(), "Must provide role.") @@ -386,7 +386,7 @@ func Test_rbacHandler_attachRole(t *testing.T) { c.SetParamNames("role") c.SetParamValues("test-role") - err := handler.attachRole(c) + err := provider.AttachRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusBadRequest) assert.Equal(t, rec.Body.String(), "Must provide username.") @@ -400,15 +400,15 @@ func Test_rbacHandler_attachRole(t *testing.T) { c.SetParamNames("role", "username") c.SetParamValues("error", "error") - err := handler.attachRole(c) + err := provider.AttachRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusInternalServerError) assert.Equal(t, rec.Body.String(), "An error occurred while attaching the role.") }) } -func Test_rbacHandler_detachRole(t *testing.T) { - handler := rbacHandler{ +func Test_Provider_detachRole(t *testing.T) { + handler := Provider{ svc: &mockRBACSvc{}, } @@ -428,7 +428,7 @@ func Test_rbacHandler_detachRole(t *testing.T) { c.SetParamNames("role", "username") c.SetParamValues("test-role", "test-user") - err := handler.detachRole(c) + err := handler.DetachRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusOK) assert.Equal(t, rec.Body.String(), "Role detached successfully.") @@ -440,7 +440,7 @@ func Test_rbacHandler_detachRole(t *testing.T) { c := e.NewContext(req, rec) c.SetPath("/api/v1/rbac/roles/:role/attach/:username") - err := handler.detachRole(c) + err := handler.DetachRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusBadRequest) assert.Equal(t, rec.Body.String(), "Must provide role.") @@ -454,7 +454,7 @@ func Test_rbacHandler_detachRole(t *testing.T) { c.SetParamNames("role") c.SetParamValues("test-role") - err := handler.detachRole(c) + err := handler.DetachRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusBadRequest) assert.Equal(t, rec.Body.String(), "Must provide username.") @@ -468,7 +468,7 @@ func Test_rbacHandler_detachRole(t *testing.T) { c.SetParamNames("role", "username") c.SetParamValues("error", "error") - err := handler.detachRole(c) + err := handler.DetachRole(c) assert.NoError(t, err) assert.Equal(t, rec.Code, http.StatusInternalServerError) assert.Equal(t, rec.Body.String(), "An error occurred while detaching the role.") diff --git a/handlers/user.go b/providers/user/provider.go similarity index 73% rename from handlers/user.go rename to providers/user/provider.go index 8fe4b975..f8e950d5 100644 --- a/handlers/user.go +++ b/providers/user/provider.go @@ -1,4 +1,4 @@ -package handlers +package user import ( "crypto/rsa" @@ -6,27 +6,29 @@ import ( "time" "github.com/dgrijalva/jwt-go" + "github.com/labstack/echo" + "github.com/gaia-pipeline/gaia" "github.com/gaia-pipeline/gaia/helper/rolehelper" "github.com/gaia-pipeline/gaia/security" - "github.com/gaia-pipeline/gaia/services" - "github.com/labstack/echo" + "github.com/gaia-pipeline/gaia/security/rbac" + "github.com/gaia-pipeline/gaia/store" ) -// jwtExpiry defines how long the produced jwt tokens -// are valid. By default 12 hours. -const jwtExpiry = 12 * 60 * 60 +// Provider represents the user handlers and contains any dependencies required by the handlers. +type Provider struct { + Store store.GaiaStore + RBACSvc rbac.Service +} -type jwtCustomClaims struct { - Username string `json:"username"` - Roles []string `json:"roles"` - jwt.StandardClaims +// NewProvider creates a new provider. +func NewProvider(store store.GaiaStore, RBACSvc rbac.Service) *Provider { + return &Provider{Store: store, RBACSvc: RBACSvc} } // UserLogin authenticates the user with // the given credentials. -func UserLogin(c echo.Context) error { - storeService, _ := services.StorageService() +func (h *Provider) UserLogin(c echo.Context) error { u := &gaia.User{} if err := c.Bind(u); err != nil { gaia.Cfg.Logger.Debug("error reading json during UserLogin", "error", err.Error()) @@ -34,23 +36,23 @@ func UserLogin(c echo.Context) error { } // Authenticate user - user, err := storeService.UserAuth(u, true) + user, err := h.Store.UserAuth(u, true) if err != nil || user == nil { gaia.Cfg.Logger.Info("invalid credentials provided", "username", u.Username) return c.String(http.StatusForbidden, "invalid username and/or password") } - perms, err := storeService.UserPermissionsGet(u.Username) + perms, err := h.Store.UserPermissionsGet(u.Username) if err != nil { return err } // Setup custom claims - claims := jwtCustomClaims{ + claims := gaia.JwtCustomClaims{ Username: user.Username, Roles: perms.Roles, StandardClaims: jwt.StandardClaims{ - ExpiresAt: time.Now().Unix() + jwtExpiry, + ExpiresAt: time.Now().Unix() + gaia.JwtExpiry, IssuedAt: time.Now().Unix(), Subject: "Gaia Session Token", }, @@ -82,10 +84,9 @@ func UserLogin(c echo.Context) error { } // UserGetAll returns all users stored in store. -func UserGetAll(c echo.Context) error { +func (h *Provider) UserGetAll(c echo.Context) error { // Get all users - storeService, _ := services.StorageService() - users, err := storeService.UserGetAll() + users, err := h.Store.UserGetAll() if err != nil { return c.String(http.StatusInternalServerError, err.Error()) } @@ -101,24 +102,23 @@ type changePasswordRequest struct { } // UserChangePassword changes the password from a user. -func UserChangePassword(c echo.Context) error { +func (h *Provider) UserChangePassword(c echo.Context) error { // Get required parameters r := &changePasswordRequest{} - storeService, _ := services.StorageService() if err := c.Bind(r); err != nil { return c.String(http.StatusBadRequest, "Invalid parameters given for password change request") } // Compare old password with current password of user by simply calling auth method. // First get user obj - user, err := storeService.UserGet(r.Username) + user, err := h.Store.UserGet(r.Username) if err != nil { return c.String(http.StatusBadRequest, "Cannot find user with the given username") } // Simply call auth by changing password user.Password = r.OldPassword - u, err := storeService.UserAuth(user, false) + u, err := h.Store.UserAuth(user, false) if err != nil { return c.String(http.StatusPreconditionFailed, "Wrong password given for password change") } @@ -130,8 +130,7 @@ func UserChangePassword(c echo.Context) error { // Change password u.Password = r.NewPassword - err = storeService.UserPut(u, true) - if err != nil { + if err := h.Store.UserPut(u, true); err != nil { return c.String(http.StatusInternalServerError, "Cannot update user in store") } @@ -140,17 +139,16 @@ func UserChangePassword(c echo.Context) error { // UserResetTriggerToken will generate and save a new Remote trigger token // for a given user. -func UserResetTriggerToken(c echo.Context) error { - // Get user which we should reset the token for - u := c.Param("username") - if u == "" { +func (h *Provider) UserResetTriggerToken(c echo.Context) error { + username := c.Param("username") + if username == "" { return c.String(http.StatusBadRequest, "Invalid username given") } - if u != "auto" { + if username != "auto" { return c.String(http.StatusBadRequest, "Only auto user can have a token reset") } - ss, _ := services.StorageService() - user, err := ss.UserGet(u) + + user, err := h.Store.UserGet(username) if err != nil { return c.String(http.StatusBadRequest, "User not found") } @@ -159,7 +157,7 @@ func UserResetTriggerToken(c echo.Context) error { } user.TriggerToken = security.GenerateRandomUUIDV5() - err = ss.UserPut(user, true) + err = h.Store.UserPut(user, true) if err != nil { return c.String(http.StatusInternalServerError, "Error while saving user") } @@ -168,25 +166,26 @@ func UserResetTriggerToken(c echo.Context) error { } // UserDelete deletes the given user -func UserDelete(c echo.Context) error { - // Get user which we should delete - u := c.Param("username") - if u == "" { +func (h *Provider) UserDelete(c echo.Context) error { + username := c.Param("username") + + if username == "" { return c.String(http.StatusBadRequest, "Invalid username given") } - storeService, _ := services.StorageService() - if u == "auto" { + + if username == "auto" { return c.String(http.StatusBadRequest, "Auto user cannot be deleted") } - // Delete user - err := storeService.UserDelete(u) - if err != nil { + + if err := h.Store.UserDelete(username); err != nil { return c.String(http.StatusNotFound, err.Error()) } - // Delete permissions - err = storeService.UserPermissionsDelete(u) - if err != nil { + if err := h.Store.UserPermissionsDelete(username); err != nil { + return c.String(http.StatusNotFound, err.Error()) + } + + if err := h.RBACSvc.DeleteUser(username); err != nil { return c.String(http.StatusNotFound, err.Error()) } @@ -194,17 +193,16 @@ func UserDelete(c echo.Context) error { } // UserAdd adds a new user to the store. -func UserAdd(c echo.Context) error { +func (h *Provider) UserAdd(c echo.Context) error { // Get user information required for add u := &gaia.User{} if err := c.Bind(u); err != nil { return c.String(http.StatusBadRequest, "Invalid parameters given for add user request") } - storeService, _ := services.StorageService() // Add user u.LastLogin = time.Now() - err := storeService.UserPut(u, true) + err := h.Store.UserPut(u, true) if err != nil { return c.String(http.StatusInternalServerError, err.Error()) } @@ -215,7 +213,7 @@ func UserAdd(c echo.Context) error { Roles: rolehelper.FlattenUserCategoryRoles(rolehelper.DefaultUserRoles), Groups: []string{}, } - err = storeService.UserPermissionsPut(perms) + err = h.Store.UserPermissionsPut(perms) if err != nil { return c.String(http.StatusInternalServerError, err.Error()) } @@ -224,26 +222,27 @@ func UserAdd(c echo.Context) error { } // UserGetPermissions returns the permissions for a user. -func UserGetPermissions(c echo.Context) error { +func (h *Provider) UserGetPermissions(c echo.Context) error { u := c.Param("username") - storeService, _ := services.StorageService() - perms, err := storeService.UserPermissionsGet(u) + + perms, err := h.Store.UserPermissionsGet(u) if err != nil { return c.String(http.StatusBadRequest, err.Error()) } + return c.JSON(http.StatusOK, perms) } // UserPutPermissions adds or updates permissions for a user. -func UserPutPermissions(c echo.Context) error { +func (h *Provider) UserPutPermissions(c echo.Context) error { var perms *gaia.UserPermission if err := c.Bind(&perms); err != nil { return c.String(http.StatusBadRequest, "Invalid parameters given for request") } - storeService, _ := services.StorageService() - err := storeService.UserPermissionsPut(perms) - if err != nil { + + if err := h.Store.UserPermissionsPut(perms); err != nil { return c.String(http.StatusBadRequest, err.Error()) } + return c.String(http.StatusOK, "Permissions have been updated") } diff --git a/handlers/user_test.go b/providers/user/provider_test.go similarity index 85% rename from handlers/user_test.go rename to providers/user/provider_test.go index 17273aac..5ae881ca 100644 --- a/handlers/user_test.go +++ b/providers/user/provider_test.go @@ -1,4 +1,4 @@ -package handlers +package user import ( "bytes" @@ -16,10 +16,31 @@ import ( "github.com/pkg/errors" "github.com/gaia-pipeline/gaia" - "github.com/gaia-pipeline/gaia/services" gStore "github.com/gaia-pipeline/gaia/store" ) +type mockUserStorageService struct { + gStore.GaiaStore + user *gaia.User + err error +} + +func (m *mockUserStorageService) UserAuth(u *gaia.User, updateLastLogin bool) (*gaia.User, error) { + return m.user, m.err +} + +func (m *mockUserStorageService) UserGet(username string) (*gaia.User, error) { + return m.user, m.err +} + +func (m *mockUserStorageService) UserPut(u *gaia.User, encryptPassword bool) error { + return nil +} + +func (m *mockUserStorageService) UserPermissionsGet(username string) (*gaia.UserPermission, error) { + return &gaia.UserPermission{}, nil +} + func TestUserLoginHMACKey(t *testing.T) { tmp, _ := ioutil.TempDir("", "TestUserLoginHMACKey") dataDir := tmp @@ -51,7 +72,13 @@ func TestUserLoginHMACKey(t *testing.T) { rec := httptest.NewRecorder() c := e.NewContext(req, rec) - if err := UserLogin(c); err != nil { + ms := &mockUserStorageService{user: &gaia.User{ + Username: "username", + Password: "password", + }, err: nil} + + provider := NewProvider(ms, nil) + if err := provider.UserLogin(c); err != nil { t.Fatal(err) } @@ -98,27 +125,14 @@ func TestDeleteUserNotAllowedForAutoUser(t *testing.T) { rec := httptest.NewRecorder() c := e.NewContext(req, rec) - _ = UserDelete(c) + provider := NewProvider(nil, nil) + _ = provider.UserDelete(c) if rec.Code != http.StatusBadRequest { t.Fatalf("expected response code %v got %v", http.StatusBadRequest, rec.Code) } } -type mockUserStorageService struct { - gStore.GaiaStore - user *gaia.User - err error -} - -func (m mockUserStorageService) UserGet(username string) (*gaia.User, error) { - return m.user, m.err -} - -func (m mockUserStorageService) UserPut(u *gaia.User, encryptPassword bool) error { - return nil -} - func TestResetAutoUserTriggerToken(t *testing.T) { dataDir, _ := ioutil.TempDir("", "TestResetAutoUserTriggerToken") @@ -137,9 +151,7 @@ func TestResetAutoUserTriggerToken(t *testing.T) { user := gaia.User{} user.Username = "auto" user.TriggerToken = "triggerToken" - m := mockUserStorageService{user: &user, err: nil} - services.MockStorageService(&m) - defer services.MockStorageService(nil) + ms := &mockUserStorageService{user: &user, err: nil} e := echo.New() req := httptest.NewRequest(echo.PUT, "/api/"+gaia.APIVersion+"/user/auto/reset-trigger-token", nil) req.Header.Set("Content-Type", "application/json") @@ -148,7 +160,8 @@ func TestResetAutoUserTriggerToken(t *testing.T) { c.SetParamNames("username") c.SetParamValues("auto") - _ = UserResetTriggerToken(c) + provider := NewProvider(ms, nil) + _ = provider.UserResetTriggerToken(c) if rec.Code != http.StatusOK { t.Fatalf("expected response code %v got %v; error: %s", http.StatusOK, rec.Code, rec.Body.String()) @@ -167,7 +180,8 @@ func TestResetAutoUserTriggerToken(t *testing.T) { c.SetParamNames("username") c.SetParamValues("auto2") - _ = UserResetTriggerToken(c) + provider := NewProvider(nil, nil) + _ = provider.UserResetTriggerToken(c) if rec.Code != http.StatusBadRequest { t.Fatalf("expected response code %v got %v; error: %s", http.StatusBadRequest, rec.Code, rec.Body.String()) @@ -194,6 +208,10 @@ func TestUserLoginRSAKey(t *testing.T) { DataPath: dataDir, Mode: gaia.ModeServer, } + ms := &mockUserStorageService{user: &gaia.User{ + Username: "username", + Password: "password", + }, err: nil} e := echo.New() @@ -207,7 +225,8 @@ func TestUserLoginRSAKey(t *testing.T) { rec := httptest.NewRecorder() c := e.NewContext(req, rec) - if err := UserLogin(c); err != nil { + provider := NewProvider(ms, nil) + if err := provider.UserLogin(c); err != nil { t.Fatal(err) } @@ -255,8 +274,6 @@ func TestUserPutPermissions(t *testing.T) { }, } - services.MockStorageService(ms) - bts, _ := json.Marshal(&gaia.UserPermission{ Username: "test-user", Roles: []string{"TestRole"}, @@ -271,7 +288,9 @@ func TestUserPutPermissions(t *testing.T) { c.SetPath("/api/v1/user/:username/permissions") c.SetParamNames("username") c.SetParamValues("test-user") - _ = UserPutPermissions(c) + + provider := NewProvider(ms, nil) + _ = provider.UserPutPermissions(c) if rec.Code != http.StatusOK { t.Fatalf("code is %d. expected %d", rec.Code, http.StatusOK) @@ -285,8 +304,6 @@ func TestUserPutPermissionsError(t *testing.T) { }, } - services.MockStorageService(ms) - bts, _ := json.Marshal(&gaia.UserPermission{ Username: "test-user", Roles: []string{"TestRole"}, @@ -301,7 +318,9 @@ func TestUserPutPermissionsError(t *testing.T) { c.SetPath("/api/v1/user/:username/permissions") c.SetParamNames("username") c.SetParamValues("test-user") - _ = UserPutPermissions(c) + + provider := NewProvider(ms, nil) + _ = provider.UserPutPermissions(c) if rec.Code != http.StatusBadRequest { t.Fatalf("code is %d. expected %d", rec.Code, http.StatusBadRequest) @@ -319,8 +338,6 @@ func TestUserGetPermissions(t *testing.T) { }, } - services.MockStorageService(ms) - e := echo.New() req := httptest.NewRequest(http.MethodGet, "/", nil) rec := httptest.NewRecorder() @@ -328,7 +345,9 @@ func TestUserGetPermissions(t *testing.T) { c.SetPath("/api/v1/user/:username/permissions") c.SetParamNames("username") c.SetParamValues("test-user") - _ = UserGetPermissions(c) + + provider := NewProvider(ms, nil) + _ = provider.UserGetPermissions(c) if rec.Code != http.StatusOK { t.Fatalf("code is %d. expected %d", rec.Code, http.StatusOK) @@ -342,8 +361,6 @@ func TestUserGetPermissionsErrors(t *testing.T) { }, } - services.MockStorageService(ms) - e := echo.New() req := httptest.NewRequest(http.MethodGet, "/", nil) rec := httptest.NewRecorder() @@ -351,7 +368,9 @@ func TestUserGetPermissionsErrors(t *testing.T) { c.SetPath("/api/v1/user/:username/permissions") c.SetParamNames("username") c.SetParamValues("test-user") - _ = UserGetPermissions(c) + + provider := NewProvider(ms, nil) + _ = provider.UserGetPermissions(c) if rec.Code != http.StatusBadRequest { t.Fatalf("code is %d. expected %d", rec.Code, http.StatusBadRequest) diff --git a/security/rbac/noop.go b/security/rbac/noop.go index bc83e416..66d1c041 100644 --- a/security/rbac/noop.go +++ b/security/rbac/noop.go @@ -1,9 +1,5 @@ package rbac -import "errors" - -var errNotEnabled = errors.New("rbac is not enabled") - type noOpService struct{} // NewNoOpService is used to instantiated a noOpService for when rbac enabled=false. @@ -19,12 +15,12 @@ func (n noOpService) Enforce(username, method, path string, params map[string]st // AddRole that errors since rbac is not enabled. func (n noOpService) AddRole(role string, roleRules []RoleRule) error { - return errNotEnabled + return nil } // DeleteRole that errors since rbac is not enabled. func (n noOpService) DeleteRole(role string) error { - return errNotEnabled + return nil } // GetAllRoles that returns nothing since rbac is not enabled. @@ -34,20 +30,24 @@ func (n noOpService) GetAllRoles() []string { // GetUserAttachedRoles that errors since rbac is not enabled. func (n noOpService) GetUserAttachedRoles(username string) ([]string, error) { - return nil, errNotEnabled + return nil, nil } // GetRoleAttachedUsers that errors since rbac is not enabled. func (n noOpService) GetRoleAttachedUsers(role string) ([]string, error) { - return nil, errNotEnabled + return nil, nil } // AttachRole that errors since rbac is not enabled. func (n noOpService) AttachRole(username string, role string) error { - return errNotEnabled + return nil } // DetachRole that errors since rbac is not enabled. func (n noOpService) DetachRole(username string, role string) error { - return errNotEnabled + return nil +} + +func (n noOpService) DeleteUser(username string) error { + return nil } diff --git a/security/rbac/noop_test.go b/security/rbac/noop_test.go index 714bc07e..0cbf1524 100644 --- a/security/rbac/noop_test.go +++ b/security/rbac/noop_test.go @@ -15,13 +15,13 @@ func TestNoOpService_Enforce_AlwaysReturnsNoError(t *testing.T) { func TestNoOpService_AddRole_ReturnsErrNotEnabled(t *testing.T) { svc := NewNoOpService() err := svc.AddRole("", []RoleRule{}) - assert.EqualError(t, err, errNotEnabled.Error()) + assert.NoError(t, err) } func TestNoOpService_DeleteRole_ReturnsErrNotEnabled(t *testing.T) { svc := NewNoOpService() err := svc.DeleteRole("") - assert.EqualError(t, err, errNotEnabled.Error()) + assert.NoError(t, err) } func TestNoOpService_GetAllRoles_ReturnsEmptySlice(t *testing.T) { @@ -33,23 +33,23 @@ func TestNoOpService_GetAllRoles_ReturnsEmptySlice(t *testing.T) { func TestNoOpService_GetUserAttachedRoles_ReturnsErrNotEnabled(t *testing.T) { svc := NewNoOpService() _, err := svc.GetUserAttachedRoles("") - assert.EqualError(t, err, errNotEnabled.Error()) + assert.NoError(t, err) } func TestNoOpService_GetRoleAttachedUsers_ReturnsErrNotEnabled(t *testing.T) { svc := NewNoOpService() _, err := svc.GetRoleAttachedUsers("") - assert.EqualError(t, err, errNotEnabled.Error()) + assert.NoError(t, err) } func TestNoOpService_AttachRole_ReturnsErrNotEnabled(t *testing.T) { svc := NewNoOpService() err := svc.AttachRole("", "") - assert.EqualError(t, err, errNotEnabled.Error()) + assert.NoError(t, err) } func TestNoOpService_DetachRole_ReturnsErrNotEnabled(t *testing.T) { svc := NewNoOpService() err := svc.DetachRole("", "") - assert.EqualError(t, err, errNotEnabled.Error()) + assert.NoError(t, err) } diff --git a/security/rbac/service.go b/security/rbac/service.go index a0a0040d..5e9b96d0 100644 --- a/security/rbac/service.go +++ b/security/rbac/service.go @@ -47,6 +47,7 @@ type ( GetRoleAttachedUsers(role string) ([]string, error) AttachRole(username string, role string) error DetachRole(username string, role string) error + DeleteUser(username string) error } enforcerService struct { @@ -199,3 +200,11 @@ func (e *enforcerService) DetachRole(username string, role string) error { } return nil } + +// DeleteUser removes the user from the rbac model. +func (e *enforcerService) DeleteUser(username string) error { + if _, err := e.enforcer.DeleteUser(username); err != nil { + return fmt.Errorf("error deleting user: %w", err) + } + return nil +} diff --git a/server/server.go b/server/server.go index 241c875a..45626129 100644 --- a/server/server.go +++ b/server/server.go @@ -20,6 +20,8 @@ import ( "github.com/gaia-pipeline/gaia/handlers" "github.com/gaia-pipeline/gaia/plugin" "github.com/gaia-pipeline/gaia/providers/pipelines" + rbacProvider "github.com/gaia-pipeline/gaia/providers/rbac" + userProvider "github.com/gaia-pipeline/gaia/providers/user" "github.com/gaia-pipeline/gaia/providers/workers" "github.com/gaia-pipeline/gaia/security" "github.com/gaia-pipeline/gaia/security/rbac" @@ -266,6 +268,8 @@ func Start() (err error) { PipelineService: pipelineService, SettingsStore: store, }) + rbacPrv := rbacProvider.NewProvider(rbacService) + userPrv := userProvider.NewProvider(store, rbacService) // initialize the worker provider workerProvider := workers.NewWorkerProvider(workers.Dependencies{ Scheduler: schedulerService, @@ -277,6 +281,8 @@ func Start() (err error) { PipelineService: pipelineService, PipelineProvider: pipelineProvider, WorkerProvider: workerProvider, + RBACProvider: rbacPrv, + UserProvider: userPrv, Certificate: ca, RBACService: rbacService, Store: store,