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.

Sign up for GitHub

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

Jump to bottom

Added new audit event sub-types #2170

Merged
merged 15 commits into from
Aug 24, 2021
Merged

Added new audit event sub-types #2170

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
2423c46
Added new audit event sub-types
apurvabhaleMS Aug 17, 2021
4018dc5
Log RU's to Audit CustomHeaders
apurvabhaleMS Aug 17, 2021
30b2f12
Correct the value for CustomAuditHeaderPrefix
apurvabhaleMS Aug 17, 2021
5372a01
Moved CustomAuditHeaderPrefix to KnownHeaders
apurvabhaleMS Aug 18, 2021
1e80e62
Merge remote-tracking branch 'origin/main' into personal/apurvabhale/…
apurvabhaleMS Aug 18, 2021
02b878b
Updated to unit tests
apurvabhaleMS Aug 18, 2021
b8048ef
Updates to Unit test
apurvabhaleMS Aug 18, 2021
ceb9283
Updates to Unit tests
apurvabhaleMS Aug 18, 2021
0b1bbaa
Updates to AuditHelper
apurvabhaleMS Aug 18, 2021
ecd093e
Updates to AuditHelper
apurvabhaleMS Aug 19, 2021
88c93ce
Fix failing unit tests
apurvabhaleMS Aug 19, 2021
dd200ae
Reverted Audit custom headers related changes for RU's
apurvabhaleMS Aug 20, 2021
25f372a
Reverted test files
apurvabhaleMS Aug 20, 2021
9ee34a0
Merge remote-tracking branch 'origin/main' into personal/apurvabhale/…
apurvabhaleMS Aug 20, 2021
30b2ade
Merge remote-tracking branch 'origin/main' into personal/apurvabhale/…
apurvabhaleMS Aug 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Microsoft.Health.Fhir.Api/Configs/FhirServerConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.Health.Api.Features.Cors;
using Microsoft.Health.Core.Configs;
using Microsoft.Health.Fhir.Core.Configs;
using Microsoft.Health.Fhir.ValueSets;

