Skip to content

Commit 963c8f1

Browse files
authored
Added missing permissions validation to rewind button (#3282)
Signed-off-by: Benjamin Perez <benjamin@bexsoft.net>
1 parent 6c50c38 commit 963c8f1

File tree

10 files changed

+217
-14
lines changed

10 files changed

+217
-14
lines changed

.github/workflows/vulncheck.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ jobs:
2222
- name: Set up Go
2323
uses: actions/setup-go@v3
2424
with:
25-
go-version: 1.21.8
25+
go-version: 1.21.9
2626
check-latest: true
2727
- name: Get official govulncheck
2828
run: go install golang.org/x/vuln/cmd/govulncheck@latest

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ require (
3333
github.com/tidwall/gjson v1.17.1
3434
github.com/unrolled/secure v1.14.0
3535
golang.org/x/crypto v0.21.0
36-
golang.org/x/net v0.22.0
36+
golang.org/x/net v0.23.0
3737
golang.org/x/oauth2 v0.18.0
3838
// Added to include security fix for
3939
// https://github.com/golang/go/issues/56152

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,8 @@ golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug
352352
golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
353353
golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
354354
golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44=
355-
golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc=
356-
golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
355+
golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs=
356+
golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
357357
golang.org/x/oauth2 v0.18.0 h1:09qnuIAgzdx1XplqJvW6CQqMCtGZykZWcXzPMPUusvI=
358358
golang.org/x/oauth2 v0.18.0/go.mod h1:Wf7knwG0MPoWIMMBgFlEaSUDaKskp0dCfrlJRJXbBi8=
359359
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=

web-app/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/ListObjects.tsx

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import get from "lodash/get";
2626
import {
2727
AccessRuleIcon,
2828
ActionsList,
29+
Badge,
2930
Box,
3031
BucketsIcon,
3132
Button,
@@ -39,7 +40,6 @@ import {
3940
RefreshIcon,
4041
ScreenTitle,
4142
ShareIcon,
42-
Badge,
4343
} from "mds";
4444
import { api } from "api";
4545
import { errorToHandler } from "api/errors";
@@ -274,6 +274,11 @@ const ListObjects = () => {
274274
[pathAsResourceInPolicy, ...sessionGrantWildCards],
275275
[IAM_SCOPES.S3_GET_OBJECT, IAM_SCOPES.S3_GET_ACTIONS],
276276
);
277+
const canRewind = hasPermission(bucketName, [
278+
IAM_SCOPES.S3_GET_OBJECT,
279+
IAM_SCOPES.S3_GET_ACTIONS,
280+
IAM_SCOPES.S3_GET_BUCKET_VERSIONING,
281+
]);
277282
const canDelete = hasPermission(
278283
[pathAsResourceInPolicy, ...sessionGrantWildCards],
279284
[IAM_SCOPES.S3_DELETE_OBJECT],
@@ -1057,7 +1062,20 @@ const ListObjects = () => {
10571062
actions={
10581063
<Fragment>
10591064
{!anonymousMode && (
1060-
<TooltipWrapper tooltip={"Rewind Bucket"}>
1065+
<TooltipWrapper
1066+
tooltip={
1067+
canRewind
1068+
? "Rewind Bucket"
1069+
: permissionTooltipHelper(
1070+
[
1071+
IAM_SCOPES.S3_GET_OBJECT,
1072+
IAM_SCOPES.S3_GET_ACTIONS,
1073+
IAM_SCOPES.S3_GET_BUCKET_VERSIONING,
1074+
],
1075+
"apply rewind in this bucket",
1076+
)
1077+
}
1078+
>
10611079
<Button
10621080
id={"rewind-objects-list"}
10631081
label={"Rewind"}
@@ -1078,13 +1096,7 @@ const ListObjects = () => {
10781096
onClick={() => {
10791097
setRewindSelect(true);
10801098
}}
1081-
disabled={
1082-
!isVersioningApplied ||
1083-
!hasPermission(bucketName, [
1084-
IAM_SCOPES.S3_GET_OBJECT,
1085-
IAM_SCOPES.S3_GET_ACTIONS,
1086-
])
1087-
}
1099+
disabled={!isVersioningApplied || !canRewind}
10881100
/>
10891101
</TooltipWrapper>
10901102
)}

web-app/tests/permissions-8/rewind.ts

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,20 @@
1717
import * as roles from "../utils/roles";
1818
import * as elements from "../utils/elements";
1919
import * as functions from "../utils/functions";
20-
import { testBucketBrowseButtonFor } from "../utils/functions";
20+
import {
21+
namedTestBucketBrowseButtonFor,
22+
setUpNamedBucket,
23+
setVersionedBucket,
24+
testBucketBrowseButtonFor,
25+
} from "../utils/functions";
26+
import { Selector } from "testcafe";
27+
import { deniedError, file } from "../permissions-6/resourceTesting";
2128

2229
fixture("Rewind Testing").page("http://localhost:9090");
2330

31+
const bucketname = "bucketname";
32+
const test3BucketBrowseButton = namedTestBucketBrowseButtonFor(bucketname);
33+
2434
test
2535
.before(async (t) => {
2636
// Create a bucket
@@ -57,3 +67,80 @@ test
5767
// Cleanup created bucket and corresponding uploads
5868
await functions.cleanUpBucketAndUploads(t, "abucketrewind");
5969
});
70+
71+
test
72+
.before(async (t) => {
73+
await functions.setUpNamedBucket(t, bucketname);
74+
await functions.setVersionedBucket(t, bucketname);
75+
await functions.uploadNamedObjectToBucket(
76+
t,
77+
bucketname,
78+
"test.txt",
79+
"web-app/tests/uploads/test.txt",
80+
);
81+
await functions.uploadNamedObjectToBucket(
82+
t,
83+
bucketname,
84+
"firstlevel/secondlevel/test.txt",
85+
"web-app/tests/uploads/test.txt",
86+
);
87+
})("Rewind button enabled in bucket", async (t) => {
88+
await t
89+
.useRole(roles.rewindEnabled)
90+
.navigateTo(`http://localhost:9090/browser`)
91+
.click(test3BucketBrowseButton)
92+
.wait(1500)
93+
.click(
94+
Selector(".ReactVirtualized__Table__rowColumn").withText("firstlevel"),
95+
)
96+
.wait(1500)
97+
.click(
98+
Selector(".ReactVirtualized__Table__rowColumn").withText("secondlevel"),
99+
)
100+
.wait(1500)
101+
.expect(elements.rewindButton.exists)
102+
.ok();
103+
})
104+
.after(async (t) => {
105+
await functions.cleanUpNamedBucketAndUploads(t, bucketname);
106+
});
107+
108+
test
109+
.before(async (t) => {
110+
await functions.setUpNamedBucket(t, bucketname);
111+
await functions.setVersionedBucket(t, bucketname);
112+
await functions.uploadNamedObjectToBucket(
113+
t,
114+
bucketname,
115+
"test.txt",
116+
"web-app/tests/uploads/test.txt",
117+
);
118+
await functions.uploadNamedObjectToBucket(
119+
t,
120+
bucketname,
121+
"firstlevel/secondlevel/test.txt",
122+
"web-app/tests/uploads/test.txt",
123+
);
124+
})("Rewind button disabled in bucket", async (t) => {
125+
await t
126+
.useRole(roles.rewindNotEnabled)
127+
.navigateTo(`http://localhost:9090/browser`)
128+
.click(test3BucketBrowseButton)
129+
.wait(1500)
130+
.click(
131+
Selector(".ReactVirtualized__Table__rowColumn").withText("firstlevel"),
132+
)
133+
.wait(1500)
134+
.click(
135+
Selector(".ReactVirtualized__Table__rowColumn").withText("secondlevel"),
136+
)
137+
.wait(1500)
138+
.expect(elements.rewindButton.exists)
139+
.ok()
140+
.wait(1500)
141+
.expect(elements.rewindButton.hasAttribute("disabled"))
142+
.ok();
143+
})
144+
.after(async (t) => {
145+
await functions.cleanUpNamedBucketAndUploads(t, bucketname);
146+
});
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
{
2+
"Version": "2012-10-17",
3+
"Statement": [
4+
{
5+
"Effect": "Deny",
6+
"Action": ["s3:CreateBucket", "s3:DeleteBucket"],
7+
"Resource": ["arn:aws:s3:::*"]
8+
},
9+
{
10+
"Effect": "Allow",
11+
"Action": ["s3:ListBucket"],
12+
"Resource": ["arn:aws:s3:::bucketname"]
13+
},
14+
{
15+
"Effect": "Allow",
16+
"Action": ["s3:GetBucketLocation", "s3:GetBucketVersioning"],
17+
"Resource": ["arn:aws:s3:::bucketname"]
18+
},
19+
{
20+
"Effect": "Allow",
21+
"Action": ["s3:GetObject"],
22+
"Resource": [
23+
"arn:aws:s3:::bucketname/firstlevel",
24+
"arn:aws:s3:::bucketname/firstlevel/*"
25+
]
26+
},
27+
{
28+
"Effect": "Allow",
29+
"Action": ["s3:*"],
30+
"Resource": [
31+
"arn:aws:s3:::bucketname/firstlevel/secondlevel*",
32+
"arn:aws:s3:::bucketname/firstlevel/secondlevel/*"
33+
]
34+
}
35+
]
36+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
{
2+
"Version": "2012-10-17",
3+
"Statement": [
4+
{
5+
"Effect": "Deny",
6+
"Action": ["s3:CreateBucket", "s3:DeleteBucket"],
7+
"Resource": ["arn:aws:s3:::*"]
8+
},
9+
{
10+
"Effect": "Allow",
11+
"Action": ["s3:ListBucket"],
12+
"Resource": ["arn:aws:s3:::bucketname"]
13+
},
14+
{
15+
"Effect": "Allow",
16+
"Action": ["s3:GetBucketLocation"],
17+
"Resource": ["arn:aws:s3:::bucketname"]
18+
},
19+
{
20+
"Effect": "Allow",
21+
"Action": ["s3:GetObject"],
22+
"Resource": [
23+
"arn:aws:s3:::bucketname/firstlevel",
24+
"arn:aws:s3:::bucketname/firstlevel/*"
25+
]
26+
},
27+
{
28+
"Effect": "Allow",
29+
"Action": ["s3:*"],
30+
"Resource": [
31+
"arn:aws:s3:::bucketname/firstlevel/secondlevel*",
32+
"arn:aws:s3:::bucketname/firstlevel/secondlevel/*"
33+
]
34+
}
35+
]
36+
}

web-app/tests/scripts/cleanup-env.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ remove_users() {
3232
mc admin user remove minio conditions-2-$TIMESTAMP
3333
mc admin user remove minio conditions-3-$TIMESTAMP
3434
mc admin user remove minio conditions-4-$TIMESTAMP
35+
mc admin user remove minio rewind-allowed-$TIMESTAMP
36+
mc admin user remove minio rewind-not-allowed-$TIMESTAMP
3537
}
3638

3739
remove_policies() {
@@ -58,6 +60,8 @@ remove_policies() {
5860
mc admin policy remove minio conditions-policy-2-$TIMESTAMP
5961
mc admin policy remove minio conditions-policy-3-$TIMESTAMP
6062
mc admin policy remove minio conditions-policy-4-$TIMESTAMP
63+
mc admin policy remove minio rewind-allowed-$TIMESTAMP
64+
mc admin policy remove minio rewind-not-allowed-$TIMESTAMP
6165
}
6266

6367
__init__() {

web-app/tests/scripts/common.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ create_policies() {
4747
mc admin policy create minio conditions-policy-2-$TIMESTAMP web-app/tests/policies/conditionsPolicy2.json
4848
mc admin policy create minio conditions-policy-3-$TIMESTAMP web-app/tests/policies/conditionsPolicy3.json
4949
mc admin policy create minio conditions-policy-4-$TIMESTAMP web-app/tests/policies/conditionsPolicy4.json
50+
mc admin policy create minio rewind-allowed-$TIMESTAMP web-app/tests/policies/rewind-allowed.json
51+
mc admin policy create minio rewind-not-allowed-$TIMESTAMP web-app/tests/policies/rewind-not-allowed.json
5052
}
5153

5254
create_users() {
@@ -78,6 +80,8 @@ create_users() {
7880
mc admin user add minio conditions-2-$TIMESTAMP conditions1234
7981
mc admin user add minio conditions-3-$TIMESTAMP conditions1234
8082
mc admin user add minio conditions-4-$TIMESTAMP conditions1234
83+
mc admin user add minio rewind-allowed-$TIMESTAMP rewindallowed1234
84+
mc admin user add minio rewind-not-allowed-$TIMESTAMP rewindnotallowed1234
8185
}
8286

8387
create_buckets() {
@@ -114,4 +118,6 @@ assign_policies() {
114118
mc admin policy attach minio conditions-policy-2-$TIMESTAMP --user conditions-2-$TIMESTAMP
115119
mc admin policy attach minio conditions-policy-3-$TIMESTAMP --user conditions-3-$TIMESTAMP
116120
mc admin policy attach minio conditions-policy-4-$TIMESTAMP --user conditions-4-$TIMESTAMP
121+
mc admin policy attach minio rewind-allowed-$TIMESTAMP --user rewind-allowed-$TIMESTAMP
122+
mc admin policy attach minio rewind-not-allowed-$TIMESTAMP --user rewind-not-allowed-$TIMESTAMP
117123
}

web-app/tests/utils/roles.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,3 +294,25 @@ export const conditions4 = Role(
294294
},
295295
{ preserveUrl: true },
296296
);
297+
298+
export const rewindEnabled = Role(
299+
loginUrl,
300+
async (t) => {
301+
await t
302+
.typeText("#accessKey", "rewind-allowed-" + unixTimestamp)
303+
.typeText("#secretKey", "rewindallowed1234")
304+
.click(submitButton);
305+
},
306+
{ preserveUrl: true },
307+
);
308+
309+
export const rewindNotEnabled = Role(
310+
loginUrl,
311+
async (t) => {
312+
await t
313+
.typeText("#accessKey", "rewind-not-allowed-" + unixTimestamp)
314+
.typeText("#secretKey", "rewindnotallowed1234")
315+
.click(submitButton);
316+
},
317+
{ preserveUrl: true },
318+
);

0 commit comments

Comments
 (0)