-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: implement GET /auth_logs/ and /auth_logs/{id} #178
Conversation
Signed-off-by: James Ramirez <[email protected]>
WalkthroughThe recent changes introduce comprehensive enhancements to the documentation and implementation of authentication logs within the Kolide framework. This includes new SQL queries and BATS tests for retrieving and validating log data, alongside modifications to existing API documentation. These improvements aim to provide clearer insights into user authentication activities and streamline data management for enhanced security and compliance tracking. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as API Client
participant Server as Kolide Server
participant Database as Auth Log DB
Client->>Server: Request authentication logs
Server->>Database: Fetch logs
Database-->>Server: Return log data
Server-->>Client: Deliver logs
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
docs/coverage.md (1)
30-31
: Clarify the status of the/auth_logs
endpoints.The status indicators for the
/auth_logs
and/auth_logs/{id}
endpoints have been changed to:question:
. This suggests uncertainty about their functionality or availability. It would be helpful to provide more specific information or context to avoid confusion for users.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- docs/coverage.md (1 hunks)
- docs/tables/kolide_auth_log.md (1 hunks)
- kolide/client/auth_log.go (1 hunks)
- kolide/plugin.go (1 hunks)
- kolide/table_kolide_auth_log.go (1 hunks)
- test/end-to-end/_query/kolide_auth_log.sql (1 hunks)
- test/end-to-end/_query/kolide_auth_log_by_id.sql (1 hunks)
- test/end-to-end/_results/kolide_auth_log.bash (1 hunks)
- test/end-to-end/kolide_auth_log.bats (1 hunks)
- test/end-to-end/kolide_auth_log_by_id.bats (1 hunks)
Files skipped from review due to trivial changes (3)
- test/end-to-end/_query/kolide_auth_log.sql
- test/end-to-end/_query/kolide_auth_log_by_id.sql
- test/end-to-end/_results/kolide_auth_log.bash
Additional context used
LanguageTool
docs/tables/kolide_auth_log.md
[grammar] ~3-~3: The verb ‘sign into’ is not standard English, except in the context of the law (“The bill was signed into law”). Write “sign in to”. For websites and computers, other options are “log in to” or “log on to”.
Context: ...attempts occurring when a user tries to sign into an App protected by Kolide Device Trust...(SIGN_INTO)
Additional comments not posted (28)
test/end-to-end/kolide_auth_log_by_id.bats (6)
3-6
: LGTM!The
setup_file
function correctly loads global settings and defines file-specific globals.
8-11
: LGTM!The
setup
function correctly loads extensions and helpers.
14-17
: LGTM!The test case
can_execute_query_via_steampipe
correctly checks if the query can be executed and if the results file exists.
19-22
: LGTM!The test case
has_no_more_than_one_result
correctly checks if the query results contain no more than one result.
24-24
: LGTM!The comment correctly indicates that the remaining functionality is covered in another BATS file.
26-30
: LGTM!The
teardown_file
function correctly removes the query results file if it exists.docs/tables/kolide_auth_log.md (3)
9-17
: LGTM!The SQL example for retrieving basic information from the
kolide_auth_log
table is correct.
21-31
: LGTM!The SQL example for listing authentication attempts from the past day is correct.
35-46
: LGTM!The SQL example for listing failed authentication attempts by a specific user is correct.
test/end-to-end/kolide_auth_log.bats (9)
3-12
: LGTM!The
setup_file
function correctly loads global settings, defines file-specific globals, and loads expected results if available.
14-17
: LGTM!The
setup
function correctly loads extensions and helpers.
20-23
: LGTM!The test case
can_execute_query_via_steampipe
correctly checks if the query can be executed and if the results file exists.
25-29
: LGTM!The test case
has_expected_number_of_results
correctly checks if the query results contain the expected number of results.
32-35
: LGTM!The test case
has_expected_timestamp
correctly checks if the query results contain the expected timestamp.
38-41
: LGTM!The test case
has_expected_person_name
correctly checks if the query results contain the expected person name.
44-47
: LGTM!The test case
has_expected_initial_status
correctly checks if the query results contain the expected initial status.
50-53
: LGTM!The test case
has_expected_result
correctly checks if the query results contain the expected result.
55-59
: LGTM!The
teardown_file
function correctly removes the query results file if it exists.kolide/plugin.go (1)
30-30
: LGTM! Verify the existence oftableKolideAuthLog(ctx)
function.The addition of the
kolide_auth_log
entry is correctly formatted.Ensure that the
tableKolideAuthLog(ctx)
function is implemented correctly in the codebase.Verification successful
Verified: The
tableKolideAuthLog(ctx)
function is correctly implemented.
- The function
tableKolideAuthLog(ctx)
is defined inkolide/table_kolide_auth_log.go
and returns a*plugin.Table
with a well-defined structure.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of `tableKolideAuthLog(ctx)` function. # Test: Search for the function implementation. Expect: The function should be implemented. rg --type go 'func tableKolideAuthLog'Length of output: 131
Script:
#!/bin/bash # Description: Retrieve the implementation details of the `tableKolideAuthLog` function. # Search and display the implementation of the `tableKolideAuthLog` function. rg --type go -A 20 'func tableKolideAuthLog' kolide/table_kolide_auth_log.goLength of output: 2651
kolide/client/auth_log.go (5)
1-5
: LGTM!The package declaration and imports are correct and necessary for the functionality.
7-28
: LGTM!The
AuthLogListResponse
andAuthLog
structs are well-defined and include necessary fields for authentication logs.
30-44
: LGTM!The
PersonInformation
,DetailedIssueInformation
, andEventInformation
structs are well-defined and include necessary fields for their respective purposes.
46-48
: LGTM!The
GetAuthLogs
function is correctly implemented and uses thefetchCollection
method.
50-52
: LGTM!The
GetAuthLogById
function is correctly implemented and uses thefetchResource
method.kolide/table_kolide_auth_log.go (4)
1-10
: LGTM!The package declaration and imports are correct and necessary for the functionality.
14-57
: LGTM!The
tableKolideAuthLog
function is well-structured and includes necessary columns and configurations.
61-67
: LGTM!The
listAuthLogs
function is correctly implemented and uses thelistAnything
method.
71-77
: LGTM!The
getAuthLog
function is correctly implemented and uses thegetAnything
method.
# Table: kolide_auth_log | ||
|
||
Lists the authentication attempts occurring when a user tries to sign into an App protected by Kolide Device Trust. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the grammatical issue in the description.
The verb ‘sign into’ is not standard English. Use “sign in to” instead.
- Lists the authentication attempts occurring when a user tries to sign into an App protected by Kolide Device Trust.
+ Lists the authentication attempts occurring when a user tries to sign in to an App protected by Kolide Device Trust.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
# Table: kolide_auth_log | |
Lists the authentication attempts occurring when a user tries to sign into an App protected by Kolide Device Trust. | |
# Table: kolide_auth_log | |
Lists the authentication attempts occurring when a user tries to sign in to an App protected by Kolide Device Trust. |
Tools
LanguageTool
[grammar] ~3-~3: The verb ‘sign into’ is not standard English, except in the context of the law (“The bill was signed into law”). Write “sign in to”. For websites and computers, other options are “log in to” or “log on to”.
Context: ...attempts occurring when a user tries to sign into an App protected by Kolide Device Trust...(SIGN_INTO)
Description
Implement GET /auth_logs and /auth_logs/{id} endpoints
Related Tickets & Documents
Close #27 , #28
Steps to Verify
Untested, insufficient data