1
0
Fork 0

issue 459.2: ensure view retrieval is denied of detailed definition is denied (#6237)

* issue 459.2: ensure view retrieval is denied of detailed definition is denied

* address test failures
This commit is contained in:
Vasiliy 2018-08-24 14:37:35 +03:00 committed by Andrey Abramov
parent 2bce75eb86
commit 2ecdfefcb8
6 changed files with 209 additions and 3 deletions

View File

@ -83,6 +83,25 @@ void RestViewHandler::getView(std::string const& nameOrId, bool detailed) {
return;
}
// skip views for which the full view definition cannot be generated, as per https://github.com/arangodb/backlog/issues/459
try {
arangodb::velocypack::Builder viewBuilder;
viewBuilder.openObject();
auto res = view->toVelocyPack(viewBuilder, true, false);
if (!res.ok()) {
generateError(res);
return; // skip view
}
} catch(...) {
generateError(arangodb::Result(TRI_ERROR_INTERNAL));
return; // skip view
}
arangodb::velocypack::Builder builder;
builder.openObject();
@ -436,6 +455,19 @@ void RestViewHandler::getViews() {
continue; // skip views that are not authorised to be read
}
// skip views for which the full view definition cannot be generated, as per https://github.com/arangodb/backlog/issues/459
try {
arangodb::velocypack::Builder viewBuilder;
viewBuilder.openObject();
if (!view->toVelocyPack(viewBuilder, true, false).ok()) {
continue; // skip view
}
} catch(...) {
continue; // skip view
}
arangodb::velocypack::Builder viewBuilder;
viewBuilder.openObject();

View File

@ -384,6 +384,21 @@ static void JS_ViewVocbase(v8::FunctionCallbackInfo<v8::Value> const& args) {
TRI_V8_THROW_EXCEPTION_MESSAGE(TRI_ERROR_FORBIDDEN, "insufficient rights to get view");
}
// skip views for which the full view definition cannot be generated, as per https://github.com/arangodb/backlog/issues/459
try {
arangodb::velocypack::Builder viewBuilder;
viewBuilder.openObject();
auto res = view->toVelocyPack(viewBuilder, true, false);
if (!res.ok()) {
TRI_V8_THROW_EXCEPTION(res); // skip view
}
} catch(...) {
TRI_V8_THROW_EXCEPTION(TRI_ERROR_INTERNAL); // skip view
}
v8::Handle<v8::Value> result = WrapView(isolate, view);
if (result.IsEmpty()) {
@ -425,6 +440,7 @@ static void JS_ViewsVocbase(v8::FunctionCallbackInfo<v8::Value> const& args) {
// already create an array of the correct size
v8::Handle<v8::Array> result = v8::Array::New(isolate);
uint32_t entry = 0;
size_t const n = views.size();
for (size_t i = 0; i < n; ++i) {
@ -435,6 +451,19 @@ static void JS_ViewsVocbase(v8::FunctionCallbackInfo<v8::Value> const& args) {
continue; // skip views that are not authorised to be read
}
// skip views for which the full view definition cannot be generated, as per https://github.com/arangodb/backlog/issues/459
try {
arangodb::velocypack::Builder viewBuilder;
viewBuilder.openObject();
if (!view->toVelocyPack(viewBuilder, true, false).ok()) {
continue; // skip view
}
} catch(...) {
continue; // skip view
}
v8::Handle<v8::Value> c = WrapView(isolate, view);
if (c.IsEmpty()) {
@ -442,7 +471,7 @@ static void JS_ViewsVocbase(v8::FunctionCallbackInfo<v8::Value> const& args) {
break;
}
result->Set(static_cast<uint32_t>(i), c);
result->Set(entry++, c);
}
if (error) {

View File

@ -308,8 +308,8 @@ function hasIResearch (db) {
if (dbLevel['rw'].has(name)) {
tasks.register(task);
wait(keySpaceId, name);
expect(getKey(keySpaceId, `${name}_status`)).to.equal(true, `${name} could not update the view with sufficient rights`);
expect(rootTestView(testViewRename)).to.equal(true, 'View renaming reported success, but updated view was not found afterwards');
expect(getKey(keySpaceId, `${name}_status`)).to.equal(colLevel['ro'].has(name) || colLevel['rw'].has(name), `${name} could not update the view with sufficient rights`);
expect(rootTestView(testViewRename)).to.equal(colLevel['ro'].has(name) || colLevel['rw'].has(name), 'View renaming reported success, but updated view was not found afterwards');
} else {
try {
tasks.register(task);

View File

@ -249,6 +249,8 @@ function hasIResearch (db) {
//FIXME: remove try/catch block after renaming will work in cluster
if (e.code === 404 && (e.errorNum === 1203 || e.errorNum === 1470)) {
return;
} else if (e.code === 403) {
return; // not authorised is valid if a non-read collection is present in the view
} else {
throw e;
}

View File

@ -712,6 +712,27 @@ SECTION("test_auth") {
CHECK((slice.hasKey(arangodb::StaticStrings::ErrorNum) && slice.get(arangodb::StaticStrings::ErrorNum).isNumber<int>() && TRI_ERROR_FORBIDDEN == slice.get(arangodb::StaticStrings::ErrorNum).getNumber<int>()));
}
// not authorized (failed detailed toVelocyPack(...)) as per https://github.com/arangodb/backlog/issues/459
{
arangodb::auth::UserMap userMap;
auto& user = userMap.emplace("", arangodb::auth::User::newUser("", "", arangodb::auth::Source::LDAP)).first->second;
user.grantDatabase(vocbase.name(), arangodb::auth::Level::RO);
user.grantCollection(vocbase.name(), "testView", arangodb::auth::Level::NONE); // for missing collections User::collectionAuthLevel(...) returns database auth::Level
userManager->setAuthInfo(userMap); // set user map to avoid loading configuration from system database
auto* testView = arangodb::LogicalView::cast<TestView>(logicalView.get());
testView->_appendVelocyPackResult = arangodb::Result(TRI_ERROR_FORBIDDEN);
auto resetAppendVelocyPackResult = std::shared_ptr<TestView>(testView, [](TestView* p)->void { p->_appendVelocyPackResult = arangodb::Result(); });
auto status = handler.execute();
CHECK((arangodb::RestStatus::DONE == status));
CHECK((arangodb::rest::ResponseCode::FORBIDDEN == responce.responseCode()));
auto slice = responce._payload.slice();
CHECK((slice.isObject()));
CHECK((slice.hasKey(arangodb::StaticStrings::Code) && slice.get(arangodb::StaticStrings::Code).isNumber<size_t>() && size_t(arangodb::rest::ResponseCode::FORBIDDEN) == slice.get(arangodb::StaticStrings::Code).getNumber<size_t>()));
CHECK((slice.hasKey(arangodb::StaticStrings::Error) && slice.get(arangodb::StaticStrings::Error).isBoolean() && true == slice.get(arangodb::StaticStrings::Error).getBoolean()));
CHECK((slice.hasKey(arangodb::StaticStrings::ErrorNum) && slice.get(arangodb::StaticStrings::ErrorNum).isNumber<int>() && TRI_ERROR_FORBIDDEN == slice.get(arangodb::StaticStrings::ErrorNum).getNumber<int>()));
}
// authorized (NONE view) as per https://github.com/arangodb/backlog/issues/459
{
arangodb::auth::UserMap userMap;
@ -806,6 +827,27 @@ SECTION("test_auth") {
CHECK((slice.hasKey(arangodb::StaticStrings::ErrorNum) && slice.get(arangodb::StaticStrings::ErrorNum).isNumber<int>() && TRI_ERROR_FORBIDDEN == slice.get(arangodb::StaticStrings::ErrorNum).getNumber<int>()));
}
// not authorized (failed detailed toVelocyPack(...))
{
arangodb::auth::UserMap userMap;
auto& user = userMap.emplace("", arangodb::auth::User::newUser("", "", arangodb::auth::Source::LDAP)).first->second;
user.grantDatabase(vocbase.name(), arangodb::auth::Level::RO);
user.grantCollection(vocbase.name(), "testView", arangodb::auth::Level::NONE); // for missing collections User::collectionAuthLevel(...) returns database auth::Level
userManager->setAuthInfo(userMap); // set user map to avoid loading configuration from system database
auto* testView = arangodb::LogicalView::cast<TestView>(logicalView.get());
testView->_appendVelocyPackResult = arangodb::Result(TRI_ERROR_FORBIDDEN);
auto resetAppendVelocyPackResult = std::shared_ptr<TestView>(testView, [](TestView* p)->void { p->_appendVelocyPackResult = arangodb::Result(); });
auto status = handler.execute();
CHECK((arangodb::RestStatus::DONE == status));
CHECK((arangodb::rest::ResponseCode::FORBIDDEN == responce.responseCode()));
auto slice = responce._payload.slice();
CHECK((slice.isObject()));
CHECK((slice.hasKey(arangodb::StaticStrings::Code) && slice.get(arangodb::StaticStrings::Code).isNumber<size_t>() && size_t(arangodb::rest::ResponseCode::FORBIDDEN) == slice.get(arangodb::StaticStrings::Code).getNumber<size_t>()));
CHECK((slice.hasKey(arangodb::StaticStrings::Error) && slice.get(arangodb::StaticStrings::Error).isBoolean() && true == slice.get(arangodb::StaticStrings::Error).getBoolean()));
CHECK((slice.hasKey(arangodb::StaticStrings::ErrorNum) && slice.get(arangodb::StaticStrings::ErrorNum).isNumber<int>() && TRI_ERROR_FORBIDDEN == slice.get(arangodb::StaticStrings::ErrorNum).getNumber<int>()));
}
// authorized (NONE view) as per https://github.com/arangodb/backlog/issues/459
{
arangodb::auth::UserMap userMap;
@ -901,6 +943,34 @@ SECTION("test_auth") {
CHECK((slice.hasKey(arangodb::StaticStrings::ErrorNum) && slice.get(arangodb::StaticStrings::ErrorNum).isNumber<int>() && TRI_ERROR_FORBIDDEN == slice.get(arangodb::StaticStrings::ErrorNum).getNumber<int>()));
}
// not authorized (failed detailed toVelocyPack(...)) as per https://github.com/arangodb/backlog/issues/459
{
arangodb::auth::UserMap userMap;
auto& user = userMap.emplace("", arangodb::auth::User::newUser("", "", arangodb::auth::Source::LDAP)).first->second;
user.grantDatabase(vocbase.name(), arangodb::auth::Level::RO);
user.grantCollection(vocbase.name(), "testView1", arangodb::auth::Level::NONE); // for missing collections User::collectionAuthLevel(...) returns database auth::Level
user.grantCollection(vocbase.name(), "testView2", arangodb::auth::Level::NONE); // for missing collections User::collectionAuthLevel(...) returns database auth::Level
userManager->setAuthInfo(userMap); // set user map to avoid loading configuration from system database
auto* testView = arangodb::LogicalView::cast<TestView>(logicalView2.get());
testView->_appendVelocyPackResult = arangodb::Result(TRI_ERROR_FORBIDDEN);
auto resetAppendVelocyPackResult = std::shared_ptr<TestView>(testView, [](TestView* p)->void { p->_appendVelocyPackResult = arangodb::Result(); });
auto status = handler.execute();
CHECK((arangodb::RestStatus::DONE == status));
CHECK((arangodb::rest::ResponseCode::OK == responce.responseCode()));
auto slice = responce._payload.slice();
CHECK((slice.isObject()));
CHECK((slice.hasKey(arangodb::StaticStrings::Code) && slice.get(arangodb::StaticStrings::Code).isNumber<size_t>() && size_t(arangodb::rest::ResponseCode::OK) == slice.get(arangodb::StaticStrings::Code).getNumber<size_t>()));
CHECK((slice.hasKey(arangodb::StaticStrings::Error) && slice.get(arangodb::StaticStrings::Error).isBoolean() && false == slice.get(arangodb::StaticStrings::Error).getBoolean()));
CHECK((slice.hasKey("result")));
slice = slice.get("result");
CHECK((slice.isArray()));
CHECK((1U == slice.length()));
slice = slice.at(0);
CHECK((slice.isObject()));
CHECK((slice.hasKey(arangodb::StaticStrings::DataSourceName) && slice.get(arangodb::StaticStrings::DataSourceName).isString() && std::string("testView1") == slice.get(arangodb::StaticStrings::DataSourceName).copyString()));
}
// authorized (NONE view) as per https://github.com/arangodb/backlog/issues/459
{
arangodb::auth::UserMap userMap;

View File

@ -977,6 +977,30 @@ SECTION("test_auth") {
CHECK((false == !view));
}
// not authorized (failed detailed toVelocyPack(...)) as per https://github.com/arangodb/backlog/issues/459
{
arangodb::auth::UserMap userMap;
auto& user = userMap.emplace("", arangodb::auth::User::newUser("", "", arangodb::auth::Source::LDAP)).first->second;
user.grantDatabase(vocbase.name(), arangodb::auth::Level::RO);
user.grantCollection(vocbase.name(), "testView", arangodb::auth::Level::NONE); // for missing collections User::collectionAuthLevel(...) returns database auth::Level
userManager->setAuthInfo(userMap); // set user map to avoid loading configuration from system database
auto* testView = arangodb::LogicalView::cast<TestView>(logicalView.get());
testView->_appendVelocyPackResult = arangodb::Result(TRI_ERROR_FORBIDDEN);
auto resetAppendVelocyPackResult = std::shared_ptr<TestView>(testView, [](TestView* p)->void { p->_appendVelocyPackResult = arangodb::Result(); });
arangodb::velocypack::Builder responce;
v8::TryCatch tryCatch(isolate.get());
auto result = v8::Function::Cast(*fn_view)->CallAsFunction(context, fn_view, args.size(), args.data());
CHECK((result.IsEmpty()));
CHECK((tryCatch.HasCaught()));
CHECK((TRI_ERROR_NO_ERROR == TRI_V8ToVPack(isolate.get(), responce, tryCatch.Exception(), false)));
auto slice = responce.slice();
CHECK((slice.isObject()));
CHECK((slice.hasKey(arangodb::StaticStrings::ErrorNum) && slice.get(arangodb::StaticStrings::ErrorNum).isNumber<int>() && TRI_ERROR_FORBIDDEN == slice.get(arangodb::StaticStrings::ErrorNum).getNumber<int>()));
auto view = vocbase.lookupView("testView");
CHECK((false == !view));
}
// authorized (NONE view) as per https://github.com/arangodb/backlog/issues/459
{
arangodb::auth::UserMap userMap;
@ -1103,6 +1127,30 @@ SECTION("test_auth") {
CHECK((false == !view));
}
// not authorized (failed detailed toVelocyPack(...))
{
arangodb::auth::UserMap userMap;
auto& user = userMap.emplace("", arangodb::auth::User::newUser("", "", arangodb::auth::Source::LDAP)).first->second;
user.grantDatabase(vocbase.name(), arangodb::auth::Level::RO);
user.grantCollection(vocbase.name(), "testView", arangodb::auth::Level::NONE); // for missing collections User::collectionAuthLevel(...) returns database auth::Level
userManager->setAuthInfo(userMap); // set user map to avoid loading configuration from system database
auto* testView = arangodb::LogicalView::cast<TestView>(logicalView.get());
testView->_appendVelocyPackResult = arangodb::Result(TRI_ERROR_FORBIDDEN);
auto resetAppendVelocyPackResult = std::shared_ptr<TestView>(testView, [](TestView* p)->void { p->_appendVelocyPackResult = arangodb::Result(); });
arangodb::velocypack::Builder responce;
v8::TryCatch tryCatch(isolate.get());
auto result = v8::Function::Cast(*fn_properties)->CallAsFunction(context, arangoView, args.size(), args.data());
CHECK((result.IsEmpty()));
CHECK((tryCatch.HasCaught()));
CHECK((TRI_ERROR_NO_ERROR == TRI_V8ToVPack(isolate.get(), responce, tryCatch.Exception(), false)));
auto slice = responce.slice();
CHECK((slice.isObject()));
CHECK((slice.hasKey(arangodb::StaticStrings::ErrorNum) && slice.get(arangodb::StaticStrings::ErrorNum).isNumber<int>() && TRI_ERROR_FORBIDDEN == slice.get(arangodb::StaticStrings::ErrorNum).getNumber<int>()));
auto view = vocbase.lookupView("testView");
CHECK((false == !view));
}
// authorized (NONE view) as per https://github.com/arangodb/backlog/issues/459
{
arangodb::auth::UserMap userMap;
@ -1229,6 +1277,31 @@ SECTION("test_auth") {
CHECK((false == !view2));
}
// not authorized (failed detailed toVelocyPack(...)) as per https://github.com/arangodb/backlog/issues/459
{
arangodb::auth::UserMap userMap;
auto& user = userMap.emplace("", arangodb::auth::User::newUser("", "", arangodb::auth::Source::LDAP)).first->second;
user.grantDatabase(vocbase.name(), arangodb::auth::Level::RO);
user.grantCollection(vocbase.name(), "testView1", arangodb::auth::Level::NONE); // for missing collections User::collectionAuthLevel(...) returns database auth::Level
user.grantCollection(vocbase.name(), "testView2", arangodb::auth::Level::NONE); // for missing collections User::collectionAuthLevel(...) returns database auth::Level
userManager->setAuthInfo(userMap); // set user map to avoid loading configuration from system database
auto* testView = arangodb::LogicalView::cast<TestView>(logicalView2.get());
testView->_appendVelocyPackResult = arangodb::Result(TRI_ERROR_FORBIDDEN);
auto resetAppendVelocyPackResult = std::shared_ptr<TestView>(testView, [](TestView* p)->void { p->_appendVelocyPackResult = arangodb::Result(); });
auto result = v8::Function::Cast(*fn_views)->CallAsFunction(context, fn_views, args.size(), args.data());
CHECK((!result.IsEmpty()));
CHECK((result.ToLocalChecked()->IsArray()));
auto* resultArray = v8::Array::Cast(*result.ToLocalChecked());
CHECK((1U == resultArray->Length()));
auto v8View = *TRI_UnwrapClass<std::shared_ptr<arangodb::LogicalView>>(resultArray->Get(0).As<v8::Object>(), WRP_VOCBASE_VIEW_TYPE);
CHECK((false == !v8View));
CHECK((std::string("testView1") == v8View->name()));
CHECK((std::string("testViewType") == v8View->type().name()));
auto view1 = vocbase.lookupView("testView1");
CHECK((false == !view1));
}
// authorized (NONE view) as per https://github.com/arangodb/backlog/issues/459
{
arangodb::auth::UserMap userMap;