Skip to content

Commit 8dbad84

Browse files
bexsoftbexsoft
authored
Improved error handling in Object Browser (#3097)
Signed-off-by: Benjamin Perez <benjamin@bexsoft.net>
1 parent 1767a37 commit 8dbad84

File tree

11 files changed

+244
-16
lines changed

11 files changed

+244
-16
lines changed

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
// You should have received a copy of the GNU Affero General Public License
1515
// along with this program. If not, see <http://www.gnu.org/licenses/>.
1616

17-
import { BucketObject } from "api/consoleApi";
17+
import { ApiError, BucketObject } from "api/consoleApi";
1818
import { IFileInfo } from "../ObjectDetails/types";
1919

2020
export interface BucketObjectItem {
@@ -38,13 +38,18 @@ 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+
Code: number;
50+
APIError: ApiError;
51+
}
52+
4853
export interface ObjectResponse {
4954
name: string;
5055
last_modified: string;

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

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,12 @@ export const objectBrowserWSMiddleware = (
8585
};
8686

8787
objectsWS.onmessage = (message) => {
88+
const basicErrorMessage = {
89+
errorMessage: "An error occurred",
90+
detailedMessage:
91+
"An unknown error occurred. Please refer to Console logs to get more information.",
92+
};
93+
8894
const response: WebsocketResponse = JSON.parse(
8995
message.data.toString(),
9096
);
@@ -94,13 +100,10 @@ export const objectBrowserWSMiddleware = (
94100
return;
95101
}
96102

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

@@ -119,10 +122,15 @@ export const objectBrowserWSMiddleware = (
119122
);
120123

121124
if (!permitItems || permitItems.length === 0) {
125+
const errorMsg = response.error.APIError;
126+
122127
dispatch(
123128
setErrorSnackMessage({
124-
errorMessage: response.error,
125-
detailedError: response.error,
129+
errorMessage:
130+
errorMsg.message || basicErrorMessage.errorMessage,
131+
detailedError:
132+
errorMsg.detailedMessage ||
133+
basicErrorMessage.detailedMessage,
126134
}),
127135
);
128136
} else {
@@ -131,6 +139,19 @@ export const objectBrowserWSMiddleware = (
131139
}
132140

133141
return;
142+
} else if (response.error) {
143+
const errorMsg = response.error.APIError;
144+
145+
dispatch(setRequestInProgress(false));
146+
dispatch(
147+
setErrorSnackMessage({
148+
errorMessage:
149+
errorMsg.message || basicErrorMessage.errorMessage,
150+
detailedError:
151+
errorMsg.detailedMessage ||
152+
basicErrorMessage.detailedMessage,
153+
}),
154+
);
134155
}
135156

136157
// 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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ type ObjectsRequest struct {
4141

4242
type WSResponse struct {
4343
RequestID int64 `json:"request_id,omitempty"`
44-
Error string `json:"error,omitempty"`
44+
Error *CodedAPIError `json:"error,omitempty"`
4545
RequestEnd bool `json:"request_end,omitempty"`
4646
Prefix string `json:"prefix,omitempty"`
4747
BucketName string `json:"bucketName,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)