Skip to content

BB2-4964/4965: Audit Event Endpoint/Scope#1647

Open
JamesDemeryNava wants to merge 26 commits into
masterfrom
jamesdemery/bb2-4964-audit-event-endpoint
Open

BB2-4964/4965: Audit Event Endpoint/Scope#1647
JamesDemeryNava wants to merge 26 commits into
masterfrom
jamesdemery/bb2-4964-audit-event-endpoint

Conversation

@JamesDemeryNava

@JamesDemeryNava JamesDemeryNava commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

JIRA Tickets:
BB2-4964
BB2-4965

What Does This PR Do?

This PR adds the ability to make BFD AuditEvent calls through the BlueButton API. It also introduces a new capabilities_protectedcapability DB record, added via the create_blue_button_scopes django management command.

For testing, some fixtures were created. We recently began to run unit/integration tests with pytest, so these fixtures extend the usage of pytest. More on those below.

What Should Reviewers Watch For?

  • Is it okay to have the new capabilities_protectedcapability for AuditEvent have default equal to true? Considerations:
    • If we do not make it true, then CAN calls with patient/AuditEvent.rs in the scope will fail, unless we add that scope to the application making the call. Note: If the CAN calls do not include patient/AuditEvent.rs in the scope param, and the capabilities_protectedcapability has default equal to False, the call will succeed, and patient/AuditEvent.rs will be automatically added to the created token's scope.
    • There are mechanisms in place to ensure AuditEvent calls are not made except for specific tokens:
      • Any v3 AuditEvent call will result in a 403 unless patient/AuditEvent.rs is on the token's scope. The only way that scope will make it onto the token is if a successful CAN call is made. We fail v1 or v2 authorize calls that include the patient/AuditEvent.rs scope. For regular v3 authorize calls, as long as the app doesn't have patient/AuditEvent.rs in its scopes, patient/AuditEvent.rs will never be on a token's scope for that app.
  • The fixtures added here are fairly minimal, and should be considered a work in progress. As of now, they replace the need for tests to be in classes to use things like self._create_user and self._create_capability, but they can certainly be improved. I added them as part of this PR, so as to not add more tech debt by using some of our prior unittest library strategies for testing.
  • Added/modified some unit tests around AuditEvent and throwing bad request errors in certain scenarios. One integration test was added as well that does get a 200 response back from BFD. Let me know if there are other things that should be tested that were missed.
  • Is the usage of the waffle_switch for audit events correct? Are there other places we should use it? What it controls currently:
    • Whether the AuditEvent endpoint is available
    • Whether the AuditEvent capability is added via create_blue_button_scopes
    • Whether we check for AuditEvent scope on non v3 regular BB auth flow requests
    • Whether we automatically add the AuditEvent scope on CAN auth flows, if it is not already present

If you're reviewing this PR, please check for these things in particular:

