Cross-Origin Bearer API Authentication#1133
Cross-Origin Bearer API Authentication#1133korbin wants to merge 10 commits intobitaxeorg:masterfrom
Conversation
|
This sounds amazing. I'll check it out as soon as I can. Is authentication on needed for API / Swarm use? ie grandma doesn't have to set it up just to run a Bitaxe? |
No authentication is needed for any part of this (maintaining this theme for better or worse) except for cross-origin requests, which now all require an API token to be set. Most users will not be making cross-origin requests unless they are using Swarm. If a user is using Swarm, the automatic scan-for-devices feature (visibly) works as it does currently, manually-added devices will require copying/pasting or typing whatever API secret the user chooses (or generates) into the Swarm UI. |
|
I added the mDNS service discovery and ripped out the iterating scan loop (can overwhelm the ESP32, is slow, etc.), it's super slick: https://github.com/korbin/ESP-Miner/tree/mdns-discovery
You can fake a bitaxe in Linux with: avahi-publish -s "fake-bitaxe" _bitaxe._tcp 80 "apiSecret=secretpass123"You can scan for fake or real bitaxes using: avahi-browse -rt _bitaxe._tcpI added a new service: |
7ece378 to
e8244da
Compare
|
Update: I merged the |
|
This needs the conflicts resolved. |
…functionality to frontend/nvs, move swarm scan functionality to backend, remove local network checks
e8244da to
b781882
Compare
mutatrum
left a comment
There was a problem hiding this comment.
Code level review with some smallish remarks. Would be really nice to get this added, IMO.
| target: esp32s3 | ||
| command: GITHUB_ACTIONS="true" idf.py build | ||
| command: | | ||
| GITHUB_ACTIONS="true" idf.py add-dependency "espressif/mdns^1.8.2" |
There was a problem hiding this comment.
Is this needed if it's already added in the root CMakeLists.txt?
|
|
||
| jobs: | ||
| build-and-test: | ||
| build: |
There was a problem hiding this comment.
Should these changes on the tests be in a separate PR?
| pTooltip="Generate random API secret" | ||
| tooltipPosition="left"></p-button> | ||
| </div> | ||
| <small class="block mt-1">Optional 12-32 character string for API authentication</small> |
| }); | ||
| } | ||
|
|
||
| private generateSuggestedHostname(macAddr: string): string { |
There was a problem hiding this comment.
This looks similar to the Wi-Fi AP name generation, is that the same?
| case 'Supra': return 'blue'; | ||
| case 'UltraHex': return 'orange'; | ||
| case 'Gamma': return 'green'; | ||
| case 'GammaHex': return 'lime'; // New color? |
There was a problem hiding this comment.
Not needed, devices give their own color, this is only used for adding old firmware versions to swarm.
|
Can you rebase this PR and remove all unnecessary changes e.g. unrelated CI changes or formatting changes. |
|
@korbin peek please again |
|
There's also #1240. To limit the scope, would it make sense to merge that in first, and when mDNS is in place, do the auth as a smaller PR? |
This PR:
Authorization: Bearerheader as well as theX-Requested-Withheader (see OWASP)This has a number of advantages: