Before this commit, webaccess.checkAccess saved the authorization in
user.padAuthorizations[padId] with padId being the read-only pad ID,
however later stages, e.g. in PadMessageHandler, use the real pad ID for
access checks. This led to authorization being denied.
This commit fixes it by only storing and comparing the real pad IDs and
not read-only pad IDs.
This fixes test case "authn user readonly pad -> 200, ok" in
src/tests/backend/specs/socketio.js.
readonly paste links should be readable even if authentication is turned
on, as long as the user provides valid login data.
This test currently fails.
Also test that readonly paste IDs can be exported under the same
condition, which currently succeeds.
This has a few benefits:
* It's more readable: It's easier for a user of the function to know
that they should use `await` when calling the function.
* Stack traces are more useful.
* Some code (e.g., the async npm package) uses introspection to
determine if a function is `async` vs. takes a callback.
Benefits of this change:
* It avoids race conditions with tests that clear cookies.
* Any attempt to get or set a value before `init()` is called will
throw an error, ensuring the API is used properly.
* Improved readability: It's easier to understand what the
`pad.noCookie` check is doing.
* Rename "current"/"other" to "user0"/"user1".
* Delete unnecessary `_createTokenFor*` functions.
* Rename helper functions to remove unnecessary leading underscore
and for brevity.
* Use jQuery's `.attr()` to build the second iframe.
* Use js-cookie to manipulate the token cookie.
* Don't attempt to set the token cookie if the pad isn't loaded.
* Use the token generated by the pad.
* Only clear the token cookie at path=/.
Move server message queue processing out of `handleUserChanges()` for
the following reasons:
* Fix a race condition: Before this change the client would stop
processing incoming messages and stop sending changes to the
server if a `NEW_CHANGES` message arrived while the user was
composing a character and waiting for an `ACCEPT_COMMIT` message.
* Improve readability: The `handleUserChanges()` function is for
handling changes from the local user, not for handling changes
from other users.
* Simplify the code.
Reusing the same op object for each iteration can result in very weird
behaviors because previously yielded op objects will get a surprise
mutation.
It is unclear why the code was written to reuse the same object. There
was no comment, nor is there a commit message providing rationale (it
has behaved this way since the very first commit). Perhaps the objects
were reused to improve performance (fewer object allocations that need
to be garbage collected). I do expect this change to reduce
performance somewhat, but not enough to warrant reverting this commit.
Richard Hansen