Validation

  • Pull down the branch and bring up the stack. Confirm there is a new capabilities_protectedcapability record, with slug equal to patient/AuditEvent.rs
    • Confirm you can see this scope in the Admin UI when viewing an application, display name is Audit Event FHIR Resource
  • Make a CAN call to get a token, with scope equal to patient/Patient.rs. Copy the "token" value in the response.
    • In the database, confirm that the newly created access token record has patient/Patient.rs patient/AuditEvent.rs for the scope value
    • In Postman, create a new call, named AuditEvent (will add this to Postman collection soon). It should be a GET call, with {{baseUrl}}/v3/fhir/AuditEvent/ as the URL
    • Under Authorization for this new call, select Bearer Token, and paste the token value copied in a prior step
    • Execute the call, confirm you get a 200 response with resourceType equal to Bundle. You will notice that an entity parameter was added to the call, that comes from the bluebutton_crosswalk.fhir_id_v3 value that is associated with the token
    • Grab one of the id values from one of the responses to use in the next validation step
  • Create another new call in Postman, this one will be Audit Event Read. It should be a GET call, with {{baseUrl}}/v3/fhir/AuditEvent/:resource_id as the URL
    • Paste the id you grabbed above as the resource_id and run the call. Confirm you only get an AuditEvent resource type and a 200 response
  • Go through the same process as above, but this time for the CAN call, have patient/Patient.rs patient/AuditEvent.rs be the scope, confirm you get a token as expected and can make the AuditEvent call again. You should see the "total" attribute of the AuditEvent response increment after making another CAN call.
  • Also in Postman, go to the Authorization tab of your local setup:
    • Add patient/AuditEvent.rs to the scope of the authorize request
    • Functionality is the same for v1, 2, and 3. Please validate v1 or 2, as well as v3
    • Click Get new access token at the bottom of the page
    • Confirm that a 400 bad request, with error equal to Invalid scopes. is immediately returned upon clicking that button
    • Do the same steps as above, but use patient/AuditEvent.s and patient/AuditEvent.r and confirm you get 400s for those as well (this happens on v1, 2 and 3)
  • Validation around the enable_auditevents switch:
    • Note: I occasionally have caching issues with waffle_switches locally. If you see unexpected behavior, try bringing the stack down and back up.
    • With enable_auditevents equal to false:
      • Delete the patient/AuditEvent.rs capabilities_protectedcapability. Run the app and ensure that the patient/AuditEvent.rs capabilities_protectedcapability was not created. For the three bullet points below, you will need the patient/AuditEvent.rs scope, so be sure to add that before doing those tests.
      • Run a CAN call without including patient/AuditEvent.rs in the scope parameter. Confirm that the scope was not added to the token
      • Run a CAN call and include patient/AuditEvent.r or patient/AuditEvent.s in the scope. Confirm that the read or search scope is removed, and patient/AuditEvent.rs is included
      • Run a CAN call with patient/AuditEvent.rs in the scope parameter. Then, try to make an AuditEvent call, it will be a 404
      • Do a regular v2 auth flow with patient/AuditEvent.rs in the scope parameter. It will fail AFTER entering credentials, not before.
    • With enable_auditevents equal to true:
      • Delete the patient/AuditEvent.rs capabilities_protectedcapability. Run the app and ensure that the patient/AuditEvent.rs capabilities_protectedcapability was created.
      • Run a CAN call without including patient/AuditEvent.rs in the scope parameter. Confirm the scope was added to the token
      • Run an AuditEvent call, confirm you get a 200 not a 404
      • Do a regular v2 auth flow with patient/AuditEvent.rs in the scope parameter. It will fail BEFORE entering credentials, not after.

@jimmyfagan Some additional validation steps with the change to default = False:

  • Run a CAN token call with only an AuditEvent scope on there. Can be any of them, ensure it still returns successfully, and that the returned scope is patient/AuditEvent.rs
  • Ensure that the 400 bad request error with the custom error message is thrown if a regular auth request is made that includes AuditEvent scopes, without the enable_auditevents waffle switch being active.
  • Do a regular auth flow for v2:
    • Don't request any scopes. Ensure no AuditEvent scopes are on the token

What Security Implications Does This PR Have?

Please indicate if this PR does any of the following:

  • Adds any new software dependencies
  • Modifies any security controls
  • Adds new transmission or storage of data
  • Any other changes that could possibly affect security?
  • Yes, one or more of the above security implications apply. This PR must not be merged without the ISSO or team
    security engineer's approval.

Any Migrations?

To add this capabilities_protectedcapability record, we will manually create it via the Django Admin UI. My thought is that should be done as part of the deployment this PR will go in, and the capabilities_protectedcapability record should be created in an environment before the code is deployed there.

To test if creating the capabilities_protectedcapability record pre-deploy would work, I ensured that the new capabilities_protectedcapability record was present, and built the app off of the master branch. I then did the following:

  • CAN flow without patient/AuditEvent.rs requested (successful)
  • CAN flow with patient/AuditEvent.rs requested (not successful - error was 400 invalid_scope, as expected)
  • Make an AuditEvent call with a valid token (404 error as expected)
  • v3 regular auth flow without patient/AuditEvent.rs requested, token refresh as well (successful)
  • v2 regular auth flow without patient/AuditEvent.rs requested, token refresh as well (successful)
  • v3 regular auth flow with patient/AuditEvent.rs requested (failure, Invalid scopes after credentials were entered)
  • v2 regular auth flow with patient/AuditEvent.rs requested (failure, Invalid scopes after credentials were entered)

To me, this indicates we will be fine in the window that the new capabilities_protectedcapability record has been created, but before the new code is live. There will be failures around patient/AuditEvent.rs in that meantime, but those are expected, and we have not yet publicized this new endpoint.

