[Freeipa-devel] Activation and password reset webapp UI
Endi Sukma Dewata
edewata at redhat.com
Tue Nov 29 22:35:12 UTC 2011
On 11/28/2011 3:11 PM, Ryan Thomson wrote:
> Hello,
>
> Attached is my amateur attempt at contributing to FreeIPA.
>
> The patch implements an account "activation" and password reset webapp
> UI to cover use cases where FreeIPA may be acting only as a backend to
> services such as Samba or other web application that do no expose a
> method for changing expired kerberos credentials.
>
> This code is based on the "migration" webapp UI.
>
> See ticket 1907.
>
> https://fedorahosted.org/freeipa/ticket/1907
Hi, thanks for the patch! I found some issues:
1. Build failed due to missing variables:
install/activation/activation.py:94: [E0602, activate] Undefined
variable 'e'
install/activation/activation.py:97: [E0602, activate] Undefined
variable 'e'
2. After fixing #1, I found that the activation folder is not installed
into /usr/share/ipa. I think this needs to be fixed in
install/Makefile.am and freeipa.spec.in.
3. After copying the folder manually I can access the activation page,
but when test it I get this error in /var/log/httpd/errors_log:
[Tue Nov 29 16:13:30 2011] [error] ERROR:root:migration context search
failed: {
'info': 'TLS error -8172:Unknown code ___f 20', 'desc': "Can't contact
LDAP serv
er"}
It probably should use ldapi instead of ldaps. See migration.py.
4. In index.html:33 I think the "Password" label should say "Current
Password" for better clarity. Also in line 41 the "Confirm" label should
say "Verify Password" for consistency with the rest of the UI.
5. In index.html:44 it should be </ul> instead of <ul>. This is an
existing problem in migration/index.html.
6. There are broken image links in ipa_activation.css. The images have
been slightly renamed and moved into install/ui/images recently.
7. There's a broken link to the CSS file in invalid.html:7.
8. The intall/activation/jquery-ui.css doesn't seem to be used anywhere.
This is also an existing problem in migration.
9. In activation.py:72 the error message still says 'migration'.
10. In activation.py:121 the return statement in the 'finally' clause is
redundant if there's an exception because it will return another page.
This return statement should be moved inside the 'try' clause. Also we
might need another return statement in the 'except' clause to handle the
case where the errno doesn't match EPERM or EIO.
11. The file actually contains 3 patches. It's better to put them in
separate files because in case one of them needs revision you won't have
to resend everything. Another option is to squash them into a single patch.
12. The second patch contains the first patch file.
13. There are some warnings about extra whitespaces when applying the
patch. Try applying the patch with --whitespace=fix then reexporting it
again.
--
Endi S. Dewata
More information about the Freeipa-devel
mailing list