Add Crate Owner Auditing

[markcatley]

Including the following audit actions:

• Invite User
• Remove User

Ticks off one of the items in #1548.

[rust-highfive]

r? @smarnach

(rust_highfive has picked a reviewer for you, use r? to override)

[smarnach]

Thanks for working on this! Unless someone else picks this up before then, I will hopefully get to this on 27 December.

[bors]

☔ The latest upstream changes (presumably #1931) made this pull request unmergeable. Please resolve the merge conflicts.

[smarnach]

There are still several open questions here that I don't know the answers to. @carols10cents since you wrote the original issue, could you please join the discussion?

• Do we want the audit information to be public? I feel it should be private, since I don't see the case for publishing it. So far this project tended to default to make information public, though.
• Do we also want to track when invitations are accepted or rejected?
• The auditing information is already partially available in the crate_owners and crate_owner_invitation tables. We should consisder deduplicating this.

                            Table "public.crate_owners"
       Column        |            Type             | Collation | Nullable | Default 
---------------------+-----------------------------+-----------+----------+---------
 crate_id            | integer                     |           | not null | 
 owner_id            | integer                     |           | not null | 
 created_at          | timestamp without time zone |           | not null | now()
 created_by          | integer                     |           |          | 
 deleted             | boolean                     |           | not null | false
 updated_at          | timestamp without time zone |           | not null | now()
 owner_kind          | integer                     |           | not null | 
 email_notifications | boolean                     |           | not null | true

                      Table "public.crate_owner_invitations"
       Column       |            Type             | Collation | Nullable | Default 
--------------------+-----------------------------+-----------+----------+---------
 invited_user_id    | integer                     |           | not null | 
 invited_by_user_id | integer                     |           | not null | 
 crate_id           | integer                     |           | not null | 
 created_at         | timestamp without time zone |           | not null | now()

[smarnach]

This publicly exposes the audit information via the API, right? We need to discuss whether this should be public. In any case, if this is available via the API, it should also be included in the database dumps and vice versa. Currently this PR excludes the information from the DB dumps.

[markcatley]

Once we resolve the issue I'll update this, and for version owner actions. I wasn't that familiar with what's expected here.

[smarnach]

I'd prefer a name like ActionKind or ActionType here, or even CrateOwnerActionKind, though this is kind of unwieldy.

We have the CrateOwnerAction and CrateAction types in this module, and while these types have quite different nature, the nature of their difference isn't reflected in the current naming.

[markcatley]

I like the suggestion. I'll have a look.

[markcatley]

Hi, thanks for the review.

This mostly follows the instructions in the issue, perhaps @carols10cents can weigh in on the rational. I will try to answer the specific questions where I can. I will note that this follows the same pattern as version owner actions that was done as part of the same issue.

Do we want the audit information to be public?
That's an interesting question. I cannot think of any strong arguments either way. What's the default answer to this question in the project? I think the argument is stronger for Version Owner Actions being public as I can see it being useful knowing who published/yanked/unyanked a crate and when. In absence of a strong argument I would probably advocate following the lead of that change.

Do we also want to track when invitations are accepted or rejected?
I think that's a good idea. I'll look at adding it when I am back from holiday next week.

The auditing information is already partially available in the crate_owners and crate_owner_invitation tables. We should consisder deduplicating this.
This is an interesting idea, I could create an issue and do it as a follow up if the information is available to backfill the information. I looked at it for version owner actions and I didn't think the information was complete enough.

[carols10cents]

I was imagining information like "user x added user y as an owner at datetime" being available publicly so that anyone could audit the risk of activities being malicious or not. One use case this could enable is what dependabot does for JavaScript: for example, in this PR, dependabot has highlighted maintainer changes:

Maintainer changes

This version was pushed to npm by eslintbot, a new releaser for eslint since your current version.

Does that help?

[carols10cents]

Do we also want to track when invitations are accepted or rejected?
I think that's a good idea. I'll look at adding it when I am back from holiday next week.

I feel like knowing when an invitation is sent and when an invitation is rejected are not useful pieces of information; it's only when an invitation is accepted and a crate has a new owner that it's interesting. What do you think?

[smarnach]

Thanks for the input, @carols10cents!

I agree that it makes sense to make changes in the ownership public, so let's make the new table public.

If that information is public, we should exclude information about when invitations are created and rejected, since this information should not be public.

The created_at, created_by and updated_at columns (and the corresponding triggers) can be removed from the crate_owners table, since we will have a better source for that information. The crate_owner_invitations table can remain unchanged. The new auditing table should be backfilled with the information from the deleted columns. This can either happen in a follow-up PR or in this one.

I think that settles all open questions. @markcatley Does this sound good to you?

[bors]

☔ The latest upstream changes (presumably #2032) made this pull request unmergeable. Please resolve the merge conflicts.

[locks]

Hi @markcatley! Are you able to address the latest discussion points?

[Turbo87]

closing due to inactivity

[carols10cents]

I know this is closed now, and I'll add this on the related issue too, but in thinking about this more, we do need to think about cases when a person might want to remove all association they have with a crate, past and present. So user_a, who was owner of crate_b at some point, now wants nothing to do with crate_b and doesn't want crates.io to show that they were ever owner of crate_b.