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.

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

Fixing #26 #31

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aquelatecnologia
Copy link

Server.bind send its info to h.context

Server.bind send its info to h.context
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 92ca5be on aquelatecnologia:master into 8dac975 on toymachiner62:master.

@coveralls
Copy link

coveralls commented Sep 23, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 3257183 on aquelatecnologia:master into 8dac975 on toymachiner62:master.

@aquelatecnologia
Copy link
Author

aquelatecnologia commented Sep 23, 2019

The first test that gives an error, the test code or test case have a problem.

it('Restricts access to protected route for multiple authorized roles that are not defined as plugin roles', (done) => {

At this point, options have hierarchy so, it will check if the role is within hierarchy and it will fail.

userRole: { role: 'ADMIN' }
requiredRole: [ 'USER', 'ADMIN' ]
hierarchy: [ 'OWNER', 'MANAGER', 'EMPLOYEE' ]

internals.isGranted = function(userRole, requiredRole, hierarchy) {

	let userRoles = null;

	// If we're using a hierarchy, get all the possible roles
	if(hierarchy) {
                 const index = hierarchy.indexOf(userRole);	// Get the index of userRole in the hierarchy

		// If the user's role is not any of the possible roles
		if (index === -1) {
			return false;  <==== This is where it fails.
		}

I am not sure if the hierarchy test should be made after the role validation or after. The tests were passing because hierarchy was always undefined as hapijs changed the server.bind behavior

@aquelatecnologia
Copy link
Author

Actually both tests case that fail have the same behavior.

@RicardoRdzG
Copy link

RicardoRdzG commented Oct 24, 2019

you should add the roles below in the hierarchy and not fail when the user role is not in the hierarchy definition

RicardoRdzG and others added 2 commits October 25, 2019 02:43
fix unit test
Update acl.js to fix unit test after bind fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants