Audit: Add token's use count to audit response

[vishalnayak]

No description provided.

[vishalnayak]

@jefferai @briankassouf LMK which other fields from TokenEntry would make sense audit response.

[jefferai]

I'm not sure about the naming here. By definition, an Auth struct corresponds to a token, so it feels redundant -- otherwise why not have token_policies and token_display_name.

[vishalnayak]

Agreed. I had reverted the name in logical.Auth struct. It makes sense to do it here as well.

[jefferai]

I think eventually we can add other fields if we decide they make sense but for the moment this is the one that caught my fancy since it's something that you can have a check on in the audit logs to explicitly correlate expected use counts with audit entry.

I do wonder, now that I'm thinking about it more, whether it makes sense to add this in the request entry as well (if non-zero), so that you can explicitly see the value at each use of the token. It's a bit of a bigger hammer though as this would require adding a field to logical.Request, attaching it to the logical request object in requestAuth, ensuring that the value can't be abused by backends in the router, and so on. So maybe we only go down this path if we are seeing requests.

What do you two think?

[vishalnayak]

I think it will be of value to add number of uses left for the token that is supplied. We should populate the request object early in handleRequest before we use the token, even though audit logging will be performed after using the token. I'll do it in a separate PR.

[jefferai]

You'd do it in requestAuth, same as the accessor and token itself.

[jefferai]

Policies/display name/metadata won't change. I wonder if on the request side, because this value does change, it should be RemainingUses. This could be accomplished by having two fields, both with omitempty. The downside is any parsing logic will have to understand both fields; on the flip side, the fields will individually be more meaningful.

Thoughts?

[vishalnayak]

I'm inclined towards doing this change. It makes sense to me.

[briankassouf]

I like the RemainingUses convention as well.

[jefferai]

One comment/question. Other than that, looks good, but let's figure that out and get @briankassouf 's input too.

[briankassouf]

Aside from that one question this looks great!