Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: Serge Petrenko <sergepetrenko@tarantool.org>
Cc: kostja@tarantool.org, tarantool-patches@freelists.org
Subject: Re: [tarantool-patches] [PATCH 1/4] Introduce separate entity object types for entity privileges.
Date: Wed, 22 Aug 2018 13:28:45 +0300	[thread overview]
Message-ID: <20180822102845.obibc7ren43m4lmq@esperanza> (raw)
In-Reply-To: <8730529d7aefb43da1dd5fde8f01a6b927f8430e.1534751862.git.sergepetrenko@tarantool.org>

On Mon, Aug 20, 2018 at 11:10:05AM +0300, Serge Petrenko wrote:
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 3007a131d..42136b7df 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -2537,10 +2537,35 @@ priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple)
>  {
>  	priv->grantor_id = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_ID);
>  	priv->grantee_id = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_UID);
> +
>  	const char *object_type =
>  		tuple_field_cstr_xc(tuple, BOX_PRIV_FIELD_OBJECT_TYPE);
> -	priv->object_id = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_OBJECT_ID);
>  	priv->object_type = schema_object_type(object_type);
> +
> +	const char *data = tuple_field(tuple, BOX_PRIV_FIELD_OBJECT_ID);
> +	if (data == NULL) {
> +		tnt_raise(ClientError, ER_NO_SUCH_FIELD,
> +		      BOX_PRIV_FIELD_OBJECT_ID + TUPLE_INDEX_BASE);
> +	}
> +	/*
> +	 * When granting or revoking privileges on a whole entity
> +	 * we pass empty string ('') to object_id to indicate
> +	 * grant on every object of that entity.
> +	 * So check for that first.
> +	 */
> +	switch (mp_typeof(*data)) {
> +	case MP_STR:
> +		if (mp_decode_strl(&data) == 0) {
> +			/* Entity-wide privilege. */
> +			priv->object_id = 0;
> +			priv->object_type = schema_entity_type(priv->object_type);
> +			break;
> +		}
> +		FALLTHROUGH;
> +	default:
> +		priv->object_id = tuple_field_u32_xc(tuple,
> +						     BOX_PRIV_FIELD_OBJECT_ID);
> +	}
>  	if (priv->object_type == SC_UNKNOWN) {
>  		tnt_raise(ClientError, ER_UNKNOWN_SCHEMA_OBJECT,
>  			  object_type);

> +enum schema_object_type
> +schema_entity_type(enum schema_object_type type)
> +{
> +	switch (type) {
> +	case SC_SPACE:
> +		return SC_ENTITY_SPACE;
> +	case SC_FUNCTION:
> +		return SC_ENTITY_FUNCTION;
> +	case SC_USER:
> +		return SC_ENTITY_USER;
> +	case SC_ROLE:
> +		return SC_ENTITY_ROLE;
> +	case SC_SEQUENCE:
> +		return SC_ENTITY_SEQUENCE;
> +	case SC_COLLATION:
> +		return SC_ENTITY_COLLATION;
> +	default:
> +		unreachable();
> +	}
> +}

You'll reach this unreachable() if you insert something like this into
_priv table

  box.space._priv:insert{1, 1, 'abc', '', 1}

I guess you should return SC_UNKNOWN instead.

Also, after looking at patch 2, I'm convinced SC_ENTITY_USER and
SC_ENTITY_ROLE should be defined there, not in this patch. Please
rebase.

Also, there's priv_def_check(), which I missed by doing review last
time. This function handles object_id == 0 for all object types, but
there's no need to anymore, because you have entity types now. Please
remove. You should add handling of entity types to that function in
this patch (you do it in patch 2 for some reason).

  reply	other threads:[~2018-08-22 10:28 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-20  8:10 [tarantool-patches] [PATCH 0/4] Finish implementation of privileges Serge Petrenko
2018-08-20  8:10 ` [tarantool-patches] [PATCH 1/4] Introduce separate entity object types for entity privileges Serge Petrenko
2018-08-22 10:28   ` Vladimir Davydov [this message]
2018-08-22 12:37   ` Vladimir Davydov
2018-08-20  8:10 ` [tarantool-patches] [PATCH 2/4] Add entities user, role to access control Serge Petrenko
2018-08-22 10:37   ` Vladimir Davydov
2018-08-22 12:53   ` Vladimir Davydov
2018-08-20  8:10 ` [tarantool-patches] [PATCH 3/4] Add single object privilege checks to access_check_ddl Serge Petrenko
2018-08-22 11:58   ` Vladimir Davydov
2018-08-20  8:10 ` [tarantool-patches] [PATCH 4/4] Add a privilege upgrade script and update tests Serge Petrenko
2018-08-22 12:48   ` Vladimir Davydov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180822102845.obibc7ren43m4lmq@esperanza \
    --to=vdavydov.dev@gmail.com \
    --cc=kostja@tarantool.org \
    --cc=sergepetrenko@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH 1/4] Introduce separate entity object types for entity privileges.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox