enable using default scopes from authorization server #1405

natergj · 2024-02-15T19:36:15Z

OAuth allows for the authorization server to provide default scopes when no scopes are specified by the requestor as described in https://datatracker.ietf.org/doc/html/rfc6749#section-3.3

This allows for the library to be configured to rely on the default scopes.

Closes/fixes # n/a

Checklist

This PR makes changes to the public API
N/A I have included links for closing relevant issue numbers

natergj · 2024-02-15T19:39:00Z

src/OidcClientSettings.ts

+     * https://datatracker.ietf.org/doc/html/rfc6749#section-3.3 describes behavior when omitting scopes from sign in requests
+     * If the IDP supports default scopes, this setting will ignore the scopes property passed to the config
+     */
+    useIdpDefaultScopes?: boolean;


the current behavior when not specifying a scope value is to default the value to "openid". Changing that would be a breaking change for the library.

This field was added to prevent a breaking change.

If the maintainers are OK with a breaking change, I think it would be more straightforward to remove the default value for scope instead of adding this new property. I can set up a PR for that approach if it is desired over adding this property.

Good point, lets keep this settings property for now. This give us time to decide when to do the breaking change. I will have to make a v4.0.0 for this change...

Can you immediately deprecate the new property?

sure thing. The deprecated tag has been added and pushed. Thanks @pamapa

I was looking more into this implementation. I think it would be awesome if the library supported this feature, but I realized that the doc I linked to in the description is the part of the OAuth spec file. The OIDC authentication request spec does state that the openid scope is required for a valid authentication request.

The OIDC specs does also state

"If the openid scope value is not present, the behavior is entirely unspecified."

If it would make it more clear, I could also rename the property to allowScopeOmission or something similar.

The OIDC authentication request spec does state that the openid scope is required for a valid authentication request.

Oh yes, that makes the @deprecated invalid :-) Also its better to keep the type of scope string. I guess you can deal with empty string...

According to https://datatracker.ietf.org/doc/html/rfc6749#section-3.3:
"If the client omits the scope parameter when requesting
authorization, the authorization server MUST either process the
request using a pre-defined default value or fail the request
indicating an invalid scope. The authorization server SHOULD
document its scope requirements and default value (if defined)."
the new property could be named omitScopeWhenRequesting?

What kind of Idp are you using for this feature?

We're using ForgeRock (now part of Ping Identity) as our IDP. We do have the ability to define the IDP behavior when scope is not provided.

I've removed the updates to the scope typing so it is no longer optional where it was not optional before. The new property has been renamed omitScopeWhenRequesting and that property value is solely responsible for including the scope query param in the authorization requests now. The overall changes in this PR are much smaller now. Thanks for all the feedback and suggestions.

codecov · 2024-03-13T15:57:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.56%. Comparing base (f0ad76e) to head (f4d11e0).
Report is 78 commits behind head on main.

❗ Current head f4d11e0 differs from pull request most recent head c192e45

Please upload reports for the commit c192e45 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1405      +/-   ##
==========================================
+ Coverage   80.53%   80.56%   +0.03%     
==========================================
  Files          45       45              
  Lines        1731     1734       +3     
  Branches      344      347       +3     
==========================================
+ Hits         1394     1397       +3     
  Misses        299      299              
  Partials       38       38

