feat(audit): auditing token addition and removal (#6649)

* auditing tokens

* adding diffs for token auditing

* added test

* generating docs

* auditing owner field
This commit is contained in:
Kira Pilot
2023-03-17 10:41:44 -07:00
committed by GitHub
parent 5b07f1e2a3
commit 090e37fc46
7 changed files with 75 additions and 32 deletions

View File

@ -18,6 +18,7 @@ import (
"github.com/tabbed/pqtype"
"golang.org/x/xerrors"
"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/coderd/httpmw"
@ -40,8 +41,19 @@ import (
// @Success 201 {object} codersdk.GenerateAPIKeyResponse
// @Router /users/{user}/keys/tokens [post]
func (api *API) postToken(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
user := httpmw.UserParam(r)
var (
ctx = r.Context()
user = httpmw.UserParam(r)
auditor = api.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.APIKey](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionCreate,
})
)
aReq.Old = database.APIKey{}
defer commitAudit()
if !api.Authorize(r, rbac.ActionCreate, rbac.ResourceAPIKey.WithOwner(user.ID.String())) {
httpapi.ResourceNotFound(rw)
@ -79,7 +91,7 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) {
return
}
cookie, _, err := api.createAPIKey(ctx, createAPIKeyParams{
cookie, key, err := api.createAPIKey(ctx, createAPIKeyParams{
UserID: user.ID,
LoginType: database.LoginTypeToken,
ExpiresAt: database.Now().Add(lifeTime),
@ -104,7 +116,7 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) {
})
return
}
aReq.New = *key
httpapi.Write(ctx, rw, http.StatusCreated, codersdk.GenerateAPIKeyResponse{Key: cookie.Value})
}
@ -314,17 +326,30 @@ func (api *API) tokens(rw http.ResponseWriter, r *http.Request) {
// @Router /users/{user}/keys/{keyid} [delete]
func (api *API) deleteAPIKey(rw http.ResponseWriter, r *http.Request) {
var (
ctx = r.Context()
user = httpmw.UserParam(r)
keyID = chi.URLParam(r, "keyid")
ctx = r.Context()
user = httpmw.UserParam(r)
keyID = chi.URLParam(r, "keyid")
auditor = api.Auditor.Load()
aReq, commitAudit = audit.InitRequest[database.APIKey](rw, &audit.RequestParams{
Audit: *auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionDelete,
})
key, err = api.Database.GetAPIKeyByID(ctx, keyID)
)
if err != nil {
api.Logger.Warn(ctx, "get API Key for audit log")
}
aReq.Old = key
defer commitAudit()
if !api.Authorize(r, rbac.ActionDelete, rbac.ResourceAPIKey.WithIDString(keyID).WithOwner(user.ID.String())) {
httpapi.ResourceNotFound(rw)
return
}
err := api.Database.DeleteAPIKeyByID(ctx, keyID)
err = api.Database.DeleteAPIKeyByID(ctx, keyID)
if errors.Is(err, sql.ErrNoRows) {
httpapi.ResourceNotFound(rw)
return

View File

@ -11,6 +11,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/coder/coder/cli/clibase"
"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/database/dbtestutil"
@ -23,8 +24,12 @@ func TestTokenCRUD(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
client := coderdtest.New(t, nil)
auditor := audit.NewMock()
numLogs := len(auditor.AuditLogs)
client := coderdtest.New(t, &coderdtest.Options{Auditor: auditor})
_ = coderdtest.CreateFirstUser(t, client)
numLogs++ // add an audit log for user creation
keys, err := client.Tokens(ctx, codersdk.Me, codersdk.TokensFilter{})
require.NoError(t, err)
require.Empty(t, keys)
@ -32,6 +37,7 @@ func TestTokenCRUD(t *testing.T) {
res, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{})
require.NoError(t, err)
require.Greater(t, len(res.Key), 2)
numLogs++ // add an audit log for token creation
keys, err = client.Tokens(ctx, codersdk.Me, codersdk.TokensFilter{})
require.NoError(t, err)
@ -46,9 +52,15 @@ func TestTokenCRUD(t *testing.T) {
err = client.DeleteAPIKey(ctx, codersdk.Me, keys[0].ID)
require.NoError(t, err)
numLogs++ // add an audit log for token deletion
keys, err = client.Tokens(ctx, codersdk.Me, codersdk.TokensFilter{})
require.NoError(t, err)
require.Empty(t, keys)
// ensure audit log count is correct
require.Len(t, auditor.AuditLogs, numLogs)
require.Equal(t, database.AuditActionCreate, auditor.AuditLogs[numLogs-2].Action)
require.Equal(t, database.AuditActionDelete, auditor.AuditLogs[numLogs-1].Action)
}
func TestTokenScoped(t *testing.T) {

View File

@ -254,9 +254,10 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
codersdk.AuditAction(alog.Action).Friendly(),
)
// API Key resources do not have targets and follow the below format:
// API Key resources (used for authentication) do not have targets and follow the below format:
// "User {logged in | logged out}"
if alog.ResourceType == database.ResourceTypeApiKey {
if alog.ResourceType == database.ResourceTypeApiKey &&
(alog.Action == database.AuditActionLogin || alog.Action == database.AuditActionLogout) {
return str
}

View File

@ -70,7 +70,11 @@ func ResourceTarget[T Auditable](tgt T) string {
case database.AuditableGroup:
return typed.Group.Name
case database.APIKey:
// this isn't used
if typed.TokenName != "nil" {
return typed.TokenName
}
// API Keys without names are used for auth
// and don't have a target
return ""
case database.License:
return strconv.Itoa(int(typed.ID))
@ -159,8 +163,10 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
}
diffRaw := []byte("{}")
// Only generate diffs if the request succeeded.
if sw.Status < 400 {
// Only generate diffs if the request succeeded
// and only if we aren't auditing authentication actions
if sw.Status < 400 &&
req.params.Action != database.AuditActionLogin && req.params.Action != database.AuditActionLogout {
diff := Diff(p.Audit, req.Old, req.New)
var err error