Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove implicit flow #152

Closed
4 tasks done
pamapa opened this issue Oct 21, 2021 · 13 comments · Fixed by #153
Closed
4 tasks done

remove implicit flow #152

pamapa opened this issue Oct 21, 2021 · 13 comments · Fixed by #153
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@pamapa
Copy link
Member

pamapa commented Oct 21, 2021

Remove the implicit flow and keep the library lightweight and best practice.

Also see #151

Remove implicit flow in:

  • source code
  • unit-tests
  • docu
  • samples
@pamapa pamapa added this to the v2.0 milestone Oct 21, 2021
@pamapa pamapa added the enhancement New feature or request label Oct 21, 2021
@pamapa pamapa self-assigned this Oct 21, 2021
pamapa added a commit that referenced this issue Oct 21, 2021
pamapa added a commit that referenced this issue Oct 21, 2021
pamapa added a commit that referenced this issue Oct 21, 2021
@pamapa
Copy link
Member Author

pamapa commented Oct 21, 2021

@kherock
maybe we should first make a 2.0.0 release and then drop the implicit flow with a 3.0.0 release? Like this we have a fallback for people who still need this. What do you think? Or they must use the oidc-client-js as fallback?

@kherock
Copy link
Collaborator

kherock commented Oct 21, 2021

If we have immediate plans to drop implicit flow, I think it would be appropriate to do so in 2.0. It doesn't make much sense when the original 1.x releases are still there and useable.

@hackermd
Copy link

hackermd commented Apr 21, 2022

I may be missing something obvious, but it appears that the implicit flow is still required in practice.

For example, Google's OIDC server requires applications to send the client_secret when using authorization code flow. If one does not want to send the client secret (which I consider a bad idea for JavaScript/TypeScript web applications), one is left with the implicit flow option (see also this StackOverflow question).

Is there any chance the implicit flow could be re-introduce until major OIDC providers fully support the OAuth 2.0 authorization code grant type with PKCE challenge?

@Badisi
Copy link
Contributor

Badisi commented Apr 22, 2022

@hackermd, this might interest you : #812

... you don't get client secrets when you build a mobile app on Google.
I was able to register an app as a mobile app, getting only a client ID, and do the OAuth authorization code flow from pure JavaScript and it worked fine.

@hackermd
Copy link

Thanks @Badisi for pointing that out. When you say "mobile app", you mean selecting OAuth Client ID credentials of Application type "iOS" or "Android", correct? While one doesn't get a client_secret, one also doesn't seem to be able to configure the JavaScript origin or the redirect URI. Could you kindly elaborate on how you configured the client for the JavaScript app?

@Badisi
Copy link
Contributor

Badisi commented Apr 23, 2022

Well I had no experience whatsoever with GoogleOIDC.
So I was just pointing out what this guy was saying and hoped it might help you.

But now that I played a bit with it myself, I realize that this guy was wrong.
You may not have a client_secret when creating an iOS/Android app, but you also won't be able to specify redirect urls.
So you are stuck with using the client web app type.
And even specifying a dummy client_secret, like mentioned in the link of my previous comment, won't work.

So yes, you either have to use the client_secret or the implicit flow.
I will then let @pamapa answer your original question :)

@hackermd
Copy link

Thank you @Badisi.

@pamapa
Copy link
Member Author

pamapa commented Apr 25, 2022

The implicit flow is gone for good. If you need it stick with the predecessor library...

domdomegg added a commit to raisenational/raise that referenced this issue Jan 19, 2023
The underlying library react-google-login uses is deprecated, and is being discontinued on 31st March 2023: https://developers.googleblog.com/2021/08/gsi-jsweb-deprecation.html

