On Mon, 7 May 2001, Jeff Trent wrote:
> Ah, this maybe a problem in the way I've adapted Struts. I reflect all UserForm
>method calls directly into the contained User object owned by the UserForm. So for
>instance, I have
>
> public class UserForm extends ActionsForm
> {
> protected User user;
>
> ...
>
> public String getName()
> {
> return user.getName();
> }
>
> public void setName(String name)
> {
> user.setName(name);
> }
>
> ...
>
> }
>
> Now can you begin to see my original concern? Maybe I need to
> separate the model from the form a little more than what I have.
>
This is where I can step in (better late than never :-) and point out that
this is *not* the recommended design pattern for form beans. They really
should be independent, and you really should decide what properties should
be copied from UserForm to User in your Action (or the business logic that
it calls).
The important issue here -- and it's not unique to Struts, the issue is
common to all web application environments -- is that you have absolutely
zero control over what the client decides to send you. For example, if
you rely on client side JavaScript for field validation, what happens when
your client turns JavaScript off? You get garbage input, so you should
always be paranoid and validate (again) on the server side.
In the case at hand, nothing stops your user from logging on (so your
security checks won't catch anything) and then hand typing a URL with
query string parameters that maliciously or accidentally try to change
things in the system. If the user is successful at doing this, it's shame
on the app developer for listening to request parameters that you
shouldn't.
Of course, you need to take other defensive measures as well (like using
the transaction control support to avoid accidental or malicious resubmits
of the same data).
For those of you going to JavaOne, I'm hosting a BOF on Thursday night at
7pm (BOF #1291) called "Approaches to User Authentication and Access
Control in Web Applications". This discussion has given me some
additional topical material to make sure that we cover.
> - jeff
>
Craig McClanahan
