Repository: authts/oidc-client-ts
Pull request #1405 (2024-02-15 19:36:15)

enable using default scopes from authorization server
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.

<!-- Please link relevant issue numbers or provide context for this change -->
Closes/fixes # n/a

### Checklist

- [x] This PR makes changes to the public API <!-- was the API report (docs/oidc-client-ts.api.md) updated by this PR? -->
- [x] N/A I have included links for closing relevant issue numbers


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/authts/oidc-client-ts/pull/1405?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=authts) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 80.56%. Comparing base [(`c805fdd`)](https://app.codecov.io/gh/authts/oidc-client-ts/commit/c805fdd58cb3719d7296f0e9987ae7d5bdff6766?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=authts) to head [(`f4d11e0`)](https://app.codecov.io/gh/authts/oidc-client-ts/pull/1405?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=authts).
> Report is 20 commits behind head on main.


<details><summary>Additional details and impacted files</summary>


```diff
@@            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](https://app.codecov.io/gh/authts/oidc-client-ts/pull/1405/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=authts) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/authts/oidc-client-ts/pull/1405/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=authts) | `80.56% <100.00%> (+0.03%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=authts#carryforward-flags-in-the-pull-request-comment) to find out more.


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/authts/oidc-client-ts/pull/1405?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=authts).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=authts).

natergj: @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: > @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. 
natergj: > 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

1. 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. 

2. 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.

3. 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.  

