Skip to content

chore: allow organization name or uuid for audit log searching #13721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 28, 2024
Merged

chore: allow organization name or uuid for audit log searching #13721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
51bedcd
chore: allow organization name or uuid for audit log searching
Emyrk Jun 28, 2024
96bcecd
add 404 error
Emyrk Jun 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion coderd/audit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) {
}

queryStr := r.URL.Query().Get("q")
filter, errs := searchquery.AuditLogs(queryStr)
filter, errs := searchquery.AuditLogs(ctx, api.Database, queryStr)
if len(errs) > 0 {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Invalid audit search query.",
Expand Down
37 changes: 36 additions & 1 deletion coderd/audit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,48 @@ func TestAuditLogs(t *testing.T) {

// Using the organization selector allows the org admin to fetch audit logs
alogs, err := orgAdmin.AuditLogs(ctx, codersdk.AuditLogsRequest{
SearchQuery: fmt.Sprintf("organization_id:%s", owner.OrganizationID.String()),
Copy link
Member

@johnstcn johnstcn Jun 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should continue testing that searching by organization_id works

EDIT: undocumented feature

SearchQuery: fmt.Sprintf("organization:%s", owner.OrganizationID.String()),
Pagination: codersdk.Pagination{
Limit: 5,
},
})
require.NoError(t, err)
require.Len(t, alogs.AuditLogs, 1)

// Also try fetching by organization name
organization, err := orgAdmin.Organization(ctx, owner.OrganizationID)
require.NoError(t, err)

alogs, err = orgAdmin.AuditLogs(ctx, codersdk.AuditLogsRequest{
SearchQuery: fmt.Sprintf("organization:%s", organization.Name),
Pagination: codersdk.Pagination{
Limit: 5,
},
})
require.NoError(t, err)
require.Len(t, alogs.AuditLogs, 1)
})

t.Run("Organization404", func(t *testing.T) {
t.Parallel()

logger := slogtest.Make(t, &slogtest.Options{
IgnoreErrors: true,
})
ctx := context.Background()
client := coderdtest.New(t, &coderdtest.Options{
Logger: &logger,
})
owner := coderdtest.CreateFirstUser(t, client)
orgAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID))

_, err := orgAdmin.AuditLogs(ctx, codersdk.AuditLogsRequest{
SearchQuery: fmt.Sprintf("organization:%s", "random-name"),
Pagination: codersdk.Pagination{
Limit: 5,
},
})
require.Error(t, err)
})
}

Expand Down
28 changes: 26 additions & 2 deletions coderd/searchquery/search.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package searchquery

import (
"context"
"database/sql"
"fmt"
"net/url"
Expand All @@ -16,7 +17,9 @@ import (
"github.com/coder/coder/v2/codersdk"
)

func AuditLogs(query string) (database.GetAuditLogsOffsetParams, []codersdk.ValidationError) {
// AuditLogs requires the database to fetch an organization by name
// to convert to organization uuid.
func AuditLogs(ctx context.Context, db database.Store, query string) (database.GetAuditLogsOffsetParams, []codersdk.ValidationError) {
// Always lowercase for all searches.
query = strings.ToLower(query)
values, errors := searchTerms(query, func(term string, values url.Values) error {
Expand All @@ -30,7 +33,6 @@ func AuditLogs(query string) (database.GetAuditLogsOffsetParams, []codersdk.Vali
const dateLayout = "2006-01-02"
parser := httpapi.NewQueryParamParser()
filter := database.GetAuditLogsOffsetParams{
OrganizationID: parser.UUID(values, uuid.Nil, "organization_id"),
ResourceID: parser.UUID(values, uuid.Nil, "resource_id"),
ResourceTarget: parser.String(values, "", "resource_target"),
Username: parser.String(values, "", "username"),
Expand All @@ -44,6 +46,28 @@ func AuditLogs(query string) (database.GetAuditLogsOffsetParams, []codersdk.Vali
if !filter.DateTo.IsZero() {
filter.DateTo = filter.DateTo.Add(23*time.Hour + 59*time.Minute + 59*time.Second)
}

// Convert the "organization" parameter to an organization uuid. This can require
// a database lookup.
organizationArg := parser.String(values, "", "organization")
if organizationArg != "" {
organizationID, err := uuid.Parse(organizationArg)
if err == nil {
filter.OrganizationID = organizationID
} else {
// Organization could be a name
organization, err := db.GetOrganizationByName(ctx, organizationArg)
if err != nil {
Comment on lines +57 to +60
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah my bad, I fixated on line 33 but missed this check 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I use the same query param of organization. Feels more straight forward

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth adding some sort of error if you use organization_id to suggest using organization.

Note that this param is not documented or used anywhere yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only problem is this might break existing queries, idk if any folks would have a reason to use this yet?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this param is not documented or used anywhere yet.

Ah, if it's not documented then all bets are off.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I added it 2 days ago, and intentionally am not documenting any of the multi-org stuff yet: #13663

parser.Errors = append(parser.Errors, codersdk.ValidationError{
Field: "organization",
Detail: fmt.Sprintf("Organization %q either does not exist, or you are unauthorized to view it", organizationArg),
})
} else {
filter.OrganizationID = organization.ID
}
}
}

parser.ErrorExcessParams(values)
return filter, parser.Errors
}
Expand Down
7 changes: 6 additions & 1 deletion coderd/searchquery/search_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package searchquery_test

import (
"context"
"database/sql"
"fmt"
"strings"
Expand All @@ -11,6 +12,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbmem"
"github.com/coder/coder/v2/coderd/searchquery"
"github.com/coder/coder/v2/codersdk"
)
Expand Down Expand Up @@ -315,7 +317,10 @@ func TestSearchAudit(t *testing.T) {
c := c
t.Run(c.Name, func(t *testing.T) {
t.Parallel()
values, errs := searchquery.AuditLogs(c.Query)
// Do not use a real database, this is only used for an
// organization lookup.
db := dbmem.New()
values, errs := searchquery.AuditLogs(context.Background(), db, c.Query)
if c.ExpectedErrorContains != "" {
require.True(t, len(errs) > 0, "expect some errors")
var s strings.Builder
Expand Down
Loading