Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: new authentication mechanism (access/refresh token) #665

Merged
merged 11 commits into from
Oct 8, 2023

Conversation

theborakompanioni
Copy link
Collaborator

@theborakompanioni theborakompanioni commented Sep 22, 2023

Resolves #663.

This PR adds handling for the new JWT auth mechanism.

From JSON-RPC-API-using-jmwalletd.md#rules-about-making-requests:

On initially creating, unlocking or recovering a wallet, a new access and refresh token will be sent in response, the former is valid for only 30 minutes and must be used for any authenticated call, the former is valid for 4 hours and can be used to request a new access token, ideally before access token expiration to avoid unauthorized response from authenticated endpoint and in any case, before refresh token expiration.

The token is refreshed every 22.5 minutes (30min * 0.75). In dev mode more often (every 60 seconds).

There is currently a problem upstream with handling wallets that contain spaces in their filename, should be fixed with JoinMarket-Org/joinmarket-clientserver#1562. Nonetheless, the code can already be reviewed and should not be affected by the changes.

How to test

Rebuild the dev docker environment npm run regtest:rebuild and verify:

  • Create wallet still works
  • Import wallet still works
  • Lock/Unlock still works
  • Token can be refreshed
  • Requests to RPC api are still successful after token refresh
  • Websocket communication still functioning after token refresh (e.g. start/stop a maker and watch the websocket data)

Misc

@theborakompanioni theborakompanioni added the enhancement New feature or request label Sep 22, 2023
@theborakompanioni theborakompanioni self-assigned this Sep 22, 2023
@theborakompanioni
Copy link
Collaborator Author

Just a heads-up: Websocket auth not addressed yet.

@theborakompanioni
Copy link
Collaborator Author

Just a heads-up: Websocket auth not addressed yet.

Worked out-of-the-box in my tests. 🙌

@theborakompanioni theborakompanioni marked this pull request as ready for review October 4, 2023 10:02
@theborakompanioni
Copy link
Collaborator Author

Ready for review.

@editwentyone
Copy link

worked smooth on my side in dev env

@theborakompanioni theborakompanioni requested a review from a team October 7, 2023 13:23
@theborakompanioni
Copy link
Collaborator Author

We definitely need more reviewers and testers. Someone also needs to audit at the code.

@theborakompanioni theborakompanioni merged commit 4c76d6a into master Oct 8, 2023
3 checks passed
@theborakompanioni theborakompanioni deleted the feat/663-auth branch October 8, 2023 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: use new authentication mechanism (access/refresh token)
2 participants