Skip to content

Commit a04f2e2

Browse files
author
Benjamin Perez
committed
Improved error handling in Object Browser
Signed-off-by: Benjamin Perez <benjamin@bexsoft.net>
1 parent 622c3a0 commit a04f2e2

File tree

11 files changed

+255
-15
lines changed

11 files changed

+255
-15
lines changed

‎portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/types.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,19 @@ export interface WebsocketRequest {
3838

3939
export interface WebsocketResponse {
4040
request_id: number;
41-
error?: string;
41+
error?: WebsocketErrorResponse;
4242
request_end?: boolean;
4343
data?: ObjectResponse[];
4444
prefix?: string;
4545
bucketName?: string;
4646
}
4747

48+
export interface WebsocketErrorResponse {
49+
status: number;
50+
message?: string;
51+
detailedMessage?: string;
52+
}
53+
4854
export interface ObjectResponse {
4955
name: string;
5056
last_modified: string;

‎portal-ui/src/websockets/objectBrowserWSMiddleware.ts‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,10 @@ export const objectBrowserWSMiddleware = (
9494
return;
9595
}
9696

97-
if (
98-
response.error ===
99-
"The Access Key Id you provided does not exist in our records."
100-
) {
101-
// Session expired.
97+
if (response.error?.status === 401) {
98+
// Session expired. We reload this page
10299
window.location.reload();
103-
} else if (response.error === "Access Denied.") {
100+
} else if (response.error?.status === 403) {
104101
const internalPathsPrefix = response.prefix;
105102
let pathPrefix = "";
106103

@@ -121,8 +118,11 @@ export const objectBrowserWSMiddleware = (
121118
if (!permitItems || permitItems.length === 0) {
122119
dispatch(
123120
setErrorSnackMessage({
124-
errorMessage: response.error,
125-
detailedError: response.error,
121+
errorMessage:
122+
response.error?.message || "An error occurred",
123+
detailedError:
124+
response.error?.detailedMessage ||
125+
"An unknown error occurred. Please refer to MinIO logs to get more information.",
126126
}),
127127
);
128128
} else {
@@ -131,6 +131,16 @@ export const objectBrowserWSMiddleware = (
131131
}
132132

133133
return;
134+
} else if (response.error) {
135+
dispatch(setRequestInProgress(false));
136+
dispatch(
137+
setErrorSnackMessage({
138+
errorMessage: response.error?.message || "An error occurred",
139+
detailedError:
140+
response.error?.detailedMessage ||
141+
"An unknown error occurred. Please refer to MinIO logs to get more information.",
142+
}),
143+
);
134144
}
135145

136146
// This indicates final messages is received.

‎portal-ui/tests/permissions-7/errorsVisibleOB.ts‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
// This file is part of MinIO Console Server
2+
// Copyright (c) 2023 MinIO, Inc.
3+
//
4+
// This program is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU Affero General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
//
9+
// This program is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU Affero General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU Affero General Public License
15+
// along with this program. If not, see <http://www.gnu.org/licenses/>.
16+
17+
import * as roles from "../utils/roles";
18+
import { Selector } from "testcafe";
19+
import * as functions from "../utils/functions";
20+
import { namedTestBucketBrowseButtonFor } from "../utils/functions";
21+
22+
fixture("Test error visibility in Object Browser Navigation").page(
23+
"http://localhost:9090/",
24+
);
25+
26+
const bucketName = "my-company";
27+
const bucketName2 = "my-company2";
28+
const bucketBrowseButton = namedTestBucketBrowseButtonFor(bucketName);
29+
const bucketBrowseButton2 = namedTestBucketBrowseButtonFor(bucketName2);
30+
export const file = Selector(".ReactVirtualized__Table__rowColumn").withText(
31+
"test.txt",
32+
);
33+
export const deniedError = Selector(".message-text").withText("Access Denied.");
34+
35+
test
36+
.before(async (t) => {
37+
await functions.setUpNamedBucket(t, bucketName);
38+
await functions.uploadNamedObjectToBucket(
39+
t,
40+
bucketName,
41+
"test.txt",
42+
"portal-ui/tests/uploads/test.txt",
43+
);
44+
await functions.uploadNamedObjectToBucket(
45+
t,
46+
bucketName,
47+
"home/UserY/test.txt",
48+
"portal-ui/tests/uploads/test.txt",
49+
);
50+
await functions.uploadNamedObjectToBucket(
51+
t,
52+
bucketName,
53+
"home/UserX/test.txt",
54+
"portal-ui/tests/uploads/test.txt",
55+
);
56+
})(
57+
"Error Notification is shown in Object Browser when no privileges are set",
58+
async (t) => {
59+
await t
60+
.useRole(roles.conditions3)
61+
.navigateTo(`http://localhost:9090/browser`)
62+
.click(bucketBrowseButton)
63+
.click(Selector(".ReactVirtualized__Table__rowColumn").withText("home"))
64+
.click(
65+
Selector(".ReactVirtualized__Table__rowColumn").withText("UserX"),
66+
)
67+
.expect(deniedError.exists)
68+
.ok();
69+
},
70+
)
71+
.after(async (t) => {
72+
await functions.cleanUpNamedBucketAndUploads(t, bucketName);
73+
});
74+
75+
test
76+
.before(async (t) => {
77+
await functions.setUpNamedBucket(t, bucketName2);
78+
await functions.setVersionedBucket(t, bucketName2);
79+
await functions.uploadNamedObjectToBucket(
80+
t,
81+
bucketName2,
82+
"test.txt",
83+
"portal-ui/tests/uploads/test.txt",
84+
);
85+
await functions.uploadNamedObjectToBucket(
86+
t,
87+
bucketName2,
88+
"home/UserY/test.txt",
89+
"portal-ui/tests/uploads/test.txt",
90+
);
91+
await functions.uploadNamedObjectToBucket(
92+
t,
93+
bucketName2,
94+
"home/UserX/test.txt",
95+
"portal-ui/tests/uploads/test.txt",
96+
);
97+
})(
98+
"Error Notification is shown in Object Browser with Rewind request set",
99+
async (t) => {
100+
await t
101+
.useRole(roles.conditions4)
102+
.navigateTo(`http://localhost:9090/browser`)
103+
.click(bucketBrowseButton2)
104+
.click(Selector("label").withText("Show deleted objects"))
105+
.wait(1500)
106+
.click(Selector(".ReactVirtualized__Table__rowColumn").withText("home"))
107+
.click(
108+
Selector(".ReactVirtualized__Table__rowColumn").withText("UserX"),
109+
)
110+
.expect(deniedError.exists)
111+
.ok();
112+
},
113+
)
114+
.after(async (t) => {
115+
await functions.cleanUpNamedBucketAndUploads(t, bucketName2);
116+
});

‎portal-ui/tests/policies/conditionsPolicy4.json‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
{
2+
"Version": "2012-10-17",
3+
"Statement": [
4+
{
5+
"Sid": "AllowUserToSeeBucketListInTheConsole",
6+
"Action": [
7+
"s3:ListAllMyBuckets",
8+
"s3:GetBucketLocation",
9+
"s3:GetBucketVersioning"
10+
],
11+
"Effect": "Allow",
12+
"Resource": ["arn:aws:s3:::*"]
13+
},
14+
{
15+
"Sid": "AllowRootAndHomeListingOfCompanyBucket",
16+
"Action": ["s3:ListBucket", "s3:List*"],
17+
"Effect": "Allow",
18+
"Resource": ["arn:aws:s3:::my-company2"],
19+
"Condition": {
20+
"StringEquals": {
21+
"s3:prefix": ["", "home/", "home/User"],
22+
"s3:delimiter": ["/"]
23+
}
24+
}
25+
},
26+
{
27+
"Sid": "AllowListingOfUserFolder",
28+
"Action": ["s3:ListBucket", "s3:List*"],
29+
"Effect": "Allow",
30+
"Resource": ["arn:aws:s3:::my-company2"],
31+
"Condition": {
32+
"StringLike": {
33+
"s3:prefix": ["home/User/*"]
34+
}
35+
}
36+
},
37+
{
38+
"Sid": "AllowAllS3ActionsInUserFolder",
39+
"Effect": "Allow",
40+
"Action": ["s3:*"],
41+
"Resource": ["arn:aws:s3:::my-company2/home/User/*"]
42+
}
43+
]
44+
}

‎portal-ui/tests/scripts/cleanup-env.sh‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ remove_users() {
3131
mc admin user remove minio conditions-$TIMESTAMP
3232
mc admin user remove minio conditions-2-$TIMESTAMP
3333
mc admin user remove minio conditions-3-$TIMESTAMP
34+
mc admin user remove minio conditions-4-$TIMESTAMP
3435
}
3536

3637
remove_policies() {
@@ -56,6 +57,7 @@ remove_policies() {
5657
mc admin policy remove minio conditions-policy-$TIMESTAMP
5758
mc admin policy remove minio conditions-policy-2-$TIMESTAMP
5859
mc admin policy remove minio conditions-policy-3-$TIMESTAMP
60+
mc admin policy remove minio conditions-policy-4-$TIMESTAMP
5961
}
6062

6163
__init__() {

‎portal-ui/tests/scripts/common.sh‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ create_policies() {
4949
mc admin policy create minio conditions-policy-$TIMESTAMP portal-ui/tests/policies/conditionsPolicy.json
5050
mc admin policy create minio conditions-policy-2-$TIMESTAMP portal-ui/tests/policies/conditionsPolicy2.json
5151
mc admin policy create minio conditions-policy-3-$TIMESTAMP portal-ui/tests/policies/conditionsPolicy3.json
52+
mc admin policy create minio conditions-policy-4-$TIMESTAMP portal-ui/tests/policies/conditionsPolicy4.json
5253
}
5354

5455
create_users() {
@@ -79,6 +80,7 @@ create_users() {
7980
mc admin user add minio conditions-$TIMESTAMP conditions1234
8081
mc admin user add minio conditions-2-$TIMESTAMP conditions1234
8182
mc admin user add minio conditions-3-$TIMESTAMP conditions1234
83+
mc admin user add minio conditions-4-$TIMESTAMP conditions1234
8284
}
8385

8486
create_buckets() {
@@ -114,4 +116,5 @@ assign_policies() {
114116
mc admin policy attach minio conditions-policy-$TIMESTAMP --user conditions-$TIMESTAMP
115117
mc admin policy attach minio conditions-policy-2-$TIMESTAMP --user conditions-2-$TIMESTAMP
116118
mc admin policy attach minio conditions-policy-3-$TIMESTAMP --user conditions-3-$TIMESTAMP
119+
mc admin policy attach minio conditions-policy-4-$TIMESTAMP --user conditions-4-$TIMESTAMP
117120
}

‎portal-ui/tests/scripts/permissions.sh‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ remove_users() {
3939
mc admin user remove minio conditions-"$TIMESTAMP"
4040
mc admin user remove minio conditions-2-"$TIMESTAMP"
4141
mc admin user remove minio conditions-3-"$TIMESTAMP"
42+
mc admin user remove minio conditions-4-"$TIMESTAMP"
4243
}
4344

4445
remove_policies() {
@@ -65,6 +66,7 @@ remove_policies() {
6566
mc admin policy remove conditions-policy-"$TIMESTAMP"
6667
mc admin policy remove conditions-policy-2-"$TIMESTAMP"
6768
mc admin policy remove conditions-policy-3-"$TIMESTAMP"
69+
mc admin policy remove conditions-policy-4-"$TIMESTAMP"
6870
}
6971

7072
remove_buckets() {

‎portal-ui/tests/utils/roles.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,14 @@ export const conditions3 = Role(
283283
},
284284
{ preserveUrl: true },
285285
);
286+
287+
export const conditions4 = Role(
288+
loginUrl,
289+
async (t) => {
290+
await t
291+
.typeText("#accessKey", "conditions-4-" + unixTimestamp)
292+
.typeText("#secretKey", "conditions1234")
293+
.click(submitButton);
294+
},
295+
{ preserveUrl: true },
296+
);

‎restapi/admin_objects.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,19 @@ type ObjectsRequest struct {
4141

4242
type WSResponse struct {
4343
RequestID int64 `json:"request_id,omitempty"`
44-
Error string `json:"error,omitempty"`
44+
Error *WSErrorResponse `json:"error,omitempty"`
4545
RequestEnd bool `json:"request_end,omitempty"`
4646
Prefix string `json:"prefix,omitempty"`
4747
BucketName string `json:"bucketName,omitempty"`
4848
Data []ObjectResponse `json:"data,omitempty"`
4949
}
5050

51+
type WSErrorResponse struct {
52+
Status int `json:"status,omitempty"`
53+
Message string `json:"message,omitempty"`
54+
DetailedMessage string `json:"detailedMessage,omitempty"`
55+
}
56+
5157
type ObjectResponse struct {
5258
Name string `json:"name,omitempty"`
5359
LastModified string `json:"last_modified,omitempty"`

‎restapi/errors.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,10 @@ func ErrorWithContext(ctx context.Context, err ...interface{}) *CodedAPIError {
108108
errorCode = 401
109109
errorMessage = ErrInvalidLogin.Error()
110110
}
111+
if strings.Contains(strings.ToLower(err1.Error()), ErrAccessDenied.Error()) {
112+
errorCode = 403
113+
errorMessage = err1.Error()
114+
}
111115
// If the last error is ErrInvalidLogin, this is a login failure
112116
if errors.Is(lastError, ErrInvalidLogin) {
113117
errorCode = 401

0 commit comments

Comments
 (0)