Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature read restriction #41

Merged
merged 25 commits into from
Jun 17, 2024
Merged

Feature read restriction #41

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
7124daf
comment out deprecated folder presigned download
Akopti8 May 15, 2024
dd01954
add new checks that will retrieve accesible prefixes
Akopti8 May 15, 2024
28456a9
add restriction on read list
Akopti8 May 15, 2024
5b4598f
added bucket limitation
Akopti8 May 16, 2024
42a5e14
add limitation for list with no detail
Akopti8 May 16, 2024
392c55c
limit read access to endpoints with allUsers
Akopti8 May 16, 2024
3626fbf
remove deprecated prefix test
Akopti8 May 16, 2024
df90a3a
Merge remote-tracking branch 'origin/main' into feature-read-restriction
Akopti8 Jun 12, 2024
9b21d30
clean up and refactor permissions logic
Akopti8 Jun 13, 2024
07156c2
change isPermittedPrefix to a public function
Akopti8 Jun 13, 2024
9ef1910
readd counter
Akopti8 Jun 13, 2024
e93254e
Improve wording of error message
ShaneMPutnam Jun 14, 2024
4d29a62
add AUTH_LIMITED_READER_ROLE to .example.env
Akopti8 Jun 14, 2024
eb7da81
disable check permisisons endpoint when auth is off
Akopti8 Jun 14, 2024
4303196
update postman
Akopti8 Jun 14, 2024
1de00ef
add delimiter option for list with detail
Akopti8 Jun 14, 2024
5e62841
make list buckets more efficient with FGAC
Akopti8 Jun 14, 2024
b83d4c4
consistent delimiter retrieval
Akopti8 Jun 14, 2024
148624a
update e2e for 422 checks
Akopti8 Jun 14, 2024
64dfca8
remove bucket from e2e for failure testing
Akopti8 Jun 15, 2024
2c6cda0
add checks for auth level to ensure claims exist
Akopti8 Jun 17, 2024
0dd9b82
add trailing / check to the detailed list endpoint
Akopti8 Jun 17, 2024
f4725ac
Merge remote-tracking branch 'origin/feature-read-restriction' into f…
Akopti8 Jun 17, 2024
fc43cf0
move adjusted prefix before the delimiter check
Akopti8 Jun 17, 2024
dc2feb9
Move variable inside if statement
ShaneMPutnam Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions blobstore/blobhandler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/http"
"os"
"strconv"
"strings"
"sync"

"github.com/Dewberry/s3api/auth"
Expand Down Expand Up @@ -309,8 +310,13 @@ func (bh *BlobHandler) PingWithAuth(c echo.Context) error {

func (bh *BlobHandler) GetS3ReadPermissions(c echo.Context, bucket string) ([]string, bool, int, error) {
permissions, fullAccess, err := bh.GetUserS3ReadListPermission(c, bucket)
httpStatus := http.StatusInternalServerError
if err != nil {
return nil, false, http.StatusInternalServerError, fmt.Errorf("error fetching user permissions: %s", err.Error())
//TEMP solution before error library is implimented and string check ups become redundant
if strings.Contains(err.Error(), "this endpoint requires authentication information that is unavailable when authorization is disabled.") {
httpStatus = http.StatusForbidden
}
return nil, false, httpStatus, fmt.Errorf("error fetching user permissions: %s", err.Error())
}
if !fullAccess && len(permissions) == 0 {
return nil, false, http.StatusForbidden, fmt.Errorf("user does not have permission to read the %s bucket", bucket)
Expand All @@ -324,12 +330,10 @@ func (bh *BlobHandler) HandleCheckS3UserPermission(c echo.Context) error {
return c.JSON(http.StatusOK, true)
}
initAuth := os.Getenv("INIT_AUTH")

if initAuth == "0" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how this would ever be true? If initAuth == "0" then line 324 would have already returned

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

errMsg := fmt.Errorf("this endpoint requires authentication information that is unavailable when authorization is disabled. Please enable authorization to use this functionality")
log.Error(errMsg.Error())
return c.JSON(http.StatusForbidden, errMsg.Error())

}
prefix := c.QueryParam("prefix")
bucket := c.QueryParam("bucket")
Expand Down
11 changes: 11 additions & 0 deletions blobstore/blobstore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package blobstore
import (
"fmt"
"net/http"
"os"
"strings"

"github.com/Dewberry/s3api/auth"
Expand Down Expand Up @@ -83,6 +84,11 @@ func isIdenticalArray(array1, array2 []string) bool {

func (bh *BlobHandler) CheckUserS3Permission(c echo.Context, bucket, prefix string, permissions []string) (int, error) {
if bh.Config.AuthLevel > 0 {
initAuth := os.Getenv("INIT_AUTH")
if initAuth == "0" {
errMsg := fmt.Errorf("this requires authentication information that is unavailable when authorization is disabled. Please enable authorization to use this functionality")
return http.StatusForbidden, errMsg
}
claims, ok := c.Get("claims").(*auth.Claims)
if !ok {
return http.StatusInternalServerError, fmt.Errorf("could not get claims from request context")
Expand Down Expand Up @@ -111,6 +117,11 @@ func (bh *BlobHandler) GetUserS3ReadListPermission(c echo.Context, bucket string
permissions := make([]string, 0)

if bh.Config.AuthLevel > 0 {
initAuth := os.Getenv("INIT_AUTH")
if initAuth == "0" {
errMsg := fmt.Errorf("this endpoint requires authentication information that is unavailable when authorization is disabled. Please enable authorization to use this functionality")
return permissions, false, errMsg
}
fullAccess := false
claims, ok := c.Get("claims").(*auth.Claims)
if !ok {
Expand Down
10 changes: 7 additions & 3 deletions blobstore/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ func (bh *BlobHandler) HandleListByPrefix(c echo.Context) error {
}

}
if delimiter && !strings.HasSuffix(prefix, "/") {

if delimiter && prefix != "" && !strings.HasSuffix(prefix, "/") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldnt this be after CheckAndAdjustPrefix like in the HandleListByPrefixWithDetails? It doesnt make sense that they are in two different places within the function

prefix = prefix + "/"
}

Expand Down Expand Up @@ -138,6 +139,8 @@ func (bh *BlobHandler) HandleListByPrefixWithDetail(c echo.Context) error {
log.Error(errMsg)
return c.JSON(statusCode, errMsg)
}
prefix = adjustedPrefix

delimiterParam := c.QueryParam("delimiter")
delimiter := true
if delimiterParam != "" {
Expand All @@ -150,7 +153,9 @@ func (bh *BlobHandler) HandleListByPrefixWithDetail(c echo.Context) error {

}

prefix = adjustedPrefix
if delimiter && prefix != "" && !strings.HasSuffix(prefix, "/") {
prefix = prefix + "/"
}

var results []ListResult
var count int
Expand Down Expand Up @@ -198,7 +203,6 @@ func (bh *BlobHandler) HandleListByPrefixWithDetail(c echo.Context) error {
}
return nil
}
fmt.Println(delimiter)
err = s3Ctrl.GetListWithCallBack(bucket, prefix, delimiter, processPage)
if err != nil {
errMsg := fmt.Errorf("error processing objects: %s", err.Error())
Expand Down
Loading