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

[AuditIgnore] on WebApi #218

Closed
holden-ar opened this issue May 11, 2019 · 19 comments
Closed

[AuditIgnore] on WebApi #218

holden-ar opened this issue May 11, 2019 · 19 comments
Assignees

Comments

@holden-ar
Copy link

I've setup Audit using Middleware in a Core Webapi.

app.UseAuditMiddleware(_ => _
.FilterByRequest(r => !r.Path.Value.EndsWith("favicon.ico"))
.IncludeHeaders()
.IncludeRequestBody()
.IncludeResponseBody()
.WithEventType("HTTP:{verb}:{url}"));

Everything is logged.

I need to exclude specific methods for being logged and im using [AuditIgnore] Attribute

[HttpGet]
[AuditIgnore]
public ActionResult GetAll()

But it seems that is notworking and the actions is also logged.

Any clue/idea

Thanks.

@thepirat000
Copy link
Owner

thepirat000 commented May 11, 2019

Check the points on the documentation here

The AuditIgnore atribute only applies for the Action Filter. It has no effect on the requests logged by the Middleware.

You will probably need to filter it out on the middleware configuration

@thepirat000
Copy link
Owner

thepirat000 commented May 12, 2019

If you don't need to log the requests that doesn't get into an action method (i.e. when the url requested does not map to an action, etc), then I recommend you to use the Global Action Filter instead of (or in addition to) the middleware.

The global filter allows more dynamic configuration and works with the AuditIgnoreAttribute

@holden-ar
Copy link
Author

Thank you very much for your explanation and your time.
Global Action Filter would be ok.

Which is the best strategy to prevent sensitive information to be audited?
I.E.
I want to track a login events but i don't want the Passwd property to be saved. Any suggestion?

Thanks in advance

@thepirat000
Copy link
Owner

thepirat000 commented May 14, 2019

[AuditIgnore] attribute can also be used on the action parameters like:

[HttpPost]
public ActionResult Save(string user, [AuditIgnore]string password)

But if it's inside a property on your model, you will need a different approach

@thepirat000
Copy link
Owner

thepirat000 commented May 14, 2019

Sorry I got confused with the #219 that is adding an AuditIgnore attribute for MVC, ignore the related commit

@thepirat000
Copy link
Owner

Which data provider do you use?

@holden-ar
Copy link
Author

Audit.NET.SqlServer

@thepirat000
Copy link
Owner

Check this for a possible solution #163

@thepirat000
Copy link
Owner

I'm closing this for inactivity, if you're still having problems, please comment here

@JohnnyFun
Copy link

@thepirat000, is there a way to use the middleware and also respect [AuditIgnore] on mvc controllers? Perhaps by setting an httpcontext item like "AuditIgnored" to true in AuditApiAdapter.IsActionIgnored and then skipping OnInsert later on in AuditMiddleware if that httpcontext item is set to true, or am I not thinking of something important?

p.s. thanks for building this and making it open source!

@thepirat000 thepirat000 reopened this May 24, 2019
@JohnnyFun
Copy link

JohnnyFun commented May 24, 2019

If it's any help, I managed to get it working for my needs by taking a chunk of your code and just adding an action filter like:

mvc.Filters.Add(typeof(AuditIgnoreFilter));
...
public class AuditIgnoreFilter : IActionFilter {
	public const string _auditIgnore = "_auditIgnore";
	public void OnActionExecuting(ActionExecutingContext context) => AddHttpItemIfAuditIgnore(context);
	public void OnActionExecuted(ActionExecutedContext context) => AddHttpItemIfAuditIgnore(context);

	private void AddHttpItemIfAuditIgnore(FilterContext context) {
		if (IsActionIgnored(context))
			context.HttpContext.Items[_auditIgnore] = true;
	}

	public static bool IgnoreAudit(HttpContext httpContext) => httpContext?.Items?.ContainsKey(_auditIgnore) == true;

	// https://github.com/thepirat000/Audit.NET/blob/master/src/Audit.WebApi/AuditApiAdapter.Core.cs
	public bool IsActionIgnored(FilterContext actionContext) {
		var actionDescriptor = actionContext?.ActionDescriptor as ControllerActionDescriptor;
		var controllerIgnored = actionDescriptor?.MethodInfo?.DeclaringType.GetTypeInfo().GetCustomAttribute<AuditIgnoreAttribute>(true);
		if (controllerIgnored != null) {
			return true;
		}
		var actionIgnored = actionDescriptor?.MethodInfo?.GetCustomAttribute<AuditIgnoreAttribute>(true);
		if (actionIgnored != null) {
			return true;
		}
		return false;
	}
}