Flag	Coverage Δ
unittests	`80.56% <100.00%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…scopes

src/OidcClientSettings.ts

This reverts commit 7667e58.

natergj · 2024-06-24T13:02:34Z

@pamapa looks like this won't be part of the 3.1.0 release? Are there any updates or anything else I can do on this PR to help get it merged?

pamapa · 2024-06-24T13:06:43Z

@pamapa looks like this won't be part of the 3.1.0 release? Are there any updates or anything else I can do on this PR to help get it merged?

Good that you ask. Do you have an actual use-case why you would need such a feature and let us know? You will also need to rebase against main, as it as now merge conflicts.

…scopes

src/OidcClient.ts

natergj · 2024-06-24T13:31:44Z

Good that you ask. Do you have an actual use-case why you would need such a feature and let us know? You will also need to rebase against main, as it as now merge conflicts.

Ah. missed the merge conflicts. Those are resolved now.

there are three use cases we are interested in for the default scopes

The issue we are starting to run into with some of our clients is request urls are too large due to a very long scope search param for scopes being requested and the requests were being rejected. The APIs our web apps interact with use very verbose scopes and we need to request a lot of them so our query string becomes very long. By using default scopes, our requests to the IDP will be reduced in size significantly without needing to make additional adjustments to our hosting infrastructure.
Our web application teams have been provided with a process to request new allowed scopes for their web applications. The web application must then be updates to request the newly allowed scope after it is added to the client. If a web app begins requesting the scope prior to it being approved, all subsequent authentication requests fails and a deployment must be rolled back. It is our hope that the default scope will help to alleviate the human error as the web apps will not become out of sync with their requested scopes.
Our mobile app uses a graphql interface. Our API teams are updating the REST APIs that back the graph. These updates will often require a new or different scope to be included in the authorization token. Default scopes will allow that work to continue without the need to deprecate and replace the graphql schema or forcing our users to continuously be updating the mobile app.

src/OidcClientSettings.ts

natergj · 2024-06-24T13:37:44Z

Thanks for your time and help with this @pamapa ! It's greatly appreciated

pamapa · 2024-06-24T13:38:05Z

Thanks for contributing!

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [oidc-client-ts](https://github.com/authts/oidc-client-ts) | dependencies | minor | [`3.0.1` -> `3.1.0`](https://renovatebot.com/diffs/npm/oidc-client-ts/3.0.1/3.1.0) | --- ### Release Notes <details> <summary>authts/oidc-client-ts (oidc-client-ts)</summary> ### [`v3.1.0`](https://github.com/authts/oidc-client-ts/releases/tag/v3.1.0) [Compare Source](authts/oidc-client-ts@v3.0.1...v3.1.0) oidc-client-ts v3.1.0 is a minor release. No longer using `crypto-js` package, but built-in browser [crypto.subtle](https://developer.mozilla.org/en-US/docs/Web/API/Crypto/subtle) module. Crypto.subtle is available only in [secure contexts](https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts) (HTTPS). Also have a look into the [migration](https://github.com/authts/oidc-client-ts/blob/main/docs/migration.md) info. #### Changelog: - Fixes: - [#1666](authts/oidc-client-ts#1666): fix link in docs to issue - [#1600](authts/oidc-client-ts#1600): updated docs about logger - [#1589](authts/oidc-client-ts#1589): fix compiler error for target=ES2022 - [#1539](authts/oidc-client-ts#1539): fix small typo in `signinCallback` doc in UserManager.ts - [#1504](authts/oidc-client-ts#1504): typo in sample app config - [#1490](authts/oidc-client-ts#1490): fix the return type of `signinCallback` - [#1443](authts/oidc-client-ts#1443): fixes typos in docs - Features: - [#1672](authts/oidc-client-ts#1672): make `signoutCallback` return signout response if request_type is so:r - [#1626](authts/oidc-client-ts#1626): add `popupSignal` property to `signinPopup` and `signoutPop` - [#1580](authts/oidc-client-ts#1580): add dpop docs - [#1569](authts/oidc-client-ts#1569): add dpop nonce support - [#1457](authts/oidc-client-ts#1457): add extra headers - [#1461](authts/oidc-client-ts#1461): add demonstrating proof of possession - [#1430](authts/oidc-client-ts#1430): add global `requestTimeoutInSeconds` setting - [#1405](authts/oidc-client-ts#1405): allow using default scopes from authorization server thanks to [@klues](https://github.com/klues), [@smujmaiku](https://github.com/smujmaiku), [@mftruso](https://github.com/mftruso), [@peetck](https://github.com/peetck), [@dbfr3qs](https://github.com/dbfr3qs), [@mottykohn](https://github.com/mottykohn), [@noshiro-pf](https://github.com/noshiro-pf), [@dbfr3qs](https://github.com/dbfr3qs), [@grjan7](https://github.com/grjan7) and [@natergj](https://github.com/natergj) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.tristess.app/alexandresoro/ouca/pulls/191 Reviewed-by: Alexandre Soro <[email protected]> Co-authored-by: renovate <[email protected]> Co-committed-by: renovate <[email protected]>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [oidc-client-ts](https://github.com/authts/oidc-client-ts) | dependencies | minor | [`3.0.1` -> `3.1.0`](https://renovatebot.com/diffs/npm/oidc-client-ts/3.0.1/3.1.0) | --- ### Release Notes <details> <summary>authts/oidc-client-ts (oidc-client-ts)</summary> ### [`v3.1.0`](https://github.com/authts/oidc-client-ts/releases/tag/v3.1.0) [Compare Source](authts/oidc-client-ts@v3.0.1...v3.1.0) oidc-client-ts v3.1.0 is a minor release. No longer using `crypto-js` package, but built-in browser [crypto.subtle](https://developer.mozilla.org/en-US/docs/Web/API/Crypto/subtle) module. Crypto.subtle is available only in [secure contexts](https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts) (HTTPS). Also have a look into the [migration](https://github.com/authts/oidc-client-ts/blob/main/docs/migration.md) info. #### Changelog: - Fixes: - [#1666](authts/oidc-client-ts#1666): fix link in docs to issue - [#1600](authts/oidc-client-ts#1600): updated docs about logger - [#1589](authts/oidc-client-ts#1589): fix compiler error for target=ES2022 - [#1539](authts/oidc-client-ts#1539): fix small typo in `signinCallback` doc in UserManager.ts - [#1504](authts/oidc-client-ts#1504): typo in sample app config - [#1490](authts/oidc-client-ts#1490): fix the return type of `signinCallback` - [#1443](authts/oidc-client-ts#1443): fixes typos in docs - Features: - [#1672](authts/oidc-client-ts#1672): make `signoutCallback` return signout response if request_type is so:r - [#1626](authts/oidc-client-ts#1626): add `popupSignal` property to `signinPopup` and `signoutPop` - [#1580](authts/oidc-client-ts#1580): add dpop docs - [#1569](authts/oidc-client-ts#1569): add dpop nonce support - [#1457](authts/oidc-client-ts#1457): add extra headers - [#1461](authts/oidc-client-ts#1461): add demonstrating proof of possession - [#1430](authts/oidc-client-ts#1430): add global `requestTimeoutInSeconds` setting - [#1405](authts/oidc-client-ts#1405): allow using default scopes from authorization server thanks to [@klues](https://github.com/klues), [@smujmaiku](https://github.com/smujmaiku), [@mftruso](https://github.com/mftruso), [@peetck](https://github.com/peetck), [@dbfr3qs](https://github.com/dbfr3qs), [@mottykohn](https://github.com/mottykohn), [@noshiro-pf](https://github.com/noshiro-pf), [@dbfr3qs](https://github.com/dbfr3qs), [@grjan7](https://github.com/grjan7) and [@natergj](https://github.com/natergj) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.tristess.app/alexandresoro/ouca/pulls/196 Reviewed-by: Alexandre Soro <[email protected]> Co-authored-by: renovate <[email protected]> Co-committed-by: renovate <[email protected]>

feat: Enable using default scopes from authorization server

f4d11e0

natergj commented Feb 15, 2024

View reviewed changes

pamapa added the enhancement New feature or request label May 7, 2024

pamapa added this to the 3.1.0 milestone May 7, 2024

pamapa changed the title ~~feat: Enable using default scopes from authorization server~~ enable using default scopes from authorization server May 7, 2024

natergj added 2 commits May 7, 2024 06:51

Add depcrecated notice for useIdpDefaultScopes property

7667e58

Merge remote-tracking branch 'upstream/main' into feat-allow-default-…

a8437ab

…scopes

pamapa reviewed May 8, 2024

View reviewed changes

src/OidcClientSettings.ts Outdated Show resolved Hide resolved

natergj added 5 commits May 8, 2024 20:16

Revert "Add depcrecated notice for useIdpDefaultScopes property"

1753dde

This reverts commit 7667e58.

rename symbol to more closely align with spec definitions

93ef2fd

undo changes to optional scope typing

82d518c

resolve unnecessary changes

36d6537

pass omitScopeWhenRequesting to createSigninRequest method

c8e3973

pamapa removed this from the 3.1.0 milestone Jun 21, 2024

Merge remote-tracking branch 'upstream/main' into feat-allow-default-…

88d62c3

…scopes

pamapa reviewed Jun 24, 2024

View reviewed changes

src/OidcClient.ts Outdated Show resolved Hide resolved

pamapa reviewed Jun 24, 2024

View reviewed changes

src/OidcClientSettings.ts Outdated Show resolved Hide resolved

use json object shorthand when passing new attribute

5b56bf8

pamapa added this to the 3.1.0 milestone Jun 24, 2024

add default value for omitScopeWhenRequesting to property doc comment

c192e45

pamapa approved these changes Jun 24, 2024

View reviewed changes

pamapa merged commit e36315d into authts:main Jun 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable using default scopes from authorization server #1405

enable using default scopes from authorization server #1405

natergj commented Feb 15, 2024 •

edited by pamapa

Loading

natergj Feb 15, 2024

pamapa May 7, 2024

natergj May 7, 2024 •

edited

Loading

pamapa May 8, 2024

natergj May 9, 2024

codecov bot commented Mar 13, 2024 •

edited

Loading

natergj commented Jun 24, 2024

pamapa commented Jun 24, 2024

natergj commented Jun 24, 2024

natergj commented Jun 24, 2024

pamapa commented Jun 24, 2024

enable using default scopes from authorization server #1405

enable using default scopes from authorization server #1405

Conversation

natergj commented Feb 15, 2024 • edited by pamapa Loading

Checklist

natergj Feb 15, 2024

Choose a reason for hiding this comment

pamapa May 7, 2024

Choose a reason for hiding this comment

natergj May 7, 2024 • edited Loading

Choose a reason for hiding this comment

pamapa May 8, 2024

Choose a reason for hiding this comment

natergj May 9, 2024

Choose a reason for hiding this comment

codecov bot commented Mar 13, 2024 • edited Loading

Codecov Report

natergj commented Jun 24, 2024

pamapa commented Jun 24, 2024

natergj commented Jun 24, 2024

natergj commented Jun 24, 2024

pamapa commented Jun 24, 2024

natergj commented Feb 15, 2024 •

edited by pamapa

Loading

natergj May 7, 2024 •

edited

Loading

codecov bot commented Mar 13, 2024 •

edited

Loading