Skip to content

Conversation

@Jusshersmith
Copy link
Contributor

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)

catch on proxy side

use http error string instead of int

a bit more lenient error string check
@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #254 into master will decrease coverage by 0.26%.
The diff coverage is 0%.

@@            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%) ⬇️
internal/auth/providers/okta.go 59.83% <0%> (-2.05%) ⬇️
internal/proxy/oauthproxy.go 50.12% <0%> (-0.37%) ⬇️
internal/auth/authenticator.go 85.81% <0%> (-0.59%) ⬇️

Copy link
Contributor

@jphines jphines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Jusshersmith Jusshersmith merged commit 917f880 into master Oct 7, 2019
@Jusshersmith Jusshersmith deleted the jusshersmith-fix-500-bad-request-bug branch October 7, 2019 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants