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

Implment configurable auditing #306

Closed
wants to merge 9 commits into from

Conversation

chadbengen
Copy link

@chadbengen chadbengen commented Jul 6, 2019

This PR is a proof of concept and is not complete..

EDIT: I can update tests if this is a chosen route to implement auditing.

This implementation relies on raising events following a store operation. Whereas the other solution from @cerginio #298 relies on implementation in the dbcontexts. I dont know which method is better or preferred. I'd love to get some weigh in. Also, this PR may overlap with the initial logging implementation from @skoruba.

What's missing
I'd like to define what goes into an audit by default. Although this PR allows a developer to write whatever they want to an audit log, I would find it helpful to define a default base. For instance, i think an audit should include the following things:

  • Source. Where did the request originate.
  • Actor. Who or what made the request.
  • Subject. Who or what was changed, if applicable.

Overview
I borrowed this initial implementation from https://github.com/RockSolidKnowledge/RSK.IdentityServer4.AuditEventSink (MIT license). Their solution provided a means for registering multiple sinks for logging using the built in Identity Server EventService. I prefer to stay as close as possible to IdentityServer's implementation of events for consistency purposes. This means that we'd need to inject EventService in our repositories and controllers where auditing is required. (Alternatively we could implement a mediator such as Mediatr)

This PR introduces an Audit.Core project where all of the logic resides. The other project SerilogMsSqlRecorder illustrates how a developer can implement a custom destination for writing audits. The Audit.Core project includes a console recorder which writes events to the debug console.

I added an event for changing password by raising an event in the Manage controller in STS. I also added an event for changing a client by raising an event in the ClientRepository.

To implement this each app needs to add the following code to the startup:

            services.AddSingleton<IdentityServerOptions>(new IdentityServerOptions()
            {
                Events = new EventsOptions()
                {
                    RaiseErrorEvents = true,
                    RaiseInformationEvents = true,
                    RaiseFailureEvents = true,
                    RaiseSuccessEvents = true
                }
            });
            services.AddTransient<IEventService, DefaultEventService>();
            services.AddSingleton<IEventSink>(sink => new EventSinkAggregator(_loggerFactory.CreateLogger("EventSinkAggregator"))
            {
                EventSinks = new List<IEventSink>()
                {
                    new DefaultEventSink(_loggerFactory.CreateLogger<DefaultEventService>()),
                    new AuditSink(new ConsoleRecorder()),
                    new AuditSink(new SerilogMSSqlRecorder(Configuration.GetConnectionString("IdentityDbConnection")))
                }
            });

The IdentityServerOptions are required by the EventService. STS implements the options so is not necessary to be included in startup. (I couldnt figure out how to register the options using the Configure(Action<>) method so i gave up).

The EventService is IdentityServer4's service for raising events. Not required in the STS startup.

The EventSinkAggregator allows us to register multiple sinks.

@skoruba
Copy link
Owner

skoruba commented Jul 7, 2019

Thanks for this PR! I want to use definetely manual auditing like you are sending here, but now I am considering how to implement it - if I will use same IEventService from IdentityServer4 or create some own implementation, but Sink agregator from RSK is cool - I like it. 👍

add events

update some tests

implement injectable sink aggregator & refactoring

fix folder structure

inject httpcontextaccessor; add AuditSinkBase

add changes column; implent compare.net

add services/repository; fix tests
@chadbengen
Copy link
Author

Hi @skoruba. I have updated the PR and squashed the commits, so i hope that doesn't break anything. Take a look and let me know if anything needs to be modified or included. If you are wanting to go a different direction and this doesn't fit your vision, please let me know. Thanks for the consideration.

The updates include:

  • The SinkAggregator and event sinks are injected rather than constructed.
  • The sink projects have builders for easy startup configurations
  • The tests now pass

I implemented a client changed event that uses Compare.Net, but i'm not sure it's the right solution. I think it will need a manual comparison in order to get property values that make sense. But currently the implementation gets the changes as an array and stores them with the audit. Do you have any thoughts on how else to do this? Even using EF tracking would require mapping to get property values that make sense.

There are three projects:

Audit.Sink
Holds all the audit logic, including the console sink.

Audit.Core
Contains dtos, entities, and interfaces for use in the admin and the api.

Audit.EntityFramework
Contains a dbcontext and implementations for data access. There are two sinks in the project, one that uses EntityFramework for writing audit logs, and a Serilog sink for using Serilog to write to mssql. The EntityFramework sink is not fully implemented. I chose to implement the serilog sink and use the serilog AuditTo functionality which writes logs immediately rather than batching them.

Writing custom sinks is pretty easy. The developer just needs to create a Recorder that implements IRecordAuditActions and a sink that derives from AuditSinkBase and then constructs with the custom sink. Add all this to the DI and done. They can then write to any source they want.

TODO

  • Confirm how to store object changes. e.g., should each property change be a separate record or can they be serialized as a list of changes.
  • Identify which objects should be audited
  • Rethink how to get object changes. Possibly need to implement manual comparisons for objects that need to be audited.
  • Inject IEventService in repositories that are auditing.
  • Define additional audit events that should be implemented.
  • Review and confirm the AuditEntity to make sure it is capturing what an audit needs. There may need to be some consideration over capturing IP address as i have read that this may not be a legal thing to capture.

@chadbengen chadbengen closed this Jul 9, 2019
@chadbengen chadbengen reopened this Jul 9, 2019
@skoruba
Copy link
Owner

skoruba commented Oct 20, 2019

@chadbengen - thanks for your PR, but I will use for audit logging stuff my own library - https://github.com/skoruba/AuditLogging - which is based on IdentityServer4 EventService.
I will definitely use some of your ideas into final preparation of audit logging.

@skoruba skoruba closed this Oct 20, 2019
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.

2 participants