From 5bb44af01c2fd5474660442b0882a32c5018d97a Mon Sep 17 00:00:00 2001 From: Tetrergeru Date: Wed, 20 Mar 2024 11:45:51 +0100 Subject: [PATCH 1/7] Add admin-only restrictions --- api/handler/contact.go | 2 +- api/handler/event.go | 2 +- api/handler/tag.go | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/api/handler/contact.go b/api/handler/contact.go index 21a508d7d..5138c6772 100644 --- a/api/handler/contact.go +++ b/api/handler/contact.go @@ -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) diff --git a/api/handler/event.go b/api/handler/event.go index 398ad96a8..c5d8602d5 100644 --- a/api/handler/event.go +++ b/api/handler/event.go @@ -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 diff --git a/api/handler/tag.go b/api/handler/tag.go index 36c04b26b..5764326d9 100644 --- a/api/handler/tag.go +++ b/api/handler/tag.go @@ -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) }) } From 2dbf034019268b26044c9bb16473dcea6caf89cf Mon Sep 17 00:00:00 2001 From: Tetrergeru Date: Wed, 20 Mar 2024 16:28:00 +0100 Subject: [PATCH 2/7] Add filed in settings for admin rights indication --- api/controller/user.go | 7 ++++-- api/controller/user_test.go | 50 +++++++++++++++++++++++++++++++------ api/dto/user.go | 3 ++- api/handler/user.go | 3 ++- 4 files changed, 51 insertions(+), 12 deletions(-) diff --git a/api/controller/user.go b/api/controller/user.go index 38a2d2b98..fe91a86f5 100644 --- a/api/controller/user.go +++ b/api/controller/user.go @@ -7,9 +7,12 @@ 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, + HasAdminRights: !auth.IsEnabled() || auth.IsAdmin(userLogin), + }, Contacts: make([]moira.ContactData, 0), Subscriptions: make([]moira.SubscriptionData, 0), } diff --git a/api/controller/user_test.go b/api/controller/user_test.go index d53a0d5cf..bae35da5c 100644 --- a/api/controller/user_test.go +++ b/api/controller/user_test.go @@ -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()} @@ -28,10 +29,10 @@ 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}, + User: dto.User{Login: login, HasAdminRights: true}, Contacts: []moira.ContactData{*contacts[0], *contacts[1]}, Subscriptions: []moira.SubscriptionData{*subscriptions[0], *subscriptions[1]}, }) @@ -42,20 +43,53 @@ 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}, + User: dto.User{Login: login, HasAdminRights: true}, Contacts: make([]moira.ContactData, 0), Subscriptions: make([]moira.SubscriptionData, 0), }) }) + Convey("Admin auth enabled", t, func() { + adminLogin := "admin_login" + auth := &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, auth) + So(err, ShouldBeNil) + So(settings, ShouldResemble, &dto.UserSettings{ + User: dto.User{Login: login, HasAdminRights: false}, + 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, auth) + So(err, ShouldBeNil) + So(settings, ShouldResemble, &dto.UserSettings{ + User: dto.User{Login: adminLogin, HasAdminRights: 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) }) @@ -63,7 +97,7 @@ func TestGetUserSettings(t *testing.T) { 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) }) @@ -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) }) @@ -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) }) diff --git a/api/dto/user.go b/api/dto/user.go index 68d45eca2..685e2c17c 100644 --- a/api/dto/user.go +++ b/api/dto/user.go @@ -18,7 +18,8 @@ func (*UserSettings) Render(w http.ResponseWriter, r *http.Request) error { } type User struct { - Login string `json:"login" example:"john"` + Login string `json:"login" example:"john"` + HasAdminRights bool `json:"has_admin_rights" example:"false"` } func (*User) Render(w http.ResponseWriter, r *http.Request) error { diff --git a/api/handler/user.go b/api/handler/user.go index 17c856698..006128646 100644 --- a/api/handler/user.go +++ b/api/handler/user.go @@ -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 From 162aba5f99116e76cc52d1a516c8c558a20b5209 Mon Sep 17 00:00:00 2001 From: Tetrergeru Date: Wed, 20 Mar 2024 16:48:15 +0100 Subject: [PATCH 3/7] Fix lint --- api/controller/user_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/controller/user_test.go b/api/controller/user_test.go index bae35da5c..f4b4e9879 100644 --- a/api/controller/user_test.go +++ b/api/controller/user_test.go @@ -54,14 +54,14 @@ func TestGetUserSettings(t *testing.T) { Convey("Admin auth enabled", t, func() { adminLogin := "admin_login" - auth := &api.Authorization{Enabled: true, AdminList: map[string]struct{}{adminLogin: {}}} + 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, auth) + settings, err := GetUserSettings(database, login, authFull) So(err, ShouldBeNil) So(settings, ShouldResemble, &dto.UserSettings{ User: dto.User{Login: login, HasAdminRights: false}, @@ -75,7 +75,7 @@ func TestGetUserSettings(t *testing.T) { 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, auth) + settings, err := GetUserSettings(database, adminLogin, authFull) So(err, ShouldBeNil) So(settings, ShouldResemble, &dto.UserSettings{ User: dto.User{Login: adminLogin, HasAdminRights: true}, From c0e8cfc51bd14c249fdd1d5fb1da3400b2837c44 Mon Sep 17 00:00:00 2001 From: Tetrergeru Date: Thu, 21 Mar 2024 12:04:03 +0100 Subject: [PATCH 4/7] Add role field --- api/controller/user.go | 5 +++-- api/controller/user_test.go | 8 ++++---- api/dto/user.go | 24 ++++++++++++++++++++++-- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/api/controller/user.go b/api/controller/user.go index fe91a86f5..21243dfe7 100644 --- a/api/controller/user.go +++ b/api/controller/user.go @@ -10,8 +10,9 @@ import ( func GetUserSettings(database moira.Database, userLogin string, auth *api.Authorization) (*dto.UserSettings, *api.ErrorResponse) { userSettings := &dto.UserSettings{ User: dto.User{ - Login: userLogin, - HasAdminRights: !auth.IsEnabled() || auth.IsAdmin(userLogin), + Login: userLogin, + AuthEnabled: auth.IsEnabled(), + Role: dto.GetRole(userLogin, auth), }, Contacts: make([]moira.ContactData, 0), Subscriptions: make([]moira.SubscriptionData, 0), diff --git a/api/controller/user_test.go b/api/controller/user_test.go index f4b4e9879..efc820c14 100644 --- a/api/controller/user_test.go +++ b/api/controller/user_test.go @@ -32,7 +32,7 @@ func TestGetUserSettings(t *testing.T) { settings, err := GetUserSettings(database, login, auth) So(err, ShouldBeNil) So(settings, ShouldResemble, &dto.UserSettings{ - User: dto.User{Login: login, HasAdminRights: true}, + User: dto.User{Login: login}, Contacts: []moira.ContactData{*contacts[0], *contacts[1]}, Subscriptions: []moira.SubscriptionData{*subscriptions[0], *subscriptions[1]}, }) @@ -46,7 +46,7 @@ func TestGetUserSettings(t *testing.T) { settings, err := GetUserSettings(database, login, auth) So(err, ShouldBeNil) So(settings, ShouldResemble, &dto.UserSettings{ - User: dto.User{Login: login, HasAdminRights: true}, + User: dto.User{Login: login}, Contacts: make([]moira.ContactData, 0), Subscriptions: make([]moira.SubscriptionData, 0), }) @@ -64,7 +64,7 @@ func TestGetUserSettings(t *testing.T) { settings, err := GetUserSettings(database, login, authFull) So(err, ShouldBeNil) So(settings, ShouldResemble, &dto.UserSettings{ - User: dto.User{Login: login, HasAdminRights: false}, + User: dto.User{Login: login, Role: dto.RoleUser, AuthEnabled: true}, Contacts: make([]moira.ContactData, 0), Subscriptions: make([]moira.SubscriptionData, 0), }) @@ -78,7 +78,7 @@ func TestGetUserSettings(t *testing.T) { settings, err := GetUserSettings(database, adminLogin, authFull) So(err, ShouldBeNil) So(settings, ShouldResemble, &dto.UserSettings{ - User: dto.User{Login: adminLogin, HasAdminRights: true}, + User: dto.User{Login: adminLogin, Role: dto.RoleAdmin, AuthEnabled: true}, Contacts: make([]moira.ContactData, 0), Subscriptions: make([]moira.SubscriptionData, 0), }) diff --git a/api/dto/user.go b/api/dto/user.go index 685e2c17c..86646884e 100644 --- a/api/dto/user.go +++ b/api/dto/user.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/moira-alert/moira" + "github.com/moira-alert/moira/api" ) type UserSettings struct { @@ -17,9 +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"` - HasAdminRights bool `json:"has_admin_rights" example:"false"` + 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 { From e12a617faa2508d268e0d8096b3edd13e60eb32f Mon Sep 17 00:00:00 2001 From: Tetrergeru Date: Mon, 25 Mar 2024 10:39:09 +0100 Subject: [PATCH 5/7] Add test for paths --- api/handler/handler_test.go | 96 +++++++++++++++++++++++++++++++++++++ notifier/registrator.go | 2 +- 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/api/handler/handler_test.go b/api/handler/handler_test.go index 43415a2d9..c84ea222c 100644 --- a/api/handler/handler_test.go +++ b/api/handler/handler_test.go @@ -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" @@ -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) + }) + }) +} diff --git a/notifier/registrator.go b/notifier/registrator.go index 009b9d66a..654cc5298 100644 --- a/notifier/registrator.go +++ b/notifier/registrator.go @@ -19,7 +19,7 @@ import ( "github.com/moira-alert/moira/senders/twilio" "github.com/moira-alert/moira/senders/victorops" "github.com/moira-alert/moira/senders/webhook" - // "github.com/moira-alert/moira/senders/kontur" + // "github.com/moira-alert/moira/senders/kontur". ) const ( From 96416067118f63213dcd4789e776f0fe2852749d Mon Sep 17 00:00:00 2001 From: Tetrergeru Date: Tue, 26 Mar 2024 11:04:24 +0100 Subject: [PATCH 6/7] Remove dot in comment --- notifier/registrator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notifier/registrator.go b/notifier/registrator.go index 654cc5298..009b9d66a 100644 --- a/notifier/registrator.go +++ b/notifier/registrator.go @@ -19,7 +19,7 @@ import ( "github.com/moira-alert/moira/senders/twilio" "github.com/moira-alert/moira/senders/victorops" "github.com/moira-alert/moira/senders/webhook" - // "github.com/moira-alert/moira/senders/kontur". + // "github.com/moira-alert/moira/senders/kontur" ) const ( From 3593fb905b5ae56d984d6eff3bec0c3798f1ef09 Mon Sep 17 00:00:00 2001 From: Tetrergeru Date: Tue, 26 Mar 2024 12:25:19 +0100 Subject: [PATCH 7/7] Close apis to get all triggers --- api/handler/triggers.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/api/handler/triggers.go b/api/handler/triggers.go index cab2bb6f7..090ed6607 100644 --- a/api/handler/triggers.go +++ b/api/handler/triggers.go @@ -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)