Add table for Gateway audit logs #816
Conversation
|
|
||
| import static java.util.Objects.requireNonNull; | ||
|
|
||
| public class AuditLogger |
There was a problem hiding this comment.
I would make AuditLogger an interface, and then add different types of AuditLoggers. e.g, a DatabaseAuditLogger for writing it to to the DB, and/or a LogAuditLogger that simply emits it out to log.info()
kjsbot
changed the title
| user_name VARCHAR(256) NOT NULL, | ||
| ip_address VARCHAR(45), | ||
| backend_name VARCHAR(256) NOT NULL, | ||
| operation VARCHAR(256) NOT NULL, |
There was a problem hiding this comment.
256 seems to be too long. isn't this one of AuditActions?
64 seems to be sufficient
| ip_address VARCHAR(45), | ||
| backend_name VARCHAR(256) NOT NULL, | ||
| operation VARCHAR(256) NOT NULL, | ||
| context VARCHAR(256) NOT NULL, |
There was a problem hiding this comment.
I am not sure what you intent by context, but it seems to be quite small. Why not text?
There was a problem hiding this comment.
Context refers to where the admin made the change - ex: curl-ing, webapp, etc
| context VARCHAR(256) NOT NULL, | ||
| success BOOLEAN NOT NULL, | ||
| user_comment VARCHAR(1024), | ||
| change_time TIMESTAMP NOT NULL, |
There was a problem hiding this comment.
Audit log should have default timestamp, no?
| ); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS gateway_audit_logs ( | ||
| audit_id BIGSERIAL, |
There was a problem hiding this comment.
| audit_id BIGSERIAL, | |
| audit_id BIGSERIAL PRIMARY KEY, |
not so sure, but can you do this instead of line 91?
| ip_address VARCHAR(45), | ||
| backend_name VARCHAR(256) NOT NULL, | ||
| operation VARCHAR(256) NOT NULL, | ||
| context VARCHAR(256) NOT NULL, |
There was a problem hiding this comment.
In my opinion, I think having different columns can make management simpler as well as being better for querying
| backend_name VARCHAR(256) NOT NULL, | ||
| operation VARCHAR(256) NOT NULL, | ||
| context VARCHAR(256) NOT NULL, | ||
| success SMALLINT NOT NULL CHECK (success IN (0, 1)), |
| operation VARCHAR(256) NOT NULL, | ||
| context VARCHAR(256) NOT NULL, | ||
| success SMALLINT NOT NULL CHECK (success IN (0, 1)), | ||
| user_comment VARCHAR(1024), |
There was a problem hiding this comment.
Ideally comments will be kept sort and brief which is why the 1024 limit is kept, anything going over should be truncated
| ip_address VARCHAR2(45), | ||
| backend_name VARCHAR(256) NOT NULL, | ||
| operation VARCHAR(256) NOT NULL, | ||
| context VARCHAR(256) NOT NULL, |
| context VARCHAR(256) NOT NULL, | ||
| success SMALLINT NOT NULL CHECK (success IN (0, 1)), | ||
| user_comment VARCHAR(1024), | ||
| change_time TIMESTAMP NOT NULL, |
There was a problem hiding this comment.
How about TIMESTAMPTZ NOT NULL DEFAULT now()?
| context VARCHAR(256) NOT NULL, | ||
| success NUMBER(1) NOT NULL CHECK (success IN (0,1)), | ||
| user_comment VARCHAR(1024), | ||
| change_time TIMESTAMP NOT NULL, |
There was a problem hiding this comment.
How about
TIMESTAMP WITH TIME ZONE CURRENT_TIMESTAMP NOT NULL
| ); | ||
|
|
||
| CREATE TABLE gateway_audit_logs ( | ||
| audit_id NUMBER GENERATED ALWAYS as IDENTITY(START with 1 INCREMENT by 1), |
There was a problem hiding this comment.
| audit_id NUMBER GENERATED ALWAYS as IDENTITY(START with 1 INCREMENT by 1), | |
| audit_id NUMBER(19) GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, |
3 participants
Description
Adds support for a audit logs table for all 3 databases.
Additional context and related issues
Following up on the discussion for issue #803, this PR adds new tables for auditing logs, it also creates supporting classes for updating the tables. Comments will be collected via the UI and from the body of curl requests (separate PR). Open to any further discussion or questions!
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
(x) Release notes are required, with the following suggested text:
* Add new table for audit logs.