Core: Check user authorization before sending protected material.

[cecilia-donnelly]

The current authentication process is flawed. We need to implement a library for securely checking user authentication before loading pages.

[cecilia-donnelly]

This one was recommended on Stack Overflow: http://usercake.com/index.php. It does have a reported vulnerability: http://xforce.iss.net/xforce/xfdb/93459, but there are examples of how to fix that in the forums (see http://usercake.com/thread.php?id=712 and http://usercake.com/thread.php?id=757).

This is the first result for "php library login secure": http://ulogin.sourceforge.net/ but it doesn't seem to be referenced anywhere else. That might be because the name "ulogin" is fairly generic and hard to Google.

[cecilia-donnelly]

Discussed in a private call with @kevinrak9 the need for a change in the program-specific access set up, which is relevant for TRP and Enlace. Enlace users need to have access to multiple programs, but not all programs. Currently the user-program relationship is many-to-one such that one user can only have access to one program, though one program may have many users associated with it. We need to change that so it is many-to-many.

While this may not quite belong in the Issue #78 thread, in practice it will probably be part of the same process.

[cecilia-donnelly]

Note that this issue has been expanded somewhat to include checks of access before executing a query to the database. For example, we will add a check of whether the logged-in user has administrative privileges before executing a query that deletes information.

[cecilia-donnelly]

In order to finish this issue, I need to:

1. Expand auth.php and copy siteconfig.php to cover all groups and core files.
2. Change UI to add new users to certain site(s) and program(s).
3. Add access check before all mysqli_query calls (!).
4. Check program access in Enlace using session cookie.
5. Check access level and display elements accordingly in all sites (done for TRP and Enlace).
6. Fix and standardize logout across sites.

[cecilia-donnelly]

@akshayagarwal, check out this issue. Once you have a chance, I'd appreciate it if you could help with part (3) from my previous comment. Since we call mysqli_query throughout the code base it would be a good opportunity for you to get more familiar with the code as a whole. For some background: the system has three different user levels. Administrators can do everything, including adding new users, changing passwords, etc. Data entry users can do most things, including adding information, but cannot delete information. View-only or read-only users cannot change any information. The code currently makes these access differentiations only via showing or hiding elements on the page. We need to add a line of code that checks whether the logged-in user has permission to execute a given command (deletion, insertion) before the mysqli_query line that executes the command.

It's pretty simple, but it will involve a fairly massive code change. We'll talk about it, but I wanted to draw your attention here.

More information: As you might be able to see from looking at the commits, I have created a set of functions in /ajax/session_test.php that check authentication and authorization. All files will include a site-specific siteconfig.php, which calls some of those functions to make sure that the browser has a cookie that matches a session ID stored in a database table. Siteconfig will also determine the level of access that the logged-in user has (this is part 1 above). Files will then show or hide elements based on that level of access (part 5 above). As we complete the issue, files will also check that access level before executing queries to the database. TRP and Enlace have special program-specific access levels that require a little extra work, as I mention in part 4 above. I'll add more information about that in commit messages and comments as I continue to work on this.

[akshayagarwal]

Hey Cecilia,

I would like to discuss this over a call with you to understand in depth. I have different calls scheduled until 3 pm Central Time today so let me know if we could connect at 11.30 pm Central time tomorrow

Thanks! Akshay

On Wed Nov 12 2014 at 11:59:34 PM Cecilia Donnelly notifications@github.com wrote:

@akshayagarwal https://github.com/akshayagarwal, check out this issue. Once you have a chance, I'd appreciate it if you could help with part (3) from my previous comment. Since we call mysqli_query throughout the code base it would be a good opportunity for you to get more familiar with the code as a whole. For some background: the system has three different user levels. Administrators can do everything, including adding new users, changing passwords, etc. Data entry users can do most things, including adding information, but cannot delete information. View-only or read-only users cannot change any information. The code currently makes these access differentiations only via showing or hiding elements on the page. We need to add a line of code that checks whether the logged-in user has permission to execute a given command (deletion, insertion) before the mysqli_query line that executes the command.

It's pretty simple, but it will involve a fairly massive code change. We'll talk about it, but I wanted to draw your attention here.

More information: As you might be able to see from looking at the commits, I have created a set of functions in /ajax/session_test.php that check authentication and authorization. All files will include a site-specific siteconfig.php, which calls some of those functions to make sure that the browser has a cookie that matches a session ID stored in a database table. Siteconfig will also determine the level of access that the logged-in user has. Files will then show or hide elements based on that level of access. As we complete the issue, files will also check that access level before executing queries to the database.

— Reply to this email directly or view it on GitHub https://github.com/OpenTechStrategies/lisc-ttm/issues/78#issuecomment-62766874 .

[kfogel]

By the way, I changed the summary (title) of this to more accurately reflect the underlying bug -- that is, to describe it in terms of the problem, not in terms of one of its possible solutions.

[cecilia-donnelly]

Hey @akshayagarwal, let's agree on a list of changes that need to be made to complete this security fix. As I understand it, we need to:

• add a function that checks the access level of a logged-in user before executing any calls to mysqli_query()
• remove $_COOKIE['user'], $_COOKIE['view_restricted'], and $_COOKIE['view_only'] and change code that uses them to instead rely on the session id stored in our User_Sessions table
• check authz and authn before loading any restricted page (i.e., any page that isn't the login screen). I'm including a subsite-specific page called "siteconfig.php" to do this in the Issue-78-personalized branch.

We may also:

• replace queries with prepared statements
• use oAuth for authentication

Is there anything else I'm missing? Once we have the full list we can make estimates on time and prioritize these changes, then complete them.

[akshayagarwal]

We can avoid the oAuth auth for now and perform the rest of the tasks. I am not sure how many files are using the MySQL queries, I am assuming all of them?

[cecilia-donnelly]

Essentially all of the files are using MySQL queries, yes.

To do:

• Write a function that will (1) check the access level of a logged-in user, (2) insert arguments into a prepared statement, and (3) execute that statement.
• Implement that function across all system files in place of calls to mysqli_query() (!)
• replace existing cookies as described above
• use auth.php and siteconfig.php across all subsites.

[cecilia-donnelly]

@akshayagarwal suggests that we break this up into stages:

1. Replace existing cookie usage in Enlace subsite with session variables.
2. Make setcookie() call more secure. Add HTTPS-only, ensure they can't be modified with Javascript.

[akshayagarwal]

Need to add captcha validation after a certain amount of wrong password login attempts to prevent bruteforce attacks.

[akshayagarwal]

How about using this https://www.owasp.org/index.php/OWASP_PHPRBAC_Project

Its actively maintained and supported by OWASP

Another offering of theirs is https://github.com/OWASP/PHP-ESAPI/ but that looks pretty old

[cecilia-donnelly]

Per my call with @akshayagarwal last week, here are our next steps:

1. We will request the data at login (1. logged in status, 2. role, and 3. permissions) and store it in $_SESSION. I added this in commits d0fbeac and 3103d3d.
   • Note: check logged-in-user's IP because otherwise we are vulnerable to session hijacking.
   • Note: add captcha after certain number of wrong passwords, then block IP after certain number of wrong captchas. We may block the IP directly in Apache or by using a firewall (?) server for more security. @kfogel, what do you think about that idea?
2. @akshayagarwal will work on adding the PHP RBAC library (see http://phprbac.net/) for authorization. He will start with the Enlace subsite.

[cecilia-donnelly]

Hey @akshayagarwal, we still need to work on this piece, right:

• Write a function that will (1) check the access level of a logged-in user and (2) execute a statement if the user has permission to do so (and return an error if s/he doesn't).
• Implement that function across all system files in place of calls to mysqli_query()

Have you begun any work on this? I'll start a draft function if not, because we really need to make that change so we can open source this thing.

[akshayagarwal]

Hey @cecilia-donnelly ,

I tried reaching you on IRC today but I think you were unavailable. I actually discussed this with @kfogel and we decided to try out an ORM implementation to address this. I have created a new Issue #98 to track the progress of that. Towards that end, I have finalized to use Propel ORM and am brainstorming an implementation strategy for the same. I will be doing the actual implementation over the weekend and will keep you updated. I understand the urgency of this task and will do everything in my hands to not let it become a bottleneck in the open sourcing milestone.

Thanks!

[cecilia-donnelly]

@akshayagarwal, oh no! Sorry I missed your IRC messages. Looking forward to talking soon. Have a good rest of the week. Cecilia

[cecilia-donnelly]

@cwebber, here are some notes on the progress on this issue:

• As I understand it, the authn and authz steps that have been added to this branch are working, but have not yet been applied to all subsites. I'd recommend checking out the recent commits on the Issue_78_personalized branch to see the work that has actually been done, rather than relying on the notes in this issue. A lot of things were mentioned above that haven't been reflected in code changes.
• As we discussed, we need to check permission before completing certain tasks. I thought that we would be wrapping the mysqli_query() calls in an authorization check (i.e., read-only users shouldn't be updating or inserting any information and data entry users shouldn't be deleting anything), but I'm very open to you suggesting other solutions.

Let's talk after you get a bit of a grip on where the code currently is so we can put together a timeline and the next steps for this change. Thanks!

[cwebber]

Okay, so I read the codebase and talked with Cecilia on the phone, and here's the summary of things.

• There's good progress on the Issue-78-personalized branch, but still work to be done.
• We can get rid of the session_id() call probably
• In the branch, the user logged in is stored in the cookie still, which means though whether you're logged in or not is stored in the session, it's trivial to escalate privilages by switching in which user you're "logged in as". Instead of $_SESSION['is_logged_in'] we should have $_SESSION['login_id'] (or user_id or whatever). The presence of this variable means that the user is logged in. If it's not set, the user isn't logged in. And also this is what's referenced to check the user and their permissions.
• I was wondering how to "bail out" if things went badly; the code already does this with exit()! The More I Learn (TM)
• There are really two levels of "permission" checking to be done
  • Checking whether the user has access to that subsite
  • Once the user has access to that subsite, there are three levels of permissions:
  • Can read
  • Can read/write
  • Admin: can read/write/delete/create users

So what's next? There are two major milestones to achieve before closing this out:

1) Log in and have access to only your subsite. This same code should be consistently usable across subsites. Cecilia thinks this is pretty close, just changing the session and using auth.php and siteconfig.php. 1b) Logout is inconsistent currently, also handle that. 2) subsite permissions come next, the read/read-write/admin stuff. These roles are not yet implemented in the DB, though currently javascript does show/hide things so people do see different things, but there's no security around it.

[kfogel]

Conceptually, are there really two levels of access-checking to be done? It could just be one continuum:

• Does the user have read access to this subsite?
• Does the user have read/write access to this subsite?
• Does the user have admin access? (is that a general thing, or is it also "to this subsite"?)

In other words, the starting state is zero -- no access to anything.

That minor conceptual quibble aside, everything you write above makes sense to me!

[cwebber]

@kfogel That makes sense, and good to hear you say that, as it was my initial reaction as well. @cecilia-donnelly expressed something that she originally thought of this as two problems, but was happy to have it changed and was flexible on it.

I'm still trying to account for the ways in which security ends up happening and take record of it to be sure that I am doing this right, but I will keep that in mind.

[cwebber]

BTW, the utilities in common/tools/auth.php currently have the session id passed in to many of them as a function parameter, but the parameter is actually ignored anyway, so I think I will remove the parameter from the arguments in those places. It might make a lot of sense to pass in something where you can specify which user we are talking about, but it probably makes more sense on the user id level than on the session id level.

Moving forward with that as my current thinking, but feel free to correct me!

[cwebber]

So (uncommitted) I began working on switching the user from the cookie to the session... would we prefer to store the user id or the username in the session? I suppose we store the username in the cookie at present, so that should be fine. But I'm not sure since getAllSiteAccess asks for user_id, whereas many existing calls which rely on the cookie rely on the user as username. Not that queries can't be adjusted, but what do you think, @cecilia-donnelly ?

[cecilia-donnelly]

Thanks @cwebber! I generally prefer using the user ID, especially since we have some duplicate usernames that need to be cleaned up (see issue #15). It's true that in many places the queries use the username, but I think it would be straightforward to use the user ID in all of them -- I think generally they actually rely on the User ID to join with the Users_Privileges table, anyway.

I agree with your earlier comment, too. We might want to use the user id as a parameter at some point, but there's no need to pass in the session id if it isn't being used.

[cwebber]

@cecilia-donnelly okay great, thanks for clarifying on that :)

[cwebber]

@xray7224's suggested that we build a User class to do permission checking; after some thinking, I realized that she's absolutely right and this is the simplest way to do checking.

However, I need some clarity at this point in writing the code....

