Skip to content

Commit 457c894

Browse files
ramondekleinramondeklein
committed
Use base64 encoded share URL
1 parent 67877d5 commit 457c894

File tree

4 files changed

+33
-76
lines changed

4 files changed

+33
-76
lines changed

‎api/public_objects.go‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package api
1818

1919
import (
20+
"encoding/base64"
2021
"fmt"
2122
"io"
2223
"net/http"
@@ -44,7 +45,7 @@ func registerPublicObjectsHandlers(api *operations.ConsoleAPI) {
4445
func getDownloadPublicObjectResponse(params public.DownloadSharedObjectParams) (middleware.Responder, *CodedAPIError) {
4546
ctx := params.HTTPRequest.Context()
4647

47-
inputURLDecoded, err := checkMinIOStringURL(params.URL)
48+
inputURLDecoded, err := decodeMinIOStringURL(params.URL)
4849
if err != nil {
4950
return nil, ErrorWithContext(ctx, err)
5051
}
@@ -90,10 +91,15 @@ func getDownloadPublicObjectResponse(params public.DownloadSharedObjectParams) (
9091
}), nil
9192
}
9293

93-
// checkMinIOStringURL decodes url and validates is a MinIO url endpoint
94-
func checkMinIOStringURL(inputURL string) (*string, error) {
94+
// decodeMinIOStringURL decodes url and validates is a MinIO url endpoint
95+
func decodeMinIOStringURL(inputURL string) (*string, error) {
96+
decodedURL, err := base64.RawURLEncoding.DecodeString(inputURL)
97+
if err != nil {
98+
return nil, err
99+
}
100+
95101
// Validate input URL
96-
parsedURL, err := xnet.ParseHTTPURL(inputURL)
102+
parsedURL, err := xnet.ParseHTTPURL(string(decodedURL))
97103
if err != nil {
98104
return nil, err
99105
}
@@ -102,7 +108,7 @@ func checkMinIOStringURL(inputURL string) (*string, error) {
102108
if parsedURL.Host != minIOHost {
103109
return nil, ErrForbidden
104110
}
105-
return swag.String(inputURL), nil
111+
return swag.String(string(decodedURL)), nil
106112
}
107113

108114
func url2BucketAndObject(u *url.URL) (bucketName, objectName string) {

‎api/public_objects_test.go‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
"github.com/stretchr/testify/assert"
2424
)
2525

26-
func Test_checkMinIOStringURL(t *testing.T) {
26+
func Test_decodeMinIOStringURL(t *testing.T) {
2727
tAssert := assert.New(t)
2828
type args struct {
2929
encodedURL string
@@ -37,31 +37,31 @@ func Test_checkMinIOStringURL(t *testing.T) {
3737
{
3838
test: "valid encoded minIO URL returns decoded URL string", // http://localhost:9000/...
3939
args: args{
40-
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",
40+
encodedURL: "aHR0cDovL2xvY2FsaG9zdDo5MDAwL2J1Y2tldDEyMy9BdWRpbyUyMGljb24lMjgxJTI5LnN2Zz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPVVCTzFMMUM3VTg3UDFCUDI1MVRTJTJGMjAyNDA0MDUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNDA1VDIxMDEzM1omWC1BbXotRXhwaXJlcz00MzIwMCZYLUFtei1TZWN1cml0eS1Ub2tlbj1leUpoYkdjaU9pSklVelV4TWlJc0luUjVjQ0k2SWtwWFZDSjkuZXlKaFkyTmxjM05MWlhraU9pSlZRazh4VERGRE4xVTROMUF4UWxBeU5URlVVeUlzSW1WNGNDSTZNVGN4TWpNNU5EQTRPU3dpY0dGeVpXNTBJam9pYldsdWFXOWhaRzFwYmlKOS5WLUtEZ3JMTVVCbG5KSEtYNlZ4SGw5LUFfLVBGRVdvazJkcFRxLTQ2YmxMbUxzdWVUeHNoVmFZNERad0dmb200VFQ1azhwaFVmZ2pjUWFuc25icmtlQSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmdmVyc2lvbklkPW51bGwmWC1BbXotU2lnbmF0dXJlPTA3Y2FkM2ViMmE2NzIyYjViYWVkMDljNmYxZmU0YTcwMWJmMTJmNDhlMTYyOGI5ZDQ1YzAxMWQ1OTU1Njc4NDU",
4141
},
4242
wantError: nil,
4343
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"),
4444
},
4545
{
4646
test: "valid encoded url but not coming from MinIO server returns forbidden error", // http://non-minio-host:9000/...
4747
args: args{
48-
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",
48+
encodedURL: "aHR0cDovL25vbi1taW5pby1ob3N0OjkwMDAvYnVja2V0MTIzL0F1ZGlvJTIwaWNvbiUyODElMjkuc3ZnP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9VUJPMUwxQzdVODdQMUJQMjUxVFMlMkYyMDI0MDQwNSUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA0MDVUMjEwMTMzWiZYLUFtei1FeHBpcmVzPTQzMjAwJlgtQW16LVNlY3VyaXR5LVRva2VuPWV5SmhiR2NpT2lKSVV6VXhNaUlzSW5SNWNDSTZJa3BYVkNKOS5leUpoWTJObGMzTkxaWGtpT2lKVlFrOHhUREZETjFVNE4xQXhRbEF5TlRGVVV5SXNJbVY0Y0NJNk1UY3hNak01TkRBNE9Td2ljR0Z5Wlc1MElqb2liV2x1YVc5aFpHMXBiaUo5LlYtS0RnckxNVUJsbkpIS1g2VnhIbDktQV8tUEZFV29rMmRwVHEtNDZibExtTHN1ZVR4c2hWYVk0RFp3R2ZvbTRUVDVrOHBoVWZnamNRYW5zbmJya2VBJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZ2ZXJzaW9uSWQ9bnVsbCZYLUFtei1TaWduYXR1cmU9MDdjYWQzZWIyYTY3MjJiNWJhZWQwOWM2ZjFmZTRhNzAxYmYxMmY0OGUxNjI4YjlkNDVjMDExZDU5NTU2Nzg0NQ",
4949
},
5050
wantError: swag.String("403 Forbidden"),
5151
expected: nil,
5252
},
5353
{
5454
test: "valid encoded url but not coming from MinIO server port returns forbidden error", // other port http://localhost:8902/...
5555
args: args{
56-
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",
56+
encodedURL: "aHR0cDovL2xvY2FsaG9zdDo4OTAyL2J1Y2tldDEyMy9BdWRpbyUyMGljb24lMjgxJTI5LnN2Zz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPVVCTzFMMUM3VTg3UDFCUDI1MVRTJTJGMjAyNDA0MDUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNDA1VDIxMDEzM1omWC1BbXotRXhwaXJlcz00MzIwMCZYLUFtei1TZWN1cml0eS1Ub2tlbj1leUpoYkdjaU9pSklVelV4TWlJc0luUjVjQ0k2SWtwWFZDSjkuZXlKaFkyTmxjM05MWlhraU9pSlZRazh4VERGRE4xVTROMUF4UWxBeU5URlVVeUlzSW1WNGNDSTZNVGN4TWpNNU5EQTRPU3dpY0dGeVpXNTBJam9pYldsdWFXOWhaRzFwYmlKOS5WLUtEZ3JMTVVCbG5KSEtYNlZ4SGw5LUFfLVBGRVdvazJkcFRxLTQ2YmxMbUxzdWVUeHNoVmFZNERad0dmb200VFQ1azhwaFVmZ2pjUWFuc25icmtlQSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmdmVyc2lvbklkPW51bGwmWC1BbXotU2lnbmF0dXJlPTA3Y2FkM2ViMmE2NzIyYjViYWVkMDljNmYxZmU0YTcwMWJmMTJmNDhlMTYyOGI5ZDQ1YzAxMWQ1OTU1Njc4NDU",
5757
},
5858
wantError: swag.String("403 Forbidden"),
5959
expected: nil,
6060
},
6161
{
6262
test: "valid url but with invalid schema returns error",
6363
args: args{
64-
encodedURL: "postgres://postgres:123456@127.0.0.1:5432/dummy", // postgres://postgres:123456@127.0.0.1:5432/dummy
64+
encodedURL: "cG9zdGdyZXM6Ly9wb3N0Z3JlczoxMjM0NTZAMTI3LjAuMC4xOjU0MzIvZHVtbXk", // postgres://postgres:123456@127.0.0.1:5432/dummy
6565

6666
},
6767
wantError: swag.String("unexpected scheme found postgres"),
@@ -70,38 +70,30 @@ func Test_checkMinIOStringURL(t *testing.T) {
7070
{
7171
test: "invalid url returns error",
7272
args: args{
73-
encodedURL: "asdsadsda", // asdsadsda
73+
encodedURL: "YXNkc2Fkc2Rh", // asdsadsda
7474

7575
},
7676
wantError: swag.String("unexpected scheme found "),
7777
expected: nil,
7878
},
79-
{
80-
test: "plain url",
81-
args: args{
82-
encodedURL: "https://localhost:9000/cestest/Audio%20icon.svg?X-Amz-Algorithm=AWS4-HMAC-SHA256",
83-
},
84-
wantError: nil,
85-
expected: swag.String("https://localhost:9000/cestest/Audio%20icon.svg?X-Amz-Algorithm=AWS4-HMAC-SHA256"),
86-
},
8779
}
8880

8981
for _, tt := range tests {
9082
t.Run(tt.test, func(_ *testing.T) {
91-
url, err := checkMinIOStringURL(tt.args.encodedURL)
83+
url, err := decodeMinIOStringURL(tt.args.encodedURL)
9284
if tt.wantError != nil {
9385
if err != nil {
9486
if err.Error() != *tt.wantError {
95-
t.Errorf("checkMinIOStringURL() error: `%v`, wantErr: `%s`, input: `%s`", err, *tt.wantError, tt.args.encodedURL)
87+
t.Errorf("decodeMinIOStringURL() error: `%v`, wantErr: `%s`", err, *tt.wantError)
9688
return
9789
}
9890
} else {
99-
t.Errorf("checkMinIOStringURL() error: `%v`, wantErr: `%s`, input: `%s`", err, *tt.wantError, tt.args.encodedURL)
91+
t.Errorf("decodeMinIOStringURL() error: `%v`, wantErr: `%s`", err, *tt.wantError)
10092
return
10193
}
10294
} else {
10395
if err != nil {
104-
t.Errorf("checkMinIOStringURL() error: `%s`, wantErr: `%v`, input: `%s`", err, tt.wantError, tt.args.encodedURL)
96+
t.Errorf("decodeMinIOStringURL() error: `%s`, wantErr: `%v`", err, tt.wantError)
10597
return
10698
}
10799
tAssert.Equal(*tt.expected, *url)

‎api/user_objects.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package api
1818

1919
import (
2020
"context"
21+
"encoding/base64"
2122
"errors"
2223
"fmt"
2324
"io"
@@ -1022,9 +1023,9 @@ func getShareObjectURL(ctx context.Context, client MCClient, r *http.Request, ve
10221023
}
10231024

10241025
requestURL := getRequestURLWithScheme(r)
1025-
escapedURL := url.QueryEscape(minioURL) // Use "QueryEscape" instead of "PathEscape", because MUX has issues dealing with colons
1026+
encodedURL := base64.RawURLEncoding.EncodeToString([]byte(minioURL))
10261027

1027-
objURL := fmt.Sprintf("%s/api/v1/download-shared-object/%s", requestURL, escapedURL)
1028+
objURL := fmt.Sprintf("%s/api/v1/download-shared-object/%s", requestURL, url.PathEscape(encodedURL))
10281029
return &objURL, nil
10291030
}
10301031

‎api/user_objects_test.go‎

Lines changed: 9 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -921,11 +921,10 @@ func Test_shareObject(t *testing.T) {
921921
shareFunc func(ctx context.Context, versionID string, expires time.Duration) (string, *probe.Error)
922922
}
923923
tests := []struct {
924-
test string
925-
args args
926-
setEnvVars func()
927-
wantError error
928-
expected string
924+
test string
925+
args args
926+
wantError error
927+
expected string
929928
}{
930929
{
931930
test: "return sharefunc url base64 encoded with host name",
@@ -942,7 +941,7 @@ func Test_shareObject(t *testing.T) {
942941
},
943942

944943
wantError: nil,
945-
expected: "http://localhost:9090/api/v1/download-shared-object/http%3A%2F%2Fsomeurl",
944+
expected: "http://localhost:9090/api/v1/download-shared-object/aHR0cDovL3NvbWV1cmw",
946945
},
947946
{
948947
test: "return https scheme if url uses TLS",
@@ -959,7 +958,7 @@ func Test_shareObject(t *testing.T) {
959958
},
960959

961960
wantError: nil,
962-
expected: "https://localhost:9090/api/v1/download-shared-object/http%3A%2F%2Fsomeurl",
961+
expected: "https://localhost:9090/api/v1/download-shared-object/aHR0cDovL3NvbWV1cmw",
963962
},
964963
{
965964
test: "returns invalid expire duration if expiration is invalid",
@@ -990,7 +989,7 @@ func Test_shareObject(t *testing.T) {
990989
},
991990
},
992991
wantError: nil,
993-
expected: "http://localhost:9090/api/v1/download-shared-object/http%3A%2F%2Fsomeurl",
992+
expected: "http://localhost:9090/api/v1/download-shared-object/aHR0cDovL3NvbWV1cmw",
994993
},
995994
{
996995
test: "return error if sharefunc returns error",
@@ -1022,62 +1021,21 @@ func Test_shareObject(t *testing.T) {
10221021
},
10231022
},
10241023
wantError: nil,
1025-
expected: "http://localhost:9090/api/v1/download-shared-object/https%3A%2F%2F127.0.0.1%3A9000%2Fcestest%2FAudio%2520icon.svg%3FX-Amz-Algorithm%3DAWS4-HMAC-SHA256",
1026-
},
1027-
{
1028-
test: "returns redirect url with share link if redirect url env variable set",
1029-
setEnvVars: func() {
1030-
t.Setenv(ConsoleBrowserRedirectURL, "http://proxy-url.com:9012/console/subpath")
1031-
},
1032-
args: args{
1033-
r: &http.Request{
1034-
TLS: nil,
1035-
Host: "localhost:9090",
1036-
},
1037-
versionID: "2121434",
1038-
expires: "30s",
1039-
shareFunc: func(_ context.Context, _ string, _ time.Duration) (string, *probe.Error) {
1040-
return "http://someurl", nil
1041-
},
1042-
},
1043-
wantError: nil,
1044-
expected: "http://proxy-url.com:9012/console/subpath/api/v1/download-shared-object/http%3A%2F%2Fsomeurl",
1045-
},
1046-
{
1047-
test: "returns redirect url with share link if redirect url env variable set with trailing slash",
1048-
setEnvVars: func() {
1049-
t.Setenv(ConsoleBrowserRedirectURL, "http://proxy-url.com:9012/console/subpath/")
1050-
},
1051-
args: args{
1052-
r: &http.Request{
1053-
TLS: nil,
1054-
Host: "localhost:9090",
1055-
},
1056-
versionID: "2121434",
1057-
expires: "30s",
1058-
shareFunc: func(_ context.Context, _ string, _ time.Duration) (string, *probe.Error) {
1059-
return "http://someurl", nil
1060-
},
1061-
},
1062-
wantError: nil,
1063-
expected: "http://proxy-url.com:9012/console/subpath/api/v1/download-shared-object/http%3A%2F%2Fsomeurl",
1024+
expected: "http://localhost:9090/api/v1/download-shared-object/aHR0cHM6Ly8xMjcuMC4wLjE6OTAwMC9jZXN0ZXN0L0F1ZGlvJTIwaWNvbi5zdmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTY",
10641025
},
10651026
}
10661027

10671028
for _, tt := range tests {
10681029
t.Run(tt.test, func(_ *testing.T) {
10691030
mcShareDownloadMock = tt.args.shareFunc
1070-
if tt.setEnvVars != nil {
1071-
tt.setEnvVars()
1072-
}
10731031
url, err := getShareObjectURL(ctx, client, tt.args.r, tt.args.versionID, tt.args.expires)
10741032
if tt.wantError != nil {
10751033
if !reflect.DeepEqual(err, tt.wantError) {
10761034
t.Errorf("getShareObjectURL() error: `%s`, wantErr: `%s`", err, tt.wantError)
10771035
return
10781036
}
10791037
} else {
1080-
tAssert.Equal(tt.expected, *url)
1038+
tAssert.Equal(*url, tt.expected)
10811039
}
10821040
})
10831041
}

0 commit comments

Comments
 (0)