Nathaniel McCallum wrote:
On Fri, 2013-04-12 at 17:39 -0400, Nathaniel McCallum wrote:
On Fri, 2013-04-12 at 11:53 -0400, Nathaniel McCallum wrote:
On Fri, 2013-04-12 at 11:34 -0400, Nathaniel McCallum wrote:
On Thu, 2013-04-11 at 14:48 -0400, Rob Crittenden wrote:
Nathaniel McCallum wrote:
On Wed, 2013-04-10 at 15:35 -0400, Rob Crittenden wrote:
I'm not sure how I'd test it if I got it built.


I'm working on this. I hope to have a clear answer next week. Bear with
me...

Overall looks really good.

I've split up the patch into multiple commits. I've also added .update
files and a patch for ipa-kdb to feed krb5 the right user string.

https://github.com/npmccallum/freeipa/commits/otp

Please take a look. I *think* I've got everything worked out so far with
the exception of bug numbers / urls. Should every patch have a separate
bug and a link to the design page?

The ticket should go into every commit. I'd probably put the design link
there too, just for completeness. Future bug fixes, et all aren't going
to require the design page, but since these commits are all related to
the initial feature it will be nice to have.

You can have multiple patches on the same ticket/bug.

https://github.com/npmccallum/freeipa/commits/otp

All four commits now have bug numbers and design page links. I'm adding
the design page link to the tickets as we speak.

Remaining issues (AFAICS):
1. The daemon (ipa-otpd) runs as root and binds anonymously
2. ipatokenRadiusSecret is readable by an anonymous bind
3. ipatokenT?OTP.* are readable by an anonymous bind

In the case of both #2 and #3, only admins should have RW. ipa-otpd
needs read access to ipatokenRadiusSecret. The DS bind plugin below (#2)
needs read access to ipatokenT?OTP.*.

Outstanding pieces:
1. CLI tool -- https://fedorahosted.org/freeipa/ticket/3368
2. DS bind plugin -- https://fedorahosted.org/freeipa/ticket/3367
3. UI -- https://fedorahosted.org/freeipa/ticket/3369
4. Self Service UI -- https://fedorahosted.org/freeipa/ticket/3370

#1 and #2 are within the scope of F19 and should hopefully land shortly
(in separate commits). #3 and #4 are probably $nextrelease.


FYI - Here is an RPM with all of the code so far:
http://koji.fedoraproject.org/koji/taskinfo?taskID=5247029

Updated RPMs, containing the new 389DS bind plugin and build for F19,
are here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=5270926

Nathaniel

BuildRequires needed for whatever provides krad.h

A bunch of the files declare functions in each other. Is it cleaner to put these into an include file? I'm gathering that this will always be self-contained, so maybe this is ok.

In entry_attr_get_berval() is it worth pointing out that there is no need to free the value, or is that assumed because it uses slapi_value_get_berval()?

If we detect that there is clock drift should we log it? Will we ever try to report to the client (e.g. future enhancement)?

I wonder if the NSPR-version of some functions should be used since this is running inside 389-ds, like PL_strcasecmp for strcasecmp()

ops.c:

pedantic: lack of spacing between if and parens

sha384 is an allowed type only in otp.c. Is that needed?

rob


_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to