Yaroslav Dynnikov authored 1 year ago and Yaroslav Dynnikov committed 1 year ago

Tested scenarios:

1. Alice is granted a privilege to create global/sharded table
2. Alice can create, read, and write it
3. Bob can't read it
4. Alice grants Bob a privilege to read it
5. Now Bob can read it, but he still can't write

This patch adds a proper support for auxiliary fields containing `Option<T>`.
As of now, KV pairs containing `Some(x)` will be printed as just `x`,
while the ones with `None` won't be printed at all.

Example:

```rust
crate::audit!(
    message: "demo audit event",
    title: "example",
    severity: Low,
    test: Some(0u32),
);
```

```json
{
  "id": "1.0.1",
  "message": "demo audit event",
  "severity": "low",
  "time": "2023-12-06T17:16:51.085+0300",
  "title": "example",
  "test": "0"
}
```

```rust
crate::audit!(
    message: "demo audit event",
    title: "example",
    severity: Low,
    test: None::<i32>,
);
```

```json
{
  "id": "1.0.1",
  "message": "demo audit event",
  "severity": "low",
  "time": "2023-12-06T17:16:51.085+0300",
  "title": "example"
}
```

This patch reintroduces instance-related events (e.g. `join_instance`)
which were accidentally disabled in a bugfix of the DML handler (see
commit 52ed9237 for more information) due to the lack of tests.

Furthermore, this patch adds the following events:

* expel_instance
* change_config, which tracks changes in `_pico_property`

This was the original intended change of the patch, but the apparent
breakage of the preexisting events called for action.

This change allows for creators of corresponding objects to consequently have all privileges on them.

Arseniy Volynets authored 1 year ago and Georgy Moshkin committed 1 year ago

- Test fails because of timeout on socket.
Current vshard multi storage call works
in 2 stages:
1. Ref needed storages
2. Execute stored procedure on the storages
When creating Ref on storage, vshard waits for all
buckets to become writable, if it happens that
rebalancer will start to move buckets, the query
execution will freeze until rebalancing is
done, which happens for this test sometimes.
- Comment out the flaky test until we switch
to new vshard multicall algorithm. Another
solution would be to increase timeout, but
there is no guarantee that new timeout is
big enough: I caught this error with 5s
timeout.

This could result in a broken instance if it terminated in between the
two storage writes. Now we always persist the commit index before
applying the persisted entries and do so in a transaction with unstable
entries or snapshot if they are present.

This patch fixes a possible DOS by checking whether the user exists
before creating a record for its unsuccessful auth attempt. This is
due to the fact that `box.session.on_auth` will be called even for
unknown users (this helps vanilla users log such events).

This patch adds two new events:
* auth_ok
* auth_fail, which may contain "verdict" describing user suspension.

Examples:

```json
{
  "id": "1.1.19",
  "message": "successfully authenticated user `guest`",
  "severity": "low",
  "time": "2023-11-30T19:02:10.708+0300",
  "title": "auth_ok",
  "user": "guest"
}
```

```json
{
  "id": "1.0.11",
  "message": "failed to authenticate user `borat`",
  "severity": "high",
  "time": "2023-11-30T18:58:48.635+0300",
  "title": "auth_failed",
  "user": "borat"
}
```

Logically, the record id consists of 3 fields:
* `raft_id` -- raft id of the current node (or `null` if uninitialized);
* `count` -- in-memory id which is auto incremented for each new record;
* `gen` -- persistent id which is incremented each time node is started.

However, in practice they are stored inside a global `LogicalClock`.

This is what the record looks like now:

```json
{
  "id": "1.0.3",
  "instance_id": "i1",
  "message": "a new instance `i1` joined the cluster",
  "raft_id": "1",
  "severity": "low",
  "time": "2023-11-29T17:07:36.524+0300",
  "title": "join_instance"
}
```

Note how those fields are concatenated using `.`; the resulting
unique id is represented by a json string type.

Furthermore, since audit record logger now depends on the
raft machine's state, we decided to move its initialization
further into the init sequence (postjoin).

The patch adds remaining access checks for cluster wide management
operations involving tables users and roles. Added checks closely follow
ones in vanilla tarantool. Basic scenario now works. See newly added
`test_create_space_smoke`.

There are some follow ups I'd like to address separately, namely:
- more tests for new ifs around box_access_check_ddl calls
  https://git.picodata.io/picodata/picodata/picodata/-/issues/420
- add tests for ownership semantics (these can be mostly ported from tarantool-module) blocked by
  https://git.picodata.io/picodata/picodata/picodata/-/issues/408
- port access.test and role.test from core (at some point later)
  https://git.picodata.io/picodata/picodata/picodata/-/issues/421
- Detect and prohibit circular role grants
  https://git.picodata.io/picodata/picodata/picodata/-/issues/415

Close #339

Arseniy Volynets authored 1 year ago and Denis Smirnov committed 1 year ago

- update sbroad submodule
- add integration tests with aggregates
on global tables
- groupby, having and aggregates are now
supported for global tbls

test: mark test_script_failure as xfail, because it's too flaky and doesn't check for anything we care about