JWT follow-up

[roshii]

• Fix JWT unit tests (async was somehow making tests always successful therefore hiding some issues)
• Fix WWW-Authenticate header construction (Decode JWT token for validation #1480 (comment))
• Encode wallet names with base64 in scopes to allow for space delimited names (Decode JWT token for validation #1480 (comment), feat: use new authentication mechanism (access/refresh token) joinmarket-webui/jam#663 (comment))
• Fix syntax errors in OpenAPI RPC documentation (Errors in the open-api generated docs #1559)

[roshii]

squashed

[AdamISZ]

Thanks.

So about the async part, I'm not sure, but I guess this arose because you are using the async syntax and not the twisted stuff, but were calling a inlineCallbacks decorated twisted-style async function (do_request) which returned a Deferred with yield, right? I mean I guess it doesn't matter that much, if it works it's fine, but at some point we'd need to probably rewrite the whole codebase with async and not twisted (that would be a lot of work, to put it mildly!), but otherwise mixing them probably just makes it harder.

Again, I have no objections, if it works it works. It's only in the test part.

About the base64, I see the point about spaces and how that mismatched with the token code. Can I assume that there is no conflict with using base64 encoding here and anywhere else we refer to the wallet name? I believe from reading that you're only using it for the token-related usage of the wallet name.

(I actually personally think filenames with spaces should not ever be allowed).

[roshii]

So about the async part, I'm not sure, but I guess this arose because you are using the async syntax and not the twisted stuff, but were calling a inlineCallbacks decorated twisted-style async function (do_request) which returned a Deferred with yield, right? I mean I guess it doesn't matter that much, if it works it's fine, but at some point we'd need to probably rewrite the whole codebase with async and not twisted (that would be a lot of work, to put it mildly!), but otherwise mixing them probably just makes it harder.

Same guess. I did assume we could do a smooth transition to async syntax but it looks like it will need to be done in one go. It was then easier to switch back to inlineCallbacks for this fix and now that I know async syntax is not only a personal preference, I may look into syntax migration rather sooner than later.

About the base64, I see the point about spaces and how that mismatched with the token code. Can I assume that there is no conflict with using base64 encoding here and anywhere else we refer to the wallet name? I believe from reading that you're only using it for the token-related usage of the wallet name.

(I actually personally think filenames with spaces should not ever be allowed).

base64 encoded wallet names are only used in token scopes (as a JWTTokenAuthority property and in issued token), its character set matching name-scope-syntax. It complicates debugging but allows to cope with filenames containing spaces (which I personally "disfavor" but understand).

[AdamISZ]

Needs rebase (presumably #1484 ) but after that, is there any blocker here or should we just merge?

[roshii]

rebased. i cannot think of any blockers, ready to merge as far as i can tell.

[kristapsk]

Agree we should just merge this, @theborakompanioni will let us now if there are problems.

[roshii]

addressed review comment