[Freeipa-devel] [PATCH] 0062 Don't crash when server returns extra output
This will make older clients usable if new output items get added to commands. Since there might be important information in the extra output, it's not ignored as the ticket asks. Instead it's printed, but not formatted nicely as the client doesn't have enough info for that. https://fedorahosted.org/freeipa/ticket/1721 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0050 Fail on unknown Command options
On 06/07/2012 11:37 AM, Petr Vobornik wrote:
On 05/28/2012 04:16 PM, Martin Kosek wrote:
On Mon, 2012-05-28 at 15:46 +0200, Petr Vobornik wrote:
On 05/25/2012 09:20 AM, Petr Vobornik wrote:
On 05/16/2012 02:11 PM, Martin Kosek wrote:
On Wed, 2012-05-16 at 10:37 +0200, Petr Viktorin wrote:
On 05/16/2012 09:58 AM, Martin Kosek wrote:
On Tue, 2012-05-15 at 13:35 +0200, Petr Viktorin wrote:
On 05/15/2012 09:55 AM, Martin Kosek wrote:
On Mon, 2012-05-14 at 14:47 +0200, Petr Viktorin wrote:
The final part of rejecting unknown Command arguments: enable the
validation, add tests.
Also fix up things that were changed since the previous patches.
https://fedorahosted.org/freeipa/ticket/2509
8--
Attaching a rebased patch.
Yup, this one is fine. Now, I did not find issues in the patch itself,
tests are clean.
However, thanks to this new check I found issues in Web UI
(automember,
selfservice, delegation screen) which use illegal options and which
should be fixed before we push your patch:
https://fedorahosted.org/freeipa/ticket/2760
Martin
I found an issue in automountmap_add_indirect. It complains that 'key'
is unknown option.
I found another options which were functional and now it complains:
* hbacsvcgroup_find: no_hbacsvc
* hbacsvc_find: not_in_hbacsvcgroup
* same issue in sudo commands and sudo command groups.
I didn't check all relationships, so it may be broken elsewhere as well.
I don't think this is an error on server side - it never had filter
options like these in the modules you referenced (though we may add them
as an RFE when needed).
When you pass these options in the UI to the server side, its just NOOP
- or an error when Petr's patch is applied.
Martin
All issues found in Web UI are fixed.
Updated and rebased patch attached.
--
Petr³
From 172acc95052fc5c3e14f15dcec6bdaefcd42e303 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 17 Apr 2012 12:42:35 -0400
Subject: [PATCH] Fail on unknown Command options
When unknown keyword arguments are passed to a Command, raise an
error instead of ignoring them.
Options used when IPA calls its commands internally are listed
in a new Command attribute called internal_options, and allowed.
Previous patches (0b01751c, c45174d6, c5689e7f) made IPA not use
unknown keyword arguments in its own commands and tests, but since
that some violations were reintroduced in permission_find and tests.
Fix those.
Tests included; both a frontend unittest and a XML-RPC test via the
ping plugin (which was untested previously).
https://fedorahosted.org/freeipa/ticket/2509
---
ipalib/frontend.py | 13 +--
ipalib/plugins/aci.py |2 ++
ipalib/plugins/automount.py |6 +++-
ipalib/plugins/permission.py| 47 +++-
tests/test_cmdline/test_cli.py |6 ++--
tests/test_ipalib/test_frontend.py |5 +++
tests/test_xmlrpc/test_host_plugin.py |2 +-
tests/test_xmlrpc/test_netgroup_plugin.py |6 ++--
tests/test_xmlrpc/test_permission_plugin.py | 12 +++
tests/test_xmlrpc/test_ping_plugin.py | 52 +++
10 files changed, 123 insertions(+), 28 deletions(-)
create mode 100644 tests/test_xmlrpc/test_ping_plugin.py
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 10087ba24bc310a036a22d4c52740825de3184ce..c28fa54ae933fe58833579b3e3aee0e1ad50f198 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -29,7 +29,8 @@
from output import Output, Entry, ListOfEntries
from text import _, ngettext
-from errors import ZeroArgumentError, MaxArgumentError, OverlapError, RequiresRoot, VersionError, RequirementError
+from errors import (ZeroArgumentError, MaxArgumentError, OverlapError,
+RequiresRoot, VersionError, RequirementError, OptionError)
from errors import InvocationError
from constants import TYPE_ERROR
from ipapython.version import API_VERSION
@@ -404,6 +405,8 @@ class Command(HasParam):
output_params = Plugin.finalize_attr('output_params')
has_output_params = tuple()
+internal_options = tuple()
+
msg_summary = None
msg_truncated = _('Results are truncated, try a more specific search')
@@ -520,7 +523,13 @@ def __args_2_params(self, values):
def __options_2_params(self, options):
for name in self.params:
if name in options:
-yield (name, options[name])
+yield (name, options.pop(name))
+# If any options remain, they are either internal or unknown
+unused_keys = set(options).difference(self.internal_options)
+unused_keys.discard('version')
+if unused_keys:
+raise OptionError(_('Unknown option: %(option)s'),
+option=unused_keys.pop())
def args_options_2_entry(self, *args, **options):
diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py
index
Re: [Freeipa-devel] [PATCH] 0062 Don't crash when server returns extra output
On Tue, 2012-06-12 at 13:12 +0200, Petr Viktorin wrote: This will make older clients usable if new output items get added to commands. Since there might be important information in the extra output, it's not ignored as the ticket asks. Instead it's printed, but not formatted nicely as the client doesn't have enough info for that. https://fedorahosted.org/freeipa/ticket/1721 Patch is missing. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0062 Don't crash when server returns extra output
On 06/12/2012 02:38 PM, Simo Sorce wrote:
On Tue, 2012-06-12 at 13:12 +0200, Petr Viktorin wrote:
This will make older clients usable if new output items get added to
commands.
Since there might be important information in the extra output, it's not
ignored as the ticket asks. Instead it's printed, but not formatted
nicely as the client doesn't have enough info for that.
https://fedorahosted.org/freeipa/ticket/1721
Patch is missing.
Simo.
My apologies
--
Petr³
From e039780d0038b57b621620eb429061a3fdfa311c Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 5 Jun 2012 10:26:35 -0400
Subject: [PATCH] Don't crash when server returns extra output
When input new optional parameter is added to server API, its still
usable with old clients with old API, it just cannot use the new parameter.
However, when a new Output parameter is added, older clients immediately
failed with a validation error.
This patch relaxes the validation so that extra Output items are allowed
in the client.
The extra items are displayed at the end of a command's output (crudely
formatted and with raw names, since the older client doesn't have any
formatting info).
This should ensure that important messages are not lost.
On the server, the validation still does not allow extra Output items.
Also, fix an error where if the expected and actual number of Output
items was the same, validation succeeded even if the keys were different.
https://fedorahosted.org/freeipa/ticket/1721
---
ipalib/frontend.py | 32 +--
tests/test_ipalib/test_frontend.py | 59 +++-
2 files changed, 67 insertions(+), 24 deletions(-)
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 10087ba24bc310a036a22d4c52740825de3184ce..e05638b2f42025338aa5c6f35e7166462ca8f196 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -904,18 +904,22 @@ def validate_output(self, output):
raise TypeError('%s: need a %r; got a %r: %r' % (
nice, dict, type(output), output)
)
-if len(output) len(self.output):
-missing = sorted(set(self.output).difference(output))
+missing = set(self.output).difference(output)
+if missing:
raise ValueError('%s: missing keys %r in %r' % (
-nice, missing, output)
-)
-if len(output) len(self.output):
-extra = sorted(set(output).difference(self.output))
-raise ValueError('%s: unexpected keys %r in %r' % (
-nice, extra, output)
+nice, sorted(missing), output)
)
+if self.api.env.in_server:
+extra = set(output).difference(self.output)
+if extra:
+raise ValueError('%s: unexpected keys %r in %r' % (
+nice, sorted(extra), output)
+)
for o in self.output():
-value = output[o.name]
+try:
+value = output[o.name]
+except KeyError:
+continue
if not (o.type is None or isinstance(value, o.type)):
raise TypeError('%s:\n output[%r]: need %r; got %r: %r' % (
nice, o.name, o.type, type(value), value)
@@ -999,6 +1003,16 @@ def output_for_cli(self, textui, output, *args, **options):
elif isinstance(result, int):
textui.print_count(result, '%s %%d' % unicode(self.output[o].doc))
+# If there is extra output from the server, it probably means that
+# the server is newer and an output item was added.
+# Since the item might be important, we show it even though we can't
+# format it correctly
+extra = set(output).difference(self.output)
+if extra:
+textui.print_dashed(unicode(_('Unexpected output from server:')))
+for item in sorted(extra):
+textui.print_line('%s: %s' % (item, output[item]))
+
return rv
# list of attributes we want exported to JSON
diff --git a/tests/test_ipalib/test_frontend.py b/tests/test_ipalib/test_frontend.py
index b717a43ad4c68940582f901308f4dd4da77834ee..dcdea694a34adbf0e351cbfa924b2f2b0666b06c 100644
--- a/tests/test_ipalib/test_frontend.py
+++ b/tests/test_ipalib/test_frontend.py
@@ -610,69 +610,98 @@ def forward(self, *args, **kw):
assert o.run.im_func is self.cls.run.im_func
assert ('forward', args, kw) == o.run(*args, **kw)
-def test_validate_output(self):
+def test_validate_output_basic(self):
Test the `ipalib.frontend.Command.validate_output` method.
-class Example(self.cls):
+class example(self.cls):
has_output = ('foo', 'bar', 'baz')
-inst = Example()
-inst.finalize()
+(api, home) = create_test_api(in_server=True)
+api.register(example)
+
+api.finalize()
+inst
Re: [Freeipa-devel] [PATCH] 1025 set fixed primary IPA server in client
Dmitri Pal wrote: On 06/11/2012 03:45 PM, Rob Crittenden wrote: Add --fixed-primary flag to control the order of ipa_server in sssd.conf. When set the discovered (or passed) server will be set first rather than _srv_. The default is to have _srv_ set first. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Have you talked to Jan Zeleny? He is working on the concept of the primary and secondary servers in sssd. It seems that these efforts should be coordinated. https://fedorahosted.org/sssd/ticket/1128 Yes, I'm aware of what he is working on. The work that Jan is doing will simply make this work better. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1024 add client session support
On 06/11/2012 06:49 PM, Martin Kosek wrote: On Thu, 2012-06-07 at 22:55 -0400, Rob Crittenden wrote: Rob Crittenden wrote: Rob Crittenden wrote: This adds client session support. The session key is stored in the kernel key ring. Your first request should go to /ipa/session/xml where it should be rejected with a 401. The next will go to /ipa/xml which will be accepted. This should all be invisible to the client. Subsequent requests should go to /ipa/session/xml which should let you in with the cookie. You can add the -vv option after ipa to see fully what is going on, e.g. ipa -vv user-show admin To manage your keyring use the keyctl command like: $ keyctl list @s 2 keys in keyring: 353548226: --alswrv 1000 -1 keyring: _uid.1000 941350591: --alswrv 1000 1000 user: ipa_session_cookie To remove a key: $ keyctl unlink 941350591 @s rob Hmm, this doesn't play too nice with the lite-server. Let me see if I can track it down. The ccache is being removed, probably as part of the session code. Sessions don't make sense with the lite server since it uses the local ccache directly. Updated patch. Don't clean up the ccache if in the lite-server. rob Good job there. I tested various scenarios (2 master, fallback with SRV records, old client (RHEL 6.2)) and most worked for me, but only I worked under the root account. This is what I got with non-root: $ ipa user-show admin ... ipa: DEBUG: stderr= ipa: DEBUG: args=keyctl search @s user ipa_session_cookie ipa: DEBUG: stdout=113632397 ipa: DEBUG: stderr= ipa: DEBUG: args=keyctl pupdate 113632397 ipa: DEBUG: stdout= ipa: DEBUG: stderr=keyctl_update: Permission denied ipa: INFO: trying https://vm-131.idm.lab.bos.redhat.com/ipa/session/xml ipa: DEBUG: NSSConnection init vm-131.idm.lab.bos.redhat.com ipa: ERROR: cannot connect to 'any of the configured servers': ... Shouldn't we use @us instead of @s for storing user session keys? Secondly, I wonder if we also plan to add some logout command? This way even if I do kdestroy, the session still exist and someone other may still execute commands. Martin Also: keyctl is in the keyutils package, which we need to depend on. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 274 Password change capability for form-based auth
On Mon, 2012-06-11 at 13:52 -0400, Rob Crittenden wrote:
Martin Kosek wrote:
On Mon, 2012-06-11 at 10:36 +0200, Martin Kosek wrote:
On Thu, 2012-06-07 at 23:07 -0400, Simo Sorce wrote:
On Thu, 2012-06-07 at 22:28 -0400, Rob Crittenden wrote:
Martin Kosek wrote:
You can use the attached script (changepw.py) to test the PW change
interface from command line (on IPA server).
---
IPA server web form-based authentication allows logins for users
which for some reason cannot use Kerberos authentication. However,
when a password for such users expires, they are unable change the
password via web interface.
This patch adds a new WSGI script attached to URL
/ipa/session/change_password which can be accessed without
authentication and which provides password change capability
for web services.
The actual password change in the script is processed with kpasswd
to be consistent with /ipa/session/login_password.
Password result is passed both in the resulting HTML page, but
also in HTTP headers for easier parsing in web services:
X-IPA-Pwchange-Result: {ok, invalid-password, policy-error}
(optional) X-IPA-Pwchange-Policy-Error: $policy_error_text
https://fedorahosted.org/freeipa/ticket/2276
It is probably more efficient to change the password using ldap. Simo,
do you know of an advantage of using one over the other? Better password
policy reporting may be reason enough.
Yes you'll get better error reporting, plus forking out kpasswd is quite
ugly, the python ldap code should be able to use the ldap passwd extend
op quite easily.
Simo.
Ok, sending a second version of the patch based on password change via
LDAP. The error reporting is indeed easier and with no hard-coded
parsing.
Martin
This patch will only work with SELinux disabled, it seems there is a
regression in SELinux policy which does not allow httpd to connect to
dirsrv socket. I logged a Bug:
https://bugzilla.redhat.com/show_bug.cgi?id=830764
This issue also disables other pages using dirsrv socket, like the
migration page or password-expiration detection in form-based auth.
Martin
For '200 Success' you can use rpcserver.HTTP_STATUS_SUCCESS.
Fixed.
This works ok and does successfully change passwords but I don't like
the logging very much.
Actually it does, it just did it in DEBUG level.
I adapted the logging style from /ipa/session/login_password WSGI
script, but I see that since this is a special page, it should have a
bit different logging.
Under normal conditions, it now prints a line when
- the WSGI script is started on INFO level, i.e. in httpd error_log by
default
- parameters are validated and we start password change for user (user
is now printed in log too - this will be useful)
- when the WSGI script finishes - with either success or error status
It should say that this is the password request
URI somewhere at a minimum. Having the HTTP response is a bit strange
too, and I don't know if a 400 should be logged as info.
I used bad_request method of HTTP_Status class. It uses info log level
for 400 statuses. I can change that, but it will be changed for all WSGI
scripts using HTTP_Status. So far, judging from what I saw in
rpcserver.py we use error log level when there is a problem on our side
and not in a user request...
I think this test program could be made into a test suite too,
particularly to check the more esoteric parts like checking for missing
options, too many options, etc.
rob
I added a test suite exercising this WSGI script. It is based on
built-in httplib instead of original pyCurl - it has much better output
parsing and is easier to handle.
The new unit test tests bad options, authentication errors and of course
successful password change, including a verification that that the
password was actually changed.
Martin
From c60cd566c7b84ebcd82730d4dfce93d610e4aca9 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Wed, 6 Jun 2012 14:38:08 +0200
Subject: [PATCH] Password change capability for form-based auth
IPA server web form-based authentication allows logins for users
which for some reason cannot use Kerberos authentication. However,
when a password for such users expires, they are unable change the
password via web interface.
This patch adds a new WSGI script attached to URL
/ipa/session/change_password which can be accessed without
authentication and which provides password change capability
for web services.
The actual password change in the script is processed by LDAP
password change command.
Password result is passed both in the resulting HTML page, but
also in HTTP headers for easier parsing in web services:
X-IPA-Pwchange-Result: {ok, invalid-password, policy-error, error}
(optional) X-IPA-Pwchange-Policy-Error: $policy_error_text
https://fedorahosted.org/freeipa/ticket/2276
---
install/conf/ipa.conf |8 ++-
Re: [Freeipa-devel] [PATCH] 1024 add client session support
Petr Viktorin wrote: On 06/11/2012 06:49 PM, Martin Kosek wrote: On Thu, 2012-06-07 at 22:55 -0400, Rob Crittenden wrote: Rob Crittenden wrote: Rob Crittenden wrote: This adds client session support. The session key is stored in the kernel key ring. Your first request should go to /ipa/session/xml where it should be rejected with a 401. The next will go to /ipa/xml which will be accepted. This should all be invisible to the client. Subsequent requests should go to /ipa/session/xml which should let you in with the cookie. You can add the -vv option after ipa to see fully what is going on, e.g. ipa -vv user-show admin To manage your keyring use the keyctl command like: $ keyctl list @s 2 keys in keyring: 353548226: --alswrv 1000 -1 keyring: _uid.1000 941350591: --alswrv 1000 1000 user: ipa_session_cookie To remove a key: $ keyctl unlink 941350591 @s rob Hmm, this doesn't play too nice with the lite-server. Let me see if I can track it down. The ccache is being removed, probably as part of the session code. Sessions don't make sense with the lite server since it uses the local ccache directly. Updated patch. Don't clean up the ccache if in the lite-server. rob Good job there. I tested various scenarios (2 master, fallback with SRV records, old client (RHEL 6.2)) and most worked for me, but only I worked under the root account. This is what I got with non-root: $ ipa user-show admin ... ipa: DEBUG: stderr= ipa: DEBUG: args=keyctl search @s user ipa_session_cookie ipa: DEBUG: stdout=113632397 ipa: DEBUG: stderr= ipa: DEBUG: args=keyctl pupdate 113632397 ipa: DEBUG: stdout= ipa: DEBUG: stderr=keyctl_update: Permission denied ipa: INFO: trying https://vm-131.idm.lab.bos.redhat.com/ipa/session/xml ipa: DEBUG: NSSConnection init vm-131.idm.lab.bos.redhat.com ipa: ERROR: cannot connect to 'any of the configured servers': ... Shouldn't we use @us instead of @s for storing user session keys? Secondly, I wonder if we also plan to add some logout command? This way even if I do kdestroy, the session still exist and someone other may still execute commands. Martin Also: keyctl is in the keyutils package, which we need to depend on. Nice catch, updated patch. I also included a bit more about why I chose @s instead of @us. Basically it is so a different shell can have a different session and therefore a different identity. I'm going to open a ticket for the logout. For the short-term one can do something like: $ keyctl purge user Or more precisely: $ keyctl list @s 2 keys in keyring: 353548226: --alswrv 1000-1 keyring: _uid.1000 207626975: --alswrv 1000 1000 user: ipa_session_cookie $ keyctl unlink 207626975 1 links removed rob From 9c28626140ceeb0c4b9ce2879a1a18273d9696da Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Wed, 6 Jun 2012 22:54:16 -0400 Subject: [PATCH] Store session cookie in ccache for cli users Try to use the URI /ipa/session/xml if there is a key in the kernel keyring. If there is no cookie or it turns out to be invalid (expired, whatever) then use the standard URI /ipa/xml. This in turn will create a session that the user can then use later. https://fedorahosted.org/freeipa/ticket/2331 --- freeipa.spec.in |1 + install/conf/ipa.conf| 10 +- ipalib/rpc.py| 84 +++-- ipapython/kernel_keyring.py | 102 +++ ipaserver/plugins/xmlserver.py |3 +- ipaserver/rpcserver.py | 229 -- tests/test_ipapython/test_keyring.py | 147 ++ 7 files changed, 497 insertions(+), 79 deletions(-) create mode 100644 ipapython/kernel_keyring.py create mode 100644 tests/test_ipapython/test_keyring.py diff --git a/freeipa.spec.in b/freeipa.spec.in index 17c748b6dc9e3b1b491bc6e0b49f915834adf951..e608979631c46dc3c7f3c10b0be5a6246ca8afa0 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -154,6 +154,7 @@ Requires(preun): python initscripts chkconfig Requires(postun): python initscripts chkconfig %endif Requires: python-dns +Requires: keyutils # We have a soft-requires on bind. It is an optional part of # IPA but if it is configured we need a way to require versions diff --git a/install/conf/ipa.conf b/install/conf/ipa.conf index 89c9849ca6656ae3da585a72392d5d2463f4d892..948fee6fc10d35c3fefb82d0f3ed09cefb9b16e8 100644 --- a/install/conf/ipa.conf +++ b/install/conf/ipa.conf @@ -1,5 +1,7 @@ # -# VERSION 4 - DO NOT REMOVE THIS LINE +# VERSION 5 - DO NOT REMOVE THIS LINE +# +# This file may be overwritten on upgrades. # # LoadModule auth_kerb_module modules/mod_auth_kerb.so @@ -66,6 +68,12 @@ KrbConstrainedDelegationLock ipa Allow from all /Location +Location /ipa/session/xml + Satisfy Any + Order Deny,Allow + Allow from all +/Location + Location /ipa/session/login_password Satisfy Any Order Deny,Allow diff --git a/ipalib/rpc.py b/ipalib/rpc.py index
[Freeipa-devel] Python debugging tip
Every so often I'm confronted with being unable to determine where an exception is being raised. Unfortunately pdb (Python debugger) cannot be set to break on an exception (oddly missing functionality). It turns out that pdb is built upon a simple Python API, a callback that is invoked whenever the interpreter steps. The callback is set with sys.settrace() (see Python doc for full explanation). To find my elusive exception I simply added this tiny bit of code that sets the tracing function and whenever a TypeError occurred it printed out the stacktrace. Caveat, it's verbose, will print out every TypeError exception including those you're not looking for and it will do so for every place on the stack. A somewhat minor inconvenience if you can't find the exception using any of the other means. You can of course tweak this to be much more specific by examining the other context information passed to the callback but for a quick dirty hack it didn't seem worthwhile. Hope its useful to someone. John import sys import traceback def tracefunc(frame, event, arg): if event == 'exception': exc, value, tb = arg if isinstance(value, TypeError): print Exception %s % value print ''.join(traceback.format_tb(tb)) return tracefunc sys.settrace(tracefunc) -- 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
Re: [Freeipa-devel] [PATCH] 274 Password change capability for form-based auth
Martin Kosek wrote:
On Mon, 2012-06-11 at 13:52 -0400, Rob Crittenden wrote:
Martin Kosek wrote:
On Mon, 2012-06-11 at 10:36 +0200, Martin Kosek wrote:
On Thu, 2012-06-07 at 23:07 -0400, Simo Sorce wrote:
On Thu, 2012-06-07 at 22:28 -0400, Rob Crittenden wrote:
Martin Kosek wrote:
You can use the attached script (changepw.py) to test the PW change
interface from command line (on IPA server).
---
IPA server web form-based authentication allows logins for users
which for some reason cannot use Kerberos authentication. However,
when a password for such users expires, they are unable change the
password via web interface.
This patch adds a new WSGI script attached to URL
/ipa/session/change_password which can be accessed without
authentication and which provides password change capability
for web services.
The actual password change in the script is processed with kpasswd
to be consistent with /ipa/session/login_password.
Password result is passed both in the resulting HTML page, but
also in HTTP headers for easier parsing in web services:
X-IPA-Pwchange-Result: {ok, invalid-password, policy-error}
(optional) X-IPA-Pwchange-Policy-Error: $policy_error_text
https://fedorahosted.org/freeipa/ticket/2276
It is probably more efficient to change the password using ldap. Simo,
do you know of an advantage of using one over the other? Better password
policy reporting may be reason enough.
Yes you'll get better error reporting, plus forking out kpasswd is quite
ugly, the python ldap code should be able to use the ldap passwd extend
op quite easily.
Simo.
Ok, sending a second version of the patch based on password change via
LDAP. The error reporting is indeed easier and with no hard-coded
parsing.
Martin
This patch will only work with SELinux disabled, it seems there is a
regression in SELinux policy which does not allow httpd to connect to
dirsrv socket. I logged a Bug:
https://bugzilla.redhat.com/show_bug.cgi?id=830764
This issue also disables other pages using dirsrv socket, like the
migration page or password-expiration detection in form-based auth.
Martin
For '200 Success' you can use rpcserver.HTTP_STATUS_SUCCESS.
Fixed.
This works ok and does successfully change passwords but I don't like
the logging very much.
Actually it does, it just did it in DEBUG level.
I adapted the logging style from /ipa/session/login_password WSGI
script, but I see that since this is a special page, it should have a
bit different logging.
Under normal conditions, it now prints a line when
- the WSGI script is started on INFO level, i.e. in httpd error_log by
default
- parameters are validated and we start password change for user (user
is now printed in log too - this will be useful)
- when the WSGI script finishes - with either success or error status
It should say that this is the password request
URI somewhere at a minimum. Having the HTTP response is a bit strange
too, and I don't know if a 400 should be logged as info.
I used bad_request method of HTTP_Status class. It uses info log level
for 400 statuses. I can change that, but it will be changed for all WSGI
scripts using HTTP_Status. So far, judging from what I saw in
rpcserver.py we use error log level when there is a problem on our side
and not in a user request...
I think this test program could be made into a test suite too,
particularly to check the more esoteric parts like checking for missing
options, too many options, etc.
rob
I added a test suite exercising this WSGI script. It is based on
built-in httplib instead of original pyCurl - it has much better output
parsing and is easier to handle.
The new unit test tests bad options, authentication errors and of course
successful password change, including a verification that that the
password was actually changed.
Martin
ACK, pushed to master. I like the tests very much.
rob
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0061 Improve ipa-client-install debug output
Petr Viktorin wrote: I went through all the output/debug messages in ipa-client-install, removed duplicates, and routed them through the log manager. I used these log levels: DEBUG - detailed messages the user probably doesn't want to see (only printed to console with --debug) INFO - progress reports (Configured /etc/sssd/sssd.conf) and instructions (You may need to restart services or reboot the machine.) WARNING - something unusual that may require attention ERROR - something went wrong Obviously there's some overlap there. On the console, the messages are now prefixed with the log level. This should bring attention to the warnings/errors. Does this format look okay? I think the current default console_format, which prefixes the logger name (ipa) and the level, is too verbose. I agree. I'm not a fan of printing the log level, it is very distracting. This is a good start but the DNS discovery area needs more work. I've found it very difficult to look at a log and figure out where the domain and hostname came from (user provided or discovered?) and trace how the discovery is working. There is currently a lot of smoke and noise and very little useful information. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 156 Action panel for service provisioning
On 6/7/2012 3:52 AM, Petr Vobornik wrote: Service provisioning status widget was modified only to display the has_keytab status. Button for 'delete key,unprovision' was moved as action to newly created action panel in the same section. This required to moved the creation of the unprovisioning dialog from that widget to new separate dialog. Action for action panel and all required status evaluators for enabling/disabling of that action were also created. https://fedorahosted.org/freeipa/ticket/2252 ACK. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 155 Action panel for host enrollment
On 6/6/2012 10:59 AM, Petr Vobornik wrote: Widgets in host enrollment sections were modified. They now serve only for displaying of has_key and has_password status. Functionality for setting otp and unprovisioning was moved to separate dialogs. Execution points for opening of these dialogs are items in new action panel in enrollment section. ACK. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 157 Added password reset capabilities to unauthorized dialog
On 6/8/2012 10:52 AM, Petr Vobornik wrote: and now the patch... On 06/08/2012 05:51 PM, Petr Vobornik wrote: For those of you who are only interest in user perspective I prepared a set of screenshots to demonstrate workflow of password reset: http://pvoborni.fedorapeople.org/ux/reset_password_workflow.png Patch depends on mkosek #274. Web UI was missing a way how to reset expired password for normal user. Recent server patch added API for such task. This patch is adding reset password form to unautorized dialog. If user tries to login using form-based authentication and his password is expired login form transforms to reset password form. The username and current password is populated by values from previous login attempt. User than have to enter new password and its verification. Then he can hit enter button on keyboard or click on reset button on dialog to perform the password reset. Error is displayed if some part of password reset fails. If it is successful new login with values entered for password reset is performed. It should login the user. In password reset form user can click on back button or hit escape on keyboard to go back to login form. https://fedorahosted.org/freeipa/ticket/2755 It works with mkosek 274-2. Some comments: 1. If you click 'form-based authentication' the dialog title still shows 'Kerberos ticket no longer valid' which is not relevant for form-based authentication. It might be better to use 'Login' as the title for all pages in this dialog. 2. Instead of having to go to a separate page for form-based authentication, would it be better to change the first page in the login dialog to show the login form? Something like this: Login - Your session has expired. Please re-login. To login with username and password: Username:[edewata ] Password:[ ] [Login] To login with Kerberos, please make sure you have valid tickets (obtainable via kinit) and [configured] the browser correctly. [Login with Kerberos] The two login mechanisms can be shown at the same time like above or in collapsible sections. If the user enters a password and it's expired, the dialog will change into: Login - Your password has expired. Please enter a new password: Username:edewata New Password:[ ] Verify Password: [ ] [Reset Password and Login] [Cancel] In this page the username is shown for info only, it's not editable. The old password is not shown again, but kept in memory. I use Cancel instead of Back to indicate that we are starting over. The Cancel button will bring you back to the first page. 3. I noticed that the password is kept in memory too long by the login dialog so if you go back and forth between the pages the fields are already populated. This might be a security risk. I think the username password should be cleaned up when you click Back/Cancel. 4. Is there a plan to provide password reset via email? -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 158 Separate reset password page
On 6/8/2012 10:54 AM, Petr Vobornik wrote: This patch adds separate reset password page. It is a complement to separate login page. It differentiate from reset password capabilities in Web UI's anauthorized dialog by not performing login. This is useful for users who wants only to reset the password and not to use Web UI. And also for users who are using the separate login page. https://fedorahosted.org/freeipa/ticket/2755 It also works with mkosek 274-2 (but needs rebase). If the password reset is successful, the page shows a link to the login page, but it also still shows the password reset form. I think it's very unlikely that the user will want to change the password again, so it might be better not to show the form, but provide a link to the form just in case the user needs to do it again. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