I also tested what would happen if we deployed the code, and then ran the command:

  • CAN flow without patient/AuditEvent.rs requested (successful)
  • CAN flow with patient/AuditEvent.rs requested (not successful - error was 400 invalid_scope, as expected)
  • Make an AuditEvent call with a valid token (403 forbidden - you do not have permission to perform this action)
  • v3 regular auth flow without patient/AuditEvent.rs requested, token refresh as well (successful)
  • v2 regular auth flow without patient/AuditEvent.rs requested, token refresh as well (successful)
  • v3 regular auth flow with patient/AuditEvent.rs requested (failure, Invalid scopes after credentials were entered)
  • v2 regular auth flow with patient/AuditEvent.rs requested (failure, Invalid scopes before credentials were entered)

Given the above findings, I don't think it really matters when we add the capabilities_protectedcapability record, but I would recommend running it before the code is deployed.

  • [] Yes, there are migrations
    • [] The migrations should be run PRIOR to the code being deployed
    • The migrations should be run AFTER the code is deployed
    • There is a more complicated migration plan (downtime,
      etc)
  • No migrations

)


def test_failure_on_authorize_non_v3_with_audit_event_scope(create_application):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note this uses a new fixture and is not part of the TestCase based classes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

NFC

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds support for retrieving BFD AuditEvent data via the BlueButton API (v3) and introduces a new protected capability/scope (patient/AuditEvent.rs) that is created via create_blue_button_scopes, along with new pytest fixtures and tests.

Changes:

  • Adds a new v3 FHIR endpoint/view for AuditEvent and allows AuditEvent as a supported resource type.
  • Introduces patient/AuditEvent.rs scope/capability creation and updates auth/token behavior to control/enable AuditEvent access.
  • Adds pytest fixtures and unit/integration tests (plus sample response fixture) for the new endpoint and scope rules.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
requirements/requirements.txt Bumps fhir-core dependency version.
requirements/requirements.dev.txt Bumps fhir-core and greenlet dev dependency versions.
conftest.py Adds pytest factory fixtures for users, tokens, capabilities, and applications.
apps/fhir/constants.py Adds AuditEvent to allowed FHIR resource types.
apps/fhir/bluebutton/views/audit_event.py New AuditEventView for v3 AuditEvent requests to BFD.
apps/fhir/bluebutton/v3/urls.py Routes /v3/fhir/AuditEvent to the new view behind v3_endpoints.
apps/fhir/bluebutton/tests/test_audit_event.py Adds tests for AuditEvent permissions/behavior (mock + integration).
apps/fhir/bluebutton/tests/sample_responses/audit_event_response.json Adds a sample BFD AuditEvent Bundle response fixture.
apps/fhir/bluebutton/exceptions.py Import reordering/typing import placement.
apps/dot_ext/views/authorization.py Blocks AuditEvent scope on non-v3 authorize; auto-adds AuditEvent scope to client_credentials token scopes.
apps/dot_ext/tests/test_views.py Extends registration test to account for the new AuditEvent scope.
apps/dot_ext/tests/test_authorization.py Adds a test asserting non-v3 authorize fails when AuditEvent scope is requested.
apps/dot_ext/tests/test_authorization_token.py Asserts AuditEvent scope is auto-added to client_credentials tokens.
apps/constants.py Adds AUDIT_EVENT_SCOPE constant.
apps/capabilities/management/commands/create_blue_button_scopes.py Adds creation of AuditEvent protected capability/scope.
apps/authorization/permissions.py Adds AuditEvent patient-matching logic in is_resource_for_patient.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/fhir/bluebutton/views/audit_event.py Outdated
Comment thread apps/fhir/bluebutton/tests/test_audit_event.py
Comment thread apps/fhir/bluebutton/tests/test_audit_event.py
Comment thread apps/fhir/bluebutton/tests/test_audit_event.py
Comment thread apps/fhir/bluebutton/tests/test_audit_event.py Outdated
Comment thread apps/dot_ext/tests/test_views.py Outdated
Comment on lines +62 to +68
elif obj['resourceType'] == 'AuditEvent':
entity = obj.get('entity', [{}])
patient_info = entity[0].get('what', {})
reference = patient_info.get('reference', '')
reference_id = reference.split('/')[1]
if reference_id != patient_id:
raise exceptions.NotFound()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Leaning towards leaving this, as it will retrieve the patient_id from the BFD response if it is in the correct path. If it is not, and IndexError may be thrown, which would be caught by the except Exception block, and False would be returned (ultimately resulting in a 403). Otherwise, we can make this change, and it would be a 404 returned instead of a 403 if there was a problem parsing the AuditEvent.