and then doing a custom data provider and adding a check prior to the insert like:

public async Task InsertAuditLogAsync(AuditEventWebApi a) {
	if (AuditIgnoreFilter.IgnoreAudit(_httpContextAccessor.HttpContext))
		return;
...

@thepirat000
Copy link
Owner

thepirat000 commented May 24, 2019

I think there is a simpler workaround. On your IActionFilter you could do:

public class AuditIgnoreActionFilter : IActionFilter
{
    private const string ScopeKey = "__private_AuditApiScope__";
    public void OnActionExecuted(ActionExecutedContext context) { }
    public void OnActionExecuting(ActionExecutingContext context)
    {
        if (IsActionIgnored(context))
        {
            var scope = context.HttpContext.Items[ScopeKey] as AuditScope;
            if (scope != null)
            {
                scope.Discard();
                context.HttpContext.Items[ScopeKey] = null;
            }
        }
    }
}

The AuditScope is already stored on the HttpContext you can just set that item key to null. The call to Discard() is just to ensure no further saving is allowed if there is any other reference to it.

The IsActionIgnored() implementation could be trivial if I make AuditApiAdapter public. I think I will do. I want to analize if it worth to provide the IActionFilter on the library.

I think I will provide an extension method on HttpContext to discard the current audit scope, so your action filter can be even simpler, like:

public class AuditIgnoreActionFilter : IActionFilter
{
    private AuditApiAdapter _adapter = new AuditApiAdapter();
    public void OnActionExecuted(ActionExecutedContext context) { }
    public void OnActionExecuting(ActionExecutingContext context)
    {
        if (_adapter.IsActionIgnored(context))
        {
            context.HttpContext.DiscardCurrentAuditScope();
        }
    }
}

@JohnnyFun
Copy link

JohnnyFun commented May 24, 2019

Sure, we also call the mvc middleware. Would it make sense to put the httpcontext logic in the filter that gets added with that?

mvc.AddAuditFilter(config => config 
	.LogAllActions()
	.IncludeHeaders()
);

No hurry, btw--what we have now will work perfectly for our needs, so you can take as much time as you like determining how/if you'd like to change the library.

@thepirat000
Copy link
Owner

Maybe I'm missing something, but the AuditApiGlobalFilter already respects the [AuditIgnoreAttribute] right?

@thepirat000
Copy link
Owner

Oh I think I got it, you mean that when using a mixed approach (middleware + audit action filter) you should not need to add a new filter; the audit action filter should take care of discarding the audit event.

@JohnnyFun
Copy link

When I only call:

mvc.AddAuditFilter(config => config
  .LogAllActions()
  .IncludeHeaders()
);

[AuditIgnore] is respected. But when I add the non-mvc middleware, [AuditIgnore] doesn't prevent an audit log from being inserted. The log that gets inserted has no MVC info (like actionname, controllername, etc), but a record gets inserted despite [AudigIgnore]:

app.UseAuditMiddleware(_ => _
	.WithEventType("{verb}:{url}")
	.IncludeHeaders()
	.IncludeResponseHeaders()
	.IncludeRequestBody());

@JohnnyFun
Copy link

JohnnyFun commented May 24, 2019

Later on, I'll maybe clone the audit.net repo and debug into it or come up with the simplest possible example of it happening, but at a glance, I wonder if these 3 spots in AuditApiGlobalFilter need to discard the audit in addition to returning?

image

My guess is that the mvc audit filter ignores it and returns out, but then later in the request pipeline, the global audit middleware still inserts an audit log, just without mvc context info?

@thepirat000
Copy link
Owner

I've added [AuditIgnore] compatibility for the middleware. It will be automatically handled if you use the mixed approach (middleware + global action filter). But if you only use the middleware, there is an action filter (AuditIgnoreActionFilter) provided that you can add to your pipeline to handle the [AuditIgnore] attribute.

Please check the documentation here.

This will be included starting on version 14.5.0.

@JohnnyFun
Copy link

Sweet, I took a quick look through your commit, and everything looks good to me. This issue can probably be re-closed.

Thanks for knocking that out so quick!

@thepirat000 thepirat000 self-assigned this Feb 19, 2021
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

No branches or pull requests

3 participants