Skip to content

api: Add support for Private Network Access header preflight requests#6089

Merged
gmalouf merged 10 commits intoalgorand:masterfrom
nullun:feature/pna-headers
Sep 19, 2024
Merged

api: Add support for Private Network Access header preflight requests#6089
gmalouf merged 10 commits intoalgorand:masterfrom
nullun:feature/pna-headers

Conversation

@nullun
Copy link
Copy Markdown
Contributor

@nullun nullun commented Jul 31, 2024

Summary

During development of Algorand smart contracts and platforms users will often run local environments consisting of algod, kmd, and indexer via sandbox or more recently algokit. By default all of these services are running on local/private network addresses (e.g. 127.0.0.1), however popular tools such as DappFlow and Lora are hosted on public network addresses and require the user to specify their local endpoints. Additionally some dapps allow their users to provide their own endpoints for a more decentralised experience.

Schedule for Google Chrome 130 (although many users are already experiencing it), PNA protections will be enabled by default, disallowing public websites from making requests to local/private resources without a specific header response during a preflight request. This PR introduces a new configuration option for both algod and kmd that will add middleware to each of their API handlers to support responding to the Private Network Access request header.

Test Plan

I simply copied the only CORS related test I could find and adjusted it to check for the PNA header. I'd be happy to add something more thorough if a suggestion can be offered.

@nullun nullun changed the title Feature/pna headers api: Add support for Private Network Access header preflight requests Jul 31, 2024
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 31, 2024

Codecov Report

Attention: Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.

Project coverage is 56.26%. Comparing base (8eca278) to head (53d2216).
Report is 33 commits behind head on master.

Files with missing lines Patch % Lines
daemon/algod/api/server/router.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6089      +/-   ##
==========================================
+ Coverage   55.85%   56.26%   +0.41%     
==========================================
  Files         488      488              
  Lines       69610    69621      +11     
==========================================
+ Hits        38879    39172     +293     
+ Misses      28045    27786     -259     
+ Partials     2686     2663      -23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jasonpaulos jasonpaulos marked this pull request as ready for review August 26, 2024 19:15
Comment thread test/e2e-go/cli/goal/expect/goalExpectCommon.exp
Comment thread daemon/kmd/config/config.go Outdated
Copy link
Copy Markdown
Contributor

@algorandskiy algorandskiy left a comment

Choose a reason for hiding this comment

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

Although the code seems correct (except the test) I cannot see Access-Control-Allow-Private-Network header in response in manual test with private net.

Comment thread test/e2e-go/cli/goal/expect/goalExpectCommon.exp
@pbennett
Copy link
Copy Markdown
Contributor

pbennett commented Sep 2, 2024

Although the code seems correct (except the test) I cannot see Access-Control-Allow-Private-Network header in response in manual test with private net.

Are you setting Origin in your request? It's required for CORS.

algorandskiy
algorandskiy previously approved these changes Sep 3, 2024
Copy link
Copy Markdown
Contributor

@algorandskiy algorandskiy left a comment

Choose a reason for hiding this comment

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

Thank you for the changes, the test works as expected now.
In my previous manual testing I used algod-listen.net instead algod.net so I confirm the header appears as expected.

Btw, there is an issue with KMD CORS helper I found while checking this new test - I'll submit a separate PR for this.

Copy link
Copy Markdown
Contributor

@gmalouf gmalouf left a comment

Choose a reason for hiding this comment

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

Couple questions but it overall makes sense.

Comment thread daemon/kmd/api/api.go
Comment thread test/e2e-go/cli/goal/expect/corsTest.exp
Copy link
Copy Markdown
Contributor

@gmalouf gmalouf left a comment

Choose a reason for hiding this comment

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

Thanks for adding the tests Steve.

@gmalouf gmalouf merged commit 2a02530 into algorand:master Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants