Feature read restriction #41
Conversation
There was a problem hiding this comment.
Looks good overall! I made the following changes: e93254e and provided some inline comments. Please also:
- Update the example env with the new AUTH_LIMITED_READER_ROLE variable
- Update postman or add an issue to do so, since the check_user_permission endpoint is missing
- Double check error messages to make sure you are being consistent with calling things objects verses keys.
| @@ -323,6 +323,14 @@ func (bh *BlobHandler) HandleCheckS3UserPermission(c echo.Context) error { | |||
| log.Info("Checked user permissions successfully") | |||
| return c.JSON(http.StatusOK, true) | |||
| } | |||
| initAuth := os.Getenv("INIT_AUTH") | |||
|
|
|||
| if initAuth == "0" { | |||
There was a problem hiding this comment.
I am not sure how this would ever be true? If initAuth == "0" then line 324 would have already returned
There was a problem hiding this comment.
This is the new init auth that will turn on and off auth in general, not the auth level which indicates whether we want FGAC or not. This is because when auth is turned off we can't get the user email from the claims which would break this endpoint.
I've just added checks to two other functions that are used in endpoints that require the claims.
blobstore/list.go
Outdated
| errMsg := fmt.Errorf("request must include a `delimiter`, options are `true` or `false`") | ||
| log.Error(errMsg.Error()) | ||
| return c.JSON(http.StatusUnprocessableEntity, errMsg.Error()) | ||
|
|
||
| } | ||
| if delimiter && !strings.HasSuffix(prefix, "/") { |
There was a problem hiding this comment.
Why dont you also need to do this in the detailed version of this function?
There was a problem hiding this comment.
It was because of the root directory check but I just added a tertiary check to see if the prefix is empty we don't add a '/' and added it for both of them. Thanks!
| @@ -76,7 +76,8 @@ func (bh *BlobHandler) HandleListByPrefix(c echo.Context) error { | |||
| } | |||
|
|
|||
| } | |||
| if delimiter && !strings.HasSuffix(prefix, "/") { | |||
|
|
|||
| if delimiter && prefix != "" && !strings.HasSuffix(prefix, "/") { | |||
There was a problem hiding this comment.
Shouldnt this be after CheckAndAdjustPrefix like in the HandleListByPrefixWithDetails? It doesnt make sense that they are in two different places within the function
Add read restriction to endpoints that can be accessed by
allUsers.