Closed Jusshersmith closed 4 years ago
Merging #254 into master will decrease coverage by
0.26%
. The diff coverage is0%
.
@@ Coverage Diff @@
## master #254 +/- ##
=========================================
- Coverage 62.37% 62.1% -0.27%
=========================================
Files 50 50
Lines 4093 4101 +8
=========================================
- Hits 2553 2547 -6
- Misses 1355 1366 +11
- Partials 185 188 +3
Impacted Files | Coverage Δ | |
---|---|---|
internal/proxy/providers/sso.go | 68.84% <0%> (-0.93%) |
:arrow_down: |
internal/auth/providers/okta.go | 59.83% <0%> (-2.05%) |
:arrow_down: |
internal/proxy/oauthproxy.go | 50.12% <0%> (-0.37%) |
:arrow_down: |
internal/auth/authenticator.go | 85.81% <0%> (-0.59%) |
:arrow_down: |
Problem
We're not catching the right error within the
oktaRequest
function, causing expired refresh tokens to be categorised under an 'unknown' error type. This results in an unclear and awkward UX in cases where the refresh token has expired.Solution
Fix the error string we're searching for, and improve the general UX around this error case a little bit to provide users with a sensible error and an option to sign back in.
Notes
The TTL of a refresh token in Okta is easily and commonly customised, whereas the TTL of a refresh token within Google is set at 6 months by default. This has meant we've ran into this error a little bit more frequently with the Okta provider which is why we're fleshing out the UX a bit. (This error is generally unseen when the Google provider is being used)