fix: interstitial of failed token verification #30
Conversation
JS-204 Direct invocation of DAPI endpoints in the browser causes a 401 redirect loop
Steps to reproduce
The new tab will enter a 401 redirect loop due to the interstitial. Note that Dashboard operates with Authorization Headers by default. We can change this to cookies as DAPI is a Same-origin API. We will also do better dog-fooding. When the API page is visited directly in the browser, there are no Authorization Headers but there is an outdated __session cookie. This issue might reveal a bug in the interstitial logic for single or multi session applications. Issue flow diagram: |
dimkl
changed the title
dimkl
force-pushed
the
js-204-interstitial
branch
from
3329972
to
3593045
Compare
dimkl
force-pushed
the
js-204-interstitial
branch
from
3593045
to
21a3e0e
Compare
lib/clerk/utils.rb
Outdated
| module_function | ||
| def camelize(term) | ||
| string = term.to_s | ||
| string = string.sub(/^[a-z\d]*/) { match.capitalize } | ||
| string.gsub!(/(?:_|(\/))([a-z\d]*)/) { "#{$1}#{$2.capitalize}" } | ||
| string | ||
| end | ||
|
|
||
| # see: https://github.com/clerkinc/javascript/blob/main/packages/backend/src/util/parsePublishableKey.ts |
There was a problem hiding this comment.
🙃 I would remove this link, it's very likely that it will become obsolete in the future.
There was a problem hiding this comment.
What do you mean by becoming obsolete? Do you mean the algorithm change or the src/util/parsePublishableKey.ts being moved?
I think we should keep a reference to something that indicates the logic of the implementation.
There was a problem hiding this comment.
I mean the file being moved. I think links in comments are risky in general, but that's just my opinion.
Of course, feel free to ignore this comment.
At the very least we can use this link instead https://github.com/clerkinc/javascript/blob/c11217cf4432aa640af67ab2fd48f4e5bb33ce6d/packages/backend/src/util/parsePublishableKey.ts
which references a specific commit which will never change.
|
@clerkinc/backend-team could you take a look ? |
dimkl
force-pushed
the
js-204-interstitial
branch
2 times, most recently
from
7e8b988
to
995a16e
Compare
lib/clerk/rack_middleware_v2.rb
Outdated
| @@ -163,11 +163,14 @@ def call(env) | |||
| end | |||
|
|
|||
| token = verify_token(cookie_token) | |||
| return signed_out(env) unless token | |||
There was a problem hiding this comment.
IIRC, verify_token() == false can also happen if the token is expired, for example. So, if I'm signed in and suddenly I close my browser, and come back after 1 hour, then verify_token() will return false here. In that case, don't we want to show the interstitial so that the token is refreshed (the long-lived FAPI cookie will still be there) and I remain signed in?
There was a problem hiding this comment.
In case we jwt is expired we re-raise the ExpiredSignature error instead of returning false from verify_token . see rescue JWT::ExpiredSignature, JWT::InvalidIatError => e below.
dimkl
force-pushed
the
js-204-interstitial
branch
from
995a16e
to
5ac0b07
Compare
dimkl
force-pushed
the
js-204-interstitial
branch
2 times, most recently
from
7038ea9
to
6b06a6e
Compare
Set signed-out as default request state and return interstitial ONLY when the token verification fails with expired or invalid_iat errors.
dimkl
force-pushed
the
js-204-interstitial
branch
from
6b06a6e
to
faa4a58
Compare
Set signed-out as default request state and return interstitial ONLY when the token verification fails with expired or invalid issued_at (iat) errors.
Extra actions:
REVIEW IT PER COMMIT