@cecilia-donnelly I'm working on the authentication system refactor. I'm a bit confused, and just want to make sure I'm doing the right thing... I see here:

    $find_site_access_sqlsafe = "SELECT Privilege_ID, Site_Privilege_ID, Program_Access FROM Users_Privileges WHERE User_ID =" . $user_id;
    // ...
    while ($access = mysqli_fetch_row($access_result)){
        $access_return[$access[0]] = array($access[1], $access[2]);
    }

Am I right in reading that:

• Privilege_ID is actually the ID for the site, so for example for Bickerdike this might be 3? It looks like that's how it's used, but it's a bit confusing because I would expect that to be named "Site_ID" or something.
• Site_Privilege_ID is thus the permission "level"
• Program_Access is whether the user has access to a specific program though it's not made clear which one it'll be. (I also see there's a comment where this should eventually become a many-to-many, since only one program can be specified at present...)

Is this an accurate description of how things work? I just want to be sure I'm doing the right thing.

[cecilia-donnelly]

@cwebber, that is exactly right. The column names are definitely confusing. Privilege_ID is the site id and is listed in the "Privileges" table. Site_Privilege_ID, as you said, is the permission level (there is another table called Site_Privileges that lists these permission levels).

I'm glad you saw the many-to-many comment on Program_Access, since I thought we might be able to look into changing that while making these other changes. If it's too complicated to do it right now then don't worry about it, but if it's simple to make that change while altering the access set up then we should do so. It's mostly relevant for Enlace, but I think TRP also uses program-specific access privileges.

[cwebber]

@cecilia-donnelly Okay, that's good to have clarified. Re: the "many to many" stuff, I think I won't do that since that isn't the critical part here, but I'll leave the code in such a state that switching in many to many support on the db side can be done in the future, while maintaining the same API as in terms of the methods called, so that multiple program access permissions per person can be added later.

[cecilia-donnelly]

@cwebber: leaving it ready to switch to many-to-many sounds perfect. I agree that it's not critical at the moment.

[cwebber]

Okay, things are working, slowly:

$u = new User(31);
echo("<br />");
echo("Username: " . $u->username . "<br />");
echo("Permissions: ");
print_r($u->site_permissions);
echo("<br />");

function do_we_have_this($user, $site_id, $site_name) {
    if ($user->has_site_access($site_id)) {
        echo("User has $site_name<br/>");
    } else {
        echo("No $site_name for user!<br/>");
    }
}

do_we_have_this($u, $TRP_id, "TRP");
do_we_have_this($u, $LSNA_id, "LSNA");
do_we_have_this($u, $SWOP_id, "SWOP");
do_we_have_this($u, $Enlace_id, "Enlace");

echo("Checking permissions....");

// Only gets past TRP

enforceUserHasAccess($u, $TRP_id);
echo("Has TRP<br/>");
enforceUserHasAccess($u, $SWOP_id);
echo("Has SWOP<br/>");
enforceUserHasAccess($u, $LSNA_id);
echo("Has LSNA<br/>");
enforceUserHasAccess($u, $Enlace_id);
echo("Has Enlace<br/>");

Spits out:

31
Username: hachanzar
Permissions: Array ( [4] => Array ( [0] => 1 [1] => a ) )
User has TRP
No LSNA for user!
No SWOP for user!
No Enlace for user!
Checking permissions....Has TRP
User does not have permissions to access this site.

Which is correct, since the user doesn't have permissions to access SWOP. The page bails out at that point.

I'm going to make it so that a $USER is set up on pretty much every page which is a User object, whenever such a user is logged in. Through this we should be able to put a enforceUserHasAccess($u, $this_site_id); at the top of each page and fail out when things aren't working.

[cecilia-donnelly]

By the way, my only quibble with the above is that we might want the do_we_have_this() function to have a more descriptive name. I assume that's just your draft name for it anyway, but figured I would mention it.

[cwebber]

@cecilia-donnelly That was just my test code above, do_we_have_this was just at throwaway function to make sure I was getting the right output and so I could repeat things a few times. It seems you guessed that though, in which case your guess was correct! :)

[cwebber]

(That close was totally unintentional!)

[cwebber]

Okay, so I've been working hard on this; here's an update of where things stand so far:

• You can see most of the progress in auth.php here
• The enforce_has_access() method on the user object is what does the bulk of the gatekeeping. It can check that the user has site access, also that they have the right permission "level", as well as making sure they have access to subsites. If the user doesn't have these things, it will give a "401 UNAUTHORIZED" error and print out an error message.

