Securesite prompts continuously unless user has permission, regardless of filter setting

Would you please provide some more information about how I can reproduce this problem? It sounds like the path check is failing. Also, which authentication method are you using? And what server software?

Would it be possible for you to run the simpletests included with Secure Site and report any failures?

At first I could not reproduce this. This sounds more like a miss configuration but I need to give this more thought.

Secure site is not a node access control module. It’s an authentication and site access module. Your normal role / node access permission need to be setup correctly before implementing secure site.

The way I was able to reproduce this was by adding a user that did not have access site permission and then trying to login. Because the filter system is assumed to only be for anonymous users and any authenticated users are assumed to be using Drupals node access and role permissions to control permissions. Your patch did work but then changes the logic of the filter system. I am not 100% sure what is the best way to proceed. As you have found out by forcing the filter system to only be for anonymous users the only way secure site would work on a multi role system is to give all authenticated users access site permission.

As I have never used secures site in a way that only one role or one user need it I have never had this problem. We could leave it as is and document it as is but this does reduce flexibility but also hands over control to other access control systems. Or we can use your patch removing the restriction altogether. Or we can implement 2 filters one for authenticated users and one for anonymous users.

I can’t be sure if there are any security problems with your patch and I can’t see why anybody would need more than one filter to use secure sites. If we need more than one filter me might as well consider build a profile based system.

By removing the restriction I do think this would make secure site function more as expected. It is possible that the restriction is a logic error.

@Junyor
What do you think?

I see the logic flaw now. There's nothing to cover logged in users without the "access secured pages" privilege. I'm not sure how to fix this yet, though. The proposed fix will only cover one use case, i.e. if the page the unprivileged user tried to access is in the bypass list. There's still the case where it isn't.

In a way, Secure Site isn't using the "access secured pages" permission now. It's more like validation of the underlying premise that logged in users have privileges and anonymous users don't. In many cases, Secure Site works best to simply block viewing of the site from anonymous users. Maybe we can clear this up, but it's going to take quite a bit of work.

@NaX: Maybe we should do this for 6.x-2.0?

Big flaw. Secure Site's only purpose is to require log-in on some pages. It seems that we made a mistake in even using the permissions system.

I can see the point in wanting to use Secure Site as a filter for logged in users, but as NaX pointed out, a node access module is better for that. Of course, if what you're trying to secure isn't a node, then those modules can't help.

I guess we might want a permission to control whether a user is allowed to log in with Secure Site. It shouldn't be used to control access to pages. That's what all the other permissions are for.

If that's the case, then it makes sense to remove the permission.

Let’s try to look for examples were the secure site permissions make sense.
If you create 2 roles A and B and you give A secure site access but not B. The permission to the protected nodes and paths is also giving to A. So A can login and access the protected paths but not B. All makes sense. If B is given access site permission but not permission to access the paths then once B has logged in B will be shown an access denied page. So it could be seen as useful in this situation to keep the permission, but the filter logic will prevent B from logging into the site completely until he his given access site permission.

I think we should commit the patch to 5.x and 6.x as is (extra testing to check for security bypass flaws) and then after 6.x is released we look at version 2 of secures site. I don’t think we need to delay the release of 6.x for this as secures site 5.x has been running with this logic for a while now and with no problems.

With version 2 I think we should do away with the permission and handle role and user access our selves. For example we could add a tab to the settings page call it something like access control and on that page allow people to select roles and also create a user list for access. So if somebody is authenticated as a Drupal users but does not pass secure sites access lists then a more meaningful message can be displayed telling them they do not have access and they should speak to their administrator. But they can still login to parts of the site that is not protected by secure site, and here we can also step in. We can add a setting (checkbox) that blocks users from accessing the protected paths, if the user is not in the access list then they will be shown a message telling them this. If this setting is not selected then it is assumed the admin wants to handle permission to protected pages some other way.

Any thoughts?

Also we could check if the user has access to the current path. Not 100% sure how to do this, but we could check the path after login and at step 1.

I think this is where we need to start looking http://api.drupal.org/api/function/_menu_item_is_accessible/5 , if that is what we want to do.

I agree that the patch can be committed as is.

I think the current logic has worked for most people because they give permission to access secure pages to all roles except the anonymous user. I would like to keep this module simple by focusing on log-ins. In fact, our goal for the next version should be to eliminate path-based access control altogether and instead to replace Drupal's access denied page with a log-in prompt for anonymous users.

When it comes to doing away with filter paths and replacing access denied pages we need to consider the httpauth module and if you we really want to overlap features. http://drupal.org/project/httpauth

DRUPAL-4-7

The logic is correct in the DRUPAL-4-7 branch. Interesting.

DRUPAL-5

CVS commit 140385

DRUPAL-6--1

CVS commit 140386

See also #312821: i18n stops password reset URL being recognized.