Re: [Freeipa-devel] [PATCH] 194 Handle case when trusted domain user access the Web UI
On 08/14/2012 03:21 AM, Endi Sukma Dewata wrote:
On 8/10/2012 6:11 AM, Petr Vobornik wrote:
WebUI catches the fact that the user can't access LDAP server with a
current ticket. It shows form-based auth login dialog. Previoustly an
ugly error was returned on an almost empty page, and user had no
recourse.
https://fedorahosted.org/freeipa/ticket/2897
I don't like the implementation much. Problem is that we don't separate
state variables and framework objects in IPA object. It is probably a
topic for fixing in 3.2.
I don't have an environment to test this, but the code looks fine, so
it's ACKed.
Some comments:
1. The logged_kerberos and logged_password cannot be true at the same
time, right?
Right.
Maybe they can be combined into a single variable (e.g.
login_status) which different values for unauthenticated, logged in via
kerberos, and logged in via password. Maybe the 'initialized' variable
can be combined too.
Yes logged_x can be combined that way. I would not merge it with
initialized though. Login and initialization are two separate steps.
The patch is pushed and I don't think the merge is important to do, so I
will leave it be. We might change it later when needed.
2. I agree about the state variables & framework objects separation.
Currently the 'IPA' object is both used as a singleton/global variable
and also as a name space for the framework. I think ideally we should
use a generic/non-IPA specific name for the framework. Probably
something like this:
// UI Framework
var UI = { ... };
UI.entity = function() { ... };
UI.facet = function() { ... };
// IPA UI
var IPA = UI();
IPA.user.entity = function() { ... };
IPA.user.details_facet = function() { ... };
IPA.init();
Copied to https://fedorahosted.org/freeipa/ticket/3030
--
Petr Vobornik
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 194 Handle case when trusted domain user access the Web UI
On 08/10/2012 07:11 AM, Petr Vobornik wrote: > WebUI catches the fact that the user can't access LDAP server with a > current ticket. It shows form-based auth login dialog. Previoustly an > ugly error was returned on an almost empty page, and user had no > recourse. > > https://fedorahosted.org/freeipa/ticket/2897 > > > I don't like the implementation much. Problem is that we don't > separate state variables and framework objects in IPA object. It is > probably a topic for fixing in 3.2. Was a ticket filed? > > ___ > Freeipa-devel mailing list > Freeipa-devel@redhat.com > https://www.redhat.com/mailman/listinfo/freeipa-devel -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. --- 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
Re: [Freeipa-devel] [PATCH] 194 Handle case when trusted domain user access the Web UI
On 08/14/2012 03:21 AM, Endi Sukma Dewata wrote:
> On 8/10/2012 6:11 AM, Petr Vobornik wrote:
>> WebUI catches the fact that the user can't access LDAP server with a
>> current ticket. It shows form-based auth login dialog. Previoustly an
>> ugly error was returned on an almost empty page, and user had no recourse.
>>
>> https://fedorahosted.org/freeipa/ticket/2897
>>
>>
>> I don't like the implementation much. Problem is that we don't separate
>> state variables and framework objects in IPA object. It is probably a
>> topic for fixing in 3.2.
>
> I don't have an environment to test this, but the code looks fine, so it's
> ACKed.
>
> Some comments:
>
> 1. The logged_kerberos and logged_password cannot be true at the same time,
> right? Maybe they can be combined into a single variable (e.g. login_status)
> which different values for unauthenticated, logged in via kerberos, and logged
> in via password. Maybe the 'initialized' variable can be combined too.
>
> 2. I agree about the state variables & framework objects separation. Currently
> the 'IPA' object is both used as a singleton/global variable and also as a
> name
> space for the framework. I think ideally we should use a generic/non-IPA
> specific name for the framework. Probably something like this:
>
> // UI Framework
> var UI = { ... };
> UI.entity = function() { ... };
> UI.facet = function() { ... };
>
> // IPA UI
> var IPA = UI();
> IPA.user.entity = function() { ... };
> IPA.user.details_facet = function() { ... };
>
> IPA.init();
>
Pushed to master.
Petr, please follow up with Endi on these comments when you return.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 194 Handle case when trusted domain user access the Web UI
On 8/10/2012 6:11 AM, Petr Vobornik wrote:
WebUI catches the fact that the user can't access LDAP server with a
current ticket. It shows form-based auth login dialog. Previoustly an
ugly error was returned on an almost empty page, and user had no recourse.
https://fedorahosted.org/freeipa/ticket/2897
I don't like the implementation much. Problem is that we don't separate
state variables and framework objects in IPA object. It is probably a
topic for fixing in 3.2.
I don't have an environment to test this, but the code looks fine, so
it's ACKed.
Some comments:
1. The logged_kerberos and logged_password cannot be true at the same
time, right? Maybe they can be combined into a single variable (e.g.
login_status) which different values for unauthenticated, logged in via
kerberos, and logged in via password. Maybe the 'initialized' variable
can be combined too.
2. I agree about the state variables & framework objects separation.
Currently the 'IPA' object is both used as a singleton/global variable
and also as a name space for the framework. I think ideally we should
use a generic/non-IPA specific name for the framework. Probably
something like this:
// UI Framework
var UI = { ... };
UI.entity = function() { ... };
UI.facet = function() { ... };
// IPA UI
var IPA = UI();
IPA.user.entity = function() { ... };
IPA.user.details_facet = function() { ... };
IPA.init();
--
Endi S. Dewata
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