This still leaves some questions:

• Probably printing out an error message doesn't always make sense... maybe we'd do different things in different environments! Maybe we should allow the user to pass in a different failure method?
• @cecilia-donnelly thinks that maybe we shouldn't always die... like, maybe that makes sense for erroring out at the start of a page, but sometimes we just want to hide a button. But that enforce_has_access() "pass in as many args as you need" seems to be a nice API, so maybe we should extend has_site_access() to also have multiple args but just return a boolean?
• If that's true, should enforce_has_access() to use that? But what about us wanting to maybe give different error messages depending on the context of how things failed? Or do we really want to do that? Are those error messages useful to us, or to the user? (Unfortunatlely, if we made heavier use of subroutines, we could make this a lot easier for ourselves by raising different types of exceptions depending on the circumstance, but that seems impossible to handle here... unless I'm missing something.)

Next steps seem to be:

• Switch to checking the user id object in the session rather than in the user
• Add global user object when the user is logged in so we can use it everywhere
• Get feedback from @cecilia-donnelly on what kind of interface we expect to present users when they don't have access in different places. What about when loading a page for the first time? What about conditionally showing user interface elements? What about ajax stuff? How does it work now? How should it work in the future? This will affect tweaks to the current API.
• Make tweaks based on that feedback.
• Test the API on a few pages, confirm this is the behavior we want.
• Blast out everywhere!

I know that sounds like a lot of remaining steps, but I think it's much closer. :)

[cecilia-donnelly]

@cwebber thank you so much for writing this up, and for doing all this work! I am excited about how close we're getting.

Here are my thoughts on our permission-denied interfaces. The cases where we deny access:

• The user isn't logged in.
• The user doesn't have access to the subsite.
• The user doesn't have access to the specific program.
• The user doesn't have permission to perform an action.

For each of the first three cases, we should die at the top of the page, and the existing system works great. It's the final case that is a little different. The actions we're concerned with are inserting, updating, and deleting data. In all (?) places in the system, these actions are accomplished by a POST/ajax-y thing.

Currently:

• we hide the UI element and hope that users will respect that

In the future, we should:

1. Hide the UI element(s)
2. Check access when they attempt to insert, update, or delete data (almost every POST in the codebase, except a few search/reporting features)
3. Give them an error in the screen or as a pop-up if they don't have permission to perform the action
4. Block the query from being executed

I think that should do it. So, we need one different error message ("You don't have permission to ...perform this action? ...change this piece of information? ... See your system administrator for details.") and, more significantly, a way to check site access level and stop the action mid-page if they don't have correct permissions.

We could check in the destination page, and send the error message as a "response" to the ajax initiator.

post(
    'some_page.php',
{
    'some info'
},
    function (response){
        alert(response); // or html_in_element, or what have you
    }
);

and on 'some_page.php':

enforce_has_access($site, $access_level_needed); // imagining that second arg
// Add the following as another piece of enforce_has_access?
if ($access_level > $access_level_needed){ //remembering that greater permission comes with a lower $access_level int
    echo "You don't have permission to change this information";
    exit; 
}

Does that make sense? It would be a lot of changes to the many 'some_page.php' destination pages, but many of the posting pages are already set up to show the response.

[cecilia-donnelly]

Update after a conversation with @cwebber:

• We are going to use the existing jquery $.post() calls to show an error message if the destination page returns a failure HTTP response (i.e., non-200). The post() function can include an inner function that will either alert on failure or show intended/existing response on success.
• Given that, we can use the existing enforce_has_access() function on the destination page, since the page will return a failed response if the user does not have the correct access level to perform the action.
  • This might be a problem in some places where the destination page performs a few different functions and they require different levels of access. -- will include an example below
• Add a check_has_access() function that will hide UI elements based on the user's level of access.

Examples of the cases in my last comment:

• The user isn't logged in.
  • Sally isn't signed in to TTM, but wants to show someone a report, so she navigates directly in her browser to /lsna/reports/reports.php. She receives a 401 Unauthorized error.
  • As we describe it above, this 401 unauthorized header isn't personalized at all for TTM. If possible, it might be more friendly to include a link to index.php that says "log in here" or something along those lines. Currently, the site redirects to /index.php.
