[OAuth2] Incompatibility with access_token responses without "expire_in"

[riptl]

Title: oauth2 filter is incompatible with authorization servers that omit expire_in on access_token responses

Description:
When configuring the oauth2 HTTP filter against an RFC 6749-compliant OAuth 2.0 authorization server, the auth flow fails if the access_token response does not contain an expire_in field.

Repro steps:
Setup Envoy oauth2 HTTP filter against an RFC 6749-compliant authorization server that does not return the expire_in field on the access_token response, such as GitHub.

Admin and Stats Output: Not relevant

Config:

http_filters:
- name: envoy.filters.http.oauth2
  typed_config:
    "@type": type.googleapis.com/envoy.extensions.filters.http.oauth2.v3alpha.OAuth2
    config:
      token_endpoint:
        cluster: github-oauth
        uri: https://github.com/login/oauth/access_token
        timeout: 3s
      authorization_endpoint: https://github.com/login/oauth/authorize
      redirect_uri: "https://example.org/callback" 
...

Logs:

Starting with the response headers of the access_token request as sent by the Envoy OAuth 2.0 client implementation.

Relevant log line: No access token or expiration after asyncGetAccessToken

[2020-12-30 13:54:59.295][138392][debug][http] [source/common/http/async_client_impl.cc:99] async http request response headers (end_stream=false):                                                                 ':status', '200'                                                                                                                                                                                                    
'date', 'Wed, 30 Dec 2020 12:54:59 GMT'                                                                                                                                                                             'content-type', 'application/json; charset=utf-8'                                                                                                                                                                   
'transfer-encoding', 'chunked'                                                                                                                                                                                      
'server', 'GitHub.com'                                                                                                                                                                                              
'status', '200 OK'                                   
'vary', 'X-PJAX,Accept-Encoding, Accept, X-Requested-With,Accept-Encoding'                                
'etag', '<redacted>'       
'cache-control', 'max-age=0, private, must-revalidate'                                                    
'strict-transport-security', 'max-age=31536000; includeSubdomains; preload'                               
'x-frame-options', 'deny'                            
'x-content-type-options', 'nosniff'                  
'x-xss-protection', '1; mode=block'                  
'referrer-policy', 'origin-when-cross-origin, strict-origin-when-cross-origin'                            
'expect-ct', 'max-age=2592000, report-uri="https://api.github.com/_private/browser/errors"'                                                                                                                         
'content-security-policy', 'default-src 'none'; base-uri 'self'; block-all-mixed-content; connect-src 'self' uploads.github.com www.githubstatus.com collector.githubapp.com api.github.com github-cloud.s3.amazonaw
s.com github-production-repository-file-5c1aeb.s3.amazonaws.com github-production-upload-manifest-file-7fdce7.s3.amazonaws.com github-production-user-asset-6210df.s3.amazonaws.com cdn.optimizely.com logx.optimize
ly.com/v1/events wss://alive.github.com; font-src github.githubassets.com; form-action 'self' github.com gist.github.com; frame-ancestors 'none'; frame-src render.githubusercontent.com; img-src 'self' data: githu
b.githubassets.com identicons.github.com collector.githubapp.com github-cloud.s3.amazonaws.com *.githubusercontent.com; manifest-src 'self'; media-src 'none'; script-src github.githubassets.com; style-src 'unsafe
-inline' github.githubassets.com; worker-src github.com/socket-worker-5029ae85.js gist.github.com/socket-worker-5029ae85.js'
'x-github-request-id', 'BC8C:90EB:916225:CC179B:5FEC78A3'                                                 
'x-envoy-upstream-service-time', '134'               

[2020-12-30 13:54:59.295][138392][trace][http] [source/common/http/async_client_impl.cc:116] async http request response data (length=236 end_stream=false)
...
[2020-12-30 13:54:59.298][138392][debug][upstream] [source/extensions/filters/http/oauth2/oauth_client.cc:92] No access token or expiration after asyncGetAccessToken
[2020-12-30 13:54:59.298][138392][debug][http] [source/common/http/filter_manager.cc:827] [C1][S16538478899231496212] Sending local reply with details 

Call Stack: Not relevant

Additional information

RFC 6749 Section 5.1 states:

expires_in
RECOMMENDED. The lifetime in seconds of the access token. For
example, the value "3600" denotes that the access token will
expire in one hour from the time the response was generated.
If omitted, the authorization server SHOULD provide the
expiration time via other means or document the default value.

In short, authorization server implementations do not have to provide the expires_in parameter in their access_token request response.

