Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

BB2-4250: Make v3_endpoints waffle switch app specific#1429

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
JamesDemeryNava wants to merge8 commits intomaster
base:master
Choose a base branch
Loading
fromjamesdemery/BB2-4250-v3_endpoint_app_specific

Conversation

@JamesDemeryNava
Copy link
Contributor

JIRA Ticket:
BB2-4250

What Does This PR Do?

This PR ensures that once we enable the v3_endpoints waffle switch, we are able to control access to v3 endpoints on an app by app basis.

What Should Reviewers Watch For?

  • Do the new unit/integration tests cover the needed cases?
  • Is the deployment plan (linkhere) sufficiently detailed to roll this out?
  • Note: You will make it all the way to the page where a user clicks 'Connect' during v3 auth flow, when an app is not in the flag. When clicking Connect, that is when the 403 will be thrown. This is to reduce the number of calls/queries made to check if an app is within the flag.

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

Validation

  • Do unit and integration tests pass?
  • Below is a way to manually validate this work that I have done as I moved through this. It's a bit to validate, so if reviewers want to split it up, feel free:
    • Pull down the branch, log into the admin UI
    • Go to Application Core -> Flags
    • Add a flag with name, 'v3_early_adopter'
    • Leave the Everyone value as Unknown
    • Add a specific user_id from your local bluebutton_crosswalk table to the Users section below (should be an integer value, should be a user you can log in as like BBUser00575)
    • To test the flow of an app being in the flag and able to access v3 endpoints:
      • Run the following statement:UPDATE dot_ext_application SET user_id = {{user_id added in django admin to flag}} WHERE id = 1;
      • That will make it so the TestApp is in the flag, and able to use v3 endpoints
      • Go through the v3 test client flow, ensure all of the v3 endpoints work once you are through to the test client
      • Validate through Postman using v3 authorize. Make sure you are able to refresh your token successfully and get a 200 back when running /v3/connect/userinfo. Note, you will need to run the same update statement as above, but swap out id = 1, with id = {{id of your local postman app}}
      • Go through v2 auth flow and confirm no issues there
    • To test the flow of an app NOT being in the flag, and not being able to access v3 endpoints:
      • Run the following statement:UPDATE dot_ext_application SET user_id = {{any user_id besides the one added to the flag in django admin}} WHERE id = 1;
      • You could also go into the Admin UI and just remove the user_id that you added previously
      • Go through the v3 testclient flow. After you have entered login info, you will see the confirmation page with the Connect button. Once you click Connect, a 403 error will be thrown with a message explaining that the app does not have access to v3 endpoints yet
      • In your local Postman, try to execute v3 read/search calls for patient/coverage/EOB. You will also see a 403, with the same message as the auth flow
      • In your local Postman, try to refresh your token, you will see a 403 in the console
      • In your local Postman, try to execute a /v3/connect/userinfo call. You will see a 403 response with the same error message as read/search calls on patient/coverage/EOB.

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?

We are not adding data to the database, but we will have to create a waffle flag as we roll this out. We will then add specific user_ids to that flag to enable v3 endpoint access.

  • 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

…er waffle_flag. Working for read/search v3 calls, v3 auth/token flows (still need to add to some other auth views)
…s in the flag. Add 403 handling for userinfo v3
returnjson_response_from_oauth2_error(error)
exceptPermissionDeniedase:
returnJsonResponse(
{'status_code':403,'message':str(e)},

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
Loading
flows to this location and may be exposed to an external user.

Copilot Autofix

AI 4 days ago

To mitigate information exposure through exception messages, the best approach is to return a generic error message to the user, rather than the actual exception string. For logging and debugging, you can log the detailed exception message and stack trace on the server side. Specifically, inTokenView.post, the block handlingPermissionDenied exceptions (lines 484-488) should be modified: instead of returning the string representation ofe to the client (line 486), return a generic message such as "You do not have permission to perform this action." Meanwhile, log the exception (with stack trace) to the server log via e.g.log.exception. All changes should be made in theTokenView.post method inapps/dot_ext/views/authorization.py. No external libraries are necessary beyond those already imported.


Suggested changeset1
apps/dot_ext/views/authorization.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git applydiff --git a/apps/dot_ext/views/authorization.py b/apps/dot_ext/views/authorization.py--- a/apps/dot_ext/views/authorization.py+++ b/apps/dot_ext/views/authorization.py@@ -482,8 +482,9 @@         except (InvalidClientError, InvalidGrantError, InvalidRequestError) as error:             return json_response_from_oauth2_error(error)         except PermissionDenied as e:+            log.exception("Permission denied during token endpoint processing.")             return JsonResponse(-                {'status_code': 403, 'message': str(e)},+                {'status_code': 403, 'message': 'You do not have permission to perform this action.'},                 status=403,             ) EOF
@@ -482,8 +482,9 @@
except (InvalidClientError,InvalidGrantError,InvalidRequestError)aserror:
returnjson_response_from_oauth2_error(error)
exceptPermissionDeniedase:
log.exception("Permission denied during token endpoint processing.")
returnJsonResponse(
{'status_code':403,'message':str(e)},
{'status_code':403,'message':'You do not have permission to perform this action.'},
status=403,
)

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JamesDemeryNava

[8]ページ先頭

©2009-2025 Movatter.jp