Patch:
diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md
index 1d9040d6..0cf154c4 100644
--- a/docs/oidc-client-ts.api.md
+++ b/docs/oidc-client-ts.api.md
@@ -301,7 +301,7 @@ export class OidcClient {
     // (undocumented)
     clearStaleState(): Promise<void>;
     // (undocumented)
-    createSigninRequest({ state, request, request_uri, request_type, id_token_hint, login_hint, skipUserInfo, nonce, url_state, response_type, scope, redirect_uri, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, extraQueryParams, extraTokenParams, }: CreateSigninRequestArgs): Promise<SigninRequest>;
+    createSigninRequest({ state, request, request_uri, request_type, id_token_hint, login_hint, skipUserInfo, nonce, url_state, response_type, scope, redirect_uri, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, extraQueryParams, extraTokenParams, omitScopeWhenRequesting, }: CreateSigninRequestArgs): Promise<SigninRequest>;
     // (undocumented)
     createSignoutRequest({ state, id_token_hint, client_id, request_type, post_logout_redirect_uri, extraQueryParams, }?: CreateSignoutRequestArgs): Promise<SignoutRequest>;
     // (undocumented)
@@ -365,6 +365,7 @@ export interface OidcClientSettings {
     metadataSeed?: Partial<OidcMetadata>;
     // (undocumented)
     metadataUrl?: string;
+    omitScopeWhenRequesting?: boolean;
     post_logout_redirect_uri?: string;
     prompt?: string;
     redirect_uri: string;
@@ -383,7 +384,7 @@ export interface OidcClientSettings {
 
 // @public
 export class OidcClientSettingsStore {
-    constructor({ authority, metadataUrl, metadata, signingKeys, metadataSeed, client_id, client_secret, response_type, scope, redirect_uri, post_logout_redirect_uri, client_authentication, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, filterProtocolClaims, loadUserInfo, requestTimeoutInSeconds, staleStateAgeInSeconds, mergeClaimsStrategy, disablePKCE, stateStore, revokeTokenAdditionalContentTypes, fetchRequestCredentials, refreshTokenAllowedScope, extraQueryParams, extraTokenParams, extraHeaders, }: OidcClientSettings);
+    constructor({ authority, metadataUrl, metadata, signingKeys, metadataSeed, client_id, client_secret, response_type, scope, redirect_uri, post_logout_redirect_uri, client_authentication, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, filterProtocolClaims, loadUserInfo, requestTimeoutInSeconds, staleStateAgeInSeconds, mergeClaimsStrategy, disablePKCE, stateStore, revokeTokenAdditionalContentTypes, fetchRequestCredentials, refreshTokenAllowedScope, extraQueryParams, extraTokenParams, extraHeaders, omitScopeWhenRequesting, }: OidcClientSettings);
     // (undocumented)
     readonly acr_values: string | undefined;
     // (undocumented)
@@ -423,6 +424,8 @@ export class OidcClientSettingsStore {
     // (undocumented)
     readonly metadataUrl: string;
     // (undocumented)
+    readonly omitScopeWhenRequesting: boolean;
+    // (undocumented)
     readonly post_logout_redirect_uri: string | undefined;
     // (undocumented)
     readonly prompt: string | undefined;
@@ -626,7 +629,7 @@ export type SigninRedirectArgs = RedirectParams & ExtraSigninRequestArgs;
 // @public (undocumented)
 export class SigninRequest {
     // (undocumented)
-    static create({ url, authority, client_id, redirect_uri, response_type, scope, state_data, response_mode, request_type, client_secret, nonce, url_state, resource, skipUserInfo, extraQueryParams, extraTokenParams, disablePKCE, ...optionalParams }: SigninRequestCreateArgs): Promise<SigninRequest>;
+    static create({ url, authority, client_id, redirect_uri, response_type, scope, state_data, response_mode, request_type, client_secret, nonce, url_state, resource, skipUserInfo, extraQueryParams, extraTokenParams, disablePKCE, omitScopeWhenRequesting, ...optionalParams }: SigninRequestCreateArgs): Promise<SigninRequest>;
     // (undocumented)
     readonly state: SigninState;
     // (undocumented)
@@ -660,6 +663,8 @@ export interface SigninRequestCreateArgs {
     // (undocumented)
     nonce?: string;
     // (undocumented)
+    omitScopeWhenRequesting?: boolean;
+    // (undocumented)
     prompt?: string;
     // (undocumented)
     redirect_uri: string;
diff --git a/src/OidcClient.ts b/src/OidcClient.ts
index 1793ec2b..8a26c944 100644
--- a/src/OidcClient.ts
+++ b/src/OidcClient.ts
@@ -111,6 +111,7 @@ export class OidcClient {
         response_mode = this.settings.response_mode,
         extraQueryParams = this.settings.extraQueryParams,
         extraTokenParams = this.settings.extraTokenParams,
+        omitScopeWhenRequesting = this.settings.omitScopeWhenRequesting,
     }: CreateSigninRequestArgs): Promise<SigninRequest> {
         const logger = this._logger.create("createSigninRequest");
 
@@ -136,6 +137,7 @@ export class OidcClient {
             skipUserInfo,
             nonce,
             disablePKCE: this.settings.disablePKCE,
+            omitScopeWhenRequesting,
         });
 
         // house cleaning
diff --git a/src/OidcClientSettings.ts b/src/OidcClientSettings.ts
index 0c90c5fb..ada34bff 100644
--- a/src/OidcClientSettings.ts
+++ b/src/OidcClientSettings.ts
@@ -142,6 +142,12 @@ export interface OidcClientSettings {
      * Defines request timeouts globally across all requests made to the authorisation server
      */
     requestTimeoutInSeconds?: number | undefined;
+
+    /**
+     * 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. (Default: false)
+     */
+    omitScopeWhenRequesting?: boolean;
 }
 
 /**
@@ -181,6 +187,7 @@ export class OidcClientSettingsStore {
     public readonly loadUserInfo: boolean;
     public readonly staleStateAgeInSeconds: number;
     public readonly mergeClaimsStrategy: { array: "replace" | "merge" };
+    public readonly omitScopeWhenRequesting: boolean;
 
     public readonly stateStore: StateStore;
 
@@ -220,6 +227,7 @@ export class OidcClientSettingsStore {
         extraQueryParams = {},
         extraTokenParams = {},
         extraHeaders = {},
+        omitScopeWhenRequesting = false,
     }: OidcClientSettings) {
 
         this.authority = authority;
@@ -260,6 +268,7 @@ export class OidcClientSettingsStore {
         this.loadUserInfo = !!loadUserInfo;
         this.staleStateAgeInSeconds = staleStateAgeInSeconds;
         this.mergeClaimsStrategy = mergeClaimsStrategy;
+        this.omitScopeWhenRequesting = omitScopeWhenRequesting;
         this.disablePKCE = !!disablePKCE;
         this.revokeTokenAdditionalContentTypes = revokeTokenAdditionalContentTypes;
 
diff --git a/src/SigninRequest.ts b/src/SigninRequest.ts
index b44bfb53..f1fae6ca 100644
--- a/src/SigninRequest.ts
+++ b/src/SigninRequest.ts
@@ -43,6 +43,7 @@ export interface SigninRequestCreateArgs {
     /** custom "state", which can be used by a caller to have "data" round tripped */
     state_data?: unknown;
     url_state?: string;
+    omitScopeWhenRequesting?: boolean;
 }
 
 /**
@@ -72,6 +73,7 @@ export class SigninRequest {
         extraQueryParams,
         extraTokenParams,
         disablePKCE,
+        omitScopeWhenRequesting,
         ...optionalParams
     }: SigninRequestCreateArgs): Promise<SigninRequest> {
         if (!url) {
@@ -114,7 +116,9 @@ export class SigninRequest {
         parsedUrl.searchParams.append("client_id", client_id);
         parsedUrl.searchParams.append("redirect_uri", redirect_uri);
         parsedUrl.searchParams.append("response_type", response_type);
-        parsedUrl.searchParams.append("scope", scope);
+        if (!omitScopeWhenRequesting) {
+            parsedUrl.searchParams.append("scope", scope);
+        }
         if (nonce) {
             parsedUrl.searchParams.append("nonce", nonce);
         }
diff --git a/src/TokenClient.ts b/src/TokenClient.ts
index ff94e4ad..43cb26c9 100644
--- a/src/TokenClient.ts
+++ b/src/TokenClient.ts
@@ -160,7 +160,10 @@ export class TokenClient {
             logger.throw(new Error("A client_id is required"));
         }
 
-        const params = new URLSearchParams({ grant_type, scope });
+        const params = new URLSearchParams({ grant_type });
+        if (!this._settings.omitScopeWhenRequesting) {
+            params.set("scope", scope);
+        }
         for (const [key, value] of Object.entries(args)) {
             if (value != null) {
                 params.set(key, value);


Test patch:
diff --git a/src/OidcClient.test.ts b/src/OidcClient.test.ts
index 8999e7d4..7bf46838 100644
--- a/src/OidcClient.test.ts
+++ b/src/OidcClient.test.ts
@@ -151,6 +151,34 @@ describe("OidcClient", () => {
             expect(url.match(/state=.*%3Burl_state/)).toBeTruthy();
         });
 
+        it("should exclude scope from SigninRequest if omitScopeWhenRequesting is true", async () => {
+            // arrange
+            jest.spyOn(subject.metadataService, "getAuthorizationEndpoint").mockImplementation(() => Promise.resolve("http://sts/authorize"));
+
+            // act
+            const request = await subject.createSigninRequest({
+                state: "foo",
+                response_type: "code",
+                scope: "baz",
+                redirect_uri: "quux",
+                prompt: "p",
+                display: "d",
+                max_age: 42,
+                ui_locales: "u",
+                id_token_hint: "ith",
+                login_hint: "lh",
+                acr_values: "av",
+                resource: "res",
+                url_state: "url_state",
+                omitScopeWhenRequesting: true,
+            });
+
+            // assert
+            expect(request.state.data).toEqual("foo");
+            const url = request.url;
+            expect(url).not.toContain("scope");
+        });
+
         it("should fail if implicit flow requested", async () => {
             // arrange
             const args = {
diff --git a/src/OidcClientSettings.test.ts b/src/OidcClientSettings.test.ts
index 9bd1adb2..d6637d2c 100644
--- a/src/OidcClientSettings.test.ts
+++ b/src/OidcClientSettings.test.ts
@@ -530,4 +530,32 @@ describe("OidcClientSettings", () => {
             expect(subject.extraHeaders).toEqual(extraHeaders);
         });
     });
+
+    describe("omitScopeWhenRequesting", () => {
+
+        it("should use default value", () => {
+            // act
+            const subject = new OidcClientSettingsStore({
+                authority: "authority",
+                client_id: "client",
+                redirect_uri: "redirect",
+            });
+
+            // assert
+            expect(subject.omitScopeWhenRequesting).toEqual(false);
+        });
+
+        it("should return value from initial settings", () => {
+            // act
+            const subject = new OidcClientSettingsStore({
+                authority: "authority",
+                client_id: "client",
+                redirect_uri: "redirect",
+                omitScopeWhenRequesting: true,
+            });
+
+            // assert
+            expect(subject.omitScopeWhenRequesting).toEqual(true);
+        });
+    });
 });
diff --git a/src/SigninRequest.test.ts b/src/SigninRequest.test.ts
index a2e779da..7081b1e3 100644
--- a/src/SigninRequest.test.ts
+++ b/src/SigninRequest.test.ts
@@ -66,6 +66,18 @@ describe("SigninRequest", () => {
             expect(url).toContain("scope=openid");
         });
 
+        it("should not include scope if omitScopeWhenRequesting is true", async () => {
+            // arrange
+            settings.omitScopeWhenRequesting = true;
+
+            // act
+            subject = await SigninRequest.create(settings);
+            url = subject.url;
+
+            // assert
+            expect(url).not.toContain("scope");
+        });
+
         it("should include state", () => {
             // assert
             expect(url).toContain("state=" + subject.state.id);
diff --git a/src/TokenClient.test.ts b/src/TokenClient.test.ts
index a3990bfd..38b5e4aa 100644
--- a/src/TokenClient.test.ts
+++ b/src/TokenClient.test.ts
@@ -361,6 +361,32 @@ describe("TokenClient", () => {
             expect(params).toHaveProperty("client_secret", "client_secret");
         });
 
+        it("should not include scope if omitScopeWhenRequesting is true", async () => {
+            settings.omitScopeWhenRequesting = true;
+            subject = new TokenClient(new OidcClientSettingsStore(settings), metadataService);
+            const getTokenEndpointMock = jest.spyOn(subject["_metadataService"], "getTokenEndpoint")
+                .mockResolvedValue("http://sts/token_endpoint");
+            const postFormMock = jest.spyOn(subject["_jsonService"], "postForm")
+                .mockResolvedValue({});
+
+            // act
+            await subject.exchangeRefreshToken({ refresh_token: "refresh_token" });
+
+            // assert
+            expect(getTokenEndpointMock).toHaveBeenCalledWith(false);
+            expect(postFormMock).toHaveBeenCalledWith(
+                "http://sts/token_endpoint",
+                expect.objectContaining({
+                    body: expect.any(URLSearchParams),
+                    basicAuth: undefined,
+                    timeoutInSeconds: undefined,
+                }),
+            );
+            const opts = postFormMock.mock.calls[0][1];
+            const params = Object.fromEntries(opts.body);
+            expect(params).not.toHaveProperty("scope");
+        });
+
         it("should call postForm", async () => {
             // arrange
             const getTokenEndpointMock = jest.spyOn(subject["_metadataService"], "getTokenEndpoint")