For example, GitHub does not set this parameter: (https://docs.github.com/en/free-pro-team@latest/developers/apps/authorizing-oauth-apps)

{
  "access_token":"e72e16c7e42f292c6912e7710c838347ae178b4a",
  "scope":"repo,gist",
  "token_type":"bearer"
}

The Envoy OAuth 2.0 client implementation interprets this field as mandatory though. It rejects any responses without the expired_in parameter

envoy/source/extensions/filters/http/oauth2/oauth_client.cc

Lines 91 to 95 in 7e3b483

	if (!response.has_access_token() || !response.has_expires_in()) {
	ENVOY_LOG(debug, "No access token or expiration after asyncGetAccessToken");
	parent_->sendUnauthorizedResponse();
	return;
	}

Proposed Fix

#14625

The Envoy oauth2 filter token config should have a field default_expires_in or similar, that when specified, allows responses without the expires_in field to pass. In the absence of the expires_in field, it should use that default value from the config file as the token expiration parameter.

[riptl]

Until then, here's an ugly patch for testing / demonstration. This patch just ignores the expired_in parameter entirely and hardcodes it to 3600.

With this patch and #14538, the oauth2 token works with the GitHub OAuth App feature.

diff --git a/source/extensions/filters/http/oauth2/oauth_client.cc b/source/extensions/filters/http/oauth2/oauth_client.cc
index 170d4a9a2..16f807c14 100644
--- a/source/extensions/filters/http/oauth2/oauth_client.cc
+++ b/source/extensions/filters/http/oauth2/oauth_client.cc
@@ -88,14 +88,14 @@ void OAuth2ClientImpl::onSuccess(const Http::AsyncClient::Request&,
 
   // TODO(snowp): Should this be a pgv validation instead? A more readable log
   // message might be good enough reason to do this manually?
-  if (!response.has_access_token() || !response.has_expires_in()) {
-    ENVOY_LOG(debug, "No access token or expiration after asyncGetAccessToken");
+  if (!response.has_access_token()) {
+    ENVOY_LOG(debug, "No access token after asyncGetAccessToken");
     parent_->sendUnauthorizedResponse();
     return;
   }
 
   const std::string access_token{PROTOBUF_GET_WRAPPED_REQUIRED(response, access_token)};
-  const std::chrono::seconds expires_in{PROTOBUF_GET_WRAPPED_REQUIRED(response, expires_in)};
+  const std::chrono::seconds expires_in{3600};
   parent_->onGetAccessTokenSuccess(access_token, expires_in);
 }

Filter:

name: envoy.filters.http.oauth2
typed_config:
  "@type": type.googleapis.com/envoy.extensions.filters.http.oauth2.v3alpha.OAuth2
  config:
    token_endpoint:
      cluster: github-oauth
      uri: https://github.com/login/oauth/access_token
      timeout: 3s
    authorization_endpoint: https://github.com/login/oauth/authorize
    redirect_uri: "<URL_TO_SELF>/callback"
    redirect_path_matcher:
      path:
        exact: /callback
    signout_path:
      path:
        exact: /signout
    credentials:
      client_id: <YOUR_GITHUB_CLIENT_ID>
      token_secret:
        name: github-oauth-client-secret
      hmac_secret:
        name: github-oauth-hmac

Cluster:

name: github-oauth
connect_timeout: 5s
type: LOGICAL_DNS
lb_policy: ROUND_ROBIN
load_assignment:
  cluster_name: github-oauth
  endpoints:
  - lb_endpoints:
    - endpoint:
        address:
          socket_address:
            address: github.com
            port_value: 443
transport_socket:
  name: tls
  typed_config:
    "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext
    sni: github.com

[rgs1]

cc: @williamsfu99

[riptl]

I'll have a go at fixing this today if you don't mind.

[williamsfu99]

@terorie
Thanks so much for raising this issue and providing so much detail! I don't mind - let me know if you need me to review or to submit a PR - I have time today.

[riptl]

@williamsfu99 Thanks for your quick response and helping me with this. I would appreciate a look at #14625. I'm very new to Envoy, so if the code isn't any good, feel free to submit your own PR. In the meantime, I'm going to add tests later today.

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[riptl]

Please remove the stale label. Envoy is still incompatible with GitHub's OAuth 2.0 provider.

[orHayoun]

Sorry to barge in, but is there a workaround or my best option are to compile envoy with hardcodesexpired_in?

[DiamondJoseph]

It seems this is still an issue when attempting to authenticate against Github.

2022-07-04T10:02:21.394561Z	debug	envoy http	async http request response headers (end_stream=false):
':status', '200'
'server', 'GitHub.com'
<snip>
2022-07-04T10:02:21.394691Z	debug	envoy upstream	No access token or expiration after asyncGetAccessToken
2022-07-04T10:02:21.394777Z	debug	envoy http	[C727][S1785646932291492729] encoding headers via codec (end_stream=false):
':status', '401'
<snip>

Frustrating issue that wasn't too clear until I found this thread: it's very hard to find results for debugging GIthub's own products!

[riptl]

@orHayoun @DiamondJoseph I am working on fixing #14625 today.