fix browser hang when listobjects is denied (#6385)

When Deny ListObjects policy is set on bucket, the browser
is hanging while trying to list the object. This is now
fixed by showing appropriate error message and an
empty object list.

Fixes #6371
master
Kanagaraj Mayilsamy 6 years ago committed by kannappanr
parent d13bd5b9b5
commit 2c46214291
  1. 77
      browser/app/js/objects/__tests__/actions.test.js
  2. 16
      browser/app/js/objects/actions.js
  3. 7
      browser/app/js/objects/reducer.js
  4. 63
      browser/ui-assets.go

@ -19,16 +19,27 @@ import thunk from "redux-thunk"
import * as actionsObjects from "../actions" import * as actionsObjects from "../actions"
import * as alertActions from "../../alert/actions" import * as alertActions from "../../alert/actions"
import { minioBrowserPrefix } from "../../constants" import { minioBrowserPrefix } from "../../constants"
import history from "../../history"
jest.mock("../../web", () => ({ jest.mock("../../web", () => ({
LoggedIn: jest.fn(() => true).mockReturnValueOnce(false), LoggedIn: jest
ListObjects: jest.fn(() => { .fn(() => true)
return Promise.resolve({ .mockReturnValueOnce(true)
objects: [{ name: "test1" }, { name: "test2" }], .mockReturnValueOnce(false)
istruncated: false, .mockReturnValueOnce(false),
nextmarker: "test2", ListObjects: jest.fn(({ bucketName }) => {
writable: false if (bucketName === "test-deny") {
}) return Promise.reject({
message: "listobjects is denied"
})
} else {
return Promise.resolve({
objects: [{ name: "test1" }, { name: "test2" }],
istruncated: false,
nextmarker: "test2",
writable: false
})
}
}), }),
RemoveObject: jest.fn(({ bucketName, objects }) => { RemoveObject: jest.fn(({ bucketName, objects }) => {
if (!bucketName) { if (!bucketName) {
@ -112,6 +123,9 @@ describe("Objects actions", () => {
objects: { currentPrefix: "" } objects: { currentPrefix: "" }
}) })
const expectedActions = [ const expectedActions = [
{
type: "alert/CLEAR"
},
{ {
type: "objects/SET_LIST", type: "objects/SET_LIST",
objects: [{ name: "test1" }, { name: "test2" }], objects: [{ name: "test1" }, { name: "test2" }],
@ -143,6 +157,9 @@ describe("Objects actions", () => {
objects: { currentPrefix: "" } objects: { currentPrefix: "" }
}) })
const expectedActions = [ const expectedActions = [
{
type: "alert/CLEAR"
},
{ {
type: "objects/APPEND_LIST", type: "objects/APPEND_LIST",
objects: [{ name: "test1" }, { name: "test2" }], objects: [{ name: "test1" }, { name: "test2" }],
@ -160,6 +177,43 @@ describe("Objects actions", () => {
}) })
}) })
it("creates objects/RESET_LIST after failing to fetch the objects from bucket with ListObjects denied for LoggedIn users", () => {
const store = mockStore({
buckets: { currentBucket: "test-deny" },
objects: { currentPrefix: "" }
})
const expectedActions = [
{
type: "alert/CLEAR"
},
{
type: "alert/SET",
alert: {
type: "danger",
message: "listobjects is denied",
id: alertActions.alertId
}
},
{
type: "object/RESET_LIST"
}
]
return store.dispatch(actionsObjects.fetchObjects()).then(() => {
const actions = store.getActions()
expect(actions).toEqual(expectedActions)
})
})
it("redirect to login after failing to fetch the objects from bucket for non-LoggedIn users", () => {
const store = mockStore({
buckets: { currentBucket: "test-deny" },
objects: { currentPrefix: "" }
})
return store.dispatch(actionsObjects.fetchObjects()).then(() => {
expect(history.location.pathname.endsWith("/login")).toBeTruthy()
})
})
it("creates objects/SET_SORT_BY and objects/SET_SORT_ORDER when sortObjects is called", () => { it("creates objects/SET_SORT_BY and objects/SET_SORT_ORDER when sortObjects is called", () => {
const store = mockStore({ const store = mockStore({
objects: { objects: {
@ -198,6 +252,7 @@ describe("Objects actions", () => {
}) })
const expectedActions = [ const expectedActions = [
{ type: "objects/SET_CURRENT_PREFIX", prefix: "abc/" }, { type: "objects/SET_CURRENT_PREFIX", prefix: "abc/" },
{ type: "alert/CLEAR" },
{ type: "objects/CHECKED_LIST_RESET" } { type: "objects/CHECKED_LIST_RESET" }
] ]
store.dispatch(actionsObjects.selectPrefix("abc/")) store.dispatch(actionsObjects.selectPrefix("abc/"))
@ -244,7 +299,11 @@ describe("Objects actions", () => {
const expectedActions = [ const expectedActions = [
{ {
type: "alert/SET", type: "alert/SET",
alert: { type: "danger", message: "Invalid bucket", id: 0 } alert: {
type: "danger",
message: "Invalid bucket",
id: alertActions.alertId
}
} }
] ]
return store.dispatch(actionsObjects.deleteObject("obj1")).then(() => { return store.dispatch(actionsObjects.deleteObject("obj1")).then(() => {

@ -24,9 +24,11 @@ import {
import { getCurrentBucket } from "../buckets/selectors" import { getCurrentBucket } from "../buckets/selectors"
import { getCurrentPrefix, getCheckedList } from "./selectors" import { getCurrentPrefix, getCheckedList } from "./selectors"
import * as alertActions from "../alert/actions" import * as alertActions from "../alert/actions"
import * as bucketActions from "../buckets/actions"
import { minioBrowserPrefix } from "../constants" import { minioBrowserPrefix } from "../constants"
export const SET_LIST = "objects/SET_LIST" export const SET_LIST = "objects/SET_LIST"
export const RESET_LIST = "object/RESET_LIST"
export const APPEND_LIST = "objects/APPEND_LIST" export const APPEND_LIST = "objects/APPEND_LIST"
export const REMOVE = "objects/REMOVE" export const REMOVE = "objects/REMOVE"
export const SET_SORT_BY = "objects/SET_SORT_BY" export const SET_SORT_BY = "objects/SET_SORT_BY"
@ -45,6 +47,10 @@ export const setList = (objects, marker, isTruncated) => ({
isTruncated isTruncated
}) })
export const resetList = () => ({
type: RESET_LIST
})
export const appendList = (objects, marker, isTruncated) => ({ export const appendList = (objects, marker, isTruncated) => ({
type: APPEND_LIST, type: APPEND_LIST,
objects, objects,
@ -58,6 +64,7 @@ export const fetchObjects = append => {
buckets: { currentBucket }, buckets: { currentBucket },
objects: { currentPrefix, marker } objects: { currentPrefix, marker }
} = getState() } = getState()
dispatch(alertActions.clear())
if (currentBucket) { if (currentBucket) {
return web return web
.ListObjects({ .ListObjects({
@ -85,8 +92,13 @@ export const fetchObjects = append => {
dispatch(setPrefixWritable(res.writable)) dispatch(setPrefixWritable(res.writable))
}) })
.catch(err => { .catch(err => {
dispatch(alertActions.set({ type: "danger", message: err.message })) if (web.LoggedIn()) {
history.push("/login") dispatch(alertActions.set({ type: "danger", message: err.message }))
dispatch(resetList())
}
else {
history.push("/login")
}
}) })
} }
} }

@ -50,6 +50,13 @@ export default (
marker: action.marker, marker: action.marker,
isTruncated: action.isTruncated isTruncated: action.isTruncated
} }
case actionsObjects.RESET_LIST:
return {
...state,
list: [],
marker: "",
isTruncated: false
}
case actionsObjects.APPEND_LIST: case actionsObjects.APPEND_LIST:
return { return {
...state, ...state,

File diff suppressed because one or more lines are too long
Loading…
Cancel
Save