ExpressGateway / express-gateway

A microservices API Gateway built on top of Express.js
https://www.express-gateway.io
Apache License 2.0
2.97k stars 344 forks source link

Oauth2-introspect change scopes to scope #926

Closed nvanheuverzwijn closed 4 years ago

nvanheuverzwijn commented 5 years ago

As per https://www.iana.org/assignments/jwt/jwt.xhtml, the currently standard way of defining scopes is through the scope key and not scopes. In this document scope is currently draft which cast some confusion on wheter or not it is standard.

See https://datatracker.ietf.org/doc/draft-ietf-oauth-token-exchange/?include_text=1

  scope
      OPTIONAL.  A list of space-delimited, case-sensitive strings, as
      defined in Section 3.3 of [RFC6749], that allow the client to
      specify the desired scope of the requested security token in the
      context of the service or resource where the token will be used.
      The values and associated semantics of scope are service specific

RFC7662 define scope as the key defining scopes in the introspection response which reinforce the idea that scope is more standard than scopes. See https://tools.ietf.org/html/rfc7662 section 2.2

   scope
      OPTIONAL.  A JSON string containing a space-separated list of
      scopes associated with this token, in the format described in
      Section 3.3 of OAuth 2.0 [RFC6749].

Which brings us to RFC6749 (the oauth2 rfc) which define scope as the authorization response. See https://tools.ietf.org/html/rfc6749 section 4.1.1

   scope
         OPTIONAL, if identical to the scope requested by the client;
         otherwise, REQUIRED.  The scope of the access token as
         described by Section 3.3.

In my opinion, we should keep the scopes for backward compatibility unless you know no one is using oauth-introspection. We could also add the ability to specify the "scope" key in the parameters of the plugins and encourage users to use scope instead of scopes.

What do you think ?

codecov[bot] commented 5 years ago

Codecov Report

Merging #926 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #926   +/-   ##
=======================================
  Coverage   88.91%   88.91%           
=======================================
  Files         137      137           
  Lines        3734     3734           
=======================================
  Hits         3320     3320           
  Misses        414      414
Impacted Files Coverage Δ
...ib/policies/oauth2-introspect/oauth2-introspect.js 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1f29dca...778bdc0. Read the comment docs.

XVincentX commented 5 years ago

@nvanheuverzwijn Hey,

Thanks a lot for this — apparently this makes sense but I'll need to dig down in the documents one more time. I recall we went with scopes because of the official document or something, but I can't really find it right now.

nvanheuverzwijn commented 5 years ago

@XVincentX Gotcha, no problem. Thanks !