From 385b98a3a11a4316e1403e7c62ae7ec91a3f78aa Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 13 May 2021 09:57:45 +0200 Subject: [PATCH] add privacy mode and role tests (fixes #185) * denied/remove * denied/add * invites/revoke * members/remove * notices/edit * notices/add also: * add members.CheckAction helper * fix muxrpc abort bug and update to v2.0.5 * strictly use SeeOther not 307 (fixes #149) --- go.mod | 2 +- go.sum | 2 + muxrpc/test/go/alias_test.go | 2 +- roomdb/types.go | 2 + roomsrv/server.go | 3 +- web/handlers/admin/aliases.go | 14 +- web/handlers/admin/aliases_test.go | 4 +- web/handlers/admin/denied_keys.go | 26 +++- web/handlers/admin/denied_keys_test.go | 189 +++++++++++++++++++----- web/handlers/admin/handler.go | 44 ++---- web/handlers/admin/invites.go | 58 +++----- web/handlers/admin/invites_test.go | 71 +++++++-- web/handlers/admin/members.go | 25 ++-- web/handlers/admin/members_test.go | 106 +++++++++++--- web/handlers/admin/notices.go | 107 +++++++++----- web/handlers/admin/notices_test.go | 195 ++++++++++++++++++++++++- web/handlers/admin/setup_test.go | 18 ++- web/handlers/http.go | 27 +--- web/handlers/notices_test.go | 14 +- web/i18n/defaults/active.de.toml | 2 + web/i18n/defaults/active.en.toml | 2 + web/members/helper.go | 116 ++++++++++++++- web/templates/admin/denied-keys.tmpl | 16 +- web/templates/admin/invite-list.tmpl | 4 +- 24 files changed, 798 insertions(+), 251 deletions(-) diff --git a/go.mod b/go.mod index 93a4dfd..430b770 100644 --- a/go.mod +++ b/go.mod @@ -30,7 +30,7 @@ require ( github.com/volatiletech/sqlboiler-sqlite3 v0.0.0-20210314195744-a1c697a68aef // indirect github.com/volatiletech/sqlboiler/v4 v4.5.0 github.com/volatiletech/strmangle v0.0.1 - go.cryptoscope.co/muxrpc/v2 v2.0.4 + go.cryptoscope.co/muxrpc/v2 v2.0.5 go.cryptoscope.co/netwrap v0.1.1 go.cryptoscope.co/secretstream v1.2.2 go.mindeco.de v1.11.0 diff --git a/go.sum b/go.sum index 4d08e49..d6a90c4 100644 --- a/go.sum +++ b/go.sum @@ -508,6 +508,8 @@ go.cryptoscope.co/muxrpc/v2 v2.0.2 h1:UdlGHY+EEYZpJz5HWnWz0r34pULYxJHfFTeqLvv+7s go.cryptoscope.co/muxrpc/v2 v2.0.2/go.mod h1:MgaeojIkWY3lLuoNw1mlMT3b3jiZwOj/fgsoGZp/VNA= go.cryptoscope.co/muxrpc/v2 v2.0.4 h1:NLN//zPt9UKFelnPNBh3fefrQ/TFylCflPZhKiDtK3U= go.cryptoscope.co/muxrpc/v2 v2.0.4/go.mod h1:MgaeojIkWY3lLuoNw1mlMT3b3jiZwOj/fgsoGZp/VNA= +go.cryptoscope.co/muxrpc/v2 v2.0.5 h1:yZEp49Qx4KWF/DD+Hg+6vPrl4cjlcH0Ex5kzaz0XpMA= +go.cryptoscope.co/muxrpc/v2 v2.0.5/go.mod h1:MgaeojIkWY3lLuoNw1mlMT3b3jiZwOj/fgsoGZp/VNA= go.cryptoscope.co/netwrap v0.1.0/go.mod h1:7zcYswCa4CT+ct54e9uH9+IIbYYETEMHKDNpzl8Ukew= go.cryptoscope.co/netwrap v0.1.1 h1:JLzzGKEvrUrkKzu3iM0DhpHmt+L/gYqmpcf1lJMUyFs= go.cryptoscope.co/netwrap v0.1.1/go.mod h1:7zcYswCa4CT+ct54e9uH9+IIbYYETEMHKDNpzl8Ukew= diff --git a/muxrpc/test/go/alias_test.go b/muxrpc/test/go/alias_test.go index 1180db3..2b2e261 100644 --- a/muxrpc/test/go/alias_test.go +++ b/muxrpc/test/go/alias_test.go @@ -111,7 +111,7 @@ func TestAliasRegister(t *testing.T) { r.Error(err) var callErr *muxrpc.CallError - r.True(errors.As(err, &callErr), "expected a call error: %T", err) + r.True(errors.As(err, &callErr), "expected a call error: %T -- %s", err, err) r.Equal(`alias ("bob") is already taken`, callErr.Message) for _, bot := range theBots { diff --git a/roomdb/types.go b/roomdb/types.go index e2870e2..b202042 100644 --- a/roomdb/types.go +++ b/roomdb/types.go @@ -83,6 +83,8 @@ const ( ModeRestricted ) +var AllPrivacyModes = []PrivacyMode{ModeOpen, ModeCommunity, ModeRestricted} + // Implements the SQL marshaling interfaces (Scanner for Scan & Valuer for Value) for PrivacyMode // Scan implements https://pkg.go.dev/database/sql#Scanner to read integers into a privacy mode diff --git a/roomsrv/server.go b/roomsrv/server.go index d51817a..be80259 100644 --- a/roomsrv/server.go +++ b/roomsrv/server.go @@ -37,7 +37,7 @@ type Server struct { closers multicloser.Closer closed bool - closedMu sync.Mutex + closedMu *sync.Mutex closeErr error Network network.Network @@ -87,6 +87,7 @@ func New( opts ...Option, ) (*Server, error) { var s Server + s.closedMu = new(sync.Mutex) s.Members = membersdb s.DeniedKeys = deniedkeysdb diff --git a/web/handlers/admin/aliases.go b/web/handlers/admin/aliases.go index fdf17b4..2fb55ff 100644 --- a/web/handlers/admin/aliases.go +++ b/web/handlers/admin/aliases.go @@ -67,13 +67,15 @@ func (h aliasesHandler) revoke(rw http.ResponseWriter, req *http.Request) { return } + defer http.Redirect(rw, req, redirectToMembers, http.StatusSeeOther) + aliasName := req.FormValue("name") ctx := req.Context() aliasEntry, err := h.db.Resolve(ctx, aliasName) if err != nil { - h.r.Error(rw, req, http.StatusBadRequest, err) + h.flashes.AddError(rw, req, err) return } @@ -81,24 +83,22 @@ func (h aliasesHandler) revoke(rw http.ResponseWriter, req *http.Request) { currentMember := members.FromContext(ctx) if currentMember == nil { err := weberrors.ErrForbidden{Details: fmt.Errorf("not an member")} - h.r.Error(rw, req, http.StatusInternalServerError, err) + h.flashes.AddError(rw, req, err) return } // ensure own alias or admin if !aliasEntry.Feed.Equal(¤tMember.PubKey) && currentMember.Role != roomdb.RoleAdmin { err := weberrors.ErrForbidden{Details: fmt.Errorf("not your alias or not an admin")} - h.r.Error(rw, req, http.StatusInternalServerError, err) + h.flashes.AddError(rw, req, err) return } - status := http.StatusTemporaryRedirect // TODO: should be SeeOther because it's method POST coming in err = h.db.Revoke(ctx, aliasName) if err != nil { h.flashes.AddError(rw, req, err) - } else { - h.flashes.AddMessage(rw, req, "AdminMemberDetailsAliasRevoked") + return } - http.Redirect(rw, req, redirectToMembers, status) + h.flashes.AddMessage(rw, req, "AdminMemberDetailsAliasRevoked") } diff --git a/web/handlers/admin/aliases_test.go b/web/handlers/admin/aliases_test.go index 8bf6888..8c6ff3c 100644 --- a/web/handlers/admin/aliases_test.go +++ b/web/handlers/admin/aliases_test.go @@ -63,7 +63,7 @@ func TestAliasesRevoke(t *testing.T) { addVals := url.Values{"name": []string{"the-name"}} rec := ts.Client.PostForm(urlRevoke, addVals) - a.Equal(http.StatusTemporaryRedirect, rec.Code) + a.Equal(http.StatusSeeOther, rec.Code) a.Equal(overviewURL.Path, rec.Header().Get("Location")) a.True(len(rec.Result().Cookies()) > 0, "got a cookie") @@ -77,7 +77,7 @@ func TestAliasesRevoke(t *testing.T) { ts.AliasesDB.RevokeReturns(roomdb.ErrNotFound) addVals = url.Values{"name": []string{"nope"}} rec = ts.Client.PostForm(urlRevoke, addVals) - a.Equal(http.StatusTemporaryRedirect, rec.Code) + a.Equal(http.StatusSeeOther, rec.Code) a.Equal(overviewURL.Path, rec.Header().Get("Location")) a.True(len(rec.Result().Cookies()) > 0, "got a cookie") diff --git a/web/handlers/admin/denied_keys.go b/web/handlers/admin/denied_keys.go index e3ef206..ae4ad68 100644 --- a/web/handlers/admin/denied_keys.go +++ b/web/handlers/admin/denied_keys.go @@ -12,6 +12,7 @@ import ( "github.com/ssb-ngi-pointer/go-ssb-room/roomdb" weberrors "github.com/ssb-ngi-pointer/go-ssb-room/web/errors" + "github.com/ssb-ngi-pointer/go-ssb-room/web/members" refs "go.mindeco.de/ssb-refs" ) @@ -20,7 +21,8 @@ type deniedKeysHandler struct { flashes *weberrors.FlashHelper - db roomdb.DeniedKeysService + db roomdb.DeniedKeysService + roomCfg roomdb.RoomConfig } const redirectToDeniedKeys = "/admin/denied" @@ -29,6 +31,15 @@ func (h deniedKeysHandler) add(w http.ResponseWriter, req *http.Request) { // always redirect defer http.Redirect(w, req, redirectToDeniedKeys, http.StatusSeeOther) + ctx := req.Context() + + _, err := members.CheckAllowed(ctx, h.roomCfg, members.ActionChangeDeniedKeys) + if err != nil { + err := weberrors.ErrNotAuthorized + h.flashes.AddError(w, req, err) + return + } + if req.Method != "POST" { err := weberrors.ErrBadRequest{Where: "HTTP Method", Details: fmt.Errorf("expected POST not %s", req.Method)} h.flashes.AddError(w, req, err) @@ -109,7 +120,16 @@ func (h deniedKeysHandler) remove(rw http.ResponseWriter, req *http.Request) { // always redirect defer http.Redirect(rw, req, redirectToDeniedKeys, http.StatusSeeOther) - err := req.ParseForm() + ctx := req.Context() + + _, err := members.CheckAllowed(ctx, h.roomCfg, members.ActionChangeDeniedKeys) + if err != nil { + err := weberrors.ErrNotAuthorized + h.flashes.AddError(rw, req, err) + return + } + + err = req.ParseForm() if err != nil { err = weberrors.ErrBadRequest{Where: "Form data", Details: err} h.flashes.AddError(rw, req, err) @@ -123,7 +143,7 @@ func (h deniedKeysHandler) remove(rw http.ResponseWriter, req *http.Request) { return } - err = h.db.RemoveID(req.Context(), id) + err = h.db.RemoveID(ctx, id) if err != nil { h.flashes.AddError(rw, req, err) } else { diff --git a/web/handlers/admin/denied_keys_test.go b/web/handlers/admin/denied_keys_test.go index 837993e..1a0feed 100644 --- a/web/handlers/admin/denied_keys_test.go +++ b/web/handlers/admin/denied_keys_test.go @@ -10,6 +10,7 @@ import ( "github.com/PuerkitoBio/goquery" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/ssb-ngi-pointer/go-ssb-room/roomdb" "github.com/ssb-ngi-pointer/go-ssb-room/web/router" @@ -33,10 +34,12 @@ func TestDeniedKeysEmpty(t *testing.T) { }) } -func TestDeniedKeysDisabledInterface(t *testing.T) { +func TestDeniedKeysAddDisabledInterface(t *testing.T) { ts := newSession(t) a := assert.New(t) + ts.ConfigDB.GetPrivacyModeReturns(roomdb.ModeRestricted, nil) + listURL := ts.URLTo(router.AdminDeniedKeysOverview) ts.User = roomdb.Member{ @@ -44,6 +47,7 @@ func TestDeniedKeysDisabledInterface(t *testing.T) { Role: roomdb.RoleAdmin, } + // check basic form html, resp := ts.Client.GetHTML(listURL) a.Equal(http.StatusOK, resp.Code, "wrong HTTP status code") @@ -65,27 +69,39 @@ func TestDeniedKeysDisabledInterface(t *testing.T) { {Name: "comment", Type: "text"}, }) + // create assertion helpers newKey := "@x7iOLUcq3o+sjGeAnipvWeGzfuYgrXl8L4LYlxIhwDc=.ed25519" - addVals := url.Values{ - "comment": []string{"some comment"}, - // just any key that looks valid - "pub_key": []string{newKey}, + addVals := url.Values{"comment": []string{"some comment"}, "pub_key": []string{newKey}} + totalAddCallCount := 0 + checkCanPostNewEntry := func(t *testing.T, shouldWork bool) { + a := assert.New(t) + r := require.New(t) + rec := ts.Client.PostForm(addURL, addVals) + + a.Equal(http.StatusSeeOther, rec.Code) + a.Equal(listURL.Path, rec.Header().Get("Location")) + + var wantedLabel = "ErrorNotAuthorized" + if shouldWork { + totalAddCallCount++ + wantedLabel = "AdminDeniedKeysAdded" + + // require call count to not panic + r.Equal(totalAddCallCount, ts.DeniedKeysDB.AddCallCount()) + _, addedKey, addedComment := ts.DeniedKeysDB.AddArgsForCall(totalAddCallCount - 1) + a.Equal(newKey, addedKey.Ref()) + a.Equal("some comment", addedComment) + } else { + r.Equal(totalAddCallCount, ts.DeniedKeysDB.AddCallCount()) + } + + webassert.HasFlashMessages(t, ts.Client, listURL, wantedLabel) } - rec := ts.Client.PostForm(addURL, addVals) - a.Equal(http.StatusSeeOther, rec.Code) - - overview := ts.URLTo(router.AdminDeniedKeysOverview) - a.Equal(overview.Path, rec.Header().Get("Location")) - webassert.HasFlashMessages(t, ts.Client, overview, "AdminDeniedKeysAdded") - - a.Equal(1, ts.DeniedKeysDB.AddCallCount()) - _, addedKey, addedComment := ts.DeniedKeysDB.AddArgsForCall(0) - a.Equal(newKey, addedKey.Ref()) - a.Equal("some comment", addedComment) /* Verify that the inputs are visible/hidden depending on user roles */ - checkInputsAreDisabled := func(shouldBeDisabled bool) { - html, resp = ts.Client.GetHTML(listURL) + checkInputsAreDisabled := func(t *testing.T, shouldBeDisabled bool) { + a := assert.New(t) + html, resp := ts.Client.GetHTML(listURL) a.Equal(http.StatusOK, resp.Code, "wrong HTTP status code") inputContainer := html.Find("#denied-keys-input-container") a.Equal(1, inputContainer.Length()) @@ -94,26 +110,41 @@ func TestDeniedKeysDisabledInterface(t *testing.T) { a.Equal(3, inputs.Length()) inputs.Each(func(i int, el *goquery.Selection) { _, disabled := el.Attr("disabled") - a.Equal(shouldBeDisabled, disabled) + name, _ := el.Attr("name") + a.Equal(shouldBeDisabled, disabled, "found diabled tag on element %q", name) }) } - // verify that inputs are enabled for RoleAdmin - checkInputsAreDisabled(false) - - // verify that inputs are enabled for RoleModerator - ts.User = roomdb.Member{ - ID: 9001, - Role: roomdb.RoleModerator, + /* test various restricted mode with the roles member, mod, admin */ + for _, mode := range roomdb.AllPrivacyModes { + t.Run(mode.String(), func(t *testing.T) { + ts.ConfigDB.GetPrivacyModeReturns(mode, nil) + t.Run("role:member", func(t *testing.T) { + ts.User = roomdb.Member{ + ID: 7331, + Role: roomdb.RoleMember, + } + checkInputsAreDisabled(t, mode != roomdb.ModeCommunity) + checkCanPostNewEntry(t, mode == roomdb.ModeCommunity) + }) + t.Run("role:moderator", func(t *testing.T) { + ts.User = roomdb.Member{ + ID: 9001, + Role: roomdb.RoleModerator, + } + checkInputsAreDisabled(t, false) + checkCanPostNewEntry(t, true) + }) + t.Run("role:admin", func(t *testing.T) { + ts.User = roomdb.Member{ + ID: 1234, + Role: roomdb.RoleAdmin, + } + checkInputsAreDisabled(t, false) + checkCanPostNewEntry(t, true) + }) + }) } - checkInputsAreDisabled(false) - - // verify that inputs are disabled for RoleMember - ts.User = roomdb.Member{ - ID: 7331, - Role: roomdb.RoleMember, - } - checkInputsAreDisabled(true) } func TestDeniedKeysAdd(t *testing.T) { @@ -295,3 +326,93 @@ func TestDeniedKeysRemove(t *testing.T) { a.Equal(overview.Path, rec.Header().Get("Location")) webassert.HasFlashMessages(t, ts.Client, overview, "ErrorNotFound") } + +func TestDeniedKeysRemovalRights(t *testing.T) { + ts := newSession(t) + + // check disabled remove button on list page + ts.DeniedKeysDB.ListReturns([]roomdb.ListEntry{ + {ID: 666, PubKey: generatePubKey(), Comment: "test-entry"}, + }, nil) + + urlRemoveConfirm := ts.URLTo(router.AdminDeniedKeysRemoveConfirm, "id", "666").String() + listURL := ts.URLTo(router.AdminDeniedKeysOverview) + listShouldShowTheRemoveButtonAsWorking := func(shouldWork bool) func(t *testing.T) { + return func(t *testing.T) { + a := assert.New(t) + html, resp := ts.Client.GetHTML(listURL) + a.Equal(http.StatusOK, resp.Code, "wrong HTTP status code") + linktToConfirm := html.Find("#theList li a") + a.Equal(1, linktToConfirm.Length()) + // pubkey, comment, submit button + linkText, has := linktToConfirm.Attr("href") + a.True(has, "anchor should have a href in any case") + if shouldWork { + a.Equal(urlRemoveConfirm, linkText) + } else { + a.True(linktToConfirm.HasClass("line-through"), "should have strikethrogh class") + a.Equal("#", linkText) + } + } + } + + // check who can actually remove + ts.DeniedKeysDB.RemoveIDReturns(nil) + urlRemove := ts.URLTo(router.AdminDeniedKeysRemove) + removeVals := url.Values{"id": []string{"666"}} + + totalRemoveCallCount := 0 + removeFromListShouldWork := func(works bool) func(t *testing.T) { + return func(t *testing.T) { + a := assert.New(t) + r := require.New(t) + rec := ts.Client.PostForm(urlRemove, removeVals) + a.Equal(http.StatusSeeOther, rec.Code, "unexpected exit code %s", rec.Result().Status) + if works { + totalRemoveCallCount++ + _, userID := ts.DeniedKeysDB.RemoveIDArgsForCall(totalRemoveCallCount - 1) + a.EqualValues(666, userID) + webassert.HasFlashMessages(t, ts.Client, listURL, "AdminDeniedKeysRemoved") + } else { + webassert.HasFlashMessages(t, ts.Client, listURL, "ErrorNotAuthorized") + } + r.Equal(totalRemoveCallCount, ts.DeniedKeysDB.RemoveIDCallCount()) + } + } + + // the users who will execute the action + memberUser := roomdb.Member{ + ID: 7331, + Role: roomdb.RoleMember, + PubKey: generatePubKey(), + } + modUser := roomdb.Member{ + ID: 9001, + Role: roomdb.RoleModerator, + PubKey: generatePubKey(), + } + adminUser := roomdb.Member{ + ID: 1337, + Role: roomdb.RoleAdmin, + PubKey: generatePubKey(), + } + + /* test various restricted mode with the roles member, mod, admin */ + for _, mode := range roomdb.AllPrivacyModes { + ts.ConfigDB.GetPrivacyModeReturns(mode, nil) + + // members can remove entries only in community mode + ts.User = memberUser + t.Run(mode.String()+" member sees link working", listShouldShowTheRemoveButtonAsWorking(mode == roomdb.ModeCommunity)) + t.Run(mode.String()+" member can actually remove", removeFromListShouldWork(mode == roomdb.ModeCommunity)) + + // mods & admins can always invite + ts.User = modUser + t.Run(mode.String()+" mod sees link working", listShouldShowTheRemoveButtonAsWorking(true)) + t.Run(mode.String()+" mod can actually remove", removeFromListShouldWork(true)) + + ts.User = adminUser + t.Run(mode.String()+" admin sees link working", listShouldShowTheRemoveButtonAsWorking(true)) + t.Run(mode.String()+" admin sees link working", removeFromListShouldWork(true)) + } +} diff --git a/web/handlers/admin/handler.go b/web/handlers/admin/handler.go index e159263..86b2a3a 100644 --- a/web/handlers/admin/handler.go +++ b/web/handlers/admin/handler.go @@ -22,7 +22,6 @@ import ( "github.com/ssb-ngi-pointer/go-ssb-room/web" weberrors "github.com/ssb-ngi-pointer/go-ssb-room/web/errors" "github.com/ssb-ngi-pointer/go-ssb-room/web/i18n" - "github.com/ssb-ngi-pointer/go-ssb-room/web/members" "github.com/ssb-ngi-pointer/go-ssb-room/web/router" ) @@ -114,6 +113,8 @@ func Handler( flashes: fh, db: dbs.DeniedKeys, + + roomCfg: dbs.Config, } mux.HandleFunc("/denied", r.HTML("admin/denied-keys.tmpl", dh.overview)) mux.HandleFunc("/denied/add", dh.add) @@ -129,6 +130,7 @@ func Handler( db: dbs.Members, fallbackAuthDB: dbs.AuthFallback, + roomCfgDB: dbs.Config, } mux.HandleFunc("/member", r.HTML("admin/member.tmpl", mh.details)) mux.HandleFunc("/members", r.HTML("admin/member-list.tmpl", mh.overview)) @@ -156,16 +158,17 @@ func Handler( var nh = noticeHandler{ r: r, + urlTo: urlTo, flashes: fh, noticeDB: dbs.Notices, pinnedDB: dbs.PinnedNotices, + roomCfg: dbs.Config, } - onlyModsAndAdmins := checkMemberRole(r.Error, roomdb.RoleModerator, roomdb.RoleAdmin) - mux.Handle("/notice/edit", onlyModsAndAdmins(r.HTML("admin/notice-edit.tmpl", nh.edit))) - mux.Handle("/notice/translation/draft", onlyModsAndAdmins(r.HTML("admin/notice-edit.tmpl", nh.draftTranslation))) - mux.Handle("/notice/translation/add", onlyModsAndAdmins(http.HandlerFunc(nh.addTranslation))) - mux.Handle("/notice/save", onlyModsAndAdmins(http.HandlerFunc(nh.save))) + mux.Handle("/notice/edit", r.HTML("admin/notice-edit.tmpl", nh.edit)) + mux.Handle("/notice/translation/draft", r.HTML("admin/notice-edit.tmpl", nh.draftTranslation)) + mux.Handle("/notice/translation/add", http.HandlerFunc(nh.addTranslation)) + mux.Handle("/notice/save", http.HandlerFunc(nh.save)) // path:/ matches everything that isn't registerd (ie. its the "Not Found handler") mux.HandleFunc("/", http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { @@ -175,35 +178,6 @@ func Handler( return customStripPrefix("/admin", mux) } -func checkMemberRole(eh render.ErrorHandlerFunc, roles ...roomdb.Role) func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - currentMember := members.FromContext(req.Context()) - if currentMember == nil { - err := weberrors.ErrRedirect{Path: "/admin/dashboard", Reason: fmt.Errorf("not an member")} - eh(rw, req, http.StatusSeeOther, err) - return - } - - var roleMatched = false - for _, r := range roles { - if currentMember.Role == r { - roleMatched = true - break - } - } - - if !roleMatched { - err := weberrors.ErrRedirect{Path: "/admin/dashboard", Reason: weberrors.ErrNotAuthorized} - eh(rw, req, http.StatusSeeOther, err) - return - } - - next.ServeHTTP(rw, req) - }) - } -} - // how many elements does a paginated page have by default const defaultPageSize = 20 diff --git a/web/handlers/admin/invites.go b/web/handlers/admin/invites.go index ea92822..a5d492d 100644 --- a/web/handlers/admin/invites.go +++ b/web/handlers/admin/invites.go @@ -55,36 +55,19 @@ func (h invitesHandler) create(w http.ResponseWriter, req *http.Request) (interf if req.Method != "POST" { return nil, weberrors.ErrBadRequest{Where: "HTTP Method", Details: fmt.Errorf("expected POST not %s", req.Method)} } + if err := req.ParseForm(); err != nil { return nil, weberrors.ErrBadRequest{Where: "Form data", Details: err} } - member := members.FromContext(req.Context()) - if member == nil { - return nil, weberrors.ErrNotAuthorized - } - pm, err := h.config.GetPrivacyMode(req.Context()) + ctx := req.Context() + + member, err := members.CheckAllowed(ctx, h.config, members.ActionInviteMember) if err != nil { return nil, err } - /* We want to check: - * 1. the room's privacy mode - * 2. the role of the member trying to create the invite - * and deny unallowed requests (e.g. member creating invite in ModeRestricted) - */ - switch pm { - case roomdb.ModeOpen: - case roomdb.ModeCommunity: - if member.Role == roomdb.RoleUnknown { - return nil, weberrors.ErrNotAuthorized - } - case roomdb.ModeRestricted: - if member.Role == roomdb.RoleMember || member.Role == roomdb.RoleUnknown { - return nil, weberrors.ErrNotAuthorized - } - } - token, err := h.db.Create(req.Context(), member.ID) + token, err := h.db.Create(ctx, member.ID) if err != nil { return nil, err } @@ -122,32 +105,37 @@ func (h invitesHandler) revokeConfirm(rw http.ResponseWriter, req *http.Request) const redirectToInvites = "/admin/invites" func (h invitesHandler) revoke(rw http.ResponseWriter, req *http.Request) { + // always redirect + defer http.Redirect(rw, req, redirectToInvites, http.StatusSeeOther) + + ctx := req.Context() + + if _, err := members.CheckAllowed(ctx, h.config, members.ActionInviteMember); err != nil { + h.flashes.AddError(rw, req, err) + return + } + err := req.ParseForm() if err != nil { err = weberrors.ErrBadRequest{Where: "Form data", Details: err} - // TODO "flash" errors - http.Redirect(rw, req, redirectToInvites, http.StatusFound) + h.flashes.AddError(rw, req, err) return } id, err := strconv.ParseInt(req.FormValue("id"), 10, 64) if err != nil { err = weberrors.ErrBadRequest{Where: "ID", Details: err} - // TODO "flash" errors - http.Redirect(rw, req, redirectToInvites, http.StatusFound) + h.flashes.AddError(rw, req, err) return } - status := http.StatusFound - err = h.db.Revoke(req.Context(), id) + err = h.db.Revoke(ctx, id) if err != nil { - if !errors.Is(err, roomdb.ErrNotFound) { - // TODO "flash" errors - h.r.Error(rw, req, http.StatusInternalServerError, err) - return + if errors.Is(err, roomdb.ErrNotFound) { + err = weberrors.ErrNotFound{What: "invite"} } - status = http.StatusNotFound + h.flashes.AddError(rw, req, err) + } else { + h.flashes.AddMessage(rw, req, "InviteRevoked") } - - http.Redirect(rw, req, redirectToInvites, status) } diff --git a/web/handlers/admin/invites_test.go b/web/handlers/admin/invites_test.go index 7b6161f..63dc19f 100644 --- a/web/handlers/admin/invites_test.go +++ b/web/handlers/admin/invites_test.go @@ -81,11 +81,13 @@ func TestInvitesOverview(t *testing.T) { Role: roomdb.RoleAdmin, } testInviteButtonDisabled(false) + ts.User = roomdb.Member{ ID: 7331, Role: roomdb.RoleModerator, } testInviteButtonDisabled(false) + ts.User = roomdb.Member{ ID: 9001, Role: roomdb.RoleMember, @@ -99,11 +101,13 @@ func TestInvitesOverview(t *testing.T) { Role: roomdb.RoleAdmin, } testInviteButtonDisabled(false) + ts.User = roomdb.Member{ ID: 7331, Role: roomdb.RoleModerator, } testInviteButtonDisabled(false) + ts.User = roomdb.Member{ ID: 9001, Role: roomdb.RoleMember, @@ -139,7 +143,7 @@ func TestInvitesCreateForm(t *testing.T) { a.Equal(addURL.String(), action) } -func TestInvitesCreate(t *testing.T) { +func TestInvitesCreateAndRevoke(t *testing.T) { ts := newSession(t) a := assert.New(t) r := require.New(t) @@ -150,7 +154,10 @@ func TestInvitesCreate(t *testing.T) { ts.InvitesDB.CreateReturns(testInvite, nil) totalCreateCallCount := 0 - createInviteShouldWork := func(works bool) *httptest.ResponseRecorder { + createInviteShouldWork := func(t *testing.T, works bool) *httptest.ResponseRecorder { + a := assert.New(t) + r := require.New(t) + rec := ts.Client.PostForm(urlCreate, url.Values{}) if works { totalCreateCallCount += 1 @@ -165,10 +172,31 @@ func TestInvitesCreate(t *testing.T) { return rec } - rec := createInviteShouldWork(true) + totalRevokeCallCount := 0 + urlRevoke := ts.URLTo(router.AdminInvitesRevoke) + canRevokeInvite := func(t *testing.T, canRevoke bool) { + a := assert.New(t) + r := require.New(t) + + rec := ts.Client.PostForm(urlRevoke, url.Values{ + "id": []string{"666"}, + }) + a.Equal(http.StatusSeeOther, rec.Code) + if canRevoke { + totalRevokeCallCount += 1 + r.Equal(totalRevokeCallCount, ts.InvitesDB.RevokeCallCount()) + _, userID := ts.InvitesDB.RevokeArgsForCall(totalRevokeCallCount - 1) + a.EqualValues(666, userID) + } else { + r.Equal(totalRevokeCallCount, ts.InvitesDB.RevokeCallCount()) + } + return + } + + rec := createInviteShouldWork(t, true) doc, err := goquery.NewDocumentFromReader(rec.Body) - require.NoError(t, err, "failed to parse response") + r.NoError(err, "failed to parse response") webassert.Localized(t, doc, []webassert.LocalizedElement{ {"title", "AdminInviteCreatedTitle"}, @@ -197,16 +225,29 @@ func TestInvitesCreate(t *testing.T) { } /* test invite creation under various restricted mode with the roles member, mod, admin */ - modes := []roomdb.PrivacyMode{roomdb.ModeRestricted, roomdb.ModeCommunity} - for _, mode := range modes { - ts.ConfigDB.GetPrivacyModeReturns(mode, nil) - ts.User = memberUser - // members can only invite in community rooms - createInviteShouldWork(mode == roomdb.ModeCommunity) - // mods & admins can always invite - ts.User = modUser - createInviteShouldWork(true) - ts.User = adminUser - createInviteShouldWork(true) + for _, mode := range roomdb.AllPrivacyModes { + t.Run(mode.String(), func(t *testing.T) { + ts.ConfigDB.GetPrivacyModeReturns(mode, nil) + + // members can only invite in community rooms + t.Run("members", func(t *testing.T) { + ts.User = memberUser + createInviteShouldWork(t, mode != roomdb.ModeRestricted) + canRevokeInvite(t, mode != roomdb.ModeRestricted) + }) + + // mods & admins can always invite + t.Run("mods", func(t *testing.T) { + ts.User = modUser + createInviteShouldWork(t, true) + canRevokeInvite(t, true) + }) + + t.Run("admins", func(t *testing.T) { + ts.User = adminUser + createInviteShouldWork(t, true) + canRevokeInvite(t, true) + }) + }) } } diff --git a/web/handlers/admin/members.go b/web/handlers/admin/members.go index 5a8a2a1..2702acf 100644 --- a/web/handlers/admin/members.go +++ b/web/handlers/admin/members.go @@ -29,6 +29,7 @@ type membersHandler struct { db roomdb.MembersService fallbackAuthDB roomdb.AuthFallbackService + roomCfgDB roomdb.RoomConfig } const redirectToMembers = "/admin/members" @@ -46,23 +47,23 @@ func (h membersHandler) add(w http.ResponseWriter, req *http.Request) { return } + defer http.Redirect(w, req, redirectToMembers, http.StatusSeeOther) + newEntry := req.Form.Get("pub_key") newEntryParsed, err := refs.ParseFeedRef(newEntry) if err != nil { err = weberrors.ErrBadRequest{Where: "Public Key", Details: err} h.flashes.AddError(w, req, err) - http.Redirect(w, req, redirectToMembers, http.StatusTemporaryRedirect) return } _, err = h.db.Add(req.Context(), *newEntryParsed, roomdb.RoleMember) if err != nil { h.flashes.AddError(w, req, err) - } else { - h.flashes.AddMessage(w, req, "AdminMemberAdded") + return } - http.Redirect(w, req, redirectToMembers, http.StatusTemporaryRedirect) + h.flashes.AddMessage(w, req, "AdminMemberAdded") } func (h membersHandler) changeRole(w http.ResponseWriter, req *http.Request) { @@ -108,7 +109,7 @@ func (h membersHandler) changeRole(w http.ResponseWriter, req *http.Request) { h.flashes.AddMessage(w, req, "AdminMemberUpdated") memberDetailsURL := h.urlTo(router.AdminMemberDetails, "id", memberID).String() - http.Redirect(w, req, memberDetailsURL, http.StatusTemporaryRedirect) + http.Redirect(w, req, memberDetailsURL, http.StatusSeeOther) } func (h membersHandler) overview(rw http.ResponseWriter, req *http.Request) (interface{}, error) { @@ -191,6 +192,8 @@ func (h membersHandler) removeConfirm(rw http.ResponseWriter, req *http.Request) } func (h membersHandler) remove(rw http.ResponseWriter, req *http.Request) { + ctx := req.Context() + if req.Method != "POST" { err := weberrors.ErrBadRequest{Where: "HTTP Method", Details: fmt.Errorf("expected POST not %s", req.Method)} h.r.Error(rw, req, http.StatusBadRequest, err) @@ -203,22 +206,26 @@ func (h membersHandler) remove(rw http.ResponseWriter, req *http.Request) { return } + defer http.Redirect(rw, req, redirectToMembers, http.StatusSeeOther) + + if _, err := members.CheckAllowed(ctx, h.roomCfgDB, members.ActionRemoveMember); err != nil { + h.flashes.AddError(rw, req, err) + return + } + id, err := strconv.ParseInt(req.FormValue("id"), 10, 64) if err != nil { err = weberrors.ErrBadRequest{Where: "ID", Details: err} h.flashes.AddError(rw, req, err) - http.Redirect(rw, req, redirectToMembers, http.StatusTemporaryRedirect) return } - err = h.db.RemoveID(req.Context(), id) + err = h.db.RemoveID(ctx, id) if err != nil { h.flashes.AddError(rw, req, err) } else { h.flashes.AddMessage(rw, req, "AdminMemberRemoved") } - - http.Redirect(rw, req, redirectToMembers, http.StatusTemporaryRedirect) } func (h membersHandler) createPasswordResetToken(rw http.ResponseWriter, req *http.Request) (interface{}, error) { diff --git a/web/handlers/admin/members_test.go b/web/handlers/admin/members_test.go index 36813e3..2826325 100644 --- a/web/handlers/admin/members_test.go +++ b/web/handlers/admin/members_test.go @@ -70,7 +70,7 @@ func TestMembersAdd(t *testing.T) { "pub_key": []string{newKey}, } rec := ts.Client.PostForm(addURL, addVals) - a.Equal(http.StatusTemporaryRedirect, rec.Code) + a.Equal(http.StatusSeeOther, rec.Code) a.Equal(1, ts.MembersDB.AddCallCount()) _, addedPubKey, addedRole := ts.MembersDB.AddArgsForCall(0) @@ -128,7 +128,7 @@ func TestMembersDontAddInvalid(t *testing.T) { "pub_key": []string{newKey}, } rec := ts.Client.PostForm(addURL, addVals) - a.Equal(http.StatusTemporaryRedirect, rec.Code) + a.Equal(http.StatusSeeOther, rec.Code) a.Equal(0, ts.MembersDB.AddCallCount()) @@ -262,36 +262,102 @@ func TestMemberDetails(t *testing.T) { wantLink = ts.URLTo(router.AdminMembersRemoveConfirm, "id", 1) a.Equal(wantLink.String(), removeLink) - testDisabledBehaviour := func(isElevated bool) { - html, resp := ts.Client.GetHTML(memberURL) - a.Equal(http.StatusOK, resp.Code, "wrong HTTP status code") + testChangeRoleDisabledBehaviour := func(t *testing.T, html *goquery.Document, canSee bool) { + a := assert.New(t) + // check for SSB ID ssbID := html.Find("#ssb-id") a.Equal(feedRef.Ref(), ssbID.Text()) // check for change-role dropdown roleDropdown := html.Find("#change-role") - if isElevated { + if canSee { a.Equal(1, roleDropdown.Length()) } else { a.Equal(0, roleDropdown.Length()) } } - testDisabledBehaviour(true) - /* Now: verify that moderators cannot make room settings changes */ - ts.User = roomdb.Member{ - ID: 7331, - Role: roomdb.RoleModerator, - } - testDisabledBehaviour(true) + testRemoveButtonHiddenBehavior := func(t *testing.T, html *goquery.Document, canSee bool) { + a := assert.New(t) - /* Finally: verify that members cannot make room settings changes */ - ts.User = roomdb.Member{ - ID: 9001, - Role: roomdb.RoleMember, + rmButton := html.Find("a#remove-member") + if canSee { + a.Equal(1, rmButton.Length()) + } else { + a.Equal(0, rmButton.Length()) + } + } + overviewURL := ts.URLTo(router.AdminMembersOverview) + removeURL := ts.URLTo(router.AdminMembersRemove) + totalRemoveCallCount := 0 + testCanDoRemoveBehavior := func(t *testing.T, html *goquery.Document, canDo bool) { + a := assert.New(t) + + resp := ts.Client.PostForm(removeURL, url.Values{"id": []string{"1"}}) + a.Equal(http.StatusSeeOther, resp.Code, "unexpected status code") + + var wantLabel string + if canDo { + totalRemoveCallCount++ + wantLabel = "AdminMemberRemoved" + } else { + wantLabel = "ErrorNotAuthorized" + } + a.Equal(totalRemoveCallCount, ts.MembersDB.RemoveIDCallCount()) + webassert.HasFlashMessages(t, ts.Client, overviewURL, wantLabel) + } + + memberUser := roomdb.Member{ + ID: 7331, + Role: roomdb.RoleMember, + PubKey: generatePubKey(), + } + modUser := roomdb.Member{ + ID: 9001, + Role: roomdb.RoleModerator, + PubKey: generatePubKey(), + } + adminUser := roomdb.Member{ + ID: 1337, + Role: roomdb.RoleAdmin, + PubKey: generatePubKey(), + } + + for _, mode := range roomdb.AllPrivacyModes { + t.Run(mode.String(), func(t *testing.T) { + ts.ConfigDB.GetPrivacyModeReturns(mode, nil) + + // members can only invite in community rooms + t.Run("members", func(t *testing.T) { + ts.User = memberUser + html, resp := ts.Client.GetHTML(memberURL) + a.Equal(http.StatusOK, resp.Code, "wrong HTTP status code") + testChangeRoleDisabledBehaviour(t, html, false) + testRemoveButtonHiddenBehavior(t, html, false) + testCanDoRemoveBehavior(t, html, false) + }) + + // mods & admins can always invite + t.Run("mods", func(t *testing.T) { + ts.User = modUser + html, resp := ts.Client.GetHTML(memberURL) + a.Equal(http.StatusOK, resp.Code, "wrong HTTP status code") + testChangeRoleDisabledBehaviour(t, html, true) + testRemoveButtonHiddenBehavior(t, html, true) + testCanDoRemoveBehavior(t, html, true) + }) + + t.Run("admins", func(t *testing.T) { + ts.User = adminUser + html, resp := ts.Client.GetHTML(memberURL) + a.Equal(http.StatusOK, resp.Code, "wrong HTTP status code") + testChangeRoleDisabledBehaviour(t, html, true) + testRemoveButtonHiddenBehavior(t, html, true) + testCanDoRemoveBehavior(t, html, true) + }) + }) } - testDisabledBehaviour(false) } func TestMembersRemoveConfirmation(t *testing.T) { @@ -337,7 +403,7 @@ func TestMembersRemove(t *testing.T) { addVals := url.Values{"id": []string{"666"}} rec := ts.Client.PostForm(urlRemove, addVals) - a.Equal(http.StatusTemporaryRedirect, rec.Code) + a.Equal(http.StatusSeeOther, rec.Code) a.Equal(1, ts.MembersDB.RemoveIDCallCount()) _, theID := ts.MembersDB.RemoveIDArgsForCall(0) @@ -355,7 +421,7 @@ func TestMembersRemove(t *testing.T) { ts.MembersDB.RemoveIDReturns(roomdb.ErrNotFound) addVals = url.Values{"id": []string{"667"}} rec = ts.Client.PostForm(urlRemove, addVals) - a.Equal(http.StatusTemporaryRedirect, rec.Code) + a.Equal(http.StatusSeeOther, rec.Code) // check flash message res = rec.Result() diff --git a/web/handlers/admin/notices.go b/web/handlers/admin/notices.go index 027d2ab..2eecb03 100644 --- a/web/handlers/admin/notices.go +++ b/web/handlers/admin/notices.go @@ -7,6 +7,8 @@ import ( "strconv" "strings" + "github.com/ssb-ngi-pointer/go-ssb-room/web" + "github.com/ssb-ngi-pointer/go-ssb-room/web/members" "github.com/ssb-ngi-pointer/go-ssb-room/web/router" "github.com/gorilla/csrf" @@ -17,17 +19,24 @@ import ( ) type noticeHandler struct { - r *render.Renderer - + r *render.Renderer + urlTo web.URLMaker flashes *weberrors.FlashHelper noticeDB roomdb.NoticesService pinnedDB roomdb.PinnedNoticesService + roomCfg roomdb.RoomConfig } func (h noticeHandler) draftTranslation(rw http.ResponseWriter, req *http.Request) (interface{}, error) { - pinnedName := req.URL.Query().Get("name") + if _, err := members.CheckAllowed(req.Context(), h.roomCfg, members.ActionChangeNotice); err != nil { + h.flashes.AddError(rw, req, err) + noticesURL := h.urlTo(router.CompleteNoticeList) + http.Redirect(rw, req, noticesURL.String(), http.StatusSeeOther) + return nil, err + } + pinnedName := req.URL.Query().Get("name") if !roomdb.PinnedNoticeName(pinnedName).Valid() { return nil, weberrors.ErrBadRequest{Where: "pinnedName", Details: fmt.Errorf("invalid pinned notice name")} } @@ -40,12 +49,7 @@ func (h noticeHandler) draftTranslation(rw http.ResponseWriter, req *http.Reques } func (h noticeHandler) addTranslation(rw http.ResponseWriter, req *http.Request) { - err := req.ParseForm() - if err != nil { - err = weberrors.ErrBadRequest{Where: "form data", Details: err} - h.r.Error(rw, req, http.StatusInternalServerError, err) - return - } + ctx := req.Context() // reply with 405 error: Method not allowed if req.Method != "POST" { @@ -54,10 +58,30 @@ func (h noticeHandler) addTranslation(rw http.ResponseWriter, req *http.Request) return } + err := req.ParseForm() + if err != nil { + err = weberrors.ErrBadRequest{Where: "form data", Details: err} + h.r.Error(rw, req, http.StatusInternalServerError, err) + return + } + + redirect := req.FormValue("redirect") + if redirect == "" { + noticesURL := h.urlTo(router.CompleteNoticeList) + redirect = noticesURL.String() + } + + defer http.Redirect(rw, req, redirect, http.StatusSeeOther) + + if _, err := members.CheckAllowed(ctx, h.roomCfg, members.ActionChangeNotice); err != nil { + h.flashes.AddError(rw, req, err) + return + } + pinnedName := roomdb.PinnedNoticeName(req.FormValue("name")) if !pinnedName.Valid() { err := weberrors.ErrBadRequest{Where: "name", Details: fmt.Errorf("invalid pinned notice name")} - h.r.Error(rw, req, http.StatusInternalServerError, err) + h.flashes.AddError(rw, req, err) return } @@ -65,7 +89,7 @@ func (h noticeHandler) addTranslation(rw http.ResponseWriter, req *http.Request) n.Title = req.FormValue("title") if n.Title == "" { err = weberrors.ErrBadRequest{Where: "title", Details: fmt.Errorf("title can't be empty")} - h.r.Error(rw, req, http.StatusInternalServerError, err) + h.flashes.AddError(rw, req, err) return } @@ -73,40 +97,45 @@ func (h noticeHandler) addTranslation(rw http.ResponseWriter, req *http.Request) n.Language = req.FormValue("language") if n.Language == "" { err := weberrors.ErrBadRequest{Where: "language", Details: fmt.Errorf("language can't be empty")} - h.r.Error(rw, req, http.StatusInternalServerError, err) + h.flashes.AddError(rw, req, err) return } n.Content = req.FormValue("content") if n.Content == "" { err = weberrors.ErrBadRequest{Where: "content", Details: fmt.Errorf("content can't be empty")} - h.r.Error(rw, req, http.StatusInternalServerError, err) + h.flashes.AddError(rw, req, err) return } // https://github.com/russross/blackfriday/issues/575 n.Content = strings.Replace(n.Content, "\r\n", "\n", -1) - err = h.noticeDB.Save(req.Context(), &n) + err = h.noticeDB.Save(ctx, &n) if err != nil { - h.r.Error(rw, req, http.StatusInternalServerError, err) + h.flashes.AddError(rw, req, err) return } - err = h.pinnedDB.Set(req.Context(), pinnedName, n.ID) + err = h.pinnedDB.Set(ctx, pinnedName, n.ID) if err != nil { - h.r.Error(rw, req, http.StatusInternalServerError, err) + h.flashes.AddError(rw, req, err) return } - // TODO: redirect to edit page of the new notice (need to add urlTo to handler) - redirect := req.FormValue("redirect") - if redirect == "" { - redirect = "/" - } - http.Redirect(rw, req, redirect, http.StatusTemporaryRedirect) + h.flashes.AddMessage(rw, req, "NoticeUpdated") + } func (h noticeHandler) edit(rw http.ResponseWriter, req *http.Request) (interface{}, error) { + ctx := req.Context() + + if _, err := members.CheckAllowed(ctx, h.roomCfg, members.ActionChangeNotice); err != nil { + h.flashes.AddError(rw, req, err) + noticesURL := h.urlTo(router.CompleteNoticeList) + http.Redirect(rw, req, noticesURL.String(), http.StatusSeeOther) + return nil, err + } + id, err := strconv.ParseInt(req.URL.Query().Get("id"), 10, 64) if err != nil { err = weberrors.ErrBadRequest{Where: "ID", Details: err} @@ -128,8 +157,6 @@ func (h noticeHandler) edit(rw http.ResponseWriter, req *http.Request) (interfac "SubmitAction": router.AdminNoticeSave, "Notice": n, "ContentPreview": template.HTML(preview), - // "Debug": string(preview), - // "DebugHex": hex.Dump(contentBytes), csrf.TemplateTag: csrf.TemplateField(req), } pageData["Flashes"], err = h.flashes.GetAll(rw, req) @@ -141,6 +168,14 @@ func (h noticeHandler) edit(rw http.ResponseWriter, req *http.Request) (interfac } func (h noticeHandler) save(rw http.ResponseWriter, req *http.Request) { + ctx := req.Context() + + if req.Method != "POST" { + err := weberrors.ErrBadRequest{Where: "http method type", Details: fmt.Errorf("add translation only accepts POST requests, sorry!")} + h.r.Error(rw, req, http.StatusMethodNotAllowed, err) + return + } + err := req.ParseForm() if err != nil { err = weberrors.ErrBadRequest{Where: "form data", Details: err} @@ -150,12 +185,16 @@ func (h noticeHandler) save(rw http.ResponseWriter, req *http.Request) { redirect := req.FormValue("redirect") if redirect == "" { - noticesURL, err := router.CompleteApp().Get(router.CompleteNoticeList).URL() - if err != nil { - h.r.Error(rw, req, http.StatusInternalServerError, err) - return - } - redirect = noticesURL.Path + noticesURL := h.urlTo(router.CompleteNoticeList) + redirect = noticesURL.String() + } + + // now, always redirect + defer http.Redirect(rw, req, redirect, http.StatusSeeOther) + + if _, err := members.CheckAllowed(ctx, h.roomCfg, members.ActionChangeNotice); err != nil { + h.flashes.AddError(rw, req, err) + return } var n roomdb.Notice @@ -163,7 +202,6 @@ func (h noticeHandler) save(rw http.ResponseWriter, req *http.Request) { if err != nil { err = weberrors.ErrBadRequest{Where: "id", Details: err} h.flashes.AddError(rw, req, err) - http.Redirect(rw, req, redirect, http.StatusSeeOther) return } @@ -171,7 +209,6 @@ func (h noticeHandler) save(rw http.ResponseWriter, req *http.Request) { if n.Title == "" { err = weberrors.ErrBadRequest{Where: "title", Details: fmt.Errorf("title can't be empty")} h.flashes.AddError(rw, req, err) - http.Redirect(rw, req, redirect, http.StatusSeeOther) return } @@ -180,7 +217,6 @@ func (h noticeHandler) save(rw http.ResponseWriter, req *http.Request) { if n.Language == "" { err = weberrors.ErrBadRequest{Where: "language", Details: fmt.Errorf("language can't be empty")} h.flashes.AddError(rw, req, err) - http.Redirect(rw, req, redirect, http.StatusSeeOther) return } @@ -188,7 +224,6 @@ func (h noticeHandler) save(rw http.ResponseWriter, req *http.Request) { if n.Content == "" { err = weberrors.ErrBadRequest{Where: "content", Details: fmt.Errorf("content can't be empty")} h.flashes.AddError(rw, req, err) - http.Redirect(rw, req, redirect, http.StatusSeeOther) return } @@ -198,10 +233,8 @@ func (h noticeHandler) save(rw http.ResponseWriter, req *http.Request) { err = h.noticeDB.Save(req.Context(), &n) if err != nil { h.flashes.AddError(rw, req, err) - http.Redirect(rw, req, redirect, http.StatusSeeOther) return } h.flashes.AddMessage(rw, req, "NoticeUpdated") - http.Redirect(rw, req, redirect, http.StatusSeeOther) } diff --git a/web/handlers/admin/notices_test.go b/web/handlers/admin/notices_test.go index 1ca75a1..926c7fa 100644 --- a/web/handlers/admin/notices_test.go +++ b/web/handlers/admin/notices_test.go @@ -50,7 +50,7 @@ func TestNoticeSaveRefusesIncomplete(t *testing.T) { loc := resp.Header().Get("Location") noticesList := ts.URLTo(router.CompleteNoticeList) - a.Equal(noticesList.Path, loc) + a.Equal(noticesList.String(), loc) // we should get noticesList here but // due to issue #35 we cant get /notices/list in package admin tests @@ -99,7 +99,8 @@ func TestNoticeAddLanguageOnlyAllowsPost(t *testing.T) { formValues := url.Values{"name": []string{roomdb.NoticeNews.String()}, "id": id, "title": title, "content": content, "language": language} resp = ts.Client.PostForm(u, formValues) - a.Equal(http.StatusTemporaryRedirect, resp.Code) + a.Equal(http.StatusSeeOther, resp.Code) + webassert.HasFlashMessages(t, ts.Client, ts.URLTo(router.AdminDashboard), "NoticeUpdated") } // Verifies that the "add a translation" page contains all the required form fields (id/title/content/language) @@ -158,3 +159,193 @@ func TestNoticeEditFormIncludesAllFields(t *testing.T) { {Tag: "textarea", Name: "content"}, }) } + +func TestNoticesRoleRightsEditing(t *testing.T) { + ts := newSession(t) + + dashboardURL := ts.URLTo(router.AdminDashboard) + editURL := ts.URLTo(router.AdminNoticeEdit, "id", 1) + saveURL := ts.URLTo(router.AdminNoticeSave) + + formValues := url.Values{ + "id": []string{"1"}, + "title": []string{"SSB Breaking News: This Test Is Great"}, + "content": []string{"Absolutely Thrilling Content"}, + "language": []string{"en-GB"}, + } + + canSeeEditForm := func(t *testing.T, shouldWork bool) { + a := assert.New(t) + + doc, resp := ts.Client.GetHTML(editURL) + + if shouldWork { + a.Equal(http.StatusOK, resp.Code, "unexpected status code") + form := doc.Find("form") + action, has := form.Attr("action") + a.True(has, "no action on a form?!") + a.Equal(saveURL.String(), action) + } else { + a.Equal(http.StatusSeeOther, resp.Code, "unexpected status code") + webassert.HasFlashMessages(t, ts.Client, dashboardURL, "ErrorNotAuthorized") + } + } + + totalSaveCallCount := 0 + canSaveNotice := func(t *testing.T, shouldWork bool) { + a := assert.New(t) + + // POST a correct request to the save handler, and verify that the save was handled using the mock database) + resp := ts.Client.PostForm(saveURL, formValues) + a.Equal(http.StatusSeeOther, resp.Code, "should have redirected") + + var wantLabel string + if shouldWork { + totalSaveCallCount++ + wantLabel = "NoticeUpdated" + } else { + wantLabel = "ErrorNotAuthorized" + } + webassert.HasFlashMessages(t, ts.Client, dashboardURL, wantLabel) + a.Equal(totalSaveCallCount, ts.NoticeDB.SaveCallCount(), "call count missmatch") + } + + memberUser := roomdb.Member{ + ID: 7331, + Role: roomdb.RoleMember, + PubKey: generatePubKey(), + } + modUser := roomdb.Member{ + ID: 9001, + Role: roomdb.RoleModerator, + PubKey: generatePubKey(), + } + adminUser := roomdb.Member{ + ID: 1337, + Role: roomdb.RoleAdmin, + PubKey: generatePubKey(), + } + + /* test invite creation under various restricted mode with the roles member, mod, admin */ + for _, mode := range roomdb.AllPrivacyModes { + t.Run(mode.String(), func(t *testing.T) { + ts.ConfigDB.GetPrivacyModeReturns(mode, nil) + + // members can only invite in community rooms + t.Run("members", func(t *testing.T) { + ts.User = memberUser + canSeeEditForm(t, mode == roomdb.ModeCommunity) + canSaveNotice(t, mode == roomdb.ModeCommunity) + }) + + // mods & admins can always invite + t.Run("mods", func(t *testing.T) { + ts.User = modUser + canSeeEditForm(t, true) + canSaveNotice(t, true) + }) + + t.Run("admins", func(t *testing.T) { + ts.User = adminUser + canSeeEditForm(t, true) + canSaveNotice(t, true) + }) + }) + } +} + +func TestNoticesRoleRightsAddingTranslation(t *testing.T) { + ts := newSession(t) + + dashboardURL := ts.URLTo(router.AdminDashboard) + draftTrURL := ts.URLTo(router.AdminNoticeDraftTranslation, "name", "NoticeNews") + addTrURL := ts.URLTo(router.AdminNoticeAddTranslation) + + formValues := url.Values{ + "id": []string{"1"}, + "title": []string{"SSB Breaking News: This Test Is Great"}, + "content": []string{"Absolutely Thrilling Content"}, + "language": []string{"en-GB"}, + + "name": []string{"NoticeNews"}, + } + + canSeeAddTranslationForm := func(t *testing.T, shouldWork bool) { + a := assert.New(t) + + doc, resp := ts.Client.GetHTML(draftTrURL) + + if shouldWork { + a.Equal(http.StatusOK, resp.Code, "unexpected status code") + form := doc.Find("form") + action, has := form.Attr("action") + a.True(has, "no action on a form?!") + a.Equal(addTrURL.String(), action) + } else { + a.Equal(http.StatusSeeOther, resp.Code, "unexpected status code") + webassert.HasFlashMessages(t, ts.Client, dashboardURL, "ErrorNotAuthorized") + } + } + + totalAddCallCount := 0 + canAddNewTranslation := func(t *testing.T, shouldWork bool) { + a := assert.New(t) + + // POST a correct request to the save handler, and verify that the save was handled using the mock database) + resp := ts.Client.PostForm(addTrURL, formValues) + a.Equal(http.StatusSeeOther, resp.Code, "should have redirected") + + var wantLabel string + if shouldWork { + totalAddCallCount++ + wantLabel = "NoticeUpdated" + } else { + wantLabel = "ErrorNotAuthorized" + } + webassert.HasFlashMessages(t, ts.Client, dashboardURL, wantLabel) + a.Equal(totalAddCallCount, ts.PinnedDB.SetCallCount(), "call count missmatch") + } + + memberUser := roomdb.Member{ + ID: 7331, + Role: roomdb.RoleMember, + PubKey: generatePubKey(), + } + modUser := roomdb.Member{ + ID: 9001, + Role: roomdb.RoleModerator, + PubKey: generatePubKey(), + } + adminUser := roomdb.Member{ + ID: 1337, + Role: roomdb.RoleAdmin, + PubKey: generatePubKey(), + } + + /* test invite creation under various restricted mode with the roles member, mod, admin */ + for _, mode := range roomdb.AllPrivacyModes { + t.Run(mode.String(), func(t *testing.T) { + ts.ConfigDB.GetPrivacyModeReturns(mode, nil) + + // members can only invite in community rooms + t.Run("members", func(t *testing.T) { + ts.User = memberUser + canSeeAddTranslationForm(t, mode == roomdb.ModeCommunity) + canAddNewTranslation(t, mode == roomdb.ModeCommunity) + }) + + // mods & admins can always invite + t.Run("mods", func(t *testing.T) { + ts.User = modUser + canSeeAddTranslationForm(t, true) + canAddNewTranslation(t, true) + }) + + t.Run("admins", func(t *testing.T) { + ts.User = adminUser + canSeeAddTranslationForm(t, true) + canAddNewTranslation(t, true) + }) + }) + } +} diff --git a/web/handlers/admin/setup_test.go b/web/handlers/admin/setup_test.go index 017f91b..b282acc 100644 --- a/web/handlers/admin/setup_test.go +++ b/web/handlers/admin/setup_test.go @@ -6,6 +6,7 @@ import ( "bytes" "context" "crypto/rand" + "fmt" "net/http" "net/url" "os" @@ -145,11 +146,18 @@ func newSession(t *testing.T) *testSession { testFuncs["list_languages"] = func(*url.URL, string) string { return "" } testFuncs["member_is_elevated"] = func() bool { return ts.User.Role == roomdb.RoleAdmin || ts.User.Role == roomdb.RoleModerator } testFuncs["member_is_admin"] = func() bool { return ts.User.Role == roomdb.RoleAdmin } - testFuncs["member_can_invite"] = func() bool { - pm, _ := ts.ConfigDB.GetPrivacyMode(context.TODO()) - memberElevated := ts.User.Role == roomdb.RoleAdmin || ts.User.Role == roomdb.RoleModerator - memberCanInvite := ts.User.Role == roomdb.RoleMember && (pm == roomdb.ModeCommunity || pm == roomdb.ModeOpen) - return memberElevated || memberCanInvite + testFuncs["member_can"] = func(what string) (bool, error) { + actionCheck, has := members.AllowedActions(what) + if !has { + return false, fmt.Errorf("unrecognized action: %s", what) + } + + pm, err := ts.ConfigDB.GetPrivacyMode(context.TODO()) + if err != nil { + return false, err + } + + return actionCheck(pm, ts.User.Role), nil } testFuncs["list_languages"] = func(*url.URL, string) string { return "" } testFuncs["relative_time"] = func(when time.Time) string { return humanize.Time(when) } diff --git a/web/handlers/http.go b/web/handlers/http.go index 6bea756..2581ca8 100644 --- a/web/handlers/http.go +++ b/web/handlers/http.go @@ -124,31 +124,6 @@ func New( } }), - render.InjectTemplateFunc("member_can_invite", func(r *http.Request) interface{} { - return func() (bool, error) { - member := members.FromContext(r.Context()) - if member == nil { - return false, nil - } - - pm, err := dbs.Config.GetPrivacyMode(r.Context()) - if err != nil { - return false, err - } - - switch pm { - case roomdb.ModeOpen: - return true, nil - case roomdb.ModeCommunity: - return member.Role > roomdb.RoleUnknown && member.Role <= roomdb.RoleAdmin, nil - case roomdb.ModeRestricted: - return member.Role == roomdb.RoleAdmin || member.Role == roomdb.RoleModerator, nil - default: - return false, nil - } - } - }), - render.InjectTemplateFunc("language_count", func(r *http.Request) interface{} { return func() int { return len(locHelper.ListLanguages()) @@ -195,7 +170,7 @@ func New( } renderOpts = append(renderOpts, locHelper.GetRenderFuncs()...) - renderOpts = append(renderOpts, members.TemplateHelpers()...) + renderOpts = append(renderOpts, members.TemplateHelpers(dbs.Config)...) r, err := render.New(web.Templates, renderOpts...) if err != nil { diff --git a/web/handlers/notices_test.go b/web/handlers/notices_test.go index 9296603..ff2a5a9 100644 --- a/web/handlers/notices_test.go +++ b/web/handlers/notices_test.go @@ -133,11 +133,11 @@ func TestNoticesEditButtonVisible(t *testing.T) { a.EqualValues(1, doc.Find(editButtonSelector).Length()) } -func TestNoticesCreateOnlyModsAndHigher(t *testing.T) { +func TestNoticesCreateOnlyModsAndHigherInRestricted(t *testing.T) { ts := setup(t) a := assert.New(t) - ts.ConfigDB.GetPrivacyModeReturns(roomdb.ModeCommunity, nil) + ts.ConfigDB.GetPrivacyModeReturns(roomdb.ModeRestricted, nil) // first, we confirm that we can't access the page when not logged in draftNotice := ts.URLTo(router.AdminNoticeDraftTranslation, "name", roomdb.NoticeNews) @@ -194,11 +194,11 @@ func TestNoticesCreateOnlyModsAndHigher(t *testing.T) { doc, resp = ts.Client.GetHTML(draftNotice) a.Equal(http.StatusSeeOther, resp.Code) - dashboardURL := ts.URLTo(router.AdminDashboard) - a.Equal(dashboardURL.Path, resp.Header().Get("Location")) + noticeListURL := ts.URLTo(router.CompleteNoticeList) + a.Equal(noticeListURL.String(), resp.Header().Get("Location")) a.True(len(resp.Result().Cookies()) > 0, "got a cookie") - webassert.HasFlashMessages(t, ts.Client, dashboardURL, "ErrorNotAuthorized") + webassert.HasFlashMessages(t, ts.Client, noticeListURL, "ErrorNotAuthorized") // also shouldnt be allowed to save/post id := []string{"1"} @@ -213,9 +213,9 @@ func TestNoticesCreateOnlyModsAndHigher(t *testing.T) { a.Equal(http.StatusSeeOther, resp.Code, "POST should work") a.Equal(0, ts.NoticeDB.SaveCallCount(), "noticedb should not save the notice") - a.Equal(dashboardURL.Path, resp.Header().Get("Location")) + a.Equal(noticeListURL.String(), resp.Header().Get("Location")) a.True(len(resp.Result().Cookies()) > 0, "got a cookie") - webassert.HasFlashMessages(t, ts.Client, dashboardURL, "ErrorNotAuthorized") + webassert.HasFlashMessages(t, ts.Client, noticeListURL, "ErrorNotAuthorized") } diff --git a/web/i18n/defaults/active.de.toml b/web/i18n/defaults/active.de.toml index 98514a6..3845adf 100644 --- a/web/i18n/defaults/active.de.toml +++ b/web/i18n/defaults/active.de.toml @@ -150,6 +150,8 @@ AdminInvitesCreatorColumn = "Erstellt von" AdminInvitesActionColumn = "Aktion" AdminInviteRevoke = "Widerrufen" +InviteRevoked = "Einladug wurde Widerrufen." + AdminInviteRevokeConfirmTitle = "Widerruf der Einladung bestätigen" AdminInviteRevokeConfirmWelcome = "Sind Sie sicher, dass Sie diese Einladung entfernen möchten? Wenn Sie sie bereits gesendet haben, können sie sie nicht verwenden." diff --git a/web/i18n/defaults/active.en.toml b/web/i18n/defaults/active.en.toml index 297eeb7..8a1cfe0 100644 --- a/web/i18n/defaults/active.en.toml +++ b/web/i18n/defaults/active.en.toml @@ -157,6 +157,8 @@ AdminInvitesCreatorColumn = "Created by" AdminInvitesActionColumn = "Action" AdminInviteRevoke = "Revoke" +InviteRevoked = "Invite Revoked." + AdminInviteRevokeConfirmTitle = "Confirm invite revocation" AdminInviteRevokeConfirmWelcome = "Are you sure you want to remove this invite? If you already sent it out, they will not be able to use it." diff --git a/web/members/helper.go b/web/members/helper.go index 42542ed..e15d817 100644 --- a/web/members/helper.go +++ b/web/members/helper.go @@ -5,6 +5,7 @@ package members import ( "context" + "fmt" "net/http" "go.mindeco.de/http/auth" @@ -101,7 +102,9 @@ func ContextInjecter(mdb roomdb.MembersService, withPassword *auth.Handler, with // {{ member_is_admin }} is a shortcut for {{ member_has_role "RoleAdmin" }} // // {{ member_is_elevated }} is a shortcut for {{ or member_has_role "RoleAdmin" member_has_role "RoleModerator"}} -func TemplateHelpers() []render.Option { +// +// {{ member_can "action" }} returns true if a member can execute a certain action. Actions are "invite" and "remove-denied-key". See allowedActions to add more. +func TemplateHelpers(roomCfg roomdb.RoomConfig) []render.Option { return []render.Option{ render.InjectTemplateFunc("is_logged_in", func(r *http.Request) interface{} { @@ -159,5 +162,116 @@ func TemplateHelpers() []render.Option { return member.Role == roomdb.RoleAdmin || member.Role == roomdb.RoleModerator } }), + + render.InjectTemplateFunc("member_can", func(r *http.Request) interface{} { + // evaluate member and privacy mode first to reduce some churn for multiple calls to this helper + // works fine since they are not changing during one request + member := FromContext(r.Context()) + if member == nil { + return func(_ string) bool { return false } + } + + pm, err := roomCfg.GetPrivacyMode(r.Context()) + if err != nil { + return func(_ string) (bool, error) { return false, err } + } + + // now return the template func which closes over pm and the member + return func(what string) (bool, error) { + actionCheck, has := allowedActionsMap[what] + if !has { + return false, fmt.Errorf("unrecognized action: %s", what) + } + + return actionCheck(pm, member.Role), nil + } + }), } } + +// AllowedFunc returns true if a member role is allowed to do a thing under the passed mode +type AllowedFunc func(mode roomdb.PrivacyMode, role roomdb.Role) bool + +// AllowedActions exposes check function by name. It exists to protected against changes of the map +func AllowedActions(name string) (AllowedFunc, bool) { + fn, has := allowedActionsMap[name] + return fn, has +} + +// member actions +const ( + ActionInviteMember = "invite" + ActionChangeDeniedKeys = "change-denied-keys" + ActionRemoveMember = "remove-member" + ActionChangeNotice = "change-notice" +) + +var allowedActionsMap = map[string]AllowedFunc{ + ActionInviteMember: func(pm roomdb.PrivacyMode, role roomdb.Role) bool { + switch pm { + case roomdb.ModeOpen: + return true + case roomdb.ModeCommunity: + return role > roomdb.RoleUnknown && role <= roomdb.RoleAdmin + case roomdb.ModeRestricted: + return role == roomdb.RoleAdmin || role == roomdb.RoleModerator + default: + return false + } + }, + + ActionChangeDeniedKeys: func(pm roomdb.PrivacyMode, role roomdb.Role) bool { + switch pm { + case roomdb.ModeCommunity: + return true + case roomdb.ModeOpen: + fallthrough + case roomdb.ModeRestricted: + return role == roomdb.RoleAdmin || role == roomdb.RoleModerator + default: + return false + } + }, + + ActionRemoveMember: func(_ roomdb.PrivacyMode, role roomdb.Role) bool { + return role == roomdb.RoleAdmin || role == roomdb.RoleModerator + }, + + ActionChangeNotice: func(pm roomdb.PrivacyMode, role roomdb.Role) bool { + switch pm { + case roomdb.ModeCommunity: + return true + case roomdb.ModeOpen: + fallthrough + case roomdb.ModeRestricted: + return role == roomdb.RoleAdmin || role == roomdb.RoleModerator + default: + return false + } + }, +} + +// CheckAllowed retreives the member from the passed context and lookups the current privacy mode from the passed cfg to determain if the action is okay or not. +// If it's not it returns an error. For convenience it also returns the member if the action is okay. +func CheckAllowed(ctx context.Context, cfg roomdb.RoomConfig, action string) (*roomdb.Member, error) { + member := FromContext(ctx) + if member == nil { + return nil, weberrors.ErrNotAuthorized + } + + pm, err := cfg.GetPrivacyMode(ctx) + if err != nil { + return nil, err + } + + allowed, ok := AllowedActions(action) + if !ok { + return nil, fmt.Errorf("unknown action: %s: %w", action, weberrors.ErrNotAuthorized) + } + + if !allowed(pm, member.Role) { + return nil, weberrors.ErrNotAuthorized + } + + return member, nil +} diff --git a/web/templates/admin/denied-keys.tmpl b/web/templates/admin/denied-keys.tmpl index c082bfc..032e2e1 100644 --- a/web/templates/admin/denied-keys.tmpl +++ b/web/templates/admin/denied-keys.tmpl @@ -22,31 +22,31 @@ {{ .csrfField }}
@@ -62,8 +62,8 @@ >{{.Comment}} {{i18n "AdminDeniedKeysRemove"}} {{end}} diff --git a/web/templates/admin/invite-list.tmpl b/web/templates/admin/invite-list.tmpl index 7f60984..c5e4cba 100644 --- a/web/templates/admin/invite-list.tmpl +++ b/web/templates/admin/invite-list.tmpl @@ -28,10 +28,10 @@ > {{ .csrfField }}