Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Fix promise errors with store preAuth hook #527

Closed
wants to merge 2 commits into from
Closed

WIP: Fix promise errors with store preAuth hook #527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3b2b93
test(store-pre-auth-hook): check returned promise resolves without error
minrwhite Jan 4, 2017
bec85ad
fix(store-pre-auth-hook): Handle early exit states gracefully
minrwhite Jan 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 30 additions & 10 deletions lib/config/store/pre-auth-hook.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ function onStorePreAuth (request, reply) {
// PouchDB’s replication sends an initial GET to CouchDB root initially
var isGetRootPath = request.path === '/hoodie/store/api/' && request.method === 'get'
if (isGetRootPath) {
return reply.continue()
return new Promise(function (resolve) {
resolve(reply.continue())
})
}

// check if store is publically accessable
Expand All @@ -30,12 +32,12 @@ function onStorePreAuth (request, reply) {

.then(function (hasAccess) {
if (hasAccess) {
return reply.continue()
return throwContinue('You have permissions for DB access')
}

var sessionToken = toSessionToken(request)
if (!sessionToken) {
return reply(Boom.unauthorized())
return throwUnauthorized('No session token given')
}

return server.plugins.account.api.sessions.find(sessionToken)
Expand All @@ -51,21 +53,25 @@ function onStorePreAuth (request, reply) {
access: requiredAccess,
role: roles
})
})

.then(function (hasAccess) {
if (hasAccess) {
return reply.continue()
}
.then(function (hasAccess) {
if (hasAccess) {
return throwContinue('Given session has permissions for DB access')
}

reply(Boom.unauthorized())
})
return throwUnauthorized('No access to database at requested level with given roles')
})

.catch(function (error) {
if (error.status === 404) { // session not found
if (error.status === 404 || error.status === 401) { // session not found
return reply(Boom.unauthorized())
}

if (error.status === 201) { // continue
return reply.continue()
}

server.log(['store', 'error'], error.message)
reply(Boom.wrap(error, 500))
})
Expand All @@ -91,3 +97,17 @@ function isRead (request) {

return false
}

function throwContinue (message) {
throwResponse(message, 201)
}

function throwUnauthorized (message) {
throwResponse(message, 401)
}

function throwResponse (message, errorCode) {
var response = new Error(message)
response.status = errorCode
throw response
}
95 changes: 54 additions & 41 deletions test/unit/config/store-pre-auth-hook-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,11 @@ test('store pre auth hook', function (t) {
roles: []
}
}
var findSessionStub = simple.stub().returnWith({ // don’t use resolveWith to avoid async
then: function (callback) {
callback(session)
return {
catch: function () {}
}
}
})
var hasAccessStub = simple.stub().returnWith({ // don’t use resolveWith to avoid async
then: function (callback) {
callback(true)
return Promise.resolve()
}
})
var findSessionStub = simple.stub().resolveWith(session)
var hasAccessStub = simple.stub().resolveWith(true).resolveWith(false)

var serverStub = {
log: simple.stub(),
plugins: {
account: {
api: {
Expand All @@ -62,15 +51,20 @@ test('store pre auth hook', function (t) {
server: serverStub
}
}
var reply = {
continue: simple.stub()
}
var reply = simple.stub()
reply.continue = simple.stub()

preAuthHook(request, reply)
var output = preAuthHook(request, reply)

t.is(reply.continue.callCount, 1, 'reply.continue() called')
t.type(output, Promise, 'returns a Promise')

t.end()
return output.then(function () {
t.is(findSessionStub.callCount, 0, 'findSession should not be called')
t.is(hasAccessStub.callCount, 1, 'hasAccess should be called once only')
t.is(reply.continue.callCount, 1, 'reply.continue() called')
t.is(reply.callCount, 0, 'reply() should not be called')
t.end()
})
})

test('store pre auth hook root path', function (t) {
Expand All @@ -92,6 +86,7 @@ test('store pre auth hook root path', function (t) {
}
})
var serverStub = {
log: simple.stub(),
plugins: {
account: {
api: {
Expand All @@ -112,21 +107,25 @@ test('store pre auth hook root path', function (t) {
server: serverStub
}
}
var reply = {
continue: simple.stub()
}
var reply = simple.stub()
reply.continue = simple.stub()

preAuthHook(request, reply)
var output = preAuthHook(request, reply)

t.is(reply.continue.callCount, 1, 'reply.continue() called')
t.type(output, Promise, 'returns a Promise')

t.end()
return output.then(function () {
t.is(reply.continue.callCount, 1, 'reply.continue() called')
t.is(reply.callCount, 0, 'reply() should not be called')
t.end()
})
})

test('store pre auth hook no authorization header', function (t) {
var findSessionStub = simple.stub().rejectWith({status: 404})
var findSessionStub = simple.stub()
var hasAccessStub = simple.stub().resolveWith(false)
var serverStub = {
log: simple.stub(),
plugins: {
account: {
api: {
Expand All @@ -149,18 +148,26 @@ test('store pre auth hook no authorization header', function (t) {
server: serverStub
}
}
var reply = simple.stub()
reply.continue = simple.stub()

t.plan(2)
preAuthHook(request, function (error) {
t.ok(error)
t.is(error.message, 'unauthorized', 'throws unauthorized error')
})
t.plan(6)
return preAuthHook(request, reply)
.then(function () {
t.is(findSessionStub.callCount, 0, 'findSession() should not be called')
t.is(hasAccessStub.callCount, 1, 'hasAccess() should be called once only')
t.is(reply.continue.callCount, 0, 'reply.continue() should not be called')
t.is(reply.callCount, 1, 'reply() called once only')
t.ok(reply.calls[0].args[0])
t.is(reply.calls[0].args[0].message, 'unauthorized', 'throws unauthorized error')
})
})

test('store pre auth hook session not found error', function (t) {
var findSessionStub = simple.stub().rejectWith({status: 404})
var hasAccessStub = simple.stub().resolveWith(false)
var serverStub = {
log: simple.stub(),
plugins: {
account: {
api: {
Expand All @@ -185,12 +192,17 @@ test('store pre auth hook session not found error', function (t) {
server: serverStub
}
}
var reply = simple.stub()
reply.continue = simple.stub()

t.plan(2)
preAuthHook(request, function (error) {
t.ok(error)
t.is(error.message, 'unauthorized', 'throws unauthorized error')
})
t.plan(4)
preAuthHook(request, reply)
.then(function () {
t.is(reply.continue.callCount, 0, 'reply.continue() should not be called')
t.is(reply.callCount, 1, 'reply() called once only')
t.ok(reply.calls[0].args[0])
t.is(reply.calls[0].args[0].message, 'unauthorized', 'throws unauthorized error')
})
})

test('store pre auth hook not public access & session found', function (t) {
Expand All @@ -209,6 +221,7 @@ test('store pre auth hook not public access & session found', function (t) {
return Promise.resolve(false) // not public access
})
var serverStub = {
log: simple.stub(),
plugins: {
account: {
api: {
Expand All @@ -235,7 +248,7 @@ test('store pre auth hook not public access & session found', function (t) {
}

t.plan(1)
preAuthHook(request, {
return preAuthHook(request, {
continue: function () {
t.pass('all good')
}
Expand Down Expand Up @@ -285,7 +298,7 @@ test('store pre auth hook read-only byy users for POST db/_all_docs', function (
}

t.plan(1)
preAuthHook(request, {
return preAuthHook(request, {
continue: function () {
t.pass('all good')
}
Expand Down Expand Up @@ -331,7 +344,7 @@ test('store pre auth hook unauthorized error', function (t) {
}

t.plan(2)
preAuthHook(request, function (error) {
return preAuthHook(request, function (error) {
t.ok(error)
t.is(error.message, 'unauthorized', 'throws unauthorized error')
})
Expand Down Expand Up @@ -377,7 +390,7 @@ test('store pre auth hook server error', function (t) {
}

t.plan(3)
preAuthHook(request, function (error) {
return preAuthHook(request, function (error) {
t.ok(error)
t.is(error.status, 500, 'throws 500 error')
t.is(error.message, 'ooops', 'oooopsie')
Expand Down