Open-EO / openeo-api

The openEO API specification
http://api.openeo.org
Apache License 2.0
93 stars 12 forks source link

Bearer token prefix may lead to issues with JWT tooling #338

Open aljacob opened 4 years ago

aljacob commented 4 years ago

Today while implementing authorization I realized that our way of defining the token we send as access token is not JWT conform and can create all kinds of issues when relying on external libraries or frameworks for parsing the token. Can we remove the /oidc/provider/ before the token, so that we just send Bearer token instead of Bearer /oidc/provider/token. If we need for some reason the information on which auth method and which provider has been chosen I would rather just add this as additional custom header fields instead of encoding them in the Bearer String.

m-mohr commented 4 years ago

The Bearer Token has the actual token after the prefix. The API doesn't require JWT to be used due to different authentication procedures, which may not even allow JWT.

See also the API description for the Bearer Token format, which was discussed in a broader round of OIDC back-ends (I think including you):

The Bearer Token MUST consist of the authentication method, a provider ID (if available) and the token itself. All separated by a forward slash /. Examples (replace TOKEN with the actual access token): (1) Basic authentication (no provider ID available): basic//TOKEN (2) OpenID Connect (provider ID is ms): oidc/ms/TOKEN. For OpenID Connect, the provider ID corresponds to the value specified for id for each provider in GET /credentials/oidc.

The TOKEN above is your JWT, so you need to split the Bearer token at the / char and the third part is your JWT you can then parse.

We can't change this easily, it would be a breaking change. Also, we need to know the auth procedure and the provider, otherwise we can't make sense of the Token. Closing for now as I hope this helps you to solve the issue and I don't see anything we can do directly.

soxofaan commented 4 years ago

I agree with @m-mohr , the bearer token that is sent from openEO client ot openEO backend should be handled as a opaque blob. I don't think there is a standard that requires it to be a JWT token.

Also, I noticed lately that JWT gets some bad press from time to time, so we shouldn't bet too big on it. It is not unlikely that some providers switch to another solution in the future.

aljacob commented 4 years ago

maybe I expressed myself poorly. The token must not be expected to be a JWT token in general. I agree with this. But if somebody relies on tools that are using JWT tokens, then the validators in those tools will fail by default with the current definition, as you are basically manipulating the token that has been send to you, so when you send it back for validation to the backend from which you got it, it does not get the same thing as it expects and hence validation fails.

m-mohr commented 4 years ago

No, I understood the issue, but what do you expect us to do now? If you want a change here you need to write a proposal to the PSC and wait for API 2.0 as a change would be breaking. I still think you can solve this in implementation though.

soxofaan commented 4 years ago

you are basically manipulating the token that has been send to you, so when you send it back for validation to the backend from which you got it, it does not get the same thing as it expects and hence validation fails.

This is not completely correct I think. There are in general 3 parties in this dance: the OIDC provider, the client and the backend. The OIDC provider sends an access token to the client, the client indeed manipulates this by adding a prefix (which creates the bearer token) and sends it to the backend, and the backend strips the prefix again to get the access token to communicate with the OIDC provider (to get user info). So it is only between the client and backend (which both are fully openEO API aware) that we use this custom bearer token format. Interaction (from client or backend) to other parties should use the original access token.

But maybe I'm still misunderstanding the issue. Do you have a concrete example where it goes wrong @aljacob ?

lforesta commented 4 years ago

The backend always needs to have the following check for each request sent with a token:

Changing this info to be in the header instead in the token itself seems more work than it's worth? (given that we would need to go through a proposal to the PSC and then all backends/clients would need to update this part)

m-mohr commented 4 years ago

So after the discussion today, I'd propose to re-consider this whenever the next breaking version will be released (i.e. API 2.0, milestone 'future') and then discuss the switch to additional HTTP headers. Until then workaround need to be implemented.

I'd guess that "normal" tooling still has issues with additional headers as with multiple IPs and auth methods you still have to have a step in-between to not mix up tokens from different IPs etc (as Luca also points out). You don't want to send a Microsoft token to Google for validation, but tooling might not expect different IPs involved. Then there would be no real gain compared to the current specification, but that likely depends on the actual libraries used and should be checked with a couple of popular libraries once this comes up again.

soxofaan commented 4 years ago

I'd guess that "normal" tooling still has issues with additional headers

Indeed, any solution or alternative will probably cause some issue somewhere because this multi-provider feature is non-standard in the first place I guess

aljacob commented 4 years ago

Additional headers, won't be an issue. The toolchains that I have looked at, just evaluate the headers interesting for them. So other "unknown" headers will just be ignored. The authorization header will only be evaluated once by the corresponding tool chain. If you have additional headers for selecting the right auth chain, you can just filter based on their presence and select the right security protocol and provider. This way especially for the more simple case of having just an individual identity provider you don't need to change anything.