namespace Microsoft.Health.Fhir.Api.Configs
{
Expand All @@ -22,7 +23,7 @@ public class FhirServerConfiguration : IApiConfiguration

public OperationsConfiguration Operations { get; } = new OperationsConfiguration();

public AuditConfiguration Audit { get; } = new AuditConfiguration("X-MS-AZUREFHIR-AUDIT-");
public AuditConfiguration Audit { get; } = new AuditConfiguration(SpecialValues.CustomAuditHeaderPrefix);
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Aug 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

new AuditConfiguration(SpecialValues.CustomAuditHeaderPrefix)

That's not right.
And it's not your fault, but if you touch this code we have chance to make it right.

It's either should be config or it should be const string, it can't be both.
I would suggest to move to away from config to const, for simpicity reasons. #Closed


public CoreFeatureConfiguration CoreFeatures { get; } = new CoreFeatureConfiguration();

Expand Down
2 changes: 2 additions & 0 deletions src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/CosmosResponseProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using Microsoft.Health.Fhir.Core.Features.Context;
using Microsoft.Health.Fhir.CosmosDb.Features.Metrics;
using Microsoft.Health.Fhir.CosmosDb.Features.Queries;
using Microsoft.Health.Fhir.ValueSets;

namespace Microsoft.Health.Fhir.CosmosDb.Features.Storage
{
Expand Down Expand Up @@ -145,6 +146,7 @@ private async Task AddRequestChargeToFhirRequestContext(double responseRequestCh
}

requestContext.ResponseHeaders[CosmosDbHeaders.RequestCharge] = responseRequestCharge.ToString(CultureInfo.InvariantCulture);
requestContext.RequestHeaders[SpecialValues.CustomAuditHeaderPrefix + CosmosDbHeaders.RequestCharge] = responseRequestCharge.ToString(CultureInfo.InvariantCulture);
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Aug 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

RequestHeaders

That doesn't sound right.

What request header has to do with request charge? #Closed

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, that collection setup on receiving request, and it's ok (actually not) to modify it while you preparing to process request, but after that, we need to work with response collection.

}

var cosmosMetrics = new CosmosStorageRequestMetricsNotification(requestContext.AuditEventType, requestContext.ResourceType)
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.Health.Fhir.Shared.Api/Controllers/FhirController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public async Task<IActionResult> Create([FromBody] Resource resource)
[HttpPost]
[ConditionalConstraint]
[Route(KnownRoutes.ResourceType)]
[AuditEventType(AuditEventSubType.Create)]
[AuditEventType(AuditEventSubType.ConditionalCreate)]
public async Task<IActionResult> ConditionalCreate([FromBody] Resource resource)
{
StringValues conditionalCreateHeader = HttpContext.Request.Headers[KnownHeaders.IfNoneExist];
Expand Down Expand Up @@ -207,7 +207,7 @@ public async Task<IActionResult> Update([FromBody] Resource resource, [ModelBind
/// <param name="resource">The resource.</param>
[HttpPut]
[Route(KnownRoutes.ResourceType)]
[AuditEventType(AuditEventSubType.Update)]
[AuditEventType(AuditEventSubType.ConditionalUpdate)]
public async Task<IActionResult> ConditionalUpdate([FromBody] Resource resource)
{
IReadOnlyList<Tuple<string, string>> conditionalParameters = GetQueriesForSearch();
Expand Down Expand Up @@ -372,7 +372,7 @@ public async Task<IActionResult> Delete(string typeParameter, string idParameter
/// <param name="idParameter">The identifier.</param>
[HttpDelete]
[Route(KnownRoutes.PurgeHistoryResourceTypeById)]
[AuditEventType(AuditEventSubType.Delete)]
[AuditEventType(AuditEventSubType.PurgeHistory)]
public async Task<IActionResult> PurgeHistory(string typeParameter, string idParameter)
{
DeleteResourceResponse response = await _mediator.DeleteResourceAsync(new ResourceKey(typeParameter, idParameter), DeleteOperation.PurgeHistory, HttpContext.RequestAborted);
Expand All @@ -388,7 +388,7 @@ public async Task<IActionResult> PurgeHistory(string typeParameter, string idPar
/// <param name="maxDeleteCount">Specifies the maximum number of resources that can be deleted.</param>
[HttpDelete]
[Route(KnownRoutes.ResourceType)]
[AuditEventType(AuditEventSubType.Delete)]
[AuditEventType(AuditEventSubType.ConditionalDelete)]
public async Task<IActionResult> ConditionalDelete(string typeParameter, [FromQuery] bool hardDelete, [FromQuery(Name = KnownQueryParameterNames.Count)] int? maxDeleteCount)
{
IReadOnlyList<Tuple<string, string>> conditionalParameters = GetQueriesForSearch();
Expand Down
8 changes: 8 additions & 0 deletions src/Microsoft.Health.Fhir.ValueSets/AuditEventSubType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,20 @@ public static class AuditEventSubType

public const string Create = "create";

public const string ConditionalCreate = "conditional-create";

public const string Read = "read";

public const string VRead = "vread";

public const string Update = "update";

public const string ConditionalUpdate = "conditional-update";

public const string Delete = "delete";

public const string ConditionalDelete = "conditional-delete";

public const string History = "history";

public const string HistoryType = "history-type";
Expand Down Expand Up @@ -66,5 +72,7 @@ public static class AuditEventSubType
public const string MemberMatch = "member-match";

public const string Everything = "everything";

public const string PurgeHistory = "purge-history";
}
}
2 changes: 2 additions & 0 deletions src/Microsoft.Health.Fhir.ValueSets/SpecialValues.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,7 @@ namespace Microsoft.Health.Fhir.ValueSets
public static class SpecialValues
{
public const string System = "http://hl7.org/fhir/special-values";

public const string CustomAuditHeaderPrefix = "http://hl7.org/fhir/special-values";
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Aug 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

http://hl7.org/fhir/special-values

Are you sure? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No :) I just updated the correct value

}
}
2 changes: 1 addition & 1 deletion test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Audit/AuditTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace Microsoft.Health.Fhir.Tests.E2E.Rest.Audit
public class AuditTests : IClassFixture<AuditTestFixture>
{
private const string RequestIdHeaderName = "X-Request-Id";
private const string CustomAuditHeaderPrefix = "X-MS-AZUREFHIR-AUDIT-";
private const string CustomAuditHeaderPrefix = SpecialValues.CustomAuditHeaderPrefix;
private const string ExpectedClaimKey = "client_id";

private readonly AuditTestFixture _fixture;
Expand Down