Skip to content

Conversation

stiwarisemanticbits
Copy link
Contributor

@stiwarisemanticbits stiwarisemanticbits commented Sep 15, 2025

JIRA Ticket:
BB2-4142

What Does This PR Do?

This PR updates the authorization views to explicitly allow POST requests on the /authorize endpoint without CSRF protection

What Should Reviewers Watch For?

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

Validation

  1. Validate using attached node app which has end to end flow for making POST request to authorize endpoint and generate token. This also verifies cross domain as server and client running on different ports
    server.js

Steps to start app

  1. Checkout this branch locally
  2. Get aws keys from cloud tammer , source them and start web server
  3. Download node server file
  4. export following in terminal (this is your local app credentials)
export CLIENT_ID='YOUR_CLIENT_ID'
export CLIENT_SECRET='YOUR_CLIENT_SECRET'
export REDIRECT_URI='http://localhost:3000/callback'

4a.

npm init -y
npm i express body-parser
  1. Start with node server.js
    Now you can make Authorize and see if its generating token
  2. In server.js remove this line <input type="hidden" name="state" value="${state}" /> to validate state parameter is required
  3. restart server.js and perform authorize requests, this time you will get error
{"status_code": 401, "message": "State required for POST requests."}

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?

  • 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

@stiwarisemanticbits stiwarisemanticbits marked this pull request as draft September 15, 2025 13:54
@stiwarisemanticbits stiwarisemanticbits force-pushed the st/BB2-4142 branch 3 times, most recently from 4643d55 to 86d42c1 Compare September 15, 2025 14:06
@stiwarisemanticbits stiwarisemanticbits marked this pull request as ready for review September 15, 2025 14:13
@stiwarisemanticbits stiwarisemanticbits force-pushed the st/BB2-4142 branch 3 times, most recently from 2f0b7b3 to 49c6923 Compare September 15, 2025 16:35
@jimmyfagan jimmyfagan marked this pull request as draft September 15, 2025 17:47
@jimmyfagan jimmyfagan self-assigned this Sep 15, 2025
@stiwarisemanticbits stiwarisemanticbits marked this pull request as ready for review September 16, 2025 13:17
Copy link
Contributor

@jimmyfagan jimmyfagan left a comment

Choose a reason for hiding this comment

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

This worked great, thanks for the detailed verification steps. I'm realizing that using curl for POST authorize probably will never really work, just don't see how that would ever bring us to the medicare login, but I think that's okay. We ought to find a place to put this kind of example, maybe let's talk about it (and have you demo the js test app) during sprint demo and see if the product team wants to proceed with something like working this into the test client or sample clients.

Had just one comment to make sure that we aren't dependent on the feature flag to enforce state for these POST commands.

jimmyfagan
jimmyfagan previously approved these changes Sep 19, 2025
Copy link
Contributor

@jimmyfagan jimmyfagan left a comment

Choose a reason for hiding this comment

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

Looks good, as long as the tests pass, we should be good here.

Copy link
Contributor

@jimmyfagan jimmyfagan left a comment

Choose a reason for hiding this comment

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

Reapproving, pending successful tests.

@stiwarisemanticbits stiwarisemanticbits merged commit cc945bc into master Sep 19, 2025
8 checks passed
@stiwarisemanticbits stiwarisemanticbits deleted the st/BB2-4142 branch September 19, 2025 17:59
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