Skip to content

Commit

Permalink
Merge pull request #48 from Michael2008S/master
Browse files Browse the repository at this point in the history
change user's RoleID to AccessRole type
  • Loading branch information
ribice authored Oct 11, 2018
2 parents 8a20e21 + 06d29d5 commit 0402da6
Show file tree
Hide file tree
Showing 15 changed files with 52 additions and 52 deletions.
2 changes: 1 addition & 1 deletion cmd/api/mw/jwt.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (j *JWT) MWFunc() echo.MiddlewareFunc {
locationID := int(claims["l"].(float64))
username := claims["u"].(string)
email := claims["e"].(string)
role := int8(claims["r"].(float64))
role := model.AccessRole(claims["r"].(float64))

c.Set("id", id)
c.Set("company_id", companyID)
Expand Down
8 changes: 4 additions & 4 deletions cmd/api/request/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ type Register struct {
PasswordConfirm string `json:"password_confirm" validate:"required"`
Email string `json:"email" validate:"required,email"`

CompanyID int `json:"company_id" validate:"required"`
LocationID int `json:"location_id" validate:"required"`
RoleID int `json:"role_id" validate:"required"`
CompanyID int `json:"company_id" validate:"required"`
LocationID int `json:"location_id" validate:"required"`
RoleID model.AccessRole `json:"role_id" validate:"required"`
}

// AccountCreate validates account creation request
Expand All @@ -30,7 +30,7 @@ func AccountCreate(c echo.Context) (*Register, error) {
if r.Password != r.PasswordConfirm {
return nil, echo.NewHTTPError(http.StatusBadRequest, "passwords do not match")
}
if r.RoleID < int(model.SuperAdminRole) || r.RoleID > int(model.UserRole) {
if r.RoleID < model.SuperAdminRole || r.RoleID > model.UserRole {
return nil, echo.NewHTTPError(http.StatusBadRequest)
}
return r, nil
Expand Down
4 changes: 2 additions & 2 deletions cmd/api/service/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestCreate(t *testing.T) {
name: "Fail on userSvc",
req: `{"first_name":"John","last_name":"Doe","username":"juzernejm","password":"hunter123","password_confirm":"hunter123","email":"[email protected]","company_id":1,"location_id":2,"role_id":2}`,
rbac: &mock.RBAC{
AccountCreateFn: func(c echo.Context, roleID, companyID, locationID int) error {
AccountCreateFn: func(c echo.Context, roleID model.AccessRole, companyID, locationID int) error {
return echo.ErrForbidden
},
},
Expand All @@ -50,7 +50,7 @@ func TestCreate(t *testing.T) {
name: "Success",
req: `{"first_name":"John","last_name":"Doe","username":"juzernejm","password":"hunter123","password_confirm":"hunter123","email":"[email protected]","company_id":1,"location_id":2,"role_id":2}`,
rbac: &mock.RBAC{
AccountCreateFn: func(c echo.Context, roleID, companyID, locationID int) error {
AccountCreateFn: func(c echo.Context, roleID model.AccessRole, companyID, locationID int) error {
return nil
},
},
Expand Down
4 changes: 2 additions & 2 deletions cmd/migration/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"fmt"
"github.com/ribice/gorsk/internal/auth"
"log"
"strings"

Expand All @@ -10,7 +11,6 @@ import (
"github.com/ribice/gorsk/internal"

"github.com/go-pg/pg"
"github.com/ribice/gorsk/internal/auth"
)

func main() {
Expand All @@ -35,7 +35,7 @@ func main() {
_, err := db.Exec(v)
checkErr(err)
}
userInsert := `INSERT INTO public.users VALUES (1, now(),now(), NULL, 'Admin', 'Admin', 'admin', '%s', '[email protected]', NULL, NULL, NULL, NULL, true, 1, 1, 1);`
userInsert := `INSERT INTO public.users VALUES (1, now(),now(), NULL, 'Admin', 'Admin', 'admin', '%s', '[email protected]', NULL, NULL, NULL, NULL, true, NULL, 1, 1, 1);`
_, err = db.Exec(fmt.Sprintf(userInsert, auth.HashPassword("admin")))
checkErr(err)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/account/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestCreate(t *testing.T) {
}{{
name: "Fail on is lower role",
rbac: &mock.RBAC{
AccountCreateFn: func(echo.Context, int, int, int) error {
AccountCreateFn: func(echo.Context, model.AccessRole, int, int) error {
return model.ErrGeneric
}},
wantErr: true,
Expand Down Expand Up @@ -60,7 +60,7 @@ func TestCreate(t *testing.T) {
},
},
rbac: &mock.RBAC{
AccountCreateFn: func(echo.Context, int, int, int) error {
AccountCreateFn: func(echo.Context, model.AccessRole, int, int) error {
return nil
}},
wantData: &model.User{
Expand Down
2 changes: 1 addition & 1 deletion internal/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ type RBACService interface {
EnforceUser(echo.Context, int) error
EnforceCompany(echo.Context, int) error
EnforceLocation(echo.Context, int) error
AccountCreate(echo.Context, int, int, int) error
AccountCreate(echo.Context, AccessRole, int, int) error
IsLowerRole(echo.Context, AccessRole) error
}
2 changes: 1 addition & 1 deletion internal/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (s *Service) User(c echo.Context) *model.AuthUser {
locationID := c.Get("location_id").(int)
user := c.Get("username").(string)
email := c.Get("email").(string)
role := c.Get("role").(int8)
role := c.Get("role").(model.AccessRole)
return &model.AuthUser{
ID: id,
Username: user,
Expand Down
4 changes: 2 additions & 2 deletions internal/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func TestRefresh(t *testing.T) {
func TestUser(t *testing.T) {
ctx := mock.EchoCtxWithKeys([]string{
"id", "company_id", "location_id", "username", "email", "role"},
9, 15, 52, "ribice", "[email protected]", int8(1))
9, 15, 52, "ribice", "[email protected]", model.AccessRole(1))
wantUser := &model.AuthUser{
ID: 9,
Username: "ribice",
Expand Down Expand Up @@ -262,7 +262,7 @@ func TestMe(t *testing.T) {
name: "Success",
ctx: mock.EchoCtxWithKeys([]string{
"id", "company_id", "location_id", "username", "email", "role"},
9, 15, 52, "ribice", "[email protected]", int8(1)),
9, 15, 52, "ribice", "[email protected]", model.AccessRole(1)),
udb: &mockdb.User{
ViewFn: func(db orm.DB, id int) (*model.User, error) {
return &model.User{
Expand Down
4 changes: 2 additions & 2 deletions internal/mock/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ type RBAC struct {
EnforceUserFn func(echo.Context, int) error
EnforceCompanyFn func(echo.Context, int) error
EnforceLocationFn func(echo.Context, int) error
AccountCreateFn func(echo.Context, int, int, int) error
AccountCreateFn func(echo.Context, model.AccessRole, int, int) error
IsLowerRoleFn func(echo.Context, model.AccessRole) error
}

Expand All @@ -37,7 +37,7 @@ func (a *RBAC) EnforceLocation(c echo.Context, id int) error {
}

// AccountCreate mock
func (a *RBAC) AccountCreate(c echo.Context, roleID, companyID, locationID int) error {
func (a *RBAC) AccountCreate(c echo.Context, roleID model.AccessRole, companyID, locationID int) error {
return a.AccountCreateFn(c, roleID, companyID, locationID)
}

Expand Down
6 changes: 3 additions & 3 deletions internal/platform/postgres/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func testAccountCreate(t *testing.T, db *pgsql.AccountDB, c *pg.DB) {
FirstName: "Tom",
LastName: "Jones",
Username: "tomjones",
RoleID: 1,
RoleID: model.AccessRole(1),
CompanyID: 1,
LocationID: 1,
Password: "pass",
Expand All @@ -75,7 +75,7 @@ func testAccountCreate(t *testing.T, db *pgsql.AccountDB, c *pg.DB) {
FirstName: "Tom",
LastName: "Jones",
Username: "tomjones",
RoleID: 1,
RoleID: model.AccessRole(1),
CompanyID: 1,
LocationID: 1,
Password: "pass",
Expand All @@ -88,7 +88,7 @@ func testAccountCreate(t *testing.T, db *pgsql.AccountDB, c *pg.DB) {
FirstName: "Tom",
LastName: "Jones",
Username: "tomjones",
RoleID: 1,
RoleID: model.AccessRole(1),
CompanyID: 1,
LocationID: 1,
Password: "pass",
Expand Down
4 changes: 2 additions & 2 deletions internal/platform/postgres/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func testUserList(t *testing.T, db *pgsql.UserDB, c *pg.DB) {
FirstName: "Tom",
LastName: "Jones",
Username: "tomjones",
RoleID: 1,
RoleID: model.AccessRole(1),
CompanyID: 1,
LocationID: 1,
Password: "newPass",
Expand All @@ -249,7 +249,7 @@ func testUserList(t *testing.T, db *pgsql.UserDB, c *pg.DB) {
FirstName: "John",
LastName: "Doe",
Username: "johndoe",
RoleID: 1,
RoleID: model.AccessRole(1),
CompanyID: 1,
LocationID: 1,
Password: "hunter2",
Expand Down
10 changes: 5 additions & 5 deletions internal/rbac/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func checkBool(b bool) error {

// EnforceRole authorizes request by AccessRole
func (s *Service) EnforceRole(c echo.Context, r model.AccessRole) error {
return checkBool(!(c.Get("role").(int8) > int8(r)))
return checkBool(!(c.Get("role").(model.AccessRole) > r))
}

// EnforceUser checks whether the request to change user data is done by the same user
Expand Down Expand Up @@ -63,17 +63,17 @@ func (s *Service) EnforceLocation(c echo.Context, ID int) error {
}

func (s *Service) isAdmin(c echo.Context) bool {
return !(c.Get("role").(int8) > int8(model.AdminRole))
return !(c.Get("role").(model.AccessRole) > model.AdminRole)
}

func (s *Service) isCompanyAdmin(c echo.Context) bool {
// Must query company ID in database for the given user
return !(c.Get("role").(int8) > int8(model.CompanyAdminRole))
return !(c.Get("role").(model.AccessRole) > model.CompanyAdminRole)
}

// AccountCreate performs auth check when creating a new account
// Location admin cannot create accounts, needs to be fixed on EnforceLocation function
func (s *Service) AccountCreate(c echo.Context, roleID, companyID, locationID int) error {
func (s *Service) AccountCreate(c echo.Context, roleID model.AccessRole, companyID, locationID int) error {
if err := s.EnforceLocation(c, locationID); err != nil {
return err
}
Expand All @@ -83,5 +83,5 @@ func (s *Service) AccountCreate(c echo.Context, roleID, companyID, locationID in
// IsLowerRole checks whether the requesting user has higher role than the user it wants to change
// Used for account creation/deletion
func (s *Service) IsLowerRole(c echo.Context, r model.AccessRole) error {
return checkBool(c.Get("role").(int8) < int8(r))
return checkBool(c.Get("role").(model.AccessRole) < r)
}
42 changes: 21 additions & 21 deletions internal/rbac/rbac_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ func TestEnforceRole(t *testing.T) {
}{
{
name: "Not authorized",
args: args{ctx: mock.EchoCtxWithKeys([]string{"role"}, int8(3)), role: model.SuperAdminRole},
args: args{ctx: mock.EchoCtxWithKeys([]string{"role"}, model.AccessRole(3)), role: model.SuperAdminRole},
wantErr: true,
},
{
name: "Authorized",
args: args{ctx: mock.EchoCtxWithKeys([]string{"role"}, int8(0)), role: model.CompanyAdminRole},
args: args{ctx: mock.EchoCtxWithKeys([]string{"role"}, model.AccessRole(0)), role: model.CompanyAdminRole},
wantErr: false,
},
}
Expand All @@ -61,17 +61,17 @@ func TestEnforceUser(t *testing.T) {
}{
{
name: "Not same user, not an admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"id", "role"}, 15, int8(3)), id: 122},
args: args{ctx: mock.EchoCtxWithKeys([]string{"id", "role"}, 15, model.AccessRole(3)), id: 122},
wantErr: true,
},
{
name: "Not same user, but admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"id", "role"}, 22, int8(0)), id: 44},
args: args{ctx: mock.EchoCtxWithKeys([]string{"id", "role"}, 22, model.AccessRole(0)), id: 44},
wantErr: false,
},
{
name: "Same user",
args: args{ctx: mock.EchoCtxWithKeys([]string{"id", "role"}, 8, int8(3)), id: 8},
args: args{ctx: mock.EchoCtxWithKeys([]string{"id", "role"}, 8, model.AccessRole(3)), id: 8},
wantErr: false,
},
}
Expand All @@ -96,22 +96,22 @@ func TestEnforceCompany(t *testing.T) {
}{
{
name: "Not same company, not an admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "role"}, 7, int8(5)), id: 9},
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "role"}, 7, model.AccessRole(5)), id: 9},
wantErr: true,
},
{
name: "Same company, not company admin or admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "role"}, 22, int8(5)), id: 22},
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "role"}, 22, model.AccessRole(5)), id: 22},
wantErr: true,
},
{
name: "Same company, company admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "role"}, 5, int8(3)), id: 5},
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "role"}, 5, model.AccessRole(3)), id: 5},
wantErr: false,
},
{
name: "Not same company but admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "role"}, 8, int8(2)), id: 9},
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "role"}, 8, model.AccessRole(2)), id: 9},
wantErr: false,
},
}
Expand All @@ -136,22 +136,22 @@ func TestEnforceLocation(t *testing.T) {
}{
{
name: "Not same location, not an admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"location_id", "role"}, 7, int8(5)), id: 9},
args: args{ctx: mock.EchoCtxWithKeys([]string{"location_id", "role"}, 7, model.AccessRole(5)), id: 9},
wantErr: true,
},
{
name: "Same location, not company admin or admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"location_id", "role"}, 22, int8(5)), id: 22},
args: args{ctx: mock.EchoCtxWithKeys([]string{"location_id", "role"}, 22, model.AccessRole(5)), id: 22},
wantErr: true,
},
{
name: "Same location, company admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"location_id", "role"}, 5, int8(3)), id: 5},
args: args{ctx: mock.EchoCtxWithKeys([]string{"location_id", "role"}, 5, model.AccessRole(3)), id: 5},
wantErr: false,
},
{
name: "Location admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"location_id", "role"}, 5, int8(4)), id: 5},
args: args{ctx: mock.EchoCtxWithKeys([]string{"location_id", "role"}, 5, model.AccessRole(4)), id: 5},
wantErr: false,
},
}
Expand All @@ -167,7 +167,7 @@ func TestEnforceLocation(t *testing.T) {
func TestAccountCreate(t *testing.T) {
type args struct {
ctx echo.Context
roleID int
roleID model.AccessRole
company_id int
location_id int
}
Expand All @@ -178,32 +178,32 @@ func TestAccountCreate(t *testing.T) {
}{
{
name: "Different location, company, creating user role, not an admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "location_id", "role"}, 2, 3, int8(5)), roleID: 5, company_id: 7, location_id: 8},
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "location_id", "role"}, 2, 3, model.AccessRole(5)), roleID: 5, company_id: 7, location_id: 8},
wantErr: true,
},
{
name: "Same location, not company, creating user role, not an admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "location_id", "role"}, 2, 3, int8(5)), roleID: 5, company_id: 2, location_id: 8},
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "location_id", "role"}, 2, 3, model.AccessRole(5)), roleID: 5, company_id: 2, location_id: 8},
wantErr: true,
},
{
name: "Different location, company, creating user role, not an admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "location_id", "role"}, 2, 3, int8(3)), roleID: 4, company_id: 2, location_id: 4},
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "location_id", "role"}, 2, 3, model.AccessRole(3)), roleID: 4, company_id: 2, location_id: 4},
wantErr: false,
},
{
name: "Same location, company, creating user role, not an admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "location_id", "role"}, 2, 3, int8(3)), roleID: 5, company_id: 2, location_id: 3},
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "location_id", "role"}, 2, 3, model.AccessRole(3)), roleID: 5, company_id: 2, location_id: 3},
wantErr: false,
},
{
name: "Same location, company, creating user role, admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "location_id", "role"}, 2, 3, int8(3)), roleID: 5, company_id: 2, location_id: 3},
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "location_id", "role"}, 2, 3, model.AccessRole(3)), roleID: 5, company_id: 2, location_id: 3},
wantErr: false,
},
{
name: "Different everything, admin",
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "location_id", "role"}, 2, 3, int8(1)), roleID: 2, company_id: 7, location_id: 4},
args: args{ctx: mock.EchoCtxWithKeys([]string{"company_id", "location_id", "role"}, 2, 3, model.AccessRole(1)), roleID: 2, company_id: 7, location_id: 4},
wantErr: false,
},
}
Expand All @@ -217,7 +217,7 @@ func TestAccountCreate(t *testing.T) {
}

func TestIsLowerRole(t *testing.T) {
ctx := mock.EchoCtxWithKeys([]string{"role"}, int8(3))
ctx := mock.EchoCtxWithKeys([]string{"role"}, model.AccessRole(3))
rbacSvc := rbac.New(nil)
if rbacSvc.IsLowerRole(ctx, model.AccessRole(4)) != nil {
t.Error("The requested user is higher role than the user requesting it")
Expand Down
2 changes: 1 addition & 1 deletion internal/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const (

// Role model
type Role struct {
ID int `json:"id"`
ID AccessRole `json:"id"`
AccessLevel AccessRole `json:"access_level"`
Name string `json:"name"`
}
6 changes: 3 additions & 3 deletions internal/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ type User struct {

Role *Role `json:"role,omitempty"`

RoleID int `json:"-"`
CompanyID int `json:"company_id"`
LocationID int `json:"location_id"`
RoleID AccessRole `json:"-"`
CompanyID int `json:"company_id"`
LocationID int `json:"location_id"`
}

// AuthUser represents data stored in JWT token for user
Expand Down

0 comments on commit 0402da6

Please sign in to comment.