Picked oidc-client over:
- @react-oauth/google: Google and React specific, makes it harder to migrate to anything else. Appreciate this is what we had beforehand, but now we seem closer to changing identity provider want to keep that flexibility.
- google-oauth-gsi: Google speicifc, so similar to above. Also seems to largely be @react-oauth/google repacked without react bits - without potential licensing issues.
- oidc-client-ts: Would have preferred to use this, but it isn't currently compatible with the way Google does OAuth (see authts/oidc-client-ts#152)
domdomegg added a commit to raisenational/raise that referenced this issue Jan 19, 2023
The underlying library react-google-login uses is deprecated, and is being discontinued on 31st March 2023: https://developers.googleblog.com/2021/08/gsi-jsweb-deprecation.html

Picked oidc-client over:
- @react-oauth/google: Google and React specific, makes it harder to migrate to anything else. Appreciate this is what we had beforehand, but now we seem closer to changing identity provider want to keep that flexibility.
- google-oauth-gsi: Google speicifc, so similar to above. Also seems to largely be @react-oauth/google repacked without react bits - without potential licensing issues.
- oidc-client-ts: Would have preferred to use this, but it isn't currently compatible with the way Google does OAuth (see authts/oidc-client-ts#152)
@sedghi
Copy link

sedghi commented Feb 9, 2023

I know the last conversation was about a year ago, so does that mean until Google changes their code grant type requirements to not include client_secret we should use it? Is it safe?

@pamapa
Copy link
Member Author

pamapa commented Feb 9, 2023

This library supports dozens of IDPs, the implicit flow is horrible and this library will never bring it back. Best would be if Google would support OAuth2.1 or at least OAuth2.0 code flow as its is specified, i am not using Google as IDP. Maybe you can convince them. At the end you still have alternative libraries that support the implicit flow, including the predecessor of this library.

@sedghi
Copy link

sedghi commented Feb 9, 2023

@pamapa Thanks for your prompt response and I expected that answer from above conversation. However, I was also asking about from the security aspect, how much does including client_secret affect the attack surface? Is it a concern? Do you use it in production? I'm not a security expert, so sorry if my question has an obvious answer

@pamapa
Copy link
Member Author

pamapa commented Feb 10, 2023

Never ever use client_secret in an application which is exposed within a customers browser. It would be very easy to extract that secret from the single page application code/memory. That secret can be miss-used for quiet a long time.

@sedghi
Copy link

sedghi commented Feb 10, 2023