Comment thread conftest.py
Comment thread conftest.py
Comment thread apps/capabilities/management/commands/create_blue_button_scopes.py Outdated
@JamesDemeryNava JamesDemeryNava marked this pull request as ready for review July 7, 2026 18:33

@jimmyfagan jimmyfagan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, I have a handful of comments and questions. On the testing front, mostly looks good, but I haven't been able to get a read request to work, I keep getting "detail": "You do not have permission to perform this action.". I might need to just update the protected capability, maybe I have a stale one. I'll reach out if I have trouble. Also, I want to test a bit more with the feature flag disabled to make sure that case is tested pretty thoroughly. But we should be on track to wrap before EOD today on this probably.


def create_audit_event_read_search_capability(group):
c = None
description = 'Allow CAN patients and 3rd party apps to retrieve audit event data that shows what apps have had a successful patient match for network calls via the Blue Button API (CAN flow).'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(optional) Might be nice to differentiate in these descriptions too, though the titles are the main thing that matters, and I like the concise differentiation there.

Suggested change
description = 'Allow CAN patients and 3rd party apps to retrieve audit event data that shows what apps have had a successful patient match for network calls via the Blue Button API (CAN flow).'
description = 'Allow CAN patients and 3rd party apps to read and search audit event data that shows what apps have had a successful patient match for network calls via the Blue Button API (CAN flow).'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Love the coverage here with very little actual test code, nice job bringing in these pytest features!

Comment thread apps/dot_ext/utils.py Outdated
Comment on lines +432 to +433
if AUDIT_EVENT_SCOPE in scope:
scope = scope.replace(AUDIT_EVENT_SCOPE, '')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My understanding is that this is removing patient/AuditEvent.rs so the replacement of patient/AuditEvent.r doesn't leave behind random s chars? I think this warrants a comment, because without thinking it through, it's a bit confusing that we're removing AUDIT_EVENT_SCOPE here. But also, you might be able to use re.sub(pattern, replacement, string) instead of replace here to make sure you only replace the regex matches. Your unit tests should verify that things continue to work correctly if you go down that path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good call, will try re.sub(pattern, replacement, string) instead!

Comment thread apps/fhir/bluebutton/v3/urls.py Outdated
re_path(
r'AuditEvent[/]?',
waffle_switch('enable_auditevents')(waffle_switch('v3_endpoints')(AuditEventView.as_view(version=3))),
name='bb_oauth_fhir_audit_event',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
name='bb_oauth_fhir_audit_event',
name='bb_oauth_fhir_audit_event_search',

Comment on lines +47 to +52
if self.version == Versions.V3 and getattr(fhir_settings, 'fhir_url_v3', None):
fhir_url = fhir_settings.fhir_url_v3
else:
fhir_url = fhir_settings.fhir_url

return f'{fhir_url}/v{self.version}/fhir/AuditEvent'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be simplified given that it's only for v3 at the moment? Or is this still necessary to properly fail on v2?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think so. In my testing, I still get a 404 on a v2 when this is hardcoded to v3.

Comment thread conftest.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is awesome, probably warrants an eng sync topic to share with the group.

@JamesDemeryNava JamesDemeryNava changed the title BB2-4964/4695: Audit Event Endpoint/Scope BB2-4964/4965: Audit Event Endpoint/Scope Jul 13, 2026

@jimmyfagan jimmyfagan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just saw one quirk in testing, but otherwise looks good!

Comment thread apps/dot_ext/views/authorization.py Outdated
Comment on lines +319 to +331
if switch_is_active('enable_auditevents') and (
AUDIT_EVENT_SCOPE in request.GET.get('scope', '')
or AUDIT_EVENT_SCOPE in request.POST.get('scope', '')
or AUDIT_EVENT_READ_SCOPE in request.GET.get('scope', '')
or AUDIT_EVENT_READ_SCOPE in request.POST.get('scope', '')
or AUDIT_EVENT_SEARCH_SCOPE in request.GET.get('scope', '')
or AUDIT_EVENT_SEARCH_SCOPE in request.POST.get('scope', '')
):
return JsonResponse(
{'status_code': HTTPStatus.BAD_REQUEST, 'message': AUDIT_EVENT_SCOPE_ERROR_MESSAGE},
status=HTTPStatus.BAD_REQUEST,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When the flag is disabled, token requests look like they are getting AuditEvent scopes back when they're requested. We might just want to always catch this and then conditionally return the custom message when the flag is enabled and otherwise return the general "invalid scopes" message.

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.

3 participants