Skip to content

Commit

Permalink
chore(CE): add audit logger and audit to models controller (Multiwove…
Browse files Browse the repository at this point in the history
…n#425)

Co-authored-by: TivonB-AI2 <[email protected]>
  • Loading branch information
github-actions[bot] and TivonB-AI2 authored Oct 22, 2024
1 parent 00036ec commit 5409e14
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 1 deletion.
12 changes: 12 additions & 0 deletions server/app/controllers/api/v1/models_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module Api
module V1
class ModelsController < ApplicationController
include Models
include AuditLogger
attr_reader :connector, :model

before_action :set_connector, only: %i[create]
Expand All @@ -12,6 +13,7 @@ class ModelsController < ApplicationController
# TODO: Enable this once we have query validation implemented for all the connectors
# before_action :validate_query, only: %i[create update]
after_action :event_logger
after_action :create_audit_log

def index
filter = params[:query_type] || "all"
Expand All @@ -23,6 +25,7 @@ def index

def show
authorize @model
@audit_resource = @model.name
render json: @model, status: :ok
end

Expand All @@ -34,6 +37,8 @@ def create
)
if result.success?
@model = result.model
@audit_resource = @model.name
@payload = model_params
render json: @model, status: :created
else
render_error(
Expand All @@ -53,6 +58,8 @@ def update

if result.success?
@model = result.model
@audit_resource = @model.name
@payload = model_params
render json: @model, status: :ok
else
render_error(
Expand All @@ -65,6 +72,7 @@ def update

def destroy
authorize model
@audit_resource = model.name
model.destroy!
head :no_content
end
Expand Down Expand Up @@ -103,6 +111,10 @@ def validate_query
)
end

def create_audit_log
audit!(resource_id: params[:id], resource: @audit_resource, payload: @payload)
end

def model_params
params.require(:model).permit(
:connector_id,
Expand Down
2 changes: 1 addition & 1 deletion server/app/controllers/concerns/audit_logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def audit!(action: nil, user: nil, resource_type: nil, resource_id: nil, resourc
resource_id:,
resource:,
workspace:,
metadata: payload ? payload.to_unsafe_h : {}
metadata: payload.try(:to_unsafe_h) || payload
)
rescue StandardError => e
Rails.logger.error({
Expand Down
144 changes: 144 additions & 0 deletions server/spec/requests/api/v1/models_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@
expect(response_hash[:data].count).to eql(6)
expect(response_hash.dig(:data, 0, :type)).to eq("models")
expect(response_hash.dig(:links, :first)).to include("http://www.example.com/api/v1/models?page=1")

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("index")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(nil)
expect(audit_log.resource).to eq(nil)
expect(audit_log.workspace_id).to eq(workspace.id)
end

it "returns success and all mode for viewer role" do
Expand All @@ -70,12 +79,30 @@
expect(response_hash[:data].count).to eql(6)
expect(response_hash.dig(:data, 0, :type)).to eq("models")
expect(response_hash.dig(:links, :first)).to include("http://www.example.com/api/v1/models?page=1")

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("index")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(nil)
expect(audit_log.resource).to eq(nil)
expect(audit_log.workspace_id).to eq(workspace.id)
end

it "filters models based on the query_type parameter" do
get "/api/v1/models?query_type=data", headers: auth_headers(user, workspace_id)
expect(response).to have_http_status(:ok)
expect(JSON.parse(response.body)["data"].map { |m| m["id"] }).not_to include(ai_ml_model.id)

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("index")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(nil)
expect(audit_log.resource).to eq(nil)
expect(audit_log.workspace_id).to eq(workspace.id)
end

it "filters models based on a different query_type" do
Expand All @@ -84,13 +111,31 @@
expect(JSON.parse(response.body)["data"].map do |m|
m["id"]
end).not_to include(raw_sql_model.id, dbt_model.id, soql_model.id)

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("index")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(nil)
expect(audit_log.resource).to eq(nil)
expect(audit_log.workspace_id).to eq(workspace.id)
end

it "returns all models" do
get "/api/v1/models", headers: auth_headers(user, workspace_id)
expect(response).to have_http_status(:ok)
model_ids = JSON.parse(response.body)["data"].map { |m| m["id"] }
expect(model_ids.count).to eql(6)

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("index")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(nil)
expect(audit_log.resource).to eq(nil)
expect(audit_log.workspace_id).to eq(workspace.id)
end
end
end
Expand All @@ -115,6 +160,15 @@
expect(response_hash.dig(:data, :attributes, :query)).to eq(models.first.query)
expect(response_hash.dig(:data, :attributes, :query_type)).to eq(models.first.query_type)
expect(response_hash.dig(:data, :attributes, :primary_key)).to eq(models.first.primary_key)

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("show")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(models.first.id)
expect(audit_log.resource).to eq(models.first.name)
expect(audit_log.workspace_id).to eq(workspace.id)
end

it "returns success and fetch model for viewer role" do
Expand Down Expand Up @@ -143,6 +197,15 @@
expect(response_hash.dig(:data, :attributes, :query)).to eq(models.first.query)
expect(response_hash.dig(:data, :attributes, :query_type)).to eq(models.first.query_type)
expect(response_hash.dig(:data, :attributes, :primary_key)).to eq(models.first.primary_key)

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("show")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(models.first.id)
expect(audit_log.resource).to eq(models.first.name)
expect(audit_log.workspace_id).to eq(workspace.id)
end

it "returns an error response while fetch model" do
Expand Down Expand Up @@ -183,6 +246,15 @@
expect(response_hash.dig(:data, :attributes, :query)).to eq(request_body.dig(:model, :query))
expect(response_hash.dig(:data, :attributes, :query_type)).to eq(request_body.dig(:model, :query_type))
expect(response_hash.dig(:data, :attributes, :primary_key)).to eq(request_body.dig(:model, :primary_key))

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("create")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(nil)
expect(audit_log.resource).to eq(request_body.dig(:model, :name))
expect(audit_log.workspace_id).to eq(workspace.id)
end

it "fails model creation for connector without catalog" do
Expand Down Expand Up @@ -216,6 +288,15 @@
expect(response_hash.dig(:data, :attributes, :query)).to eq(request_body.dig(:model, :query))
expect(response_hash.dig(:data, :attributes, :query_type)).to eq(request_body.dig(:model, :query_type))
expect(response_hash.dig(:data, :attributes, :primary_key)).to eq(request_body.dig(:model, :primary_key))

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("create")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(nil)
expect(audit_log.resource).to eq(request_body.dig(:model, :name))
expect(audit_log.workspace_id).to eq(workspace.id)
end

it "creates a new model with query type table selector" do
Expand All @@ -231,6 +312,15 @@
expect(response_hash.dig(:data, :attributes, :query)).to eq(request_body.dig(:model, :query))
expect(response_hash.dig(:data, :attributes, :query_type)).to eq("table_selector")
expect(response_hash.dig(:data, :attributes, :primary_key)).to eq(request_body.dig(:model, :primary_key))

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("create")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(nil)
expect(audit_log.resource).to eq(request_body.dig(:model, :name))
expect(audit_log.workspace_id).to eq(workspace.id)
end

context "when creating a model with query_type = ai_ml and configuration is present" do
Expand All @@ -255,6 +345,15 @@
expect(response_hash.dig(:data, :attributes, :name)).to eq(request_body.dig(:model, :name))
expect(response_hash.dig(:data, :attributes, :query_type)).to eq("ai_ml")
expect(response_hash.dig(:data, :attributes, :configuration)).to eq(request_body.dig(:model, :configuration))

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("create")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(nil)
expect(audit_log.resource).to eq(request_body.dig(:model, :name))
expect(audit_log.workspace_id).to eq(workspace.id)
end
end

Expand Down Expand Up @@ -306,6 +405,15 @@
expect(response_hash.dig(:data, :attributes, :query)).to eq(request_body.dig(:model, :query))
expect(response_hash.dig(:data, :attributes, :query_type)).to eq(request_body.dig(:model, :query_type))
expect(response_hash.dig(:data, :attributes, :primary_key)).to eq(request_body.dig(:model, :primary_key))

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("update")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(models.second.id)
expect(audit_log.resource).to eq(request_body.dig(:model, :name))
expect(audit_log.workspace_id).to eq(workspace.id)
end

it "fails model update for connector without catalog for ai model" do
Expand Down Expand Up @@ -346,6 +454,15 @@
expect(response_hash.dig(:data, :attributes, :query)).to eq(request_body.dig(:model, :query))
expect(response_hash.dig(:data, :attributes, :query_type)).to eq(request_body.dig(:model, :query_type))
expect(response_hash.dig(:data, :attributes, :primary_key)).to eq(request_body.dig(:model, :primary_key))

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("update")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(models.second.id)
expect(audit_log.resource).to eq(request_body.dig(:model, :name))
expect(audit_log.workspace_id).to eq(workspace.id)
end

context "when updating a model with query_type = ai_ml and configuration is present" do
Expand All @@ -370,6 +487,15 @@
expect(response_hash.dig(:data, :attributes, :name)).to eq(request_body.dig(:model, :name))
expect(response_hash.dig(:data, :attributes, :query_type)).to eq("ai_ml")
expect(response_hash.dig(:data, :attributes, :configuration)).to eq(request_body.dig(:model, :configuration))

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("update")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(models.second.id)
expect(audit_log.resource).to eq(request_body.dig(:model, :name))
expect(audit_log.workspace_id).to eq(workspace.id)
end
end

Expand Down Expand Up @@ -407,12 +533,30 @@
it "returns success and delete model " do
delete "/api/v1/models/#{models.first.id}", headers: auth_headers(user, workspace_id)
expect(response).to have_http_status(:no_content)

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("destroy")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(models.first.id)
expect(audit_log.resource).to eq(models.first.name)
expect(audit_log.workspace_id).to eq(workspace.id)
end

it "returns success and delete model for member role" do
workspace.workspace_users.first.update(role: member_role)
delete "/api/v1/models/#{models.first.id}", headers: auth_headers(user, workspace_id)
expect(response).to have_http_status(:no_content)

audit_log = AuditLog.last
expect(audit_log).not_to be_nil
expect(audit_log.user_id).to eq(user.id)
expect(audit_log.action).to eq("destroy")
expect(audit_log.resource_type).to eq("Model")
expect(audit_log.resource_id).to eq(models.first.id)
expect(audit_log.resource).to eq(models.first.name)
expect(audit_log.workspace_id).to eq(workspace.id)
end

it "returns for viwer role " do
Expand Down

0 comments on commit 5409e14

Please sign in to comment.