Proposal: fetchOauthGrantToken filter to complement oauthGrant

[mohabusama]

Is your feature request related to a problem? Please describe.

Assuming the following routes in a custom skipper deployment:

backendService:
    Path("/apis/backend-service/*resources")
    -> oauthGrant()
    ...
    -> "https://backend-service.example.org";
ui:
    *
    -> oauthGrant()
    ...

The idea here is that an employee logs in using the oauthGrant filter and immediately we get a session cookie with the access token. Now the client (i.e. the browser JS code) will make subsequent REST API calls to one or more backends (e.g. backendService), with the session cookie in the requests. Having oauthGrant and oauth2-access-token-header-name would guarantee passing the token (stored in the cookie) to the backend which is what is expected and the client is not dealing with authentication details anymore, which is really cool.

The problem here is that in case a user is no longer logged in (for any reason), oauthGrant filter will respond with a redirect to authenticate the user instead of returning 401 (which a call to the backendService is expected to return in case of unauthenticated request). This behavior modifies the client expectation of API responses.

Describe the solution you would like

One proposal here is to introduce a new filter (e.g. fetchOauthGrantToken) to actually extract the access token from the cookie without performing further authentication, the token is then forwarded according to oauth2-access-token-header-name (if exists) and authentication is then handled in the backend service. API response will be consistent between browser client sessions and other clients as well.

Describe alternatives you've considered (optional) N/A

Additional context (optional)

Risk: the backendService route is no longer protected by the oauthGrant filter, so users should be aware of this side-effect. This also assumes backend-service.example.org implements authentication.

Would you like to work on it? Yes

[mohabusama]

@szuecs @aryszka please let me know what you think.

[AlexanderYastrebov]

Hi,

The problem here is that in case a user is no longer logged in (for any reason), oauthGrant filter will respond with a redirect to authenticate the user instead of returning 401

Could you clarify what could be the reason for client to be no longer logged? Did they remove the cookie? And if they removed the cookie how would the proposed solution extract token out of it?

[mohabusama]

This might be caused by couple of reasons:

1. Token revoked.
2. The API call is coming from a headless client (i.e. the cookie is missing). The route technically can serve both session based clients and clients calling with Authorization header.

[szuecs]

@mohabusama so you want to have a new filter that extracts data from one or more cookies and put it into a header as far as I understand? If so we should not call it grantWhatever, but cookieToStateBag(cookie-name, key, statebag-entry) and then we need to read from stateBag and put it into a header, which we already have https://opensource.zalando.com/skipper/reference/filters/#setcontextrequestheader.

So it would look like:

backendService:
    Path("/apis/backend-service/*resources")
    -> cookieToStateBag("mycookie", "access-token", "app1-token")
    -> setContextRequestHeader("Authorization", "app1-token") // here we have lost "Bearer ", but I guess with some templating it should work!
    -> "https://backend-service.example.org";
ui:
    *
    -> oauthGrant()
    ...

[AlexanderYastrebov]

Token revoked.

What is the point of extracting it then?

The API call is coming from a headless client (i.e. the cookie is missing). The route technically can serve both session based clients and clients calling with Authorization header.

Would a separate route for "headless clients" work then?

backendServiceHeadless:
    Path("/apis/backend-service/*resources") && HeaderRegexp("Authorization", ".+")
    -> oauthTokeninfoAllScope("whatever") // if needed
    ...
    -> "https://backend-service.example.org";

[mohabusama]

@szuecs Thanks, I will look into that and get back if I have any questions.

@AlexanderYastrebov Thanks for the suggestion, but we are trying to avoid duplicating routes in the setup.

What is the point of extracting it then?

The idea is to offload authentication to the backend service while handling a single route. The backend service will respond according to its defined API specs, something that should not leak into skipper.

[aryszka]

I like the idea, and also the mentioned risk seems to be less of a problem, because if we extract the token from the cookie into the Authorization header, then the backend route can have a subsequent tokeninfo filter that would validate the token and would return 401 if it is invalid.

[mohabusama]

@szuecs I looked into possible implementation of cookieToStateBag in the context of oauthGrant and found a blocker. Mainly skipper differentiates between cookies filter(s) and ouathGrant cookie, since the grantcookie encrypts the cookie based on oauthGrant configuration.

Thought about adjusting cookieToStateBag to decrypt via an optional arg, but it might not be a good idea:

1. Breaks separation of concerns; since grantcookie has its own cookie variant implementation.
2. Could be fragile; since in the future encryption/decryption could be extended and handled differently than assuming the grantcookie is always responsible for the decryption.

One solution is to introduce grantCookieToStateBag, which is explicit enough to handle the scope of oauthGrant cookies (also consistent with other grant filters). This filter could be parallel to the cookieToStateBag implementation.

or we can abstract the token extraction in case of oauthGrant as mentioned in the issue description (via fetchGrantToken or a variant thereof).

wdyt?

[szuecs]

What if we would put the cookie (or whatever you need) always into the statebag?