Skip to content

Commit

Permalink
feat: add GET_SELF handler in acl.go (#548)
Browse files Browse the repository at this point in the history 
* feat: add get self handler in acl

* fix: userid convertor format
  • Loading branch information
@boojack
boojack committed Feb 6, 2022
1 parent f6d0e92 commit 4a92f46
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 45 deletions.
4 changes: 2 additions & 2 deletions frontend/src/store/modules/inbox.ts
View file
Expand Up @@ -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}`;
}
Expand All @@ -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 });
Expand Down
64 changes: 46 additions & 18 deletions server/acl.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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") {
Expand All @@ -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,
}
Expand All @@ -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,
}
Expand All @@ -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,
}
Expand All @@ -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,
}
Expand All @@ -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,
}
Expand All @@ -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
}
6 changes: 3 additions & 3 deletions server/acl_casbin_policy_dba.csv
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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
p, DBA, /sheet/{id}, DELETE_SELF
6 changes: 3 additions & 3 deletions server/acl_casbin_policy_developer.csv
View file
Expand Up @@ -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
Expand All @@ -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
p, DEVELOPER, /sheet/{id}, DELETE_SELF
6 changes: 3 additions & 3 deletions server/acl_casbin_policy_owner.csv
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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
p, OWNER, /sheet/{id}, DELETE_SELF
28 changes: 12 additions & 16 deletions server/inbox.go
View file
Expand Up @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit 4a92f46

Please sign in to comment.