• The user doesn't have access to the subsite.
  • Sam works at TRP, but is curious about how SWOP is using their system, so, after logging in, he navigates from /index.php to /swop/index.php. He receives a 401 Unauthorized error.
  • Here, I don't think we need to add specific text to the error, though we could. I think it's intuitive to receive an access denied page when they follow a link or construct a link to another org's site. Currently I believe they're also redirected to /index.php.
• The user doesn't have access to the specific program.
  • Joe works at an Enlace grantee program called Beyond the Ball. He wants to look at a student's information, but that student isn't in the Beyond the Ball program. He clicks on the student's name in the search results on /enlace/participants/participants.php and instead of seeing /enlace/participants/profile.php, he gets a 401 Unauthorized page.
  • Currently, users see an error message from /enlace/participants/permission_check_page.php that says either they don't have permission to access the page (can't see any participant records) or don't have permission to see this particular participant because s/he isn't in the user's defined program.
• The user doesn't have permission to perform an action.
  • Sally is a data entry user at LSNA, but she accidentally first makes visible and then clicks on a delete button in the participant search results. She is on the /lsna/participants/participants.php page, and the search results are generated on /lsna/ajax/search_participants.php. After she clicks "delete," she sees a message that states, "You do not have permission to perform this action."
  • Currently, as mentioned above, there is no actual safeguard against people performing actions they shouldn't. The only thing stopping them is that the UI elements are hidden.

[cwebber]

I believe the branch is at the point where it needs to be to start updating things, or at least discuss it.

Unfortunately, though I made some demo files to drop into the checkout, I wasn't sure where to put them since this repo is not public yet, so scp'ing them to my own site did not make sense, github lacks an attachment feature, etc... so I have created an "evil" branch. Check out the Issue-78-personalized-withsandbox branch.

There were a few other things I could have done; I considered emailing a tarball to @cecilia-donnelly and @kfogel, but then that's lost to the issue tracker.

I'll find out what @kfogel thinks is TRTTD later :)

[cecilia-donnelly]

@cwebber As you can see in commit 17635da, I've started adding the new security apparatus to some Enlace pages. Based on that exercise, I think we need to add:

1. A way to quickly check whether a person has access to "all" programs. That might be deprecated with the issue #121 change(s), but it would make access to person and program profiles simpler.
2. A standard way to show or hide UI elements. This doesn't necessarily need to be a method in the API, but I want to make sure that we document it clearly.

What do you think?

[kfogel]

Hey @cwebber,

I didn't understand your last comment. How does " this repo is not public yet" affect what you were trying to do? What are these demo files and why would you not just add them to the regular issue branch?

[cecilia-donnelly]

@cwebber also mentioned in IRC that we should be sure to close db connections when we exit a page because of insufficient permission.

[cwebber]

@kfogel Probably I didn't really need to ping you about it, I overworried about the method of distribution, since all felt suboptimal.

So there's a .gitignore'd "sandbox/" directory in the git repo where you can dump test files and mess with things as a "poor person's REPL". I wrote some demos for @cecilia-donnelly to test, but they hardcode make public various properties of users, whether you're logged in or not, which would clearly be dangerous in production, so I didn't want to commit to any branch where that might accidentally happen.

In a public repo, I would have just put together a tarball of these demo .php files and dumped them on http://dustycloud.org/tmp/ and asked @cecilia-donnelly to download that, but since the project isn't released I worried that isn't kosher. But it's probably no problem really

Anyway I was overworrying.

[cwebber]

@cecilia-donnelly I think that your first requirement is fulfilled, as per our recent conversations on IRC. Correct me if wrong!

Thinking about the standard show/hide UI stuff now.

[cwebber]

So I think regarding showing/harding UI elements, it partly depends on the scenario right? Things are pretty different if you're getting a JSON response and then editing the DOM in your ajax'y handler than if you're just rendering things once or using .innerHTML() to replace things after you get a response from the server.

But, the TTM way of doing things does seem to focus around doing one of two things: rendering the page the first time, or doing a .innerHTML() response replacing with whatever you got from the server... right?

So there's really two routes:

• have a css class for "hidden" that's set when the user fails some test. This can be a function that makes use of user_has_site_access() very easily.
• in fact, this then works the same way, very simply, for responses where a .innerHTML() is done.
• If you want to avoid rendering an entire section of HTML, that's easy enough: just check if the user has site access before you render it out.

If TTM works differently than the above, then maybe a different technique will be needed. The fact that UI elements are "hidden" is okay.

@cecilia-donnelly have I caught things right?

[cecilia-donnelly]

@cwebber, yup, first requirement fulfilled! We can check for access to all programs using site_access_check() as updated in c71088b.

As far as the UI stuff goes, that seems reasonable. There actually are two css classes ("hide_on_view" and "no_view") that currently hide elements that shouldn't be visible to data entry and read-only users, respectively. Currently, those elements are literally just hidden (".").hide() is called in the header). It's my impression that we should do better than that and, as you suggest in your third bullet above, check access before rendering the HTML section. What do you think?

[cecilia-donnelly]

As we split up the work of applying the new API , here is a preliminary list of things that we need to check and change in each file:

• at top of page, call user_enforce_has_access() to check whether a user is logged-in and has access to subsite. Occasionally this also needs to check for special access to program (e.g. for a program or participant profile in Enlace).
• All posts: their destination (invoked page) should check the level of access necessary for the posted action (usually $DataEntryAccess, occasionally $AdminAccess for deletions)
  • and the invoker/originator/post-er should add: .fail(function() {alert('You do not have permission to perform this action.');}) or similar to all post() functions. We probably need a shared/global function to call in these cases.
• Hide UI elements that have class "hide_on_view" or "no_view" as explained in my comment from Feb 20 (above)
• Remove old system: all cookie checks, including those that cause certain classes to be hidden. Instead of just hiding those elements with CSS we should only create them for qualified users (i.e., include a conditional access check around them)

TL;DR:

• So: for all user-visible pages, do enforce_has_access at the top
• Then: for all non-visible (e.g., destination pages) do enforce_has_access with access level check
• Then: remove cookies.
• Then: remove/replace styles

What else?

[cwebber]

We also need to add "database cleanup" functions so that when the enforcement code runs and decides it needs to bail out, all connections are shut down appropriately

My suggestion is the following:

• Any time something sets up something that needs to be shut down by the end of the request handling, it should add a shutdown function to a new global array called $CLEANUP_TASKS or etc.
• Add another function called run_cleanup() which iterates through the $CLEANUP_TASKS list and calls each function, shutting down all open connections or whatever.
• Run run_cleanup() at the end of each page.

(We could then make use of run_cleanup() more globally too?)

[cwebber]

There are some other things worth cleaning up too once we splat out all these updates:

• There's lots of legacy code in /ajax/login_submit.php that supports the old cooke-based auth behavior and should be removed after the conversion is finished. (This might break some things: we'll want to do a bit of testing to ensure that things are okay at this point.)
• /include/auth.php can be removed, I think.
• There are a bunch of functions in core/tools/auth.php that should either be factored out or removed, as they are deprecated by the new API. We should do a recursive grep on each one to make sure the codebase doesn't make use of it before we tear it out of course!

[cwebber]

I've added new tools in core/tools/db.php which handle setting up and shutting down connections. Working on splatting them out in the right places.

Luckily, this is a lot easier than putting users everywhere, because we can leverage the existing include files that we have. Due to the way the "copy from template include files" work, whis will require updating the dbconnopen files on everyones' servers, but this will be a one-time task.

[cwebber]

Okay! I've set up a connection manager thingy and set up all the dbconnopen.php things to use it. Still need to get to the close and etc stuff. But yeah, good week! :)

[cecilia-donnelly]

@cwebber, @kfogel, I've converted the Enlace subsite to use the new access API and it seems to be working as expected. You can see my notes on making the access changes and testing them in all my commit messages, but especially in commit 82847fd. To convert pages to the new API:

1. Add include_once() of the user setup file and the core db connection file to the top of each page.
2. Run enforce_user_has_access() with the appropriate subsite. If the file is an invoked page in an AJAX call (usually just the files in the ajax/ directory), then add the appropriate permission level for "insert," "update," and "delete" statements as a second argument to the call.
3. Search for ".post(" on each page, and at the end of the parenthetic expression add .fail(failAlert), a function that pops out a failure message if the invoked page returns a 401 (as it will if the enforce_user_has_access() function returns an error).
4. Search for $_COOKIE['user'] and $cnnLISC on all pages. These are indications that some specialized access tests are already being run and adjustments will need to be made. The user cookie is going away, so we definitely have to remove references to it wherever they appear.
5. Where UI elements have classes "hide_on_view" and "no_view", treat them as in my comments from Feb 20 and 24 (the latter of which is an earlier version of this).