diff --git a/frontend/src/store/modules/inbox.ts b/frontend/src/store/modules/inbox.ts index b91d95fe74ed4e..d514e7b1d43645 100644 --- a/frontend/src/store/modules/inbox.ts +++ b/frontend/src/store/modules/inbox.ts @@ -67,7 +67,7 @@ const actions = { readCreatedAfterTs, }: { userId: PrincipalId; readCreatedAfterTs?: number } ) { - let url = `/api/inbox?user=${userId}`; + let url = `/api/inbox/user/${userId}`; if (readCreatedAfterTs) { url += `&created=${readCreatedAfterTs}`; } @@ -81,7 +81,7 @@ const actions = { }, async fetchInboxSummaryByUser({ commit }: any, userId: PrincipalId) { - const inboxSummary = (await axios.get(`/api/inbox/summary?user=${userId}`)) + const inboxSummary = (await axios.get(`/api/inbox/user/${userId}/summary`)) .data; commit("setInboxSummaryByUser", { userId, inboxSummary }); diff --git a/server/acl.go b/server/acl.go index fcc7f631a26cf4..0f64500e89ade8 100644 --- a/server/acl.go +++ b/server/acl.go @@ -58,10 +58,10 @@ func aclMiddleware(l *zap.Logger, s *Server, ce *casbin.Enforcer, next echo.Hand return echo.NewHTTPError(http.StatusUnauthorized, "This user has been deactivated by the admin") } - // If the requests is trying to PATCH/DELETE herself, we will change the method signature to + // If the request is trying to GET/PATCH/DELETE itself, we will change the method signature to // XXX_SELF so that the policy can differentiate between XXX and XXX_SELF - if method == "PATCH" || method == "DELETE" { - if isSelf, err := isUpdatingSelf(ctx, c, s, principalID); err != nil { + if method == "GET" || method == "PATCH" || method == "DELETE" { + if isSelf, err := isOperatingSelf(ctx, c, s, principalID, method); err != nil { return err } else if isSelf { method = method + "_SELF" @@ -94,6 +94,29 @@ func aclMiddleware(l *zap.Logger, s *Server, ce *casbin.Enforcer, next echo.Hand } } +func isOperatingSelf(ctx context.Context, c echo.Context, s *Server, curPrincipalID int, method string) (bool, error) { + if method == "GET" { + return isGettingSelf(ctx, c, s, curPrincipalID) + } else if method == "PATCH" || method == "DELETE" { + return isUpdatingSelf(ctx, c, s, curPrincipalID) + } + + return false, nil +} + +func isGettingSelf(ctx context.Context, c echo.Context, s *Server, curPrincipalID int) (bool, error) { + if strings.HasPrefix(c.Path(), "/api/inbox/user") { + userID, err := strconv.Atoi(c.Param("userId")) + if err != nil { + return false, echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("User ID is not a number: %s", c.Param("userId"))).SetInternal(err) + } + + return userID == curPrincipalID, nil + } + + return false, nil +} + func isUpdatingSelf(ctx context.Context, c echo.Context, s *Server, curPrincipalID int) (bool, error) { const defaultErrMsg = "Failed to process authorize request." if strings.HasPrefix(c.Path(), "/api/principal") { @@ -105,10 +128,9 @@ func isUpdatingSelf(ctx context.Context, c echo.Context, s *Server, curPrincipal if activityIDStr := c.Param("activityID"); activityIDStr != "" { activityID, err := strconv.Atoi(activityIDStr) if err != nil { - msg := "Activity ID is not a number: " + activityIDStr - httpErr := echo.NewHTTPError(http.StatusBadRequest, msg) - return false, httpErr + return false, echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Activity ID is not a number: %s"+activityIDStr)).SetInternal(err) } + activityFind := &api.ActivityFind{ ID: &activityID, } @@ -119,16 +141,16 @@ func isUpdatingSelf(ctx context.Context, c echo.Context, s *Server, curPrincipal if activity == nil { return false, echo.NewHTTPError(http.StatusNotFound, fmt.Sprintf("Activity ID not found: %d", activityID)) } + return activity.CreatorID == curPrincipalID, nil } } else if strings.HasPrefix(c.Path(), "/api/bookmark") { if bookmarkIDStr := c.Param("bookmarkID"); bookmarkIDStr != "" { bookmarkID, err := strconv.Atoi(bookmarkIDStr) if err != nil { - msg := "Bookmark ID is not a number: " + bookmarkIDStr - httpErr := echo.NewHTTPError(http.StatusBadRequest, msg) - return false, httpErr + return false, echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Bookmark ID is not a number: %s"+bookmarkIDStr)).SetInternal(err) } + bookmarkFind := &api.BookmarkFind{ ID: &bookmarkID, } @@ -137,18 +159,18 @@ func isUpdatingSelf(ctx context.Context, c echo.Context, s *Server, curPrincipal return false, echo.NewHTTPError(http.StatusInternalServerError, defaultErrMsg).SetInternal(err) } if bookmark == nil { - return false, echo.NewHTTPError(http.StatusNotFound, fmt.Sprintf("Bookmark ID not found: %d", bookmarkID)) + return false, echo.NewHTTPError(http.StatusNotFound, fmt.Sprintf("Bookmark ID not found: %d", bookmarkID)).SetInternal(err) } + return bookmark.CreatorID == curPrincipalID, nil } } else if strings.HasPrefix(c.Path(), "/api/inbox") { if inboxIDStr := c.Param("inboxID"); inboxIDStr != "" { inboxID, err := strconv.Atoi(inboxIDStr) if err != nil { - msg := "Inbox ID is not a number: " + inboxIDStr - httpErr := echo.NewHTTPError(http.StatusBadRequest, msg) - return false, httpErr + return false, echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Inbox ID is not a number: %s", inboxIDStr)).SetInternal(err) } + inboxFind := &api.InboxFind{ ID: &inboxID, } @@ -157,16 +179,18 @@ func isUpdatingSelf(ctx context.Context, c echo.Context, s *Server, curPrincipal return false, echo.NewHTTPError(http.StatusInternalServerError, defaultErrMsg).SetInternal(err) } if inbox == nil { - return false, echo.NewHTTPError(http.StatusNotFound, fmt.Sprintf("Inbox ID not found: %d", inboxID)) + return false, echo.NewHTTPError(http.StatusNotFound, fmt.Sprintf("Inbox ID not found: %d", inboxID)).SetInternal(err) } + return inbox.ReceiverID == curPrincipalID, nil } } else if strings.HasPrefix(c.Path(), "/api/savedquery") { if idStr := c.Param("id"); idStr != "" { id, err := strconv.Atoi(idStr) if err != nil { - return false, echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Saved query ID is not a number: %s", idStr)) + return false, echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Saved query ID is not a number: %s", idStr)).SetInternal(err) } + savedQueryFind := &api.SavedQueryFind{ ID: &id, } @@ -175,16 +199,18 @@ func isUpdatingSelf(ctx context.Context, c echo.Context, s *Server, curPrincipal return false, echo.NewHTTPError(http.StatusInternalServerError, defaultErrMsg).SetInternal(err) } if savedQuery == nil { - return false, echo.NewHTTPError(http.StatusNotFound, fmt.Sprintf("Saved query ID not found: %d", id)) + return false, echo.NewHTTPError(http.StatusNotFound, fmt.Sprintf("Saved query ID not found: %d", id)).SetInternal(err) } + return savedQuery.CreatorID == curPrincipalID, nil } } else if strings.HasPrefix(c.Path(), "/api/sheet") { if idStr := c.Param("id"); idStr != "" { id, err := strconv.Atoi(idStr) if err != nil { - return false, echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Sheet ID is not a number: %s", idStr)) + return false, echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Sheet ID is not a number: %s", idStr)).SetInternal(err) } + sheetFind := &api.SheetFind{ ID: &id, } @@ -193,10 +219,12 @@ func isUpdatingSelf(ctx context.Context, c echo.Context, s *Server, curPrincipal return false, echo.NewHTTPError(http.StatusInternalServerError, defaultErrMsg).SetInternal(err) } if sheet == nil { - return false, echo.NewHTTPError(http.StatusNotFound, fmt.Sprintf("Sheet ID not found: %d", id)) + return false, echo.NewHTTPError(http.StatusNotFound, fmt.Sprintf("Sheet ID not found: %d", id)).SetInternal(err) } + return sheet.CreatorID == curPrincipalID, nil } } + return false, nil } diff --git a/server/acl_casbin_policy_dba.csv b/server/acl_casbin_policy_dba.csv index 9f13215b8702d0..0a9d96a57a2da7 100644 --- a/server/acl_casbin_policy_dba.csv +++ b/server/acl_casbin_policy_dba.csv @@ -57,8 +57,8 @@ p, DBA, /activity, POST p, DBA, /activity, GET p, DBA, /activity/{id}, PATCH_SELF p, DBA, /activity/{id}, DELETE_SELF -p, DBA, /inbox, GET -p, DBA, /inbox/summary, GET +p, DBA, /inbox/user/{userID}, GET_SELF +p, DBA, /inbox/user/{userID}/summary, GET_SELF p, DBA, /inbox/{id}, PATCH_SELF p, DBA, /bookmark, POST p, DBA, /bookmark, GET @@ -91,4 +91,4 @@ p, DBA, /sheet, POST p, DBA, /sheet, GET p, DBA, /sheet/{id}, GET p, DBA, /sheet/{id}, PATCH -p, DBA, /sheet/{id}, DELETE_SELF \ No newline at end of file +p, DBA, /sheet/{id}, DELETE_SELF diff --git a/server/acl_casbin_policy_developer.csv b/server/acl_casbin_policy_developer.csv index 36ce000c21481e..315b5fdee80e34 100644 --- a/server/acl_casbin_policy_developer.csv +++ b/server/acl_casbin_policy_developer.csv @@ -53,8 +53,8 @@ p, DEVELOPER, /activity, POST p, DEVELOPER, /activity, GET p, DEVELOPER, /activity/{id}, PATCH_SELF p, DEVELOPER, /activity/{id}, DELETE_SELF -p, DEVELOPER, /inbox, GET -p, DEVELOPER, /inbox/summary, GET +p, DEVELOPER, /inbox/user/{userID}, GET_SELF +p, DEVELOPER, /inbox/user/{userID}/summary, GET_SELF p, DEVELOPER, /inbox/{id}, PATCH_SELF p, DEVELOPER, /bookmark, POST p, DEVELOPER, /bookmark, GET @@ -80,4 +80,4 @@ p, DEVELOPER, /sheet, POST p, DEVELOPER, /sheet, GET p, DEVELOPER, /sheet/{id}, GET p, DEVELOPER, /sheet/{id}, PATCH -p, DEVELOPER, /sheet/{id}, DELETE_SELF \ No newline at end of file +p, DEVELOPER, /sheet/{id}, DELETE_SELF diff --git a/server/acl_casbin_policy_owner.csv b/server/acl_casbin_policy_owner.csv index fa96c6b92b21b3..80521200ec539a 100644 --- a/server/acl_casbin_policy_owner.csv +++ b/server/acl_casbin_policy_owner.csv @@ -62,8 +62,8 @@ p, OWNER, /activity, POST p, OWNER, /activity, GET p, OWNER, /activity/{id}, PATCH_SELF p, OWNER, /activity/{id}, DELETE_SELF -p, OWNER, /inbox, GET -p, OWNER, /inbox/summary, GET +p, OWNER, /inbox/user/{userID}, GET_SELF +p, OWNER, /inbox/user/{userID}/summary, GET_SELF p, OWNER, /inbox/{id}, PATCH_SELF p, OWNER, /bookmark, POST p, OWNER, /bookmark, GET @@ -97,4 +97,4 @@ p, OWNER, /sheet, POST p, OWNER, /sheet, GET p, OWNER, /sheet/{id}, GET p, OWNER, /sheet/{id}, PATCH -p, OWNER, /sheet/{id}, DELETE_SELF \ No newline at end of file +p, OWNER, /sheet/{id}, DELETE_SELF diff --git a/server/inbox.go b/server/inbox.go index e9cfd983b24b68..b456885f0bc036 100644 --- a/server/inbox.go +++ b/server/inbox.go @@ -13,17 +13,17 @@ import ( ) func (s *Server) registerInboxRoutes(g *echo.Group) { - g.GET("/inbox", func(c echo.Context) error { + g.GET("/inbox/user/:userId", func(c echo.Context) error { ctx := context.Background() - inboxFind := &api.InboxFind{} - userIDStr := c.QueryParams().Get("user") - if userIDStr != "" { - userID, err := strconv.Atoi(userIDStr) - if err != nil { - return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Query parameter user is not a number: %s", userIDStr)).SetInternal(err) - } - inboxFind.ReceiverID = &userID + userID, err := strconv.Atoi(c.Param("userId")) + if err != nil { + return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("User ID is not a number: %s", c.Param("userId"))).SetInternal(err) } + + inboxFind := &api.InboxFind{ + ReceiverID: &userID, + } + createdAfterStr := c.QueryParams().Get("created") if createdAfterStr != "" { createdTs, err := strconv.ParseInt(createdAfterStr, 10, 64) @@ -50,15 +50,11 @@ func (s *Server) registerInboxRoutes(g *echo.Group) { return nil }) - g.GET("/inbox/summary", func(c echo.Context) error { + g.GET("/inbox/user/:userId/summary", func(c echo.Context) error { ctx := context.Background() - userIDStr := c.QueryParams().Get("user") - if userIDStr == "" { - return echo.NewHTTPError(http.StatusBadRequest, "Missing query parameter user") - } - userID, err := strconv.Atoi(userIDStr) + userID, err := strconv.Atoi(c.Param("userId")) if err != nil { - return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Query parameter user is not a number: %s", userIDStr)).SetInternal(err) + return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("User ID is not a number: %s", c.Param("userId"))).SetInternal(err) } summary, err := s.InboxService.FindInboxSummary(ctx, userID)