.. -*- coding: utf-8 -*-

Ticket #3154558 [security] rdefs using default read permissions: just do nothing
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
:type: enhancement
:load: 1.000
:state: open

Because:

* the default rule is (u'managers', u'users', u'guests'), which actually means
'everyone'

* semantically it parallels what is now done with the entity update rule
(https://www.cubicweb.org/patch/2932064)

Comments ::

  On 2013/09/19 08:23 - acampeas wrote :
  First problem seen while implementing this: the test
  unittest_security.test_update_rql_permission breaks.
  
  It works on the following (reduced) schema, with read perms shown::
  
  class Affaire:
  __permissions__ = ('managers', 'X owned_by U', 'X concerne S?, S owned_by U')
  sujet = String()
  concerne = Relation(Societe)
  
  class Societe:
  __permissions__ = ('managers', 'users', 'guests')
  
  We have a scenario where a "user" (belonging only to the users group):
  
  1. creates an Affaire
  2. creates a Societe and links to the Affaire through "concerne"
  3. wants to set the "sujet" attribute of the Societe
  
  As of today it all works. Let's detail point 3 security check:
  
  * the querier performs a read group check on the "sujet" attribute
  * since the default sec on attributes is ('managers', 'users', 'guests') it
  passes (!)
  * later, the rqlrewriter interpolates the rql expressions, hence since the user
  owns both ends of the "concerne" relation it works ok
  
  It think this way of life is a pb., because:
  
  * when writing a schema like this, the typical end user NEVER thinks she just
  gave ('managers', 'users', 'guests') group permissions to the Societe
  attributes,
  
  * instead she believes the permission rule at entity level controls the whole
  entity (that is, attributes-security), unless a specific attribute permission
  rule is written
  
  I have the following scenario to implement: a "readonly" profile. Unfortunately,
  it cannot be made to work (with a "readonly" group at least) unless every
  attribute of every entity type is explicitly given a non-default, e.g.
  ('managers', 'users', 'guests', 'readonly') rule. Which is a bit unfriendly...
  
  Opinions ?
  > On 2013/09/19 09:48 - acampeas wrote :
  > Well, doing nothind wrt rdefs read permissions == default read permissions
  > "just works".
  On 2013/09/28 10:24 - acampeas wrote :
  this actually must be extended to write security checks
  On 2014/08/01 13:23 - acampeas wrote :
  As of 01-08-2014, the need rises again. Let's do it !

Ticket #1681974 cubicweb-ctl shell and migration ignores uid
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
:type: enhancement
:load: 2.000
:state: open

This often lead to executing code as root on production instance.

Comments ::

  On 2011/05/19 08:54 - pydavid wrote :
  After discussion with Sylvain we have to distinct two kinds of migration and
  shell:
  
  * Migration that process generic code and modify the database (that should
  respect the "uid").
  
  * Migration that modify config, source file, i18n stuff etc. (that should still
  be run as root).

Ticket #2965518 attribute permissions: have an 'add' permission distinct from 'update'
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
:type: enhancement
:load: 1.000
:state: resolved

As of today, attribute 'update' permissions are checked at entity creation time.

In the past, we have gone from 'add'/'delete' permissions in the manner of
relations to 'update' perms, like for entities, but while simplifying it to
'update', we lost something.

There are definitely cases when a specific attribute 'update' permission forbids
entity _creation_. The most common case (no permission at all) is actually
hard-coded in `check_entity_attributes`, but there's no deep reason more cases
cannot be handled.

Proposal:

* stop checking attributes update perms at entity creation time * introduce
attribute 'add' perms and check them at entity creation time * make the default
attribute 'add' perms the _same_ as the current default 'update' perm, for
backward compatibility.

Comments ::

  On 2013/10/23 14:35 - acampeas wrote :
  The "add" permission is proving controversial. People argue about use cases.
  
  It was introduced because of this: https://www.cubicweb.org/task/2932167
  
  Other than that I believe the only potential use case is completely computed
  attributes. When the bw compat is lifted, giving "add": (), "update": () will
  ensure the attribute never crosses the ui boundary (hence no need for additional
  uicfg tweaks (e.g. use a hidden input).
  
  This is arguably a small benefit, but not without merit.

Ticket #2932033 [security] consider operation checking only in securityafterupdateentity hook
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
:type: enhancement
:load: 0.500
:state: resolved

As of today, the entity is checked in the hook, and if Unauthorized the check is
deferred to an Operation (hoping that things will work better at this time).

I contend that this strategy is too costly for the case when only an operation
will yield a successful permission check, and that this case is quite common.

I propose we drop the immediate permission check and only defer to the
operation.

Comments ::

  On 2013/06/11 10:16 - sthenault wrote :
  IMO what we want is to control when the check occurs, as this is done for
  relation.
  
  Though I'm afraid such change may lead to very nasty bw incompatibility
  problems.
  > On 2013/06/11 10:23 - acampeas wrote :
  > regarding bw compat: we have good security test coverage haven't we ? :) but
  > indeed, a knob might be more reasonnable

Ticket #2930861 [security] update perms fire on attributes at entity creation time
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
:type: bug
:load: 1.000
:state: resolved

This makes hard to define some otherwise straightforward security rules::

Scenario.__permissions__ = { 'read': ('managers', 'modellers', 'users',
'agents'), 'add': ('managers', 'modellers', 'users'), 'update': ('managers',
'modellers', ERQLExpression('V has_scenario X, V is_master False')), 'delete':
('managers', ERQLExpression('NOT EXISTS(TS has_scenario X)')) }

The following snippets chokes::

with debugged(DBG_SEC): scen = req.execute('INSERT Scenario S: S name "BABAR"')
cnx.commit()

Security tracing shows this::

check_perm: 'add' 'Scenario': user user matches frozenset(['modellers',
'managers', 'users']) with set([u'users']) check_perm: 'update' 'Scenario'
[(ERQLExpression(Any X WHERE V has_scenario X, V is_master False, X eid %(x)s),
{'eid': 5623}, False)] check_perm: 'update' 'attribute Scenario.name[String]'
[(ERQLExpression(Any X WHERE U has_update_permission X, X eid %(x)s, U eid
%(u)s), {'eid': 5623}, False)] check_perm: 'update' 'Scenario'
[(ERQLExpression(Any X WHERE V has_scenario X, V is_master False, X eid %(x)s),
{'eid': 5623}, False)]

Since I didn't explicitely overrid the attribute permissions settings it should
not be checked. There might be a special marker for these situations, that
allows to say::

if rdef.permissions is not DEFAULT: # do the attribute check else: # do nothing,
the etype permissions are all we wanted to check

Comments ::

  On 2013/06/10 19:52 - sthenault wrote :
  I don't get your later remark. if you associate an empty tuple to the 'update'
  key in the permissions dictionary, you don't get "the default security", you get
  an empty tuple. And iirc that's the target.
  > On 2013/06/10 19:57 - acampeas wrote :
  > I didn't associate anything to the Scenario.name per se and I have seen
  > rdef.permissions.get('update') yield the default cw permission.
  On 2013/06/11 09:57 - acampeas wrote :
  The two "check_perm: 'update' 'Scenario'" are quite disturbing also.
  > On 2013/06/11 10:20 - acampeas wrote :
  > I understand the second comes as a followup on the "U has_update_permission X"
  > of the Scenario.name check.
  On 2013/06/11 10:43 - acampeas wrote :
  an hypothetical first step::
  
  diff --git a/hooks/security.py b/hooks/security.py
  --- a/hooks/security.py
  +++ b/hooks/security.py
  @@ -23,6 +23,8 @@ the user connected to a session
  from logilab.common.registry import objectify_predicate
  +from yams import buildobjs
  +
  from cubicweb import Unauthorized
  from cubicweb.server import BEFORE_ADD_RELATIONS, ON_COMMIT_ADD_RELATIONS, hook
  @@ -41,7 +43,11 @@ def check_entity_attributes(session, ent
  rdef = eschema.rdef(attr)
  if rdef.final: # non final relation are checked by other hooks
  # add/delete should be equivalent (XXX: unify them into 'update' ?)
  - if creation and not rdef.permissions.get('update'):
  + updateperm = rdef.permissions.get('update')
  + if creation and not updateperm:
  + # someone stuck an empty tuple, probably meaning immutability
  + continue
  + if updateperm == buildobjs.DEFAULT_ATTRPERMS['update']:
  continue
  rdef.check_perm(session, 'update', eid=eid)
  > On 2013/06/25 11:16 - sthenault wrote :
  > This probably won't work because of the presence of an ERQLExpression in the
  > DEFAULT_ATTRPERMS. Also order may matters.
  > 
  > While I would prefer a proper solution, I would be fine with such patch in the
  > mean time, provided an XXX line before the attribute update perms comparison.
  On 2013/06/25 13:14 - acampeas wrote :
  example workaround::
  
  Scenario.__permissions__ = {
  'read': ('managers', 'modellers', 'users', 'agents'),
  'add': ('managers', 'modellers', 'users'),
  'update': ('managers', 'modellers',
  # Handled by a hook til https://www.cubicweb.org/2930861 is fixed
  # ERQLExpression('V has_scenario X, V is_master False')),
  'users'
  ),
  'delete': ('managers', ERQLExpression('NOT EXISTS(TS has_scenario X)'))
  }
  
  # look at the third 'if'
  class DoNotModifyBasecaseName(Hook):
  __regid__ = 'do_not_modify_basecase_name'
  __select__ = Hook.__select__ & is_instance('Scenario',)
  events = ('before_update_entity',)
  category = 'scenarioset'
  def __call__(self):
  old_name, new_name = self.entity.cw_edited.oldnewvalue('name')
  if old_name != new_name:
  if old_name == variables.DEFAULT_SCENARIO_NAME:
  msg = _('You are not allowed to rename the BaseCase')
  raise ValidationError(self.entity.eid, {'name': msg})
  if not self._cw.user.matching_groups(('managers', 'modellers')):
  # workaround for https://www.cubicweb.org/2930861, see
  # Scenario __permissions__ in schema.py
  raise Unauthorized('update', 'Scenario.name')
  
  As you can see it is not too nice. The UI is uninformed of the hook rule.
  (Also while doing this I introduced a bug wrt the spec; can you spot it ?)

Ticket #2920304 [security] add a security debugging tool
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
:type: enhancement
:load: 1.000
:state: resolved

DBG_SECURITY & al. ::

def test_contract_config_update(self): with self.perm_checker() as t: from
cubicweb.server import DBG_SEC, debugged, tunesecurity with debugged(DBG_SEC):
with tunesecurity(items=('rolling_dates',), capabilities=('update',)): ccf =
t.efi(self.ccf) cc = ccf.reverse_commodity_config[0]
cc.set_attributes(rolling_dates_offset=42) t.commit()

Would yield::

check_perm: 'update' 'attribute ContractConfig.rolling_dates_offset[Int]'
[(ERQLExpression(Any X WHERE U has_update_permission X, X eid %(x)s, U eid
%(u)s), {'eid': 2167}, True)]

Ticket #2465904 upgrade weak hashes automatically on login
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
:type: enhancement
:state: resolved

When a weak password hash is stored for a user, we should be able to
automatically upgrade it to sha512 on their next login.

Ticket #2103684 use passlib (?)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
:type: task
:load: 0.500
:state: resolved

passlib is a password hashing library for Python 2 & 3, which provides
cross-platform implementations of over 20 password hashing algorithms, as well
as a framework for managing existing password hashes. It’s designed to be useful
for a large range of tasks, including:

* quick-start password hashing for new python applications ~ quickstart guide *
constructing a configurable hashing policy to match the needs of any python
application ~ passlib.context * reading & writing Apache htpasswd / htdigest
files ~ passlib.apache * creating & verifying hashes used by MySQL, PostgreSQL,
OpenLDAP, and other applications ~ passlib.apps * creating & verifying hashes
found in Unix “shadow” files ~ passlib.hosts

CW uses different hash algo on unix and windows, making migrations of users
difficult (we need a password reset). If only for this, using passlib would be
nice.

Comments ::

  On 2012/01/11 17:04 - Unknown author wrote :
  http://pypi.python.org/pypi/passlib

Ticket #1642893 Unauthorized exception gives a 500 error message
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
:type: bug
:load: 0.250
:state: resolved

Publisher handle Unauthorized exception as any other and return a 500 HTTP
returns code. Returning a 403 Code would be more accurate.