oauth2-proxy sends expired access tokens to backend

[cazpr]

We've configured the oauth2-proxy (v7.3.0) to integrate with Keycloak via the oidc provider and to forward access tokens to our backend. Most of the time everything works as expected, but from time to time we notice that the access tokens (JWTs) that are forwarded by the proxy to our backend are rejected by the backend because they are expired.

The access tokens issued by Keycloak are valid for 5 minutes and we've configured to proxy to refresh its session after 4m30s. Keycloak has the concept of a "max session lifetime" which causes access tokens issues at the end of the Keycloak session to have a lifetime of less than 5 minutes. After obtaining an access token, the proxy seems to assume that its SessionState cookie is at least valid for the duration of cookie_refresh (4m30s in this case) but this isn't necessarily correct near the end of a Keycloak session.

Let me illustrate this with an example.

Keycloak max session lifetime is 1h. A user logs in via the proxy at 00:00:00. The access token in SessionState expires at 00:05:00. The users keeps sending request to the proxy.

00:04:00 - SessionState isn't refreshed. Access token expires at 00:05:00.
00:04:30 - SessionState is refreshed. The new access token expires at 00:09:30.
00:55:00 - SessionState is refreshed. The new access token expires at 01:00:00.
00:59:30 - SessionState is refreshed. The new access token also expires at 01:00:00 as the Keycloak 'max session lifetime' is 1h.
01:01:00 - SessionState isn't refreshed. Access token expired at 01:00:00 but is forwarded to the backend nonetheless.
01:04:00 - SessionState can't be refreshed as refresh token is no longer valid. Access token is also expired so the user is redirected to the login page.

All requests sent to the backend between 01:00:00 and 01:04:00 failed with a 401 response as the backend rejects the expired access token, but the proxy didn't redirect the user to the login page.

The following config is representative for what we're doing.

oauth2-proxy.cfg

reverse_proxy = true

redirect_url = "http://localhost:4180/oauth2/callback"
whitelist_domains = ["localhost:*"]
email_domains = ["*"]

cookie_refresh = "4m30s"
cookie_secret = "HNsbEdlRG9i5JGoRb6TJlS5YQgrLMDNR"

alpha-config.yml

server:
  BindAddress: 127.0.0.1:4180
providers:
  - id: oidc
    clientID: example
    clientSecret: 123456789
    provider: oidc
    oidcConfig:
      emailClaim: email
      userIDClaim: sub
      audienceClaims: [aud]
      issuerURL: http://localhost:8091/auth/realms/test
    scope: openid profile
    code_challenge_method: S256
upstreamConfig:
  upstreams:
    - id: backend
      path: /
      uri: http://localhost:8080
      timeout: 30s
injectRequestHeaders:
  - name: Authorization
    values:
      - claim: access_token
        prefix: 'Bearer '

Expected Behavior

The oauth2-proxy should validate the SessionState regardless of whether or not the session needs to be refreshed.

Current Behavior

The oauth2-proxy seems to skip session validation when the session doesn't need to be refreshed.

//pkg/middleware/stored_session.go:109
func (s *storedSessionLoader) getValidatedSession(rw http.ResponseWriter, req *http.Request) (*sessionsapi.SessionState, error) {
    session, err := s.store.Load(req)
    if err != nil || session == nil {
        // No session was found in the storage or error occurred, nothing more to do
        return nil, err
    }

    err = s.refreshSessionIfNeeded(rw, req, session)
    if err != nil {
        return nil, fmt.Errorf("error refreshing access token for session (%s): %v", session, err)
    }

    return session, nil
}

//pkg/middleware/stored_session.go:127
func (s *storedSessionLoader) refreshSessionIfNeeded(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) error {
    if !needsRefresh(s.refreshPeriod, session) {
        // Refresh is disabled or the session is not old enough, do nothing
        return nil
    }

       ...

    return s.validateSession(req.Context(), session)
}

Possible Solution

Check session.IsExpired() regardless of whether the session was refreshed or not.

Your Environment

• Version used: 7.3.0 and master branch at a6c8f6f04a762c7abe51c946d446621669af4d6f

[wvdhaute]

We are having the same troubles with our oauth2-proxy / Keycloak integration This could indeed make the oauth2-proxy better

[JoelSpeed]

This has come up many times, and different people want different behaviour here. Realistically, we need to break the logic out so that there can be configuration of how someone would like to configure this, PRs are welcome for that if folks have ideas

[cazpr]

Is it possible to describe different behaviours wanted by other people or link related tickets?

[miguelborges99]

Instead of adding more options that need to be maintained, improving the needsRefresh method could be the way to go. If we check the expire date of IDToken when provider is oidc, then this might solve the problem. At the moment, this method relies only in the session age.

https://github.com/oauth2-proxy/oauth2-proxy/blob/f753ec1ca5c42cd4c4e400afa1f76817638f3010/pkg/middleware/stored_session.go#L202

[miguelborges99]

Check pull request. Are you able to verify this? It is just a proposal.

[cazpr]

@miguelborges99 I briefly tested your changes but they seem to trigger a token refresh on every request so I guess something isn't working as expected.

[cazpr]

Simply tweaking the needsRefresh method in stored_session.go as follows seems to do the trick for how we are using the proxy, but it might break stuff for other customers ....

// needsRefresh determines whether we should attempt to refresh a session or not.
func needsRefresh(refreshPeriod time.Duration, session *sessionsapi.SessionState) bool {
    return refreshPeriod > time.Duration(0) && (session.Age() > refreshPeriod || session.IsExpired())
}

[miguelborges99]

Simply tweaking the needsRefresh method in stored_session.go as follows seems to do the trick for how we are using the proxy, but it might break stuff for other customers ....

// needsRefresh determines whether we should attempt to refresh a session or not.
func needsRefresh(refreshPeriod time.Duration, session *sessionsapi.SessionState) bool {
  return refreshPeriod > time.Duration(0) && (session.Age() > refreshPeriod || session.IsExpired())
}

Yes,you are right. It is also necessary to check if cookie has expired, I've incorporated that change. I'm also checking if ID token has expired, but I had the logic inverted. It was corrected.

[cazpr]

re-tested it and seems to work.

[miguelborges99]

@cazpr Thanks for your support.

[guilhermedalmarco]

Hi guys, is there any expectation to merge this https://github.com/oauth2-proxy/oauth2-proxy/pull/1955 ?

[miguelborges99]

Only the mantainers can answer that.

[JoelSpeed]

I'm struggling to find time to review the PRs in this project right now, I will try to review the related PR and see if it resolves this issue

[github-actions[bot]]

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

[miguelborges99]

waiting for review...

[github-actions[bot]]

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

[cazpr]

To late to respond in time, but this issue is still relevant for us.

[github-actions[bot]]

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

[marbon87]

Issue still exists.

[github-actions[bot]]

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

[rbange]

This issue is still relevant

[github-actions[bot]]

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

[rbange]

This issue is still relevant

[github-actions[bot]]

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

[marbon87]

This issue is still relevant

[achetronic]

Still relevant for us too :)

[github-actions[bot]]

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

[rbange]

The issue is still relevant

[github-actions[bot]]

This issue has been inactive for 60 days. If the issue is still relevant please comment to re-activate the issue. If no action is taken within 7 days, the issue will be marked closed.

[rbange]

still relevant