Re: Proof of concept structs for 2.0

[Chris Travers <..hidden..>]

On Fri, Mar 12, 2010 at 5:40 PM, Adam Thompson <..hidden..> wrote:
> Chris Travers wrote:
>> Here are some proof of concept struct definitions for 2.0.  What do folks think?
>
> I'm assuming you are indicating data types, not literal strings of '$'
> et al., right?  Working on that assumption, for now.

$ is scalar
*% is hashref
@ is list.
>
>> Also do you like right-justification or left justification of hash keys?
>
> Either works for me.  I'd probably pick left-justification because it's
> easier and I'm lazy.  Plus if you use tabs it won't break horribly when
> using proportional fonts.  (Not that I do so particularly often, I'm
> just saying...)

I will use left-justification.
>
>>
>> struct LedgerSMB::User, {
>>         role_prefix =>  '$', # formerly from $request
>>            username =>  '$', # normally we had to get credentials
>>               roles =>  '@', # formerly from $request
>
> Can't $role_prefix be dropped if we have @roles?
>
>>                 dbh =>  'DBI' # formerly from $request
>
> I can honestly say that I don't care for the current method of
> maintaining sessions... not being able to look at things in two windows
> simultaneously is occasionally a royal P.I.T.A.  I feel the burden of
> serialization (process, not data) has been pushed out onto the user,
> whereas IMHO it belongs in the web app framework.

The 1.2 behavior was strictly to mitigate a series of security issues
that were simply not practical to properly fix in 1.2.  It does not
occur in 1.3.

>
> Also, doesn't doing it this way completely prevent session pooling?

That was the whole point of the behavior in 1.2.  In 1.3, session pooling is ok.


> [Oops... see below.]
>
>> struct LedgerSMB::Web::Request, {
>>      params         =>  '*%', # formerly top-level hash
>>      method         =>  '$',  # GET/PUT/POST
>>      headers        =>  '*%', # Defined for the interface
>>      url            =>  '$',  # full url requested
>>      script_nsp     =>  '$',  # namespace of the script to run
>>      dispatch_point =>  '$',  # formerly 'action' on the top-level
>> };
>
> I realize that CGI.pm may not be in use forever, and there is therefore
> some value in abstracting its interfaces instead of calling them
> natively, but is it worth the extra setup code?  What about non-web
> requests, e.g. thick clients - does this concept make writing those
> clients easier or harder?

LedgerSMB::Web::Request is designed specifically to provide HTTP
wrappers for workflow scripts.  Thick clients wouldn't use it.
>
>> struct LedgerSMB::Session, {
>
> [Oh.  Ignore my earlier comments about dbh in the ::User object.]
>
> I would probably combine this with ::Web::Request - if we're using some
> sort of web-based infrastructure, the two are inextricable, and if we're
> not (e.g. thick client), will we need to keep track of the ::Session
> object at all?

Sessions in thick clients would still be used in a few cases.
Bascially in 1.3, the session has two specific functions:

1)  Anti-XSRF (web-app-only) (moved to LedgerSMB::Web::Form)

2)  Management of discretionary, cross-transactional locks.  This
would be useful in thick client environments.

In 2.0, I see the anti-XSRF functions still tied to the session, but
since the emphasis would be on allowing the same session framework to
be useful across thick and web clients (for example, having long batch
payment processes allow "locking" of transactions to a session on a
thick client without preventing most other operations).  There might
be other ways of doing this with advisory locks and thick clients but
it might be simpler not to reinvent the wheel.

>
>> struct LedgerSMB::Web::Form {
>
> Same comments as about ::Session apply here, too.

The original idea here was specifically to implement an anti-XSRF
measure.  In 1.3, this is tied to the session, but stored on the
request object.  However, since session handles some things beyond web
interfaces, and this is web-interface-specific, I figured it shoudl be
broken off.  Other views are welcome however.
>
>> struct LedgerSMB::Auth, {
>  > subclass LedgerSMB::Auth::HTTP_Basic...
>  > subclass LedgerSMB::Auth::HTTP_KRB5....
>
> Similarly, I would not expect this to be something that needs to be
> exposed to the application innards.

That's a good point.  Let's drop this structure and simply make it a
module with some functions.

>  Either the web framework takes care
> of it, or something else does, and the ::User object is the result of
> the authentication process... I would also probably consider it an error
> to treat a user suddenly switching auth mechanisms from one request to
> the next as a single "session"

Hmmm...  A couple things occur to me here.....

First, ideally the authentication mechanism for a given installation
should be admin-defined.  For example, if you use HTTP_KRB5, you
really don't want users confused if they get authentication prompts so
it would be rather concerning if this were to change.  However,
suppose you have a second installation of the web app against the same
db for extranet users, and it requires username/password.  Then it
seems that it would be a real concern if this were to switch.

OTOH, I wonder about a single session for the user of the web app and
another for an add-on thick client app.

So if we worry about that we have to track it.  This suggests to me
the following modification:

struct LedgerSMB::Session, {
    id     => '$',
    token  => '$',
    dbclient => '$', # Pg type inet, connection of client connecting to Pg
    type => '$', #type of session (web, etc)
    auth_module => '$', #name of auth module
};

This could be subclassed for web interfaces by also adding a remote_addr field:
struct LedgerSMB::Session::Web, {
    id     => '$',
    token  => '$',
    dbclient => '$', # Pg type inet, connection of client connecting to Pg
    type => '$', #type of session (web, etc)
    auth_module => '$', #name of auth module
    remote_addr => '$', #type inet, remote address from web server perspective
};


>
> Now you tell me where I misinterpreted things or am making up demons of
> smoke and mirrors out of whole cloth.  :-)

that's what dialog is about :-)

Does that help?

Best Wishes,
Chris Travers