On Fri, 2014-02-21 at 16:29 +0100, Jan Cholasta wrote:
> Hi,
>
> On 21.2.2014 16:09, Nathaniel McCallum wrote:
> > On Fri, 2014-02-21 at 09:45 -0500, Nathaniel McCallum wrote:
> >> We had originally decided to provide defaults on the server side so that
> >> they could be part of a global config for the admin. However, on further
> >> reflection, only certain defaults really make sense given the
> >> limitations of Google Authenticator. Similarly, other defaults may be
> >> token specific.
> >>
> >> Attempting to handle defaults on the server side also makes both the UI
> >> and the generated documentation unclear.
> >>
> >> NOTE: this patch changes an existing API. VERSION says that we should
> >> bump the major version in this case. But we haven't actually released
> >> this API yet. Please advise.
>
> There were similar changes in the past and bumping minor version was
> always enough (we never ever bump major version).
Fixed.
> >
> > I forgot to mention something else about this patch. The default_from in
> > the key parameter generates a warning. This is because the default is a
> > bytes string and internally a check is done against NULLS
> > (constants.py:36). The u'' in NULLS forces an attempt to convert the
> > byte string to unicode. When this fails (because ipatokenotpkey is *NOT*
> > a string but a byte array), a warning is thrown.
> >
> > Since '' and u'' evaluate as equal, what is the point of having u'' in
> > NULLS? I don't see any way to suppress this warning except to remove u''
> > from NULLS.
>
> I think we can get rid of NULLS entirely and do something better instead
> (like "if not value and value is not False:").
>
> Is this worth filing a ticket?
How about a patch?
https://www.redhat.com/archives/freeipa-devel/2014-February/msg00426.html
Nathaniel
>From 046a80c190004dc178a094611d9134f96c79da37 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Thu, 20 Feb 2014 13:21:32 -0500
Subject: [PATCH] Rework how otptoken defaults are handled
We had originally decided to provide defaults on the server side so that they
could be part of a global config for the admin. However, on further reflection,
only certain defaults really make sense given the limitations of Google
Authenticator. Similarly, other defaults may be token specific.
Attempting to handle defaults on the server side also makes both the UI and
the generated documentation unclear.
---
API.txt | 46 ++++++++++++++++----------------
VERSION | 4 +--
ipalib/plugins/otptoken.py | 65 +++++++++++++++++++++++-----------------------
3 files changed, 58 insertions(+), 57 deletions(-)
diff --git a/API.txt b/API.txt
index 504a60ff31686cfa828c3a8f17debd6dad3bb60d..9898da918b3f61d39648baa77d7fe5d8306c2375 100644
--- a/API.txt
+++ b/API.txt
@@ -2221,34 +2221,34 @@ output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
output: Output('value', <type 'unicode'>, None)
command: otptoken_add
args: 1,21,3
-arg: Str('ipatokenuniqueid', attribute=True, cli_name='id', multivalue=False, primary_key=True, required=False)
+arg: Str('ipatokenuniqueid', attribute=True, autofill=True, cli_name='id', multivalue=False, primary_key=True, required=False)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=False)
option: Bool('ipatokendisabled', attribute=True, cli_name='disabled', multivalue=False, required=False)
-option: Int('ipatokenhotpcounter', attribute=True, cli_name='counter', minvalue=0, multivalue=False, required=False)
-option: Str('ipatokenmodel', attribute=True, cli_name='model', multivalue=False, required=False)
+option: Int('ipatokenhotpcounter', attribute=True, autofill=True, cli_name='counter', default=0, minvalue=0, multivalue=False, required=False)
+option: Str('ipatokenmodel', attribute=True, autofill=True, cli_name='model', multivalue=False, required=False)
option: Str('ipatokennotafter', attribute=True, cli_name='not_after', multivalue=False, required=False)
option: Str('ipatokennotbefore', attribute=True, cli_name='not_before', multivalue=False, required=False)
-option: StrEnum('ipatokenotpalgorithm', attribute=True, cli_name='algo', multivalue=False, required=False, values=(u'sha1', u'sha256', u'sha384', u'sha512'))
-option: IntEnum('ipatokenotpdigits', attribute=True, cli_name='digits', multivalue=False, required=False, values=(6, 8))
-option: OTPTokenKey('ipatokenotpkey', attribute=True, cli_name='key', multivalue=False, required=False)
+option: StrEnum('ipatokenotpalgorithm', attribute=True, autofill=True, cli_name='algo', default=u'sha1', multivalue=False, required=False, values=(u'sha1', u'sha256', u'sha384', u'sha512'))
+option: IntEnum('ipatokenotpdigits', attribute=True, autofill=True, cli_name='digits', default=6, multivalue=False, required=False, values=(6, 8))
+option: OTPTokenKey('ipatokenotpkey', attribute=True, autofill=True, cli_name='key', multivalue=False, required=False)
option: Str('ipatokenowner', attribute=True, cli_name='owner', multivalue=False, required=False)
-option: Str('ipatokenserial', attribute=True, cli_name='serial', multivalue=False, required=False)
-option: Int('ipatokentotpclockoffset', attribute=True, cli_name='offset', multivalue=False, required=False)
-option: Int('ipatokentotptimestep', attribute=True, cli_name='interval', minvalue=5, multivalue=False, required=False)
-option: Str('ipatokenvendor', attribute=True, cli_name='vendor', multivalue=False, required=False)
+option: Str('ipatokenserial', attribute=True, autofill=True, cli_name='serial', multivalue=False, required=False)
+option: Int('ipatokentotpclockoffset', attribute=True, autofill=True, cli_name='offset', default=0, multivalue=False, required=False)
+option: Int('ipatokentotptimestep', attribute=True, autofill=True, cli_name='interval', default=30, minvalue=5, multivalue=False, required=False)
+option: Str('ipatokenvendor', attribute=True, autofill=True, cli_name='vendor', default=u'FreeIPA', multivalue=False, required=False)
option: Flag('qrcode?', autofill=True, default=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
-option: StrEnum('type', attribute=False, cli_name='type', multivalue=False, required=False, values=(u'totp', u'hotp'))
+option: StrEnum('type', attribute=False, autofill=True, cli_name='type', default=u'totp', multivalue=False, required=False, values=(u'totp', u'hotp'))
option: Str('version?', exclude='webui')
output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
output: Output('value', <type 'unicode'>, None)
command: otptoken_del
args: 1,2,3
-arg: Str('ipatokenuniqueid', attribute=True, cli_name='id', multivalue=True, primary_key=True, query=True, required=True)
+arg: Str('ipatokenuniqueid', attribute=True, autofill=True, cli_name='id', multivalue=True, primary_key=True, query=True, required=True)
option: Flag('continue', autofill=True, cli_name='continue', default=False)
option: Str('version?', exclude='webui')
output: Output('result', <type 'dict'>, None)
@@ -2260,23 +2260,23 @@ arg: Str('criteria?', noextrawhitespace=False)
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, query=True, required=False)
option: Bool('ipatokendisabled', attribute=True, autofill=False, cli_name='disabled', multivalue=False, query=True, required=False)
-option: Int('ipatokenhotpcounter', attribute=True, autofill=False, cli_name='counter', minvalue=0, multivalue=False, query=True, required=False)
+option: Int('ipatokenhotpcounter', attribute=True, autofill=False, cli_name='counter', default=0, minvalue=0, multivalue=False, query=True, required=False)
option: Str('ipatokenmodel', attribute=True, autofill=False, cli_name='model', multivalue=False, query=True, required=False)
option: Str('ipatokennotafter', attribute=True, autofill=False, cli_name='not_after', multivalue=False, query=True, required=False)
option: Str('ipatokennotbefore', attribute=True, autofill=False, cli_name='not_before', multivalue=False, query=True, required=False)
-option: StrEnum('ipatokenotpalgorithm', attribute=True, autofill=False, cli_name='algo', multivalue=False, query=True, required=False, values=(u'sha1', u'sha256', u'sha384', u'sha512'))
-option: IntEnum('ipatokenotpdigits', attribute=True, autofill=False, cli_name='digits', multivalue=False, query=True, required=False, values=(6, 8))
+option: StrEnum('ipatokenotpalgorithm', attribute=True, autofill=False, cli_name='algo', default=u'sha1', multivalue=False, query=True, required=False, values=(u'sha1', u'sha256', u'sha384', u'sha512'))
+option: IntEnum('ipatokenotpdigits', attribute=True, autofill=False, cli_name='digits', default=6, multivalue=False, query=True, required=False, values=(6, 8))
option: Str('ipatokenowner', attribute=True, autofill=False, cli_name='owner', multivalue=False, query=True, required=False)
option: Str('ipatokenserial', attribute=True, autofill=False, cli_name='serial', multivalue=False, query=True, required=False)
-option: Int('ipatokentotpclockoffset', attribute=True, autofill=False, cli_name='offset', multivalue=False, query=True, required=False)
-option: Int('ipatokentotptimestep', attribute=True, autofill=False, cli_name='interval', minvalue=5, multivalue=False, query=True, required=False)
+option: Int('ipatokentotpclockoffset', attribute=True, autofill=False, cli_name='offset', default=0, multivalue=False, query=True, required=False)
+option: Int('ipatokentotptimestep', attribute=True, autofill=False, cli_name='interval', default=30, minvalue=5, multivalue=False, query=True, required=False)
option: Str('ipatokenuniqueid', attribute=True, autofill=False, cli_name='id', multivalue=False, primary_key=True, query=True, required=False)
-option: Str('ipatokenvendor', attribute=True, autofill=False, cli_name='vendor', multivalue=False, query=True, required=False)
+option: Str('ipatokenvendor', attribute=True, autofill=False, cli_name='vendor', default=u'FreeIPA', multivalue=False, query=True, required=False)
option: Flag('pkey_only?', autofill=True, default=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Int('sizelimit?', autofill=False, minvalue=0)
option: Int('timelimit?', autofill=False, minvalue=0)
-option: StrEnum('type', attribute=False, autofill=False, cli_name='type', multivalue=False, query=True, required=False, values=(u'totp', u'hotp'))
+option: StrEnum('type', attribute=False, autofill=False, cli_name='type', default=u'totp', multivalue=False, query=True, required=False, values=(u'totp', u'hotp'))
option: Str('version?', exclude='webui')
output: Output('count', <type 'int'>, None)
output: ListOfEntries('result', (<type 'list'>, <type 'tuple'>), Gettext('A list of LDAP entries', domain='ipa', localedir=None))
@@ -2284,7 +2284,7 @@ output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
output: Output('truncated', <type 'bool'>, None)
command: otptoken_mod
args: 1,16,3
-arg: Str('ipatokenuniqueid', attribute=True, cli_name='id', multivalue=False, primary_key=True, query=True, required=True)
+arg: Str('ipatokenuniqueid', attribute=True, autofill=True, cli_name='id', multivalue=False, primary_key=True, query=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
option: Str('delattr*', cli_name='delattr', exclude='webui')
@@ -2295,9 +2295,9 @@ option: Str('ipatokennotafter', attribute=True, autofill=False, cli_name='not_af
option: Str('ipatokennotbefore', attribute=True, autofill=False, cli_name='not_before', multivalue=False, required=False)
option: Str('ipatokenowner', attribute=True, autofill=False, cli_name='owner', multivalue=False, required=False)
option: Str('ipatokenserial', attribute=True, autofill=False, cli_name='serial', multivalue=False, required=False)
-option: Str('ipatokenvendor', attribute=True, autofill=False, cli_name='vendor', multivalue=False, required=False)
+option: Str('ipatokenvendor', attribute=True, autofill=False, cli_name='vendor', default=u'FreeIPA', multivalue=False, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
-option: Str('rename', cli_name='rename', multivalue=False, primary_key=True, required=False)
+option: Str('rename', autofill=True, cli_name='rename', multivalue=False, primary_key=True, required=False)
option: Flag('rights', autofill=True, default=False)
option: Str('setattr*', cli_name='setattr', exclude='webui')
option: Str('version?', exclude='webui')
@@ -2306,7 +2306,7 @@ output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
output: Output('value', <type 'unicode'>, None)
command: otptoken_show
args: 1,4,3
-arg: Str('ipatokenuniqueid', attribute=True, cli_name='id', multivalue=False, primary_key=True, query=True, required=True)
+arg: Str('ipatokenuniqueid', attribute=True, autofill=True, cli_name='id', multivalue=False, primary_key=True, query=True, required=True)
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Flag('rights', autofill=True, default=False)
diff --git a/VERSION b/VERSION
index b1dcb9c05416fb611f523c9c10599b5fc0e18045..64472ada17b9f30d3e3cf0a01d178b1d6ab8dd9f 100644
--- a/VERSION
+++ b/VERSION
@@ -89,5 +89,5 @@ IPA_DATA_VERSION=20100614120000
# #
########################################################
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=75
-# Last change: npmccallum - HOTP support
+IPA_API_VERSION_MINOR=76
+# Last change: npmccallum - otptoken defaults change
diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index 8e76ada907c161ffc6a7e83c02d41daa5849e515..92853dec3048fd98f98f4113ef8b5874f2500919 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -53,7 +53,10 @@ EXAMPLES:
register = Registry()
-TOKEN_TYPES = (u'totp', u'hotp')
+TOKEN_TYPES = {
+ u'totp': ['ipatokentotpclockoffset', 'ipatokentotptimestep'],
+ u'hotp': ['ipatokenhotpcounter']
+}
# NOTE: For maximum compatibility, KEY_LENGTH % 5 == 0
KEY_LENGTH = 10
@@ -117,12 +120,16 @@ class otptoken(LDAPObject):
Str('ipatokenuniqueid',
cli_name='id',
label=_('Unique ID'),
+ default_from=lambda: unicode(uuid.uuid4()),
+ autofill=True,
primary_key=True,
flags=('optional_create'),
),
StrEnum('type?',
label=_('Type'),
- values=TOKEN_TYPES,
+ default=u'totp',
+ autofill=True,
+ values=tuple(TOKEN_TYPES.keys()),
flags=('virtual_attribute', 'no_update'),
),
Str('description?',
@@ -148,23 +155,33 @@ class otptoken(LDAPObject):
Str('ipatokenvendor?',
cli_name='vendor',
label=_('Vendor'),
+ default=u'FreeIPA',
+ autofill=True,
),
Str('ipatokenmodel?',
cli_name='model',
label=_('Model'),
+ default_from=lambda type: type,
+ autofill=True,
),
Str('ipatokenserial?',
cli_name='serial',
label=_('Serial'),
+ default_from=lambda id: id,
+ autofill=True,
),
OTPTokenKey('ipatokenotpkey?',
cli_name='key',
label=_('Key'),
+ default_from=lambda: "".join(random.SystemRandom().sample(map(chr, range(256)), 10)),
+ autofill=True,
flags=('no_display', 'no_update', 'no_search'),
),
StrEnum('ipatokenotpalgorithm?',
cli_name='algo',
label=_('Algorithm'),
+ default=u'sha1',
+ autofill=True,
flags=('no_update'),
values=(u'sha1', u'sha256', u'sha384', u'sha512'),
),
@@ -172,22 +189,30 @@ class otptoken(LDAPObject):
cli_name='digits',
label=_('Display length'),
values=(6, 8),
+ default=6,
+ autofill=True,
flags=('no_update'),
),
Int('ipatokentotpclockoffset?',
cli_name='offset',
label=_('Clock offset'),
+ default=0,
+ autofill=True,
flags=('no_update'),
),
Int('ipatokentotptimestep?',
cli_name='interval',
label=_('Clock interval'),
+ default=30,
+ autofill=True,
minvalue=5,
flags=('no_update'),
),
Int('ipatokenhotpcounter?',
cli_name='counter',
label=_('Counter'),
+ default=0,
+ autofill=True,
minvalue=0,
flags=('no_update'),
),
@@ -208,37 +233,13 @@ class otptoken_add(LDAPCreate):
)
def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
- # These are values we always want to write to LDAP. So if they are
- # specified as a value that evaluates to False (i.e. None), delete them
- # and fill in the defaults below.
- for attr in ('ipatokentotpclockoffset', 'ipatokentotptimestep',
- 'ipatokenotpalgorithm', 'ipatokenotpdigits',
- 'ipatokenotpkey'):
- if attr in entry_attrs and not entry_attrs[attr]:
- del entry_attrs[attr]
-
- # Set defaults. This needs to happen on the server side because we may
- # have global configurable defaults in the near future.
- options.setdefault('type', TOKEN_TYPES[0])
- if entry_attrs.get('ipatokenuniqueid', None) is None:
- entry_attrs['ipatokenuniqueid'] = str(uuid.uuid4())
- dn = DN("ipatokenuniqueid=%s" % entry_attrs['ipatokenuniqueid'], dn)
- entry_attrs.setdefault('ipatokenvendor', u'FreeIPA')
- entry_attrs.setdefault('ipatokenmodel', options['type'])
- entry_attrs.setdefault('ipatokenserial', entry_attrs['ipatokenuniqueid'])
- entry_attrs.setdefault('ipatokenotpalgorithm', u'sha1')
- entry_attrs.setdefault('ipatokenotpdigits', 6)
- entry_attrs.setdefault('ipatokenotpkey',
- "".join(map(chr, random.SystemRandom().sample(range(255), KEY_LENGTH))))
-
# Set the object class and defaults for specific token types
- if options['type'] == 'totp':
- entry_attrs['objectclass'] = otptoken.object_class + ['ipatokentotp']
- entry_attrs.setdefault('ipatokentotpclockoffset', 0)
- entry_attrs.setdefault('ipatokentotptimestep', 30)
- elif options['type'] == 'hotp':
- entry_attrs['objectclass'] = otptoken.object_class + ['ipatokenhotp']
- entry_attrs.setdefault('ipatokenhotpcounter', 0)
+ entry_attrs['objectclass'] = otptoken.object_class + ['ipatoken' + options['type']]
+ for ttype, tattrs in TOKEN_TYPES.items():
+ if ttype != options['type']:
+ for tattr in tattrs:
+ if tattr in entry_attrs:
+ del entry_attrs[tattr]
# Resolve the user's dn
_normalize_owner(self.api.Object.user, entry_attrs)
--
1.8.5.3
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
