Skip to content
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.

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

Add missing fields to AuditEntry #2786

Merged
merged 7 commits into from May 24, 2023
Merged

Add missing fields to AuditEntry #2786

merged 7 commits into from May 24, 2023

Conversation

patriknordlen
Copy link
Contributor

@patriknordlen patriknordlen commented May 24, 2023

Fixes: #2785.

Adds the following fields to the AuditEntry struct and populates them when they exist in a response from the API:

  • ActorIP - IP of actor (if enabled in GitHub audit logs)
  • ActorLocation - location of actor (country code etc)
  • HashedToken - hashed token identifier when an action is performed using an access token
  • JobWorkflowRef - reference to workflow job when a workflow runs
  • OAuthApplicationID - OAuth application identifier when an OAuth application performs an action
  • OrgID - organization numeric ID
  • ProgrammaticAccessType - type of programmatic access (i.e. via token, cli, oauth app...)
  • PullRequestID - self-explanatory
  • PullRequestTitle - self-explanatory
  • PullRequestURL - self-explanatory
  • Reasons - Used when a user overrides branch protection, includes info on user's stated reason why
  • RunNumber - the numerical ID of the workflow run
  • TokenID - token ID when an action is performed using an access token
  • TokenScopes - string containing scopes assigned to a token
  • Topic - topic used in workflow runs
  • UserAgent - user-agent used in performing an action

@gmlewis gmlewis changed the title feat(audit-log): Add more fields Add missing fields to AuditEntry May 24, 2023
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @patriknordlen!
Just a few tweaks, please, then we will be ready to merge.

@@ -29,69 +29,96 @@ type HookConfig struct {
Secret *string `json:"secret,omitempty"`
}

type ActorLocation struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add godoc-style comments to each new exported struct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added!

github/orgs_audit_log.go Outdated Show resolved Hide resolved
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
@gmlewis
Copy link
Collaborator

gmlewis commented May 24, 2023

Please make sure to run go generate ./... in the top-level of the repo and push (not force-push since we always use squash and merge) the changes to the PR.
Please see CONTRIBUTING.md for more details.

@patriknordlen
Copy link
Contributor Author

Resolving type-related issues in tests now.

@gmlewis
Copy link
Collaborator

gmlewis commented May 24, 2023

Almost there... please update your unit tests so that "go test ./..." passes on your local system, then push the changes.

@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Merging #2786 (fe4b0df) into master (f96b64e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2786   +/-   ##
=======================================
  Coverage   98.06%   98.06%           
=======================================
  Files         132      132           
  Lines       11650    11650           
=======================================
  Hits        11424    11424           
  Misses        154      154           
  Partials       72       72           
Impacted Files Coverage Δ
github/orgs_audit_log.go 100.00% <ø> (ø)

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @patriknordlen !
LGTM.
Merging after tests pass.

@gmlewis gmlewis merged commit f19d239 into google:master May 24, 2023
9 checks passed
@patriknordlen patriknordlen deleted the more-audit-log-fields branch May 28, 2023 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AuditEntry missing relevant fields
2 participants