Skip to content

Commit 917f880

Browse files
authored
Merge pull request #254 from buzzfeed/jusshersmith-fix-500-bad-request-bug
sso_*: fix 500 error caused by expired Okta refresh token
2 parents 1b44055 + 746524b commit 917f880

File tree

4 files changed

+11
-2
lines changed

4 files changed

+11
-2
lines changed

internal/auth/authenticator.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,9 @@ func (p *Authenticator) SignIn(rw http.ResponseWriter, req *http.Request) {
259259
p.ProxyOAuthRedirect(rw, req, session, tags)
260260
case http.ErrNoCookie:
261261
p.SignInPage(rw, req, http.StatusOK)
262+
case providers.ErrTokenRevoked:
263+
p.sessionStore.ClearSession(rw, req)
264+
p.SignInPage(rw, req, http.StatusOK)
262265
case sessions.ErrLifetimeExpired, sessions.ErrInvalidSession:
263266
p.sessionStore.ClearSession(rw, req)
264267
p.SignInPage(rw, req, http.StatusOK)

internal/auth/providers/okta.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"io/ioutil"
1010
"net/http"
1111
"net/url"
12+
"strings"
1213
"time"
1314

1415
"github.com/buzzfeed/sso/internal/auth/circuit"
@@ -206,7 +207,7 @@ func (p *OktaProvider) oktaRequest(method, endpoint string, params url.Values, t
206207
ErrorDescription string `json:"error_description"`
207208
}
208209
e := json.Unmarshal(respBody, &response)
209-
if e == nil && response.ErrorDescription == "Token expired or revoked" {
210+
if e == nil && strings.Contains(strings.ToLower(response.ErrorDescription), "token is invalid or expired") {
210211
p.StatsdClient.Incr("provider.token_revoked", tags, 1.0)
211212
return ErrTokenRevoked
212213
}
@@ -373,7 +374,6 @@ func (p *OktaProvider) RefreshSessionIfNeeded(s *sessions.SessionState) (bool, e
373374
if s == nil || !s.RefreshPeriodExpired() || s.RefreshToken == "" {
374375
return false, nil
375376
}
376-
377377
newToken, duration, err := p.RefreshAccessToken(s.RefreshToken)
378378
if err != nil {
379379
return false, err

internal/proxy/oauthproxy.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,9 @@ func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) {
679679
// We know the user is not authorized for the request, we show them a forbidden page
680680
p.ErrorPage(rw, req, http.StatusForbidden, "Forbidden", "You're not authorized to view this page")
681681
return
682+
case providers.ErrTokenRevoked:
683+
p.ErrorPage(rw, req, http.StatusUnauthorized, "Unauthorized", "Token Expired or Revoked")
684+
return
682685
default:
683686
logger.Error(err, "unknown error authenticating user")
684687
tags = append(tags, "error:internal_error")

internal/proxy/providers/sso.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ var (
3131
var (
3232
ErrMissingRefreshToken = errors.New("missing refresh token")
3333
ErrAuthProviderUnavailable = errors.New("auth provider unavailable")
34+
ErrTokenRevoked = errors.New("token revoked or expired")
3435
)
3536

3637
var userAgentString string
@@ -321,6 +322,8 @@ func (p *SSOProvider) redeemRefreshToken(refreshToken string) (token string, exp
321322
if resp.StatusCode != http.StatusCreated {
322323
if isProviderUnavailable(resp.StatusCode) {
323324
err = ErrAuthProviderUnavailable
325+
} else if resp.StatusCode == http.StatusUnauthorized {
326+
err = ErrTokenRevoked
324327
} else {
325328
err = fmt.Errorf("got %d from %q %s", resp.StatusCode, p.RefreshURL.String(), body)
326329
}

0 commit comments

Comments
 (0)