[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 2/6/2012 12:35 PM, John Dennis wrote:
rebased because patch 61-2 did not apply to master.

I've looked at the ipa.js, attached is a patch that fixes several issues:

1. The error_handler_login() does nothing if it gets an error other than 401, it was causing the unit tests to fail. It's supposed to call the original error_handler().

2. In line 62 the login_url is not needed for the other case because I don't think we can generate error 401 with the test data.

3. There were some jslint warnings in line 312 and 424, it's missing a semi colon at the end of the line.

4. It replaces the tabs used for indentation with spaces in IPA.get_credentials().

Feel free to merge this patch into yours or apply it separately.

I found some other issues but it probably can be addressed separately:

5. If the ipa_memcached is restarted (to simulate session expiration), subsequent UI operations will generate the 'Kerberos ticket no longer valid' error dialog. After about 45 seconds it will work again. Ideally the users should not see this. I'm not sure if a similar situation will happen when the session times out in a normal situation.

6. The curl command like in install/ui/test/bin/update_ipa_init.sh will not work anymore because it doesn't use session. We need to figure out how to enable sessions on curl.

--
Endi S. Dewata
From ab96c8a24d1b4c88118dc6cb845473111d7e82a5 Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" <edewata redhat com>
Date: Wed, 8 Feb 2012 11:38:39 -0600
Subject: [PATCH] Fixed ipa.js for sessions.

The patch fixes a problem in error_handler_login() when it gets
an error other than 401.

The login_url is not needed for fixtures because it does not need
authentication.

The patch also fixes jslint warnings and formatting issues.
---
 install/ui/ipa.js |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index a424fe95ab9fa5674116d9c88186bc638b56b4e5..82e8920433b855ae920505a4cc10f862fb1d579d 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -59,10 +59,11 @@ var IPA = function() {
         // if current path matches live server path, use live data
         if (that.url && window.location.pathname.substring(0, that.url.length) === that.url) {
             that.json_url = params.url || '/ipa/json';
-            that.login_url = params.url || '/ipa/login'; // FIXME, what about the other case below?
+            that.login_url = params.url || '/ipa/login';
 
         } else { // otherwise use fixtures
             that.json_path = params.url || "test/data";
+            // that.login_url is not needed for fixtures
         }
 
         $.ajaxSetup(that.ajax_options);
@@ -300,8 +301,8 @@ IPA.get_credentials = function() {
 
     var request = {
         url: IPA.login_url,
-	async: false,
-	type: "GET",
+        async: false,
+        type: "GET",
         success: success_handler,
         error: error_handler
     };
@@ -309,7 +310,7 @@ IPA.get_credentials = function() {
     $.ajax(request);
 
     return status;
-}
+};
 
 /**
  * Call an IPA command over JSON-RPC.
@@ -421,13 +422,13 @@ IPA.command = function(spec) {
                 var login_status = IPA.get_credentials();
 
                 if (login_status === 200) {
-                    that.request.error = error_handler
+                    that.request.error = error_handler;
                     $.ajax(that.request);
-                } else {
-                    // error_handler() calls IPA.hide_activity_icon()
-                    error_handler.call(this, xhr, text_status, error_thrown);
+                    return;
                 }
             }
+            // error_handler() calls IPA.hide_activity_icon()
+            error_handler.call(this, xhr, text_status, error_thrown);
         }
 
         /*
-- 
1.7.6.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]