On 02/09/2012 12:16 AM, Endi Sukma Dewata wrote:
On 2/8/2012 6:29 PM, John Dennis wrote:
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.
Is there a way to use the same URLs to support both mechanisms? For
example, can the client specify the mechanism to use in a request
header, or some other tricks? If it's not possible I think we would have
to use separate URLs and maintain backward compatibility.
No. The URL is what tells us how the client is authenticating and which
RPC mechanism they indent to use. The reason why the URL determines the
auth method is because Apache enforces authentication based on a
"location" i.e. a URL. This occurs before we see the request, in fact we
won't even see the request unless Apache has validated the
authentication. For sessions we don't use Apache based authentication,
instead we leave the URL "open" without Apache authentication and
perform the authentication inside the handler attached to the URL.
It might be possible to fold the json and xmlrpc URL's into a single RPC
URL if we could examine the request and determine the RPC mechanism from
the data in the request but I don't see much advantage in this.
We've actually been around this tree before with using different URL's
and have concluded it's the only viable approach. Our choice is to
either keep the existing URL's and introduce new incompatible behavior
which was the decision we came to a few weeks back. Or we introduce new
URL's, at the time the feeling was we wanted to keep the same URL's for
location consistency sacrificing behavior consistency. Now we've
reversed this decison which I'm O.K. with, but we can't have our cake
and eat it too, we have to pick one or the other approach.
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?
Currently when the UI is loaded for the first time it will execute an
ipa_init operation which consists of:
1. Loading I18 messages.
2. Getting user info (whoami).
3. Loading environment variables.
4. Checking whether DNS is enabled.
5. Loading objects& commands metadata for labels and validations.
Right now if the UI detects a change in the principal name or server
version it will reload itself and so it will execute the ipa_init again.
With your patch #61 the UI will automatically renew the session, but if
the principal stays the same it won't execute the ipa_init again.
I think the only thing that might change after session renewal is user
info (#2). The others are pretty much static. So we probably can move
whoami into another method that can be called separately.
I agree this represents a good modification to the client logic however
it don't see how it's specific to sessions. Wouldn't the same hold true
for the current case when the principal changes?
Bottom line, it's a good optimization which should be done but it seems
independent of the session work.
In other words, the UI needs to be changed so that during initial load
the UI should do ipa_init and whoami, and after each session renewal it
should only do the whoami only.
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.
Right.
I think the UI changes can be done separately, I'll open the tickets.
What I meant here is that since we're keeping backward compatibility you
don't really have to modify the UI in the same patch because it will
still work like before. Your UI changes with the patch that I sent in
the earlier email would be good enough to push, but if you decide to
delay it to make additional changes it should be fine too.
Not sure I see the need for new tickets, could you elaborate?
On top of your changes there would be some additional UI changes:
1. As described above, we need to move whoami out of ipa_init and call
it after each session renewal.
2. The whoami output contains the user's authz info (group/role
membership). We need to redraw the UI components if the user's authz has
changed and make sure they are re-initialized correctly.
3. We need to redirect the user to the UI main page if the current page
is no longer accessible due to authz changes.
O.K. got it, here are my thoughts:
1) The enhanced logic is independent of sessions.
2) We need to test and exercise the new session auth so that code
should be there.
3) However, adding the session logic in item 2 will be affected by the
code changes in item 1.
Therefore both should be done ASAP. We can either add the session code
immediately and modify it later when the code changes in item 1 are done
or put the session changes in immediately and modify it to use the new
logic in item 1 when it's ready.
I don't have strong feelings either way. However I would prefer if you
or another UI guru made the changes you outline above rather than me. I
think that would be more efficient and someone who intimately knows the
UI code would be less likely to introduce a problem I might not be aware of.
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.
Yes, that too.
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".
Right, no need to worry about this, the UI will just use one of the methods.
See also the discussion about session expiration:
https://fedorahosted.org/freeipa/ticket/2361
Interesting discussion. I tend to agree with Simo that session auth
duration should be shorter. I think 5 minutes is too short but that's
easy to adjust, we already have a config item for that, it just means
editing the config.
I do think we need a logout button which will invalidate the session
auth. The current patch does not include an RPC command to accomplish
that, but it's on my to-do list. Since we have to redo the patch to
handle both session and non-session auth I could add that in at the same
time (or we could open a new ticket and defer).
--
John Dennis <jden...@redhat.com>
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