I see, so I guess the conclusion is that:
we can not use your library [oidc-client-ts](https://github.com/authts/oidc-client-ts) for google oauth since 1) Google requires the client_secret 2) You have removed the implicit flow.

Is that the right conclusion?

github-actions bot pushed a commit to ncats/oidc-client-ts that referenced this issue Feb 28, 2023
# 1.0.0 (2023-02-28)

### Bug Fixes

* [authts#201](https://github.com/ncats/oidc-client-ts/issues/201) add sourcemaps ([6c1c965](6c1c965))
* [authts#225](https://github.com/ncats/oidc-client-ts/issues/225) move revoke from TokenRevocationClient into TokenClient ([e4570ee](e4570ee))
* [authts#257](https://github.com/ncats/oidc-client-ts/issues/257) signoutRedirect does not work ([79ebe27](79ebe27))
* [authts#314](https://github.com/ncats/oidc-client-ts/issues/314) document signingKeys is unused ([d9d79fd](d9d79fd))
* [authts#415](https://github.com/ncats/oidc-client-ts/issues/415) allow to get and process claims when scope does not contain openid ([558319b](558319b))
* [authts#435](https://github.com/ncats/oidc-client-ts/issues/435) allow passing post_logout_redirect_uri to signoutRedirect ([ad209f3](ad209f3))
* [authts#441](https://github.com/ncats/oidc-client-ts/issues/441) only validate sub during refresh token path when the optional id_token is present ([00b9072](00b9072))
* [authts#450](https://github.com/ncats/oidc-client-ts/issues/450) do not use workspaces ([4e993f4](4e993f4))
* [authts#466](https://github.com/ncats/oidc-client-ts/issues/466) clear stale states when creating new one ([1b39579](1b39579))
* [authts#569](https://github.com/ncats/oidc-client-ts/issues/569) if there's no session_state on the response, then take it from initial sign-in ([06de889](06de889))
* [authts#580](https://github.com/ncats/oidc-client-ts/issues/580) add id_token_hint if present ([0a495bb](0a495bb))
* [authts#688](https://github.com/ncats/oidc-client-ts/issues/688) id_token is lost on silent refresh token renewal ([0a49522](0a49522))
* [authts#754](https://github.com/ncats/oidc-client-ts/issues/754) preserve id_token and profile even if no openid is in scope of refresh request ([edfef08](edfef08))
* [authts#769](https://github.com/ncats/oidc-client-ts/issues/769) allow multiple resource parameters ([e0188fe](e0188fe))
* add change set ([ccf65d0](ccf65d0))
* add dist folder ([1e261f4](1e261f4))
* add release workflow ([b556057](b556057))
* add scope to refresh tokens ([62bc823](62bc823)), closes [authts#364](https://github.com/ncats/oidc-client-ts/issues/364)
* allow to override acr_values in signinRedirect ([5772d5d](5772d5d))
* avoid redundant error event when a window is closed by user ([9491eea](9491eea))
* build ([d99fe4d](d99fe4d))
* code style ([562c1d4](562c1d4))
* custom state ([542e7c3](542e7c3))
* deprecated unused clockSkewInSeconds and userInfoJwtIssuer settings ([52e8799](52e8799))
* ensure we only throw something of type Error ([b9c2a38](b9c2a38))
* fully specify crypto-js imports ([8a6f772](8a6f772))
* handle case of refresh token response not containing id_token ([50fbb50](50fbb50))
* improve test asserts ([d43f46b](d43f46b))
* improve unit-tests for JsonService ([2decbab](2decbab))
* improve unit-tests for MetadataService ([b57541e](b57541e))
* improve unit-tests for ResponseValidator ([be401a6](be401a6))
* improve unit-tests for SigninResponse ([8723167](8723167))
* indent of signingKeys ([985e28f](985e28f))
* no need for open+path ([6daa33b](6daa33b))
* remove dist folder ([b1520c7](b1520c7))
* remove npm release ([d39f4a5](d39f4a5))
* signout callback ([813fda0](813fda0))
* simplify _validateIdTokenAttributes ([84935b2](84935b2))
* store keypair ([22d36af](22d36af))
* test ([bda9a69](bda9a69))
* test setup update ([3154679](3154679))
* type of AccessTokenCallback ([af51769](af51769))
* update ([15c568c](15c568c))
* update config ([5e741ab](5e741ab))
* update package ([bf00d1e](bf00d1e))
* update package file ([73720a9](73720a9))
* update package-lock file ([22115a8](22115a8))
* update release file ([00bf5dc](00bf5dc))
* update release yml file ([9c05fc8](9c05fc8))
* update token ([10ff74f](10ff74f))
* **authts#278:** expose the internal logger ([361403f](361403f)), closes [authts#278](https://github.com/ncats/oidc-client-ts/issues/278)
* **authts#780:** filterProtocolClaims deletes properties required by the IdTokenClaims type ([b7fadd8](b7fadd8)), closes [authts#780](https://github.com/ncats/oidc-client-ts/issues/780)
* **CheckSessionIFrame:** normalize iframe origin before comparing to the MessageEvent origin ([e8e38a7](e8e38a7))
* **docs:** update oidc-client-ts-api.md ([e785030](e785030))
* **IFrameWindow:** wait for iframe content to unload before removing from DOM ([dea5ca0](dea5ca0))
* **JsonService:** throw an ErrorResponse instance for invalid responses ([dff9cdf](dff9cdf))
* **parcel-sample:** use COOP/COEP policies compatible with window.opener ([d491e94](d491e94))
* **PopupWindow:** clear the interval checking popup closed on dispose ([346f7c1](346f7c1))
* **RedirectNavigator:** wait for 'unload' instead of 'beforeunload' ([608668b](608668b))
* **RedirectNavigator:** wait until the window is unloading to resolve the returned promise ([86f124c](86f124c))
* **TokenClient:** incorrect secret value sent for client_secret_post ([8fc20c2](8fc20c2))
* **UserManager:** fix anonymous session monitoring ([577200e](577200e))
* [#11](#11) eslint errors wherever possible ([91a4e55](91a4e55))
* [#13](#13) convert checkSessionInterval (milliseconds) to checkSessionIntervalInSeconds in settings ([7d41514](7d41514))
* [#13](#13) convert silentRequestTimeout (milliseconds) to silentRequestTimeoutInSeconds in settings ([0f7005c](0f7005c))
* [#13](#13) rename accessTokenExpiringNotificationTime to accessTokenExpiringNotificationTimeInSeconds in settings ([be7733b](be7733b))
* [#13](#13) rename clockSkew to clockSkewInSeconds in settings ([ca09d98](ca09d98))
* [#13](#13) rename duration to durationInSeconds in Timer ([3715389](3715389))
* [#13](#13) rename staleStateAge to staleStateAgeInSeconds in settings ([56bcaa6](56bcaa6))
* [authts#158](https://github.com/ncats/oidc-client-ts/issues/158) default response mode is explicit query ([b442b84](b442b84))
* [#16](#16) marshal OidcClient instead of inherit ([8009969](8009969))
* [authts#166](https://github.com/ncats/oidc-client-ts/issues/166) move samples/Angular into separate repository ([9d03379](9d03379))
* [authts#167](https://github.com/ncats/oidc-client-ts/issues/167) round tripping "state" is missing ([2d5da0f](2d5da0f))
* [authts#170](https://github.com/ncats/oidc-client-ts/issues/170) revert removing too much from commit "remove implicit flow" (320168b) and adapt ([310b0f2](310b0f2))
* [authts#173](https://github.com/ncats/oidc-client-ts/issues/173) refresh_token is undefined ([95d8443](95d8443))
* [authts#175](https://github.com/ncats/oidc-client-ts/issues/175) treat eslint warnings as errors ([13251d4](13251d4))
* [authts#184](https://github.com/ncats/oidc-client-ts/issues/184) improve esbuild ([26bdc17](26bdc17))
* [#23](#23) avoid multiple instances of MetaDataService ([00b7319](00b7319))
* [#4](#4) add explicit-function-return-type ([02606f1](02606f1))
* [authts#64](https://github.com/ncats/oidc-client-ts/issues/64) drop Cordova ([14c0be8](14c0be8))
* [#7](#7) drop Global.ts ([142d883](142d883))
* [#9](#9) change default settings to minimal + best practice ([536dc68](536dc68))
* add missing devDependencies ([0c48b70](0c48b70))
* add missing metadataSeed ([fa8f78c](fa8f78c))
* add npm package link icon ([e5d6cad](e5d6cad))
* adding url check to iframe _message ([e8bd1b2](e8bd1b2))
* adjust return type, _signinCallback never returns a user ([04831f6](04831f6))
* Angular 8 build error: ([4688b62](4688b62))
* avoid test warning by awaiting promise ([3b1bed5](3b1bed5))
* bump version ([d51c50e](d51c50e))
* change classes to use named exports instead of default exports ([712986f](712986f))
* enforce importsNotUsedAsValues ([975363b](975363b))
* explicit use window.setInterval, avoid NodeJS.Time vs number warning ([207c962](207c962))
* export modules from index directly instead of requiring and then re-exporting ([bcc98ff](bcc98ff))
* export more types (esbuild is more conservative than tsdx) ([9b7123f](9b7123f))
* for typescript v4.4.2 ([11cfd18](11cfd18))
* improve code ([5c2d93f](5c2d93f))
* improve description for UserSignedIn and UserSignedOut ([50f3b1a](50f3b1a))
* improve event logger name ([13448a8](13448a8))
* improve ParsedJwt type ([4842559](4842559))
* move eslint to devDependencies ([e6be1c8](e6be1c8))
* parcel npm scripts ([27da3f3](27da3f3))
* pipeline icon ([e8556dc](e8556dc))
* prepare for test release ([211e304](211e304))
* prevent scrollbars in browsers that refuse to render iframes with zero height ([cac300f](cac300f))
* remove argument/type mapping hack ([d66cf72](d66cf72))
* remove getEpochTime from OidcClientSettings ([6b5020c](6b5020c)), closes [/github.com/IdentityModel/oidc-client-js/pull/1271#issuecomment-769877657](https://github.com//github.com/IdentityModel/oidc-client-js/pull/1271/issues/issuecomment-769877657)
* remove useless jsx config ([28662f2](28662f2))
* remove User.state, its not used at all ([703b941](703b941))
* restore Angular sample project functionality ([2a4b2b5](2a4b2b5))
* signinSilent fails when no user is available ([0c9ea9c](0c9ea9c))
* simplify the arguments types ([be75227](be75227))
* spaces ([2f623ac](2f623ac))
* support typescript 4.4.2 ([860b116](860b116))
* target ES5 to have accessors (getter) without warnings ([4d26e6e](4d26e6e))
* type ([f77455d](f77455d))
* type of jwt aud field ([a1337d4](a1337d4))
* types ([ac61a78](ac61a78))
* update repository location ([aac326e](aac326e))
* updating with changes ([319ac38](319ac38))
* url can be undefined ([047f4b1](047f4b1))
* use type exports in entrypoint ([ba3186c](ba3186c))
* window is not defined ([7a1efc0](7a1efc0))
* workflow file ([81ad9de](81ad9de))

### Features

* [#15](#15) ensure typedoc works before we merge ([f8a7669](f8a7669))
* [#15](#15) move text from index.md into source code ([c7f2a85](c7f2a85))
* [authts#152](https://github.com/ncats/oidc-client-ts/issues/152) remove implicit flow ([320168b](320168b))
* [authts#152](https://github.com/ncats/oidc-client-ts/issues/152) update docs, implicit flow is gone ([5eba1a5](5eba1a5))
* [authts#152](https://github.com/ncats/oidc-client-ts/issues/152) update samples/Parcel, implicit flow is gone ([1e8522d](1e8522d))
* [authts#251](https://github.com/ncats/oidc-client-ts/issues/251) retry silent renew for fetch timeout ([0078fd5](0078fd5))
* [authts#251](https://github.com/ncats/oidc-client-ts/issues/251) retry silent renew for iframe timeout ([b8e6c84](b8e6c84))
* [#4](#4) introduce SigningKey type ([63fccb1](63fccb1))
* [#4](#4) reduce any types ([2575030](2575030))
* [authts#73](https://github.com/ncats/oidc-client-ts/issues/73) improve logging ([eccb5dc](eccb5dc))
* add ability to specify iframe script origin and iframe notify parent for silent auth ([authts#514](https://github.com/ncats/oidc-client-ts/issues/514)) ([e28dcef](e28dcef))
* add DPoP feature ([c228b30](c228b30))
* add github pages via typedoc ([7064f82](7064f82))
* add minimal SSR support ([2a6f632](2a6f632))
* add some more typedoc comments ([9d11051](9d11051))
* add some more unit-tests for UserManager ([b520ed6](b520ed6))
* add unit-test for TokenClient ([f15ba9e](f15ba9e))
* automatically center popups in front of the opener window ([8c6f799](8c6f799))
* default of loadUserInfo changed from true to false ([a0c056c](a0c056c))
* execute "npm dedupe" ([a35caf7](a35caf7))
* extend Parcel with revoke access token ([dcde654](dcde654))
* fine tune export ([e092726](e092726))
* improve sample Parcel logging ([2e1db1a](2e1db1a))
* increase open-pull-requests-limit from 5 to 10 ([a346acb](a346acb))
* introduce ErrorTimeout ([23f8e33](23f8e33))
* introduce Logger.createStatic ([a4ce6b6](a4ce6b6))
* make use of improved logger ([f811496](f811496))
* move ErrorResponse into error sub-dir ([b8fa443](b8fa443))
* move random into CryptoUtils ([9fb4aa0](9fb4aa0))
* return destructor functions from event listener methods ([6fa9632](6fa9632))
* set sandbox attribute for iframe ([dc28584](dc28584))
* update parcel to v2.0.1 ([c037f2b](c037f2b))
* **Claims:** rebase with main ([4bfae02](4bfae02))
* **ErrorResponse:** attach the request body to POST errors ([2f793f6](2f793f6))
* **PopupWindow:** compute a default width based on the size of the window opener ([4b5f377](4b5f377))
* **User:** add types for standard claims to UserProfile ([authts#241](https://github.com/ncats/oidc-client-ts/issues/241)) ([bb33e10](bb33e10))
* [#1](#1) adapt unit-tests ([0e533b1](0e533b1))
* [#1](#1) port build system to tsdx ([cb74a41](cb74a41))
* [#1](#1) port to typescript ([bb02e6f](bb02e6f))
* [#12](#12) add eslint for unit-tests ([9e01e6f](9e01e6f))
* [#20](#20) make client_id, authority, extraQueryParams and extraTokenParams settings readonly ([8de0cf6](8de0cf6))
* [#23](#23) avoid multiple metadataUrl implementations ([a575bce](a575bce))
* [#23](#23) cache matadata and signingKeys in MetadataService ([5479346](5479346))
* [#3](#3) replace .then with async/await pattern ([35318a2](35318a2))
* [#4](#4) fix lint warnings ([f14b8c9](f14b8c9))
* [#4](#4) reduce any type ([e86bedc](e86bedc))
* [#4](#4) reduce any type in navigators ([9c82d50](9c82d50))
* [#4](#4) reduce any type in SilentRenewService ([267c5f9](267c5f9))
* [#4](#4) reduce any type in TokenClient ([2ec9624](2ec9624))
* [#4](#4) reduce any type of signin and signout chain ([cefd12a](cefd12a))
* [#4](#4) reduce any types ([312b341](312b341))
* [#4](#4) reduce eslint errors of misused-promises ([e6e0bbd](e6e0bbd))
* [#4](#4) reduce eslint errors of restrict-plus-operands ([c8973f6](c8973f6))
* [#4](#4) reduce typescript warning/error ignores ([c8f70f0](c8f70f0))
* [#4](#4) reduce usage of any type ([f067d1b](f067d1b))
* [#4](#4) reduce usage of any type ([4e56fb3](4e56fb3))
* [#4](#4) reduce usage of any type ([862b2a4](862b2a4))
* [#4](#4) reduce usage of any type: session_state is typeof string ([5e81347](5e81347))
* [#4](#4) response_mode can only be "query" or "fragment" ([7c73645](7c73645))
* [authts#48](https://github.com/ncats/oidc-client-ts/issues/48) add redirectMethod to settings ([14a481f](14a481f))
* [#5](#5) use external jsrsasign and drop rsa.js ([19b0f62](19b0f62))
* [#6](#6) make use of public/protected keyword ([172bc96](172bc96))
* [#8](#8) replace XMLHttpRequest with fetch ([4fec4d7](4fec4d7))
* [#9](#9) redirect_uri was and is a required setting ([368b25e](368b25e))
* add docusaurus website [#15](#15) ([14ce282](14ce282))
* add typedoc ([0302c21](0302c21))
* introduce sub-directories utils and navigators ([3e6d591](3e6d591))
* merge openid-configuration and metdata ([36d5add](36d5add)), closes [authts#1067](https://github.com/ncats/oidc-client-ts/issues/1067)
* prevent mangling of function names ([34b5bdb](34b5bdb))
* split responses state into state_id (query id state) and "state" (callers custom state) ([0423efa](0423efa))
* support passing extra params to signin and signout requests ([7b27ccf](7b27ccf))
* support passing window params to signin/signout methods ([0e66f8a](0e66f8a))
* use Webpack 4 ([b49b3ce](b49b3ce))

### Reverts

* Revert "filter more stuff from npm?" ([44f3fbe](44f3fbe))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants