Patch WFM and looks clean. My only question is on the overall
prompting.
Currently we have:
* 1FA:
- "Password"
* 2FA (required):
- "First Factor"
- "Second Factor"
* 2FA (optional):
- "First Factor or Password"
- "Second Factor, press return for Password authentication"
This all seems a little verbose to me. It also seems like it reveals
technical details that don't matter and/or might confuse the user.
Is there a reason not to do the following?
* 1FA:
- "Password"
* 2FA (required):
- "Password"
- "Token
Code"
* 2FA (optional):
- "Password"
- "Token Code (optional)"
This seems to me simpler, shorter and more descriptive. Alternatively:
* 1FA:
- "Password"
* 2FA (required):
- "Password"
-
"Second Factor"
* 2FA (optional):
- "Password"
- "Second Factor
(optional)"
Whatever we decide to do, this patch has my ACK. So feel free to just
change the string and merge it.
On Mon, 2016-06-13 at 14:01 +0200, Sumit Bose wrote:
> and now with patch ...
>
> On Mon, Jun 13, 2016 at 02:00:37PM +0200, Sumit Bose wrote:
> >
> > On Fri, Jun 10, 2016 at 02:28:16PM +0200, Lukas Slebodnik wrote:
> > >
> > > On (30/05/16 17:07), Sumit Bose wrote:
> > > >
> > > > Hi,
> > > >
> > > > this patch is the SSSD part of the Authentication Indicator
> > > > related
> > > > changes in FreeIPA. The basic part is that now it is possible
> > > > to
> > > > authenticate at will with either password or 2FA as described
> > > > on
> > > >
https://fedorahosted.org/sssd/wiki/DesignDocs/PromptingForMulti
> > > > pleAuthenticationTypes.
> > > >
> > > > To test you need a FreeIPA server build from the FreeIPA master
> > > > branch
> > > > and optionally Nathaniel's '[PATCH 0093] Enable service
> > > > authentication
> > > > indicator management' which is currently under review for the
> > > > kvno
> > > > related test. Additionally you need MIT Kerberos packages which
> > > > contain
> > > > the fix for
https://bugzilla.redhat.com//show_bug.cgi?id=134030
> > > > 4.
> > > >
> > > > Since this patch changes how libkrb5 gets the password for
> > > > password
> > > > authentication with newer version of MIT Kerberos it would be
> > > > nice if
> > > > someone can run some regression tests with the AD or plain KRB5
> > > > auth_provider.
> > > >
> > > I can see failures in password change for AD provider.
> > > I haven't tried KRB5 or IPA yet.
> > >
> >
> > Thank you for testing and reviewing. I guess it was a general
> > issue, the
> > new patch should fix it.
> >
> > >
> > >
> > ...
> > >
> > > >
> > > > + DEBUG(SSSDBG_TRACE_ALL, "Prompt
[%u][%s].\n",
> > > > c,
> > > ^^^
> > > type of "c" is size_t an
> > > therefore
> > > there should be "%zu".
> > > otherwise
> > > there is a warning on 64
> > > bit platforms
> > > CC src/providers/krb5/krb5_child-krb5_child.o
> > > ../sssd/src/providers/krb5/krb5_child.c: In function
> > > ‘sss_krb5_prompter’:
> > > ../sssd/src/providers/krb5/krb5_child.c:665:17: error: format
> > > ‘%u’ expects argument of type ‘unsigned int’, but argument 6 has
> > > type ‘size_t {aka long unsigned int}’ [-Werror=format=]
> > > DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n",
c,
> > > ^~~~~
> > > cc1: all warnings being treated as errors
> >
> > Fixed as well. New version attached.
> >
> > bye,
> > Sumit
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahos
>
ted.org