Open-EO / openeo-js-client

JavaScript and TypeScript client for the openEO API.
https://open-eo.github.io/openeo-js-client/latest/
Apache License 2.0
15 stars 6 forks source link

Default to Auth code flow (with PKCE) instead of implicit flow #39

Closed soxofaan closed 3 years ago

soxofaan commented 3 years ago

From my initial testing of OIDC auth against our VITO backend, I think the web editor is using the implicit flow, is this correct?

I was wondering why the auth code flow with PKCE isn't used as a lot of online resources advise against the implicit flow and recommend auth code + PKCE instead. https://oauth.net/2/grant-types/implicit/ is a good starting point more background

m-mohr commented 3 years ago

I haven't set this anywhere, I think, so it seems it's the default of the library I'm using. Actually, some issues from 2018 imply that the library doesn't support Auth Code + PKCE while others imply it's supported. I may need to switch the library anyway for mobile support (@jneijt), so that may solve it.

jneijt commented 3 years ago

I've been testing this as well and setting response_type: 'code' on the sign-in request seems to make the library use the auth code flow with PKCE. At least then the login on the VITO backend works in a normal browser-based environment. I have not tested this in the web editor code though, just in my sandbox application.

m-mohr commented 3 years ago

Would be nice if the library would just choose what is supported, I guess?!

soxofaan commented 3 years ago

Would be nice if the library would just choose what is supported

I guess the reasoning is that picking the right OIDC flow should be a conscious choice by the app developer (because of practical and security related details) instead of something that can be resolved automatically. But that only works if the (set of) OIDC provider(s) is known in advance, which is not exactly the case for the openeo web editor.

But apart from that, I think that PKCE is already established enough to assume it is supported by all major providers used by current openeo backends.

m-mohr commented 3 years ago

It seems it would work like that: https://github.com/IdentityModel/oidc-client-js/issues/991#issuecomment-555937606

m-mohr commented 3 years ago

This is actually implemented and defined in the JS client, so I have to move this to the JS client.

I'll override this in the Web Editor so that it uses PKCE instead of Implicit.

I also found that there's a Hybrid Flow, but unfortunately it is something separate and not a Flow that is just choosing from the available methods.

m-mohr commented 3 years ago

I've tried to response_type to code, which should activate Auth Code w/ PKCE, but it doesn't work for me. It looks like a bug in the oidc library to me, but I'm not 100% sure. Hope https://github.com/IdentityModel/oidc-client-js/issues/1256 helps...

Anyway, I'd need to keep Implicit Flow as default to avoid breaking changes. If response_type code would work, I'd just need to document how to change the behavior.

m-mohr commented 3 years ago

Waiting for https://github.com/IdentityModel/oidc-client-js/pull/1258 to be merged (or solved differently). It's actually a bug in the OIDC library.

m-mohr commented 3 years ago

The author of the OIDC library has closed the bugfix I submitted as PR without any detailed checks so we won't get any AuthCode support in the JS client and will have to stay with Implicit until a new library gets developed. So closing here, too. I can't do anything.

m-mohr commented 3 years ago

I got it reconsidered and it's now fixed so that I can now include the flow. It's available since v1.11.3 of the oidc-client-js

m-mohr commented 3 years ago

This will be released with 1.0.3, so it supports AuthCode w/PKCE but the default is still Implicit. The default will change to AuthCode in 2.0.0, as it's a breaking change. Still, the Web Editor will implement AuthCode w/PKCE now and use it in the next version already. Implicit is then removed from the Web Editor completely.