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

feat: audit logs #994

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Anirudhxx
Copy link
Contributor

Opening this PR so it makes it easier to discuss adding audit logs to parseable.

Fixes #765.

What kind of events do we want to capture?

  • Right now I am only tracking the Action events
  • Since these are accessible in the auth middleware I have sent the actual audit log post request in the auth middleware itself
  • This most likely add a performance penalty to our server and should be moved elsewhere (Looking for some suggestions)

Adding any other details to the audit log

  • Right now I am only capturing details like <action, ip address, request type>

This PR has:

  • been tested to ensure log ingestion and log query works.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added documentation for new or modified features or behaviors.

Co-authored-by: Akshat Agarwal <[email protected]>
pub audit_log_target_username: Option<String>,
pub audit_log_target_password: Option<String>,
pub audit_log_target_tls_verify: bool,
pub audit_log_target_headers: HashMap<String, String>,
Copy link
Contributor

@de-sh de-sh Nov 19, 2024

Choose a reason for hiding this comment

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

This could just as well be an http::HeaderMap after collection from the CLI args

.get("user-agent")
.and_then(|value| value.to_str().ok())
.unwrap_or("unknown"),
"id": "user123"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this hardcoded, and what is the intention behind using user-agent as type?

.unwrap_or("unknown"),
"id": "user123"
},
"ip-address":&req
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you intending to use the addr of the client?

@@ -164,6 +171,31 @@ where
/* ## Section end */

let auth_result: Result<_, Error> = (self.auth_method)(&mut req, self.action);
let body = json!([
Copy link
Contributor

Choose a reason for hiding this comment

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

I will work on the Schema, will get back to you in a week's time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @de-sh, just following up. Did you get a chance to work on this?

Copy link
Contributor

@de-sh de-sh Dec 2, 2024

Choose a reason for hiding this comment

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

Hey, sorry that this got lost, here is what I was able to come up with and Nitish has updated:

{
    "version": "1",
    "deploymentID": "String: uuid",
    "auditID": "String: uuid",
    "timestamp": "String: Datetime",
    "stream": "String",
    "actor": {
        "remoteHost": "127.0.0.1",
        "userAgent": "String",
        "userName": "String: username",
        "authorizationMethod": "Enum: basic/oauth/jwt"
    },
    "request": {
        "method": "Enum: POST/GET/PUT",
        "path": "String",
        "host": "String",
        "protocol": "Enum: kafka/http/https",
        "headers": {
            "key": "value"
        }
    },
    "response": {
        "statusCode": int
    }
}

@de-sh
Copy link
Contributor

de-sh commented Dec 2, 2024

This most likely add a performance penalty to our server and should be moved elsewhere (Looking for some suggestions)

I feel like the right way to do this would involve a dedicated thread that handles the serialization and reqwest part of your code. This since audit logging shouldn't impact normal functioning of request in any-ways and hence should be done, removed from the data flow path. I am exploring the feasibility of using a specialized tracing subscriber, to do this for us, similar to how we log to stdout now and how otel loggers work.

@Anirudhxx
Copy link
Contributor Author

I am exploring the feasibility of using a specialized tracing subscriber, to do this for us, similar to how we log to stdout now and how otel loggers work.

@de-sh I'll implement a audit macro and replace it with the current reqwest code and create a new tracing subscriber which will look for these audit spans and send the audit log to the relevant parseable server. lmk if this works?

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.

feat: audit logging for Parseable
2 participants