Skip to content

Commit

Permalink
feat(api): limit handlers to admins only (#1001)
Browse files Browse the repository at this point in the history
  • Loading branch information
Tetrergeru authored Mar 27, 2024
1 parent f1e51ba commit 32a5011
Show file tree
Hide file tree
Showing 9 changed files with 174 additions and 15 deletions.
8 changes: 6 additions & 2 deletions api/controller/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ import (
)

// GetUserSettings gets user contacts and subscriptions.
func GetUserSettings(database moira.Database, userLogin string) (*dto.UserSettings, *api.ErrorResponse) {
func GetUserSettings(database moira.Database, userLogin string, auth *api.Authorization) (*dto.UserSettings, *api.ErrorResponse) {
userSettings := &dto.UserSettings{
User: dto.User{Login: userLogin},
User: dto.User{
Login: userLogin,
AuthEnabled: auth.IsEnabled(),
Role: dto.GetRole(userLogin, auth),
},
Contacts: make([]moira.ContactData, 0),
Subscriptions: make([]moira.SubscriptionData, 0),
}
Expand Down
46 changes: 40 additions & 6 deletions api/controller/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ func TestGetUserSettings(t *testing.T) {
defer mockCtrl.Finish()
database := mock_moira_alert.NewMockDatabase(mockCtrl)
login := "user"
auth := &api.Authorization{}

Convey("Success get user data", t, func() {
subscriptionIDs := []string{uuid.Must(uuid.NewV4()).String(), uuid.Must(uuid.NewV4()).String()}
Expand All @@ -28,7 +29,7 @@ func TestGetUserSettings(t *testing.T) {
database.EXPECT().GetSubscriptions(subscriptionIDs).Return(subscriptions, nil)
database.EXPECT().GetUserContactIDs(login).Return(contactIDs, nil)
database.EXPECT().GetContacts(contactIDs).Return(contacts, nil)
settings, err := GetUserSettings(database, login)
settings, err := GetUserSettings(database, login, auth)
So(err, ShouldBeNil)
So(settings, ShouldResemble, &dto.UserSettings{
User: dto.User{Login: login},
Expand All @@ -42,7 +43,7 @@ func TestGetUserSettings(t *testing.T) {
database.EXPECT().GetSubscriptions(make([]string, 0)).Return(make([]*moira.SubscriptionData, 0), nil)
database.EXPECT().GetUserContactIDs(login).Return(make([]string, 0), nil)
database.EXPECT().GetContacts(make([]string, 0)).Return(make([]*moira.ContactData, 0), nil)
settings, err := GetUserSettings(database, login)
settings, err := GetUserSettings(database, login, auth)
So(err, ShouldBeNil)
So(settings, ShouldResemble, &dto.UserSettings{
User: dto.User{Login: login},
Expand All @@ -51,19 +52,52 @@ func TestGetUserSettings(t *testing.T) {
})
})

Convey("Admin auth enabled", t, func() {
adminLogin := "admin_login"
authFull := &api.Authorization{Enabled: true, AdminList: map[string]struct{}{adminLogin: {}}}

Convey("User is not admin", func() {
database.EXPECT().GetUserSubscriptionIDs(login).Return(make([]string, 0), nil)
database.EXPECT().GetSubscriptions(make([]string, 0)).Return(make([]*moira.SubscriptionData, 0), nil)
database.EXPECT().GetUserContactIDs(login).Return(make([]string, 0), nil)
database.EXPECT().GetContacts(make([]string, 0)).Return(make([]*moira.ContactData, 0), nil)
settings, err := GetUserSettings(database, login, authFull)
So(err, ShouldBeNil)
So(settings, ShouldResemble, &dto.UserSettings{
User: dto.User{Login: login, Role: dto.RoleUser, AuthEnabled: true},
Contacts: make([]moira.ContactData, 0),
Subscriptions: make([]moira.SubscriptionData, 0),
})
})

Convey("User is admin", func() {
database.EXPECT().GetUserSubscriptionIDs(adminLogin).Return(make([]string, 0), nil)
database.EXPECT().GetSubscriptions(make([]string, 0)).Return(make([]*moira.SubscriptionData, 0), nil)
database.EXPECT().GetUserContactIDs(adminLogin).Return(make([]string, 0), nil)
database.EXPECT().GetContacts(make([]string, 0)).Return(make([]*moira.ContactData, 0), nil)
settings, err := GetUserSettings(database, adminLogin, authFull)
So(err, ShouldBeNil)
So(settings, ShouldResemble, &dto.UserSettings{
User: dto.User{Login: adminLogin, Role: dto.RoleAdmin, AuthEnabled: true},
Contacts: make([]moira.ContactData, 0),
Subscriptions: make([]moira.SubscriptionData, 0),
})
})
})

Convey("Errors", t, func() {
Convey("GetUserSubscriptionIDs", func() {
expected := fmt.Errorf("can not read ids")
database.EXPECT().GetUserSubscriptionIDs(login).Return(nil, expected)
settings, err := GetUserSettings(database, login)
settings, err := GetUserSettings(database, login, auth)
So(err, ShouldResemble, api.ErrorInternalServer(expected))
So(settings, ShouldBeNil)
})
Convey("GetSubscriptions", func() {
expected := fmt.Errorf("can not read subscriptions")
database.EXPECT().GetUserSubscriptionIDs(login).Return(make([]string, 0), nil)
database.EXPECT().GetSubscriptions(make([]string, 0)).Return(nil, expected)
settings, err := GetUserSettings(database, login)
settings, err := GetUserSettings(database, login, auth)
So(err, ShouldResemble, api.ErrorInternalServer(expected))
So(settings, ShouldBeNil)
})
Expand All @@ -72,7 +106,7 @@ func TestGetUserSettings(t *testing.T) {
database.EXPECT().GetUserSubscriptionIDs(login).Return(make([]string, 0), nil)
database.EXPECT().GetSubscriptions(make([]string, 0)).Return(make([]*moira.SubscriptionData, 0), nil)
database.EXPECT().GetUserContactIDs(login).Return(nil, expected)
settings, err := GetUserSettings(database, login)
settings, err := GetUserSettings(database, login, auth)
So(err, ShouldResemble, api.ErrorInternalServer(expected))
So(settings, ShouldBeNil)
})
Expand All @@ -85,7 +119,7 @@ func TestGetUserSettings(t *testing.T) {
database.EXPECT().GetSubscriptions(subscriptionIDs).Return(subscriptions, nil)
database.EXPECT().GetUserContactIDs(login).Return(contactIDs, nil)
database.EXPECT().GetContacts(contactIDs).Return(nil, expected)
settings, err := GetUserSettings(database, login)
settings, err := GetUserSettings(database, login, auth)
So(err, ShouldResemble, api.ErrorInternalServer(expected))
So(settings, ShouldBeNil)
})
Expand Down
23 changes: 22 additions & 1 deletion api/dto/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/http"

"github.com/moira-alert/moira"
"github.com/moira-alert/moira/api"
)

type UserSettings struct {
Expand All @@ -17,8 +18,28 @@ func (*UserSettings) Render(w http.ResponseWriter, r *http.Request) error {
return nil
}

type Role string

var (
RoleUndefined Role = ""
RoleUser Role = "user"
RoleAdmin Role = "admin"
)

func GetRole(login string, auth *api.Authorization) Role {
if !auth.IsEnabled() {
return RoleUndefined
}
if auth.IsAdmin(login) {
return RoleAdmin
}
return RoleUser
}

type User struct {
Login string `json:"login" example:"john"`
Login string `json:"login" example:"john"`
Role Role `json:"role,omitempty" example:"user"`
AuthEnabled bool `json:"auth_enabled" example:"true"`
}

func (*User) Render(w http.ResponseWriter, r *http.Request) error {
Expand Down
2 changes: 1 addition & 1 deletion api/handler/contact.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
)

func contact(router chi.Router) {
router.Get("/", getAllContacts)
router.With(middleware.AdminOnlyMiddleware()).Get("/", getAllContacts)
router.Put("/", createNewContact)
router.Route("/{contactId}", func(router chi.Router) {
router.Use(middleware.ContactContext)
Expand Down
2 changes: 1 addition & 1 deletion api/handler/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (

func event(router chi.Router) {
router.With(middleware.TriggerContext, middleware.Paginate(0, 100)).Get("/{triggerId}", getEventsList)
router.Delete("/all", deleteAllEvents)
router.With(middleware.AdminOnlyMiddleware()).Delete("/all", deleteAllEvents)
}

// nolint: gofmt,goimports
Expand Down
96 changes: 96 additions & 0 deletions api/handler/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"testing"

"github.com/golang/mock/gomock"
"github.com/moira-alert/moira"
"github.com/moira-alert/moira/api"
"github.com/moira-alert/moira/api/dto"
"github.com/moira-alert/moira/logging/zerolog_adapter"
Expand Down Expand Up @@ -109,3 +110,98 @@ func TestReadonlyMode(t *testing.T) {
})
})
}

func TestAdminOnly(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

mockDb := mock_moira_alert.NewMockDatabase(mockCtrl)
database = mockDb

logger, _ := zerolog_adapter.GetLogger("Test")

adminLogin := "admin_login"
userLogin := "user_login"
config := &api.Config{Authorization: api.Authorization{Enabled: true, AdminList: map[string]struct{}{adminLogin: {}}}}
webConfig := &api.WebConfig{
SupportEmail: "test",
Contacts: []api.WebContact{},
}
handler := NewHandler(mockDb, logger, nil, config, nil, webConfig)

Convey("Get all contacts", t, func() {
Convey("For non-admin", func() {
trigger := &dto.Trigger{}
triggerBytes, err := json.Marshal(trigger)
So(err, ShouldBeNil)

testRequest := httptest.NewRequest(http.MethodGet, "/api/contact", bytes.NewReader(triggerBytes))
testRequest.Header.Add("x-webauth-user", userLogin)

responseWriter := httptest.NewRecorder()
handler.ServeHTTP(responseWriter, testRequest)

response := responseWriter.Result()
defer response.Body.Close()
So(response.StatusCode, ShouldEqual, http.StatusForbidden)
})

Convey("For admin", func() {
trigger := &dto.Trigger{}
triggerBytes, err := json.Marshal(trigger)
So(err, ShouldBeNil)

mockDb.EXPECT().GetAllContacts().Return([]*moira.ContactData{}, nil)

testRequest := httptest.NewRequest(http.MethodGet, "/api/contact", bytes.NewReader(triggerBytes))
testRequest.Header.Add("x-webauth-user", adminLogin)

responseWriter := httptest.NewRecorder()
handler.ServeHTTP(responseWriter, testRequest)

response := responseWriter.Result()
defer response.Body.Close()

So(response.StatusCode, ShouldEqual, http.StatusOK)
})
})

Convey("Get tag stats", t, func() {
Convey("For non-admin", func() {
trigger := &dto.Trigger{}
triggerBytes, err := json.Marshal(trigger)
So(err, ShouldBeNil)

testRequest := httptest.NewRequest(http.MethodGet, "/api/tag/stats", bytes.NewReader(triggerBytes))
testRequest.Header.Add("x-webauth-user", userLogin)

responseWriter := httptest.NewRecorder()
handler.ServeHTTP(responseWriter, testRequest)

response := responseWriter.Result()
defer response.Body.Close()
So(response.StatusCode, ShouldEqual, http.StatusForbidden)
})

Convey("For admin", func() {
trigger := &dto.Trigger{}
triggerBytes, err := json.Marshal(trigger)
So(err, ShouldBeNil)

mockDb.EXPECT().GetTagNames().Return([]string{"tag_1"}, nil)
mockDb.EXPECT().GetTagsSubscriptions([]string{"tag_1"}).Return([]*moira.SubscriptionData{}, nil)
mockDb.EXPECT().GetTagTriggerIDs("tag_1").Return([]string{"tag_1_trigger_id"}, nil)

testRequest := httptest.NewRequest(http.MethodGet, "/api/tag/stats", bytes.NewReader(triggerBytes))
testRequest.Header.Add("x-webauth-user", adminLogin)

responseWriter := httptest.NewRecorder()
handler.ServeHTTP(responseWriter, testRequest)

response := responseWriter.Result()
defer response.Body.Close()

So(response.StatusCode, ShouldEqual, http.StatusOK)
})
})
}
3 changes: 2 additions & 1 deletion api/handler/tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ import (
func tag(router chi.Router) {
router.Post("/", createTags)
router.Get("/", getAllTags)
router.Get("/stats", getAllTagsAndSubscriptions)
router.With(middleware.AdminOnlyMiddleware()).Get("/stats", getAllTagsAndSubscriptions)
router.Route("/{tag}", func(router chi.Router) {
router.Use(middleware.TagContext)
router.Use(middleware.AdminOnlyMiddleware())
router.Delete("/", removeTag)
})
}
Expand Down
6 changes: 4 additions & 2 deletions api/handler/triggers.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ func triggers(metricSourceProvider *metricSource.SourceProvider, searcher moira.
return func(router chi.Router) {
router.Use(middleware.MetricSourceProvider(metricSourceProvider))
router.Use(middleware.SearchIndexContext(searcher))
router.Get("/", getAllTriggers)
router.Get("/unused", getUnusedTriggers)

router.With(middleware.AdminOnlyMiddleware()).Get("/", getAllTriggers)
router.With(middleware.AdminOnlyMiddleware()).Get("/unused", getUnusedTriggers)

router.Put("/", createTrigger)
router.Put("/check", triggerCheck)
router.Route("/{triggerId}", trigger)
Expand Down
3 changes: 2 additions & 1 deletion api/handler/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ func getUserName(writer http.ResponseWriter, request *http.Request) {
// @router /user/settings [get]
func getUserSettings(writer http.ResponseWriter, request *http.Request) {
userLogin := middleware.GetLogin(request)
userSettings, err := controller.GetUserSettings(database, userLogin)
auth := middleware.GetAuth(request)
userSettings, err := controller.GetUserSettings(database, userLogin, auth)
if err != nil {
render.Render(writer, request, err) //nolint
return
Expand Down

0 comments on commit 32a5011

Please sign in to comment.