Fix cubbyhole and token revocation for legacy service tokens

[nsimons]

Legacy service tokens generated in Vault 1.10+ with env var VAULT_DISABLE_SERVER_SIDE_CONSISTENT_TOKENS=true are not assigned a cubbyhole ID. The implication is that cubbyhole/ cannot be used, nor can the tokens be revoked.

This commit assigns a cubbyhole ID to these tokens and adds a new test case to see that cubbyhole and revocation works correctly.

Fixes #18304

[akshya96]

LGTM!!

[Kardi5]

Thanks a lot for the fast PR. I guess this fixes the problem from occurring. What do you think about logging the cubbyhole error when destroying the old tokens instead of throwing it? This would allow current "broken" tokens to be deleted as well (via tidy call).

vault/vault/token_store.go

Lines 1826 to 1829 in 16e9c14

	err = ts.cubbyholeDestroyer(ctx, ts, entry)
	if err != nil {
	return err
	}

[nsimons]

Philosophically I think it makes sense. cubbyholeDestroyer wouldn't have anything to destroy if there is no cubbyhole ID linked with the token. With this reasoning, we could change the error to a nil here:

vault/vault/token_store.go

Lines 129 to 131 in d2a09ad

	if te.CubbyholeID == "" {
	return fmt.Errorf("missing cubbyhole ID while destroying")
	}

Then again I'm just an outsider without the full picture, so we'd need opinion from the maintainers :)

[VioletHynes]

Thanks for the PR! I think it makes most sense to leave as-is, in this case. In particular, the missing cubbyhole ID would be something we'd like to keep erroring on as non-legacy tokens should always have a cubbyhole ID, which is the usual case for tokens, and adding too much code to determine between the two kinds of tokens is something we'd like to avoid.

I'm going to merge this PR, I think. Thank you for the fix, and thank you for adding the additional test coverage!