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.

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
Show file tree
Hide file tree
Changes from 8 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
1 change: 1 addition & 0 deletions .example.env
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ S3API_SERVICE_PORT='5005'
KEYCLOAK_PUBLIC_KEYS_URL='public-keys-url-string'
AUTH_LEVEL=1 # Options: [0, 1] corresponds to [no FGAC, FGAC]. This integer value configures the initialization mode in docker-compose.
AUTH_LIMITED_WRITER_ROLE='s3_limited_writer'
AUTH_LIMITED_READER_ROLE='s3_limited_reader'

## DB for Auth:
POSTGRES_CONN_STRING='postgres://user:password@postgres:5432/db?sslmode=disable'
Expand Down
8 changes: 8 additions & 0 deletions blobstore/blobhandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
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")
operation := c.QueryParam("operation")
Expand Down
25 changes: 12 additions & 13 deletions blobstore/buckets.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ func (bh *BlobHandler) HandleListBuckets(c echo.Context) error {
bh.Mu.Lock()
defer bh.Mu.Unlock()

fullAccess := false
// Check user's overall read access level
_, fullAccess, err := bh.GetUserS3ReadListPermission(c, "")
if err != nil {
return c.JSON(http.StatusInternalServerError, fmt.Errorf("error fetching user permissions: %s", err.Error()))
}

for _, controller := range bh.S3Controllers {
if bh.AllowAllBuckets {
Expand All @@ -108,13 +112,14 @@ func (bh *BlobHandler) HandleListBuckets(c echo.Context) error {

// Extract the bucket names from the response and append to allBuckets
for i, bucket := range controller.Buckets {
permissions, fullAccessTmp, err := bh.GetUserS3ReadListPermission(c, bucket)
if err != nil {
return c.JSON(http.StatusInternalServerError, fmt.Errorf("error fetching user permissions: %s", err.Error()))
canRead := fullAccess
if !fullAccess {
permissions, _, err := bh.GetUserS3ReadListPermission(c, bucket)
if err != nil {
return c.JSON(http.StatusInternalServerError, fmt.Errorf("error fetching user permissions: %s", err.Error()))
}
canRead = len(permissions) > 0
}
fullAccess = fullAccess || fullAccessTmp // Update full access based on any bucket returning full access

canRead := len(permissions) > 0 || fullAccessTmp // Set canRead based on permissions or full access
allBuckets = append(allBuckets, BucketInfo{
ID: i,
Name: bucket,
Expand All @@ -123,12 +128,6 @@ func (bh *BlobHandler) HandleListBuckets(c echo.Context) error {
}
}

if fullAccess { // If full access is true, set CanRead to true for all buckets
for i := range allBuckets {
allBuckets[i].CanRead = true
}
}

// Sorting allBuckets slice by CanRead true first and then by Name field alphabetically
sort.Slice(allBuckets, func(i, j int) bool {
if allBuckets[i].CanRead == allBuckets[j].CanRead {
Expand Down
25 changes: 16 additions & 9 deletions blobstore/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,15 @@ func (bh *BlobHandler) HandleListByPrefix(c echo.Context) error {
}

delimiterParam := c.QueryParam("delimiter")
var delimiter bool
if delimiterParam == "true" || delimiterParam == "false" {
delimiter := true
if delimiterParam != "" {
delimiter, err = strconv.ParseBool(delimiterParam)
if err != nil {
errMsg := fmt.Errorf("error parsing `delimiter` param: %s", err.Error())
log.Error(errMsg.Error())
return c.JSON(http.StatusUnprocessableEntity, errMsg.Error())
}

} else {
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, "/") {
Copy link
Member

Choose a reason for hiding this comment

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

Why dont you also need to do this in the detailed version of this function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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!

prefix = prefix + "/"
Expand Down Expand Up @@ -143,6 +138,18 @@ func (bh *BlobHandler) HandleListByPrefixWithDetail(c echo.Context) error {
log.Error(errMsg)
return c.JSON(statusCode, errMsg)
}
delimiterParam := c.QueryParam("delimiter")
delimiter := true
if delimiterParam != "" {
delimiter, err = strconv.ParseBool(delimiterParam)
if err != nil {
errMsg := fmt.Errorf("error parsing `delimiter` param: %s", err.Error())
log.Error(errMsg.Error())
return c.JSON(http.StatusUnprocessableEntity, errMsg.Error())
}

}

prefix = adjustedPrefix

var results []ListResult
Expand Down Expand Up @@ -191,8 +198,8 @@ func (bh *BlobHandler) HandleListByPrefixWithDetail(c echo.Context) error {
}
return nil
}

err = s3Ctrl.GetListWithCallBack(bucket, prefix, true, processPage)
fmt.Println(delimiter)
err = s3Ctrl.GetListWithCallBack(bucket, prefix, delimiter, processPage)
if err != nil {
errMsg := fmt.Errorf("error processing objects: %s", err.Error())
log.Error(errMsg.Error())
Expand Down
6 changes: 1 addition & 5 deletions e2e-test/e2eCollection.json
Original file line number Diff line number Diff line change
Expand Up @@ -2043,7 +2043,7 @@
"method": "GET",
"header": [],
"url": {
"raw": "{{s3_api_root_url}}/prefix/list?bucket={{bucket}}",
"raw": "{{s3_api_root_url}}/prefix/list?bucket=",
"host": [
"{{s3_api_root_url}}"
],
Expand All @@ -2061,10 +2061,6 @@
"key": "delimiter",
"value": "{{e2eoverride}}",
"disabled": true
},
{
"key": "bucket",
"value": "{{bucket}}"
}
]
}
Expand Down
48 changes: 45 additions & 3 deletions postman/S3api.postman_collection.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"info": {
"_postman_id": "89da2e0a-96d6-418f-964d-c00912cabda2",
"_postman_id": "e8b7d353-22af-417c-bdb6-b44d4be5288e",
"name": "S3api",
"description": "# 🚀 Get started here\n\nThis collection guides you through CRUD operations (GET, POST, PUT, DELETE), variables, and tests.\n\n## 🔖 **How to use this collection**\n\n#### **Step 1: Send requests**\n\nRESTful APIs allow you to perform CRUD operations using the POST, GET, PUT, and DELETE HTTP methods.\n\nThis collection contains each of these request types. Open each request and click \"Send\" to see what happens.\n\n#### **Step 2: View responses**\n\nObserve the response tab for status code (200 OK), response time, and size.\n\n#### **Step 3: Send new Body data**\n\nUpdate or add new data in \"Body\" in the POST request. Typically, Body data is also used in PUT and PATCH requests.\n\n```\n{\n \"name\": \"Add your name in the body\"\n}\n\n```\n\n#### **Step 4: Update the variable**\n\nVariables enable you to store and reuse values in Postman. We have created a variable called `base_url` with the sample request [https://postman-api-learner.glitch.me](https://postman-api-learner.glitch.me). Replace it with your API endpoint to customize this collection.\n\n#### **Step 5: Add tests in the \"Tests\" tab**\n\nTests help you confirm that your API is working as expected. You can write test scripts in JavaScript and view the output in the \"Test Results\" tab.\n\n<img src=\"https://content.pstmn.io/b5f280a7-4b09-48ec-857f-0a7ed99d7ef8/U2NyZWVuc2hvdCAyMDIzLTAzLTI3IGF0IDkuNDcuMjggUE0ucG5n\">\n\n## 💪 Pro tips\n\n- Use folders to group related requests and organize the collection.\n- Add more scripts in \"Tests\" to verify if the API works as expected and execute flows.\n \n\n## ℹ️ Resources\n\n[Building requests](https://learning.postman.com/docs/sending-requests/requests/) \n[Authorizing requests](https://learning.postman.com/docs/sending-requests/authorization/) \n[Using variables](https://learning.postman.com/docs/sending-requests/variables/) \n[Managing environments](https://learning.postman.com/docs/sending-requests/managing-environments/) \n[Writing scripts](https://learning.postman.com/docs/writing-scripts/intro-to-scripts/)",
"schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json",
"_exporter_id": "18773467",
"_collection_link": "https://universal-comet-524706.postman.co/workspace/Dewberry~fe115dcb-2f48-4ca3-a618-e462c6ac4255/collection/18773467-89da2e0a-96d6-418f-964d-c00912cabda2?action=share&source=collection_link&creator=18773467"
"_collection_link": "https://universal-comet-524706.postman.co/workspace/Dewberry~fe115dcb-2f48-4ca3-a618-e462c6ac4255/collection/18773467-e8b7d353-22af-417c-bdb6-b44d4be5288e?action=share&source=collection_link&creator=18773467"
},
"item": [
{
Expand Down Expand Up @@ -630,7 +630,8 @@
"var jsonData = JSON.parse(responseBody);\r",
"postman.setEnvironmentVariable(\"bearer_token\", jsonData.access_token);"
],
"type": "text/javascript"
"type": "text/javascript",
"packages": {}
}
}
],
Expand Down Expand Up @@ -719,6 +720,47 @@
},
"response": []
},
{
"name": "check_user_permission",
"request": {
"auth": {
"type": "bearer",
"bearer": [
{
"key": "token",
"value": "{{bearer_token}}",
"type": "string"
}
]
},
"method": "GET",
"header": [],
"url": {
"raw": "{{url}}/check_user_permission?prefix=antonTestingFolder/anton_testing/10Files%20-%20Copy%20(3)/&operation=write&bucket=ffrd-trinity",
"host": [
"{{url}}"
],
"path": [
"check_user_permission"
],
"query": [
{
"key": "prefix",
"value": "antonTestingFolder/anton_testing/10Files%20-%20Copy%20(3)/"
},
{
"key": "operation",
"value": "write"
},
{
"key": "bucket",
"value": "ffrd-trinity"
}
]
}
},
"response": []
},
{
"name": "DeleteListOfObjects",
"request": {
Expand Down
Loading