Skip to content

Commit

Permalink
Fix share link issue (#3382) (#3384)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ramondeklein
ramondeklein committed Jun 13, 2024
1 parent fa32d78 commit f4d98a4
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 23 deletions.
16 changes: 11 additions & 5 deletions api/public_objects.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package api

import (
"encoding/base64"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -44,7 +45,7 @@ func registerPublicObjectsHandlers(api *operations.ConsoleAPI) {
func getDownloadPublicObjectResponse(params public.DownloadSharedObjectParams) (middleware.Responder, *CodedAPIError) {
ctx := params.HTTPRequest.Context()

inputURLDecoded, err := checkMinIOStringURL(params.URL)
inputURLDecoded, err := decodeMinIOStringURL(params.URL)
if err != nil {
return nil, ErrorWithContext(ctx, err)
}
Expand Down Expand Up @@ -90,10 +91,15 @@ func getDownloadPublicObjectResponse(params public.DownloadSharedObjectParams) (
}), nil
}

// checkMinIOStringURL decodes url and validates is a MinIO url endpoint
func checkMinIOStringURL(inputURL string) (*string, error) {
// decodeMinIOStringURL decodes url and validates is a MinIO url endpoint
func decodeMinIOStringURL(inputURL string) (*string, error) {
decodedURL, err := base64.RawURLEncoding.DecodeString(inputURL)
if err != nil {
return nil, err
}

// Validate input URL
parsedURL, err := xnet.ParseHTTPURL(inputURL)
parsedURL, err := xnet.ParseHTTPURL(string(decodedURL))
if err != nil {
return nil, err
}
Expand All @@ -102,7 +108,7 @@ func checkMinIOStringURL(inputURL string) (*string, error) {
if parsedURL.Host != minIOHost {
return nil, ErrForbidden
}
return swag.String(inputURL), nil
return swag.String(string(decodedURL)), nil
}

func url2BucketAndObject(u *url.URL) (bucketName, objectName string) {
Expand Down
22 changes: 11 additions & 11 deletions api/public_objects_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"github.com/stretchr/testify/assert"
)

func Test_checkMinIOStringURL(t *testing.T) {
func Test_decodeMinIOStringURL(t *testing.T) {
tAssert := assert.New(t)
type args struct {
encodedURL string
Expand All @@ -37,31 +37,31 @@ func Test_checkMinIOStringURL(t *testing.T) {
{
test: "valid encoded minIO URL returns decoded URL string", // http://localhost:9000/...
args: args{
encodedURL: "http://localhost:9000/bucket123/Audio%20icon%281%29.svg?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=UBO1L1C7U87P1BP251TS%2F20240405%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20240405T210133Z&X-Amz-Expires=43200&X-Amz-Security-Token=eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJhY2Nlc3NLZXkiOiJVQk8xTDFDN1U4N1AxQlAyNTFUUyIsImV4cCI6MTcxMjM5NDA4OSwicGFyZW50IjoibWluaW9hZG1pbiJ9.V-KDgrLMUBlnJHKX6VxHl9-A_-PFEWok2dpTq-46blLmLsueTxshVaY4DZwGfom4TT5k8phUfgjcQansnbrkeA&X-Amz-SignedHeaders=host&versionId=null&X-Amz-Signature=07cad3eb2a6722b5baed09c6f1fe4a701bf12f48e1628b9d45c011d595567845",
encodedURL: "aHR0cDovL2xvY2FsaG9zdDo5MDAwL2J1Y2tldDEyMy9BdWRpbyUyMGljb24lMjgxJTI5LnN2Zz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPVVCTzFMMUM3VTg3UDFCUDI1MVRTJTJGMjAyNDA0MDUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNDA1VDIxMDEzM1omWC1BbXotRXhwaXJlcz00MzIwMCZYLUFtei1TZWN1cml0eS1Ub2tlbj1leUpoYkdjaU9pSklVelV4TWlJc0luUjVjQ0k2SWtwWFZDSjkuZXlKaFkyTmxjM05MWlhraU9pSlZRazh4VERGRE4xVTROMUF4UWxBeU5URlVVeUlzSW1WNGNDSTZNVGN4TWpNNU5EQTRPU3dpY0dGeVpXNTBJam9pYldsdWFXOWhaRzFwYmlKOS5WLUtEZ3JMTVVCbG5KSEtYNlZ4SGw5LUFfLVBGRVdvazJkcFRxLTQ2YmxMbUxzdWVUeHNoVmFZNERad0dmb200VFQ1azhwaFVmZ2pjUWFuc25icmtlQSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmdmVyc2lvbklkPW51bGwmWC1BbXotU2lnbmF0dXJlPTA3Y2FkM2ViMmE2NzIyYjViYWVkMDljNmYxZmU0YTcwMWJmMTJmNDhlMTYyOGI5ZDQ1YzAxMWQ1OTU1Njc4NDU",
},
wantError: nil,
expected: swag.String("http://localhost:9000/bucket123/Audio%20icon%281%29.svg?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=UBO1L1C7U87P1BP251TS%2F20240405%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20240405T210133Z&X-Amz-Expires=43200&X-Amz-Security-Token=eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJhY2Nlc3NLZXkiOiJVQk8xTDFDN1U4N1AxQlAyNTFUUyIsImV4cCI6MTcxMjM5NDA4OSwicGFyZW50IjoibWluaW9hZG1pbiJ9.V-KDgrLMUBlnJHKX6VxHl9-A_-PFEWok2dpTq-46blLmLsueTxshVaY4DZwGfom4TT5k8phUfgjcQansnbrkeA&X-Amz-SignedHeaders=host&versionId=null&X-Amz-Signature=07cad3eb2a6722b5baed09c6f1fe4a701bf12f48e1628b9d45c011d595567845"),
},
{
test: "valid encoded url but not coming from MinIO server returns forbidden error", // http://non-minio-host:9000/...
args: args{
encodedURL: "http://non-minio-host:9000/bucket123/Audio%20icon%281%29.svg?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=UBO1L1C7U87P1BP251TS%2F20240405%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20240405T210133Z&X-Amz-Expires=43200&X-Amz-Security-Token=eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJhY2Nlc3NLZXkiOiJVQk8xTDFDN1U4N1AxQlAyNTFUUyIsImV4cCI6MTcxMjM5NDA4OSwicGFyZW50IjoibWluaW9hZG1pbiJ9.V-KDgrLMUBlnJHKX6VxHl9-A_-PFEWok2dpTq-46blLmLsueTxshVaY4DZwGfom4TT5k8phUfgjcQansnbrkeA&X-Amz-SignedHeaders=host&versionId=null&X-Amz-Signature=07cad3eb2a6722b5baed09c6f1fe4a701bf12f48e1628b9d45c011d595567845",
encodedURL: "aHR0cDovL25vbi1taW5pby1ob3N0OjkwMDAvYnVja2V0MTIzL0F1ZGlvJTIwaWNvbiUyODElMjkuc3ZnP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9VUJPMUwxQzdVODdQMUJQMjUxVFMlMkYyMDI0MDQwNSUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA0MDVUMjEwMTMzWiZYLUFtei1FeHBpcmVzPTQzMjAwJlgtQW16LVNlY3VyaXR5LVRva2VuPWV5SmhiR2NpT2lKSVV6VXhNaUlzSW5SNWNDSTZJa3BYVkNKOS5leUpoWTJObGMzTkxaWGtpT2lKVlFrOHhUREZETjFVNE4xQXhRbEF5TlRGVVV5SXNJbVY0Y0NJNk1UY3hNak01TkRBNE9Td2ljR0Z5Wlc1MElqb2liV2x1YVc5aFpHMXBiaUo5LlYtS0RnckxNVUJsbkpIS1g2VnhIbDktQV8tUEZFV29rMmRwVHEtNDZibExtTHN1ZVR4c2hWYVk0RFp3R2ZvbTRUVDVrOHBoVWZnamNRYW5zbmJya2VBJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZ2ZXJzaW9uSWQ9bnVsbCZYLUFtei1TaWduYXR1cmU9MDdjYWQzZWIyYTY3MjJiNWJhZWQwOWM2ZjFmZTRhNzAxYmYxMmY0OGUxNjI4YjlkNDVjMDExZDU5NTU2Nzg0NQ",
},
wantError: swag.String("403 Forbidden"),
expected: nil,
},
{
test: "valid encoded url but not coming from MinIO server port returns forbidden error", // other port http://localhost:8902/...
args: args{
encodedURL: "http://localhost:8902/bucket123/Audio%20icon%281%29.svg?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=UBO1L1C7U87P1BP251TS%2F20240405%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20240405T210133Z&X-Amz-Expires=43200&X-Amz-Security-Token=eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJhY2Nlc3NLZXkiOiJVQk8xTDFDN1U4N1AxQlAyNTFUUyIsImV4cCI6MTcxMjM5NDA4OSwicGFyZW50IjoibWluaW9hZG1pbiJ9.V-KDgrLMUBlnJHKX6VxHl9-A_-PFEWok2dpTq-46blLmLsueTxshVaY4DZwGfom4TT5k8phUfgjcQansnbrkeA&X-Amz-SignedHeaders=host&versionId=null&X-Amz-Signature=07cad3eb2a6722b5baed09c6f1fe4a701bf12f48e1628b9d45c011d595567845",
encodedURL: "aHR0cDovL2xvY2FsaG9zdDo4OTAyL2J1Y2tldDEyMy9BdWRpbyUyMGljb24lMjgxJTI5LnN2Zz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPVVCTzFMMUM3VTg3UDFCUDI1MVRTJTJGMjAyNDA0MDUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNDA1VDIxMDEzM1omWC1BbXotRXhwaXJlcz00MzIwMCZYLUFtei1TZWN1cml0eS1Ub2tlbj1leUpoYkdjaU9pSklVelV4TWlJc0luUjVjQ0k2SWtwWFZDSjkuZXlKaFkyTmxjM05MWlhraU9pSlZRazh4VERGRE4xVTROMUF4UWxBeU5URlVVeUlzSW1WNGNDSTZNVGN4TWpNNU5EQTRPU3dpY0dGeVpXNTBJam9pYldsdWFXOWhaRzFwYmlKOS5WLUtEZ3JMTVVCbG5KSEtYNlZ4SGw5LUFfLVBGRVdvazJkcFRxLTQ2YmxMbUxzdWVUeHNoVmFZNERad0dmb200VFQ1azhwaFVmZ2pjUWFuc25icmtlQSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmdmVyc2lvbklkPW51bGwmWC1BbXotU2lnbmF0dXJlPTA3Y2FkM2ViMmE2NzIyYjViYWVkMDljNmYxZmU0YTcwMWJmMTJmNDhlMTYyOGI5ZDQ1YzAxMWQ1OTU1Njc4NDU",
},
wantError: swag.String("403 Forbidden"),
expected: nil,
},
{
test: "valid url but with invalid schema returns error",
args: args{
encodedURL: "postgres://postgres:123456@127.0.0.1:5432/dummy", // postgres://postgres:123456@127.0.0.1:5432/dummy
encodedURL: "cG9zdGdyZXM6Ly9wb3N0Z3JlczoxMjM0NTZAMTI3LjAuMC4xOjU0MzIvZHVtbXk", // postgres://postgres:123456@127.0.0.1:5432/dummy

},
wantError: swag.String("unexpected scheme found postgres"),
Expand All @@ -70,7 +70,7 @@ func Test_checkMinIOStringURL(t *testing.T) {
{
test: "invalid url returns error",
args: args{
encodedURL: "asdsadsda", // asdsadsda
encodedURL: "YXNkc2Fkc2Rh", // asdsadsda

},
wantError: swag.String("unexpected scheme found "),
Expand All @@ -79,7 +79,7 @@ func Test_checkMinIOStringURL(t *testing.T) {
{
test: "plain url",
args: args{
encodedURL: "https://localhost:9000/cestest/Audio%20icon.svg?X-Amz-Algorithm=AWS4-HMAC-SHA256",
encodedURL: "aHR0cHM6Ly9sb2NhbGhvc3Q6OTAwMC9jZXN0ZXN0L0F1ZGlvJTIwaWNvbi5zdmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTY",
},
wantError: nil,
expected: swag.String("https://localhost:9000/cestest/Audio%20icon.svg?X-Amz-Algorithm=AWS4-HMAC-SHA256"),
Expand All @@ -88,20 +88,20 @@ func Test_checkMinIOStringURL(t *testing.T) {

for _, tt := range tests {
t.Run(tt.test, func(_ *testing.T) {
url, err := checkMinIOStringURL(tt.args.encodedURL)
url, err := decodeMinIOStringURL(tt.args.encodedURL)
if tt.wantError != nil {
if err != nil {
if err.Error() != *tt.wantError {
t.Errorf("checkMinIOStringURL() error: `%v`, wantErr: `%s`, input: `%s`", err, *tt.wantError, tt.args.encodedURL)
t.Errorf("decodeMinIOStringURL() error: `%v`, wantErr: `%s`", err, *tt.wantError)
return
}
} else {
t.Errorf("checkMinIOStringURL() error: `%v`, wantErr: `%s`, input: `%s`", err, *tt.wantError, tt.args.encodedURL)
t.Errorf("decodeMinIOStringURL() error: `%v`, wantErr: `%s`", err, *tt.wantError)
return
}
} else {
if err != nil {
t.Errorf("checkMinIOStringURL() error: `%s`, wantErr: `%v`, input: `%s`", err, tt.wantError, tt.args.encodedURL)
t.Errorf("decodeMinIOStringURL() error: `%s`, wantErr: `%v`", err, tt.wantError)
return
}
tAssert.Equal(*tt.expected, *url)
Expand Down
5 changes: 4 additions & 1 deletion api/user_objects.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package api

import (
"context"
"encoding/base64"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -1022,7 +1023,9 @@ func getShareObjectURL(ctx context.Context, client MCClient, r *http.Request, ve
}

requestURL := getRequestURLWithScheme(r)
objURL := fmt.Sprintf("%s/api/v1/download-shared-object/%s", requestURL, url.PathEscape(minioURL))
encodedURL := base64.RawURLEncoding.EncodeToString([]byte(minioURL))

objURL := fmt.Sprintf("%s/api/v1/download-shared-object/%s", requestURL, url.PathEscape(encodedURL))
return &objURL, nil
}

Expand Down
12 changes: 6 additions & 6 deletions api/user_objects_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,7 @@ func Test_shareObject(t *testing.T) {
},

wantError: nil,
expected: "http://localhost:9090/api/v1/download-shared-object/http:%2F%2Fsomeurl",
expected: "http://localhost:9090/api/v1/download-shared-object/aHR0cDovL3NvbWV1cmw",
},
{
test: "return https scheme if url uses TLS",
Expand All @@ -959,7 +959,7 @@ func Test_shareObject(t *testing.T) {
},

wantError: nil,
expected: "https://localhost:9090/api/v1/download-shared-object/http:%2F%2Fsomeurl",
expected: "https://localhost:9090/api/v1/download-shared-object/aHR0cDovL3NvbWV1cmw",
},
{
test: "returns invalid expire duration if expiration is invalid",
Expand Down Expand Up @@ -990,7 +990,7 @@ func Test_shareObject(t *testing.T) {
},
},
wantError: nil,
expected: "http://localhost:9090/api/v1/download-shared-object/http:%2F%2Fsomeurl",
expected: "http://localhost:9090/api/v1/download-shared-object/aHR0cDovL3NvbWV1cmw",
},
{
test: "return error if sharefunc returns error",
Expand Down Expand Up @@ -1022,7 +1022,7 @@ func Test_shareObject(t *testing.T) {
},
},
wantError: nil,
expected: "http://localhost:9090/api/v1/download-shared-object/https:%2F%2F127.0.0.1:9000%2Fcestest%2FAudio%2520icon.svg%3FX-Amz-Algorithm=AWS4-HMAC-SHA256",
expected: "http://localhost:9090/api/v1/download-shared-object/aHR0cHM6Ly8xMjcuMC4wLjE6OTAwMC9jZXN0ZXN0L0F1ZGlvJTIwaWNvbi5zdmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTY",
},
{
test: "returns redirect url with share link if redirect url env variable set",
Expand All @@ -1041,7 +1041,7 @@ func Test_shareObject(t *testing.T) {
},
},
wantError: nil,
expected: "http://proxy-url.com:9012/console/subpath/api/v1/download-shared-object/http:%2F%2Fsomeurl",
expected: "http://proxy-url.com:9012/console/subpath/api/v1/download-shared-object/aHR0cDovL3NvbWV1cmw",
},
{
test: "returns redirect url with share link if redirect url env variable set with trailing slash",
Expand All @@ -1060,7 +1060,7 @@ func Test_shareObject(t *testing.T) {
},
},
wantError: nil,
expected: "http://proxy-url.com:9012/console/subpath/api/v1/download-shared-object/http:%2F%2Fsomeurl",
expected: "http://proxy-url.com:9012/console/subpath/api/v1/download-shared-object/aHR0cDovL3NvbWV1cmw",
},
}

Expand Down

0 comments on commit f4d98a4

Please sign in to comment.