NOTE: Sorry for "thinking out loud" here, but I usually enjoy reading other people's posts where they go through a design thought process in some area, so I thought it may be okay for me to do it here...
This topic obviously got me thinking :-) and I have determined that my recommendation to do a check in the action itself may be inconsistent with my complaint about redundent db calls if we were to develop a "pluggable" system where you would have security checking modules that are separate from your actions. The inconsistency is that doing the check in the action will often result in redundent db calls, too, making it no better than the "pluggable" solution in that respect. Here's an example: The action that shows a sales_report item (/viewSalesReportItem.do?sales_report_id=3) will probably load the sales_report object from the db to get the territory_id as part of the security check. But then it will likely delegate the loading and population of the SalesReportActionForm to some other class that will also make a call to the db to get the sales_report object from the db. Thus, we still end up with two calls to the db, when really only one is needed. One solution for this simple case is to have the action call the delegate to populate the action form, and then pick the territory out of the now-populated ActionForm to do the security check. That works in this simple case, but only if we are careful to ignore any territory_id info that came in with the request. This is not an issue for the other two cases, since in one of them the territory_id comes directly from the request (no db calls required) and in the other one, our db query will limit the results to data that we know the user is allowed to see. I think my problem here is that I like to do the security check up front in the action processing to avoid having to do any unecessary processing (i.e. stop the request as soon as you know the user doesn't have permission), and that leads to redundent calls. I am having a bit of trouble coming up with a specific example where the redundent db calls can't be avoided by doing the security check at the end of the action processing (right before forwarding to the JSP or other view component for display). But I imagine that some levels of indirection in the data model might result in not being able to get the territory_id for a security check. Here's an example that is slightly stilted, but perhaps reasonable: /viewOrder.do?orderId=10 Let's say that sales users should only be able to see orders in the territories that they have access to, but that the action to view an order is intended primarily for customers (who don't care what the territory is) rather than sales users. So, it might be that the call that the ViewOrder action makes to the business delegate to load the order information does not return the territory_id. In that case, the action would then have to do its own db calls to get a territory_id for the requested order to do a security check. Perhaps the most reasonable solution here would be to change the delegate and ActionForm (or DTO, etc. -- whatever the business delegate call returns or populates) to retrieve and contain the territory_id information so the check could be made at the end of the ViewOrder action. The security check would probably allow the request if the user is the customer that placed the order, or a user with the 'sales' role and the order is within a territory that have access to. But the security check is getting somewhat complicated now, and will probably be duplicated elsewhere if we put it directly in our ViewOrder action. So this lack of re-use and the potential for complicated security checking rules mixed in with action processing still leaves me wanting a way to separate it from the action itself. Implementing it separately also makes it easier to divide the work among multiple developers. But separating it out usually means redundent db calls. I can think of two possible solutions. One is new (in this thread), and the other one is a continuation of the "pluggable" security module design. Solution 1: Have the business layer do the security checking. This is somewhat appropriate anyway, since the security constraints are usually driven by business rules. Passing the username into the business layer would not be much of a problem, but you might want to develop a custom UserProfile class to encapsulate not only the username, but also other security info, such as what territories they have access to in our sales_report example. As a mechanism for communicating this information upward toward the view layer, the business layer could throw a custom SecurityException if an access violation is detected, and that could be caught by the action, the base action for your application, or even Struts itself for proper processing (i.e. to show the "access denied" page). This solution gets the security checks out of the actions, but moves it into the business layer, which might not be any better. But it does solve the problem of redundent calls to the db for the same info. Solution 2: I am still attracted to the idea of using a cache for db access that is limited to the scope of a single request. This would support the separate, pluggable security checking modules architecture without leading to redundent db calls. It wouldn't matter how many times different parts of the request processing chain want to load a certain object from the db, since it would be cached after the first load. Perhaps the cache could be attached to the thread (as a ThreadLocal variable), and it would be discarded by a filter at the end of processing for each request, as the server will be pooling the threads for use in processing other requests. (I have used a similar technique to limit the number of db connections used in processing a request to 1 to avoid "dining philosphers" deadlock issues.) If anyone has any knowledge or ideas about persistence layers (EJB, JDO, custom, etc.) that support this kind of thing, I would love to hear it. It would be great to be able to write the constraints declaratively, and the pluggable architecture backed by a peristence layer that avoid redundent calls seems to get us closer to that goal. It would be cool if you could write your security constraints in some simplified langauge (perhaps in some query language or an XML format) and then attach the constraint to a Struts action or a url-pattern. Simplifying the constraint language might improve communication between customers/users, business analysts, and developers, and would make it easier to change the constraints when the requirements change. And they would be easier to write in the first place. This would also open the door for tools vendors to make it even simpler still. I'd love to hear other ideas for implementing security requirements of this nature. What is your solution, and what are the advantages/disadvantages? If I am out of the loop on some product/project/standard/technology in this area, I'd like to hear about those, too. -Max ----- Original Message ----- From: "Max Cooper" <[EMAIL PROTECTED]> To: "Struts Users Mailing List" <[EMAIL PROTECTED]> Sent: Saturday, October 04, 2003 8:00 PM Subject: Re: Verifying integrity of URLs > The requirement to allow a given to user to see only a subset of the data in > a particular table is a relatively common one. For instance, in a sales > reporting application, you may have a table of total monthly sales for a > number of territories and a given user may only be allowed to view/edit one > territory or several but not all of the territory data. There may also be > users who aren't allowed to see any sales data at all. > > For instance, here is some sample data from our fictitious Sales_Report > table: > > id, territory_id, date, total_sales > 1, 1024, May-2003, $1000 > 2, 1024, Jun-2003, $1200 > 3, 1908, May-2003, $800 > 4, 2100, May-2003, $1300 > > To handle these requirements, you could use a combination of role-based and > programmatic security. You can use the role-based part to block access to > the sales reporting actions for users that aren't in sales. In other words, > a user would have to have the 'sales' role to view them. That takes care of > the users who can't see any sales data. > > Then you need to restrict users who do have the 'sales' role from seeing > data from territories they don't have access to. Consider these three > actions: > > /viewSalesReportItem.do?sales_report_id=3 > /viewSalesReport.do?territory_id=1024 > /viewAllMySalesReports.do > > For the first one, you can load the sales_report from the db and then get > the territory_id from it. Don't trust a territory_id coming in from the > request (e.g. > /viewSalesReportItem.do?sales_report_id=1908&territory_id=1024) because the > sales_report_id is really what determines what your app should show, and you > can see in my example URL that a user could just provide a territory_id that > they do have access to, but that isn't the right one for the sales_report > item they are requesting. That may seem obvious, but it is a common mistake > on update actions where the territory_id might be in the request as a POST > parameter in a hidden field. Users can fake POST parameters, too. Once you > have a trustworthy territory_id, check that the user has access to that > territory. There should be some join table in the db that indicates what > territories the current user has access to. If they don't have access, you > want to show some kind of "access denied" page. If it is okay for the access > denied page to be somewhat generic (and you aren't going to show the user > any links that would get them there, so I think this is okay), you can setup > a global forward to the access_denied page and have your Action return that. > > The second one is even easier, because it is the territory_id that directly > drives what the app is going to show. Again, just check that the user has > access to the requested territory, and return the "access_denied" global > forward if the check fails. On one app that I worked on, we put a > convenience method in our application's ActionBase class, which all of our > Actions used as their base class. It is a little clunky still, but our > convenience method would return the "access_denied" ActionForward if the > user did not have access, and null if they did have access. The code in each > action that used it looks something like this: > > ActionForward accessDenied = checkAccess(mapping, territoryId, request); > if (accessDenied != null) return accessDenied; > > We pass the mapping in so that the checkAccess method could get the > "access_denied" global forward. We pass the request in for two reasons. The > first is that the method needs to call request.getRemoteUser() to figure out > who the current user is. The second reason is that we have a utility class > that will cache the list of territories the user has access to in the > session, and it can get to the session from the request. > > The third action I listed up there (/viewAllMySalesReports.do) should show a > list of all sales_report rows that the user who requests it has access to. > In this case, you will want to adjust the db query to only return data that > the user can see. You could do that with a WHERE clause like "territory_id > IN (1024, 1908)" or perhaps better yet just join to the table in the db that > says what user has access to which territories. By limiting the results in > the db query, you avoid getting more data than you need and having to weed > through and discard the ones the user isn't supposed to see. > > > I have kicked around another idea where the programmatic security code would > not be in the action, but rather you would write a security module for a > given request and "plug it in" somewhere. For instance, for the > /viewSalesReportItem.do?sales_report_id=3 request, you would write a module > that would load the sales_report to get the territory_id and then determine > if the user has access to that territory. I was thinking of incorporating an > interface like this in SecurityFilter, where you might write a security > module that would allow or deny access and then register it with the filter > to be used for request URLs that match "/viewSalesReportItem.do". A > mechanism that allowed you to register a security module with a particular > (or a set of) Struts modules is also possible. The system would allow you to > register different modules with different actions, etc. The possible > advantages to using this kind of approach are that security code is > separated from the action itself (divide and conquer at a conceptual and/or > work assignment level) and that you could likely reuse a given module across > multiple actions (like the view and update actions for a sales_report, for > instance). > > However, there is a downside that I don't have a good solution for. Notice > how in this example that we had to load a sales_report from the db in the > security module to get the territory_id so that we could decide if the user > has access or not. In this case the action is going to load the sales_report > from the db again, which means we have more db calls than should really be > needed to process this request. One option to avoid the extra call would be > to have the security module save the sales_report object as a request > attribute for the action to retrieve later, but that couples the sales > module and the action too much, in my opinion. Another solution would be to > have some kind of generic, request-scoped cache of any db objects that have > been loaded. That would eliminate the coupling between the security module > and the action, since the action would just say "I want sales_report with > id=3" and it would either be retrieved from the cache or loaded from the db. > The action does not have to know or care if the object had already been > loaded from the db by the security module. But for that to work, you would > need to acquire or implement such a caching system (perhaps there are some > standard solutions, but I haven't researched it much), and then you would > also need to use it whenever you wanted to get something from the db. And > thus, I don't know of any good way to make this kind of thing work yet. > > You could use inheritance among your actions to get some of this benefit, > but that is less flexible than the "pluggable" architecture I was proposing > above. For instance, you could have an inheritance structure like this for > your actions: > > Action (Struts Action class) > ^ > | > MyAppBaseAction (base class for all of the Actions in your app) > ^ > | > SalesReportItemAction (base class that does security for SalesReportItem > actions) > ^ ^ > | | > ViewSalesReportItem UpdateSalesReportItem (the actions that handle requests) > > But that isn't very flexible, and may break down in practice. It would be > easy to inherit from the wrong action, perhaps run into situations where you > would want to inherit from multiple base actions (can't do that), override > the wrong method or forget to call a method in the superclass or your > action, or break the security on an action like ViewSalesReportItem because > you made changes to SalesReportItemAction while implementing some > requirement for UpdateSalesReportItem. Perhaps all of that is manageable > after all, but my current thoughts are that action inheritance like this > could be trouble and should thus be avoided for our security checking > purposes. > > > And back to your original proposal... It is fairly common for someone to > email a URL to some page in an app to someone else (by chance or by request > from a user who can't see a page that someone else can). If it is just the > request URL+parameters that are encyrpted into the key, anyone with that URL > can get to the page if you don't have any other security checks. You could > encypt the URL+parameters+username to make the key unique to each user. But > that's a lot of stuff to encypt into a query string parameter. But even then > anyone who figures out how to encode their own params would have free reign > over the system. I think it is best to do active checking to avoid creating > any loop-holes that could be exploited to run wild on the system. With > active checking, you might accidentally create a few holes here and there, > but there is no opportunity for someone to make a "master key" like there > would be with the encryption scheme. If you ever had to change the > encryption key, it would invalidate any bookmarks that your users may have > made to pages in your application, too, which may upset them. > > -Max > > ----- Original Message ----- > From: "Ross Sargant" <[EMAIL PROTECTED]> > To: "Struts Users Mailing List" <[EMAIL PROTECTED]> > Sent: Monday, September 01, 2003 2:59 PM > Subject: Verifying integrity of URLs > > > Hi, > This may be a little off topic, but I thought I'd ask what the experts > thought. > Many actions often take URL parameters. For example a catalog system's > "viewItem" action might include an id number for the > item you wish to view. The action uses this ID to pull something out of your > RDBMS (ideally through a service interface), sets a request attribute and > then forwards to a JSP to show the relevant data. > > Now imagine the case where there are complex rules governing who should be > able to view what items. Say for example that a certain user can only view > one category of items (which would obviously be enforced by the search > interface-you would never generate a link to something they weren't supposed > to see in your search results). > > What is the best way in general to ensure that the user is not playing > around with url parameters (in this example by manipulating the id > parameter) to get at things they shouldn't. Assume for the moment that the > RDBMS cannot help you solve the problem.I don't really see how Roles help > here because my understanding of Roles is that they only enable and restrict > actions. > > One option is to include the security validation in the ViewItem action > itself so it will not just blindly display whatever object the parameters > referred to.This seems like a problem that might occur in numerous different > places though which means it might be benificial to have something more > generic. > > My other thought was to append a URL parameter that is basically a digital > signature of the entire URL+query string (before appending the signature). > Assuming for now that the private key is safe on the server, then this would > provide a generic way to prove that an incoming request came from a link > generated by the site. This approach takes the view that if the user is not > meant to see or do something, they are never presented a link in the first > place to access it and it is impossible for them to fake a get request that > works without knowing the private key. This might introduce peformance > concerns.Note that I'm not even talking about public/private key crypto > here.. just simple single private key encrypt/decrypt. This would really > work for any combination of action+parameters you were concerned about. > > I was wondering if anyone out there had any better ideas or advice on how > they handle this type of problem in an MVC style application while keeping > things fairly simple. > > Ross > > > > > > > > > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
