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

Author: speza

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"`

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",
 		},

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

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

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
+}

handlers/rbac.go 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.")