[Freeipa-devel] [PATCH 61] Cache authentication in session

[John Dennis]

On 02/08/2012 03:23 PM, Endi Sukma Dewata wrote:
> John,
>
> Per discussion with Rob, Simo, and Adam we're considering to make these
> changes:
>
> 1. For backward compatibility with curl or 3rd party apps, we should
> keep the existing authentication without session in /ipa/json and /ipa/xml.

I originally wanted to use different URL's but was persuaded not to. I'm 
happy to see this recommendation, generally I think it's a good idea but 
see my caveat below.

If I have any concern it's simply with the combinatoric matrix which has 
to be tested with this change. With something as critical as 
authentication having just one code path to validate has merit. But then 
again allowing both session and non-session based auth also has merit. 
Pick and choose your evils :-) Given we have already heavily tested the 
old mechanism I don't think there is too much risk in keeping it and 
just adding a new mechanism. Plus we will want handle form based auth 
soon and this adds yet another code path, so we probably should just be 
prepared to deal with the various combinations.

> 2. For the UI we can use the sessions using different URIs:
>      * /ipa/login for authentication
>      * /ipa/session/json for the actual operations
>
> 3. If we modify the CLI later to use the sessions it will use the
> following URIs:
>      * /ipa/login for authentication
>      * /ipa/session/xml for the actual operations
>
> Is this OK? How difficult is it to make the above changes?

Yes it's OK. I don't think it will be terribly difficult. We register 
RPC handlers for each URL in the server. It would require adding new 
handlers. But I suspect some of the code in the duplicated handlers 
would be identical and I'm not a fan of cut-n-paste code logic, I'd 
rather see things call into common code. So although duplicating the 
handlers would be straight forward and quick we might want to refactor 
some logic to keep duplication to a minimum so we only differ in the 
actual auth logic in the handlers but otherwise share common code to 
dispatch the RPC in the handler. My estimate would be 1-2 days of work 
to recode and test depending on the extent of any necessary refactoring.

> We also want to tie the authorization to the sessions, so whenever the
> session expires the UI will reauthenticate using /ipa/login and then
> reload the authorization info in a separate operation using
> /ipa/session/json and then redraw the UI if necessary. This way we can
> keep the /ipa/login generic enough to be used by both XML and JSON clients.

Unless I've misunderstood, the existing code pretty much does this 
already, albeit with different URL's. Aside from adjusting the URL's was 
there something else you were looking for?

Just to be clear, here is the sequence of operations:

Client sends post to /ipa/session/json (or /ipa/session/xml). (This 
could be the first post for which there is no existing session or a post 
after the session has expired, it doesn't matter which case it is)

Server responds with auth failure (and session cookie)

Client sends GET to /ipa/login along with session cookie which refreshes 
credentials in session (or fails because the credentials could not be 
refreshed, in which case this is treated as a hard error and processing 
stops on the client signaling an error to the user)

Presuming the credential refresh succeeded the client resends the 
previous post that failed due to insufficient auth (along with session 
cookie) to /ipa/session/{json,xml} which now succeeds because session is 
populated with valid credentials from the previous step. From the user's 
perspective nothing is different other than a possible delay due to the 
extra protocol exchanges occurring under the covers.

Note: for the json case there is no need to reload the UI unless the 
previous principal does not match the current principal (i.e. switch 
user). To the best of my knowledge the existing code already handles 
this case.

> I think the UI changes can be done separately, I'll open the tickets.

Not sure I see the need for new tickets, could you elaborate? I think 
the UI code pretty much already does what we want, it just needs to be 
tweaked a bit for the URL changes. That small adjustment can be handled 
in the updated patch.

The only thing which might be unaccounted for would be if the web UI for 
some reason wanted to use the old /ipa/json and not use sessions. It 
would need some extra logic to handle this but I don't see any need for 
this, after the server is updated to support sessions it sends back an 
ipa.js javascript file to the client which always elects to use the 
/ipa/session/json URL. If for some reason the browser is still using an 
old copy of ipa.js it simply ends up using the old /ipa/json URL without 
sessions, which should "just work".

Let me know your thoughts.

-- 
John Dennis <jdennis at redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/