Node16 EoL: Remove deprecated API (#886)

* Remove deprecated deny signature * Remove deprecated approceAccessRequest signature * Remove deprecated getAccessgrantAll signature * Fix deprecated signatures in e2e tests
inrupt · Dec 22, 2023 · 9d5060c · 9d5060c · vercel · Dec 22, 2023
1 parent db5abae
commit 9d5060c
Show file tree

Hide file tree

Showing 9 changed files with 165 additions and 587 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,11 @@ The following changes are pending, and will be applied on the next major release
   `getter` functions to get these attributes.
 - **Node 16 is no longer supported**: The global `fetch` function is now used instead of
   `@inrupt/universal-fetch`. This means this library now only works with Node 18 and higher.
+- **Deprecated signatures removed**:
+  - `denyAccessRequest` no longer supports the `resourceOwner` argument, it must be removed.
+  - `approveAccessRequest` no longer supports the `resourceOwner` argument, it must be removed.
+  - `getAccessGrantAll` no longer supports the `resource` argument, which should be merged into
+    the `params` argument along the other `AccessParameter`.
 
 ## [2.6.2](https://github.com/inrupt/solid-client-access-grants-js/releases/tag/v2.6.2) - 2023-11-16
 

diff --git a/e2e/node/e2e.test.ts b/e2e/node/e2e.test.ts
@@ -319,8 +319,7 @@ describe(`End-to-end access grant tests for environment [${environment}] `, () =
           ).resolves.toMatchObject({ errors: [] });
 
           const grantedAccess = await getAccessGrantAll(
-            sharedFileIri,
-            undefined,
+            { resource: sharedFileIri },
             {
               fetch: addUserAgent(resourceOwnerSession.fetch, TEST_USER_AGENT),
               accessEndpoint: vcProvider,
@@ -731,10 +730,13 @@ describe(`End-to-end access grant tests for environment [${environment}] `, () =
         });
 
         it("can filter VCs held by the service based on target resource", async () => {
-          const allGrants = getAccessGrantAll(sharedFilterTestIri, undefined, {
-            fetch: addUserAgent(resourceOwnerSession.fetch, TEST_USER_AGENT),
-            accessEndpoint: vcProvider,
-          });
+          const allGrants = getAccessGrantAll(
+            { resource: sharedFilterTestIri },
+            {
+              fetch: addUserAgent(resourceOwnerSession.fetch, TEST_USER_AGENT),
+              accessEndpoint: vcProvider,
+            },
+          );
           // There should be at least one grant
           await expect(allGrants).resolves.not.toHaveLength(0);
 
@@ -1487,9 +1489,12 @@ describe(`End-to-end access grant tests for environment [${environment}] `, () =
           await expect(requestorFile.text()).resolves.toBe(testFileContent);
 
           // Lookup grants for the target resource, while it has been issued for the container.
-          const grants = await getAccessGrantAll(testFileIri, undefined, {
-            fetch: addUserAgent(resourceOwnerSession.fetch, TEST_USER_AGENT),
-          });
+          const grants = await getAccessGrantAll(
+            { resource: testFileIri },
+            {
+              fetch: addUserAgent(resourceOwnerSession.fetch, TEST_USER_AGENT),
+            },
+          );
           expect(grants.map((grant) => grant.proof)).toContainEqual(
             accessGrant.proof,
           );
@@ -1514,9 +1519,12 @@ describe(`End-to-end access grant tests for environment [${environment}] `, () =
 
           // Lookup grants for the target resource, while it has been issued for the container.
           // There should be no matching grant, because the issued grant is not recursive.
-          const grants = await getAccessGrantAll(testFileIri, undefined, {
-            fetch: addUserAgent(resourceOwnerSession.fetch, TEST_USER_AGENT),
-          });
+          const grants = await getAccessGrantAll(
+            { resource: testFileIri },
+            {
+              fetch: addUserAgent(resourceOwnerSession.fetch, TEST_USER_AGENT),
+            },
+          );
           expect(grants).not.toContainEqual(accessGrant);
         });
 

diff --git a/src/gConsent/discover/redirectToAccessManagementUi.ts b/src/gConsent/discover/redirectToAccessManagementUi.ts
@@ -35,8 +35,6 @@ import type { FetchOptions } from "../../type/FetchOptions";
 import type { RedirectOptions } from "../../type/RedirectOptions";
 import { getResources } from "../../common";
 
-// DEPRECATED: the VC should be sent by IRI, and not by value.
-export const REQUEST_VC_PARAM_NAME = "requestVc";
 export const REQUEST_VC_URL_PARAM_NAME = "requestVcUrl";
 export const REDIRECT_URL_PARAM_NAME = "redirectUrl";
 

diff --git a/src/gConsent/manage/approveAccessRequest.test.ts b/src/gConsent/manage/approveAccessRequest.test.ts
@@ -772,67 +772,6 @@ describe("approveAccessRequest", () => {
     );
   });
 
-  it("issues a proper access grant from a request VC using the deprecated signature", async () => {
-    mockAcpClient();
-    mockAccessApiEndpoint();
-    const mockedVcModule = jest.requireMock(
-      "@inrupt/solid-client-vc",
-    ) as typeof VcClient;
-    const spiedIssueRequest = jest.spyOn(
-      mockedVcModule,
-      "issueVerifiableCredential",
-    );
-    spiedIssueRequest.mockResolvedValueOnce(accessGrantVc);
-    await approveAccessRequest(
-      "https://some.resource.owner",
-      accessRequestVc,
-      undefined,
-      {
-        fetch: jest.fn<typeof fetch>(),
-      },
-    );
-
-    // Tests like this are failing because of the fact that
-    expect(spiedIssueRequest).toHaveBeenCalledWith(
-      `${MOCKED_ACCESS_ISSUER}/issue`,
-      expect.objectContaining({
-        providedConsent: {
-          mode: accessRequestVc.credentialSubject.hasConsent.mode,
-          hasStatus: "https://w3id.org/GConsent#ConsentStatusExplicitlyGiven",
-          forPersonalData:
-            accessRequestVc.credentialSubject.hasConsent.forPersonalData,
-          isProvidedTo: accessRequestVc.credentialSubject.id,
-          forPurpose: [],
-        },
-        inbox: accessRequestVc.credentialSubject.inbox,
-      }),
-      expect.objectContaining({
-        type: ["SolidAccessGrant"],
-      }),
-      expect.anything(),
-    );
-  });
-
-  it("throws if the returned VC is not an Access Grant using the deprecated signature", async () => {
-    mockAcpClient();
-    mockAccessApiEndpoint();
-    const mockedIssue = jest.spyOn(
-      jest.requireMock("@inrupt/solid-client-vc") as typeof VcClient,
-      "issueVerifiableCredential",
-    );
-    mockedIssue.mockResolvedValueOnce(accessRequestVc);
-    await expect(
-      approveAccessRequest(
-        "https://some.resource.owner",
-        accessRequestVc,
-        undefined,
-        {
-          fetch: jest.fn<typeof fetch>(),
-        },
-      ),
-    ).rejects.toThrow();
-  });
-
   it("normalizes equivalent JSON-LD VCs", async () => {
     mockAcpClient();
     mockAccessApiEndpoint();

diff --git a/src/gConsent/manage/approveAccessRequest.ts b/src/gConsent/manage/approveAccessRequest.ts
@@ -19,7 +19,7 @@
 // SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 //
 
-import type { UrlString, WebId } from "@inrupt/solid-client";
+import type { UrlString } from "@inrupt/solid-client";
 // eslint-disable-next-line camelcase
 import { acp_ess_2 } from "@inrupt/solid-client";
 import type {
@@ -140,75 +140,6 @@ async function addVcMatcher(
   );
 }
 
-// The following may be removed and merged back in `approveAccessRequest` once
-// the deprecated signature is removed.
-// eslint-disable-next-line camelcase
-async function internal_approveAccessRequest(
-  // If the VC is specified, all the overrides become optional
-  requestVc: DatasetWithId | URL | UrlString,
-  requestOverride?: Partial<ApproveAccessRequestOverrides>,
-  options?: AccessBaseOptions & WithLegacyJsonFlag,
-): Promise<DatasetWithId>;
-// eslint-disable-next-line camelcase
-async function internal_approveAccessRequest(
-  requestVc: undefined,
-  // If the VC is undefined, then some of the overrides become mandatory
-  requestOverride: ApproveAccessRequestOverrides,
-  options?: AccessBaseOptions & WithLegacyJsonFlag,
-): Promise<DatasetWithId>;
-// eslint-disable-next-line camelcase
-async function internal_approveAccessRequest(
-  requestVc?: DatasetWithId | URL | UrlString,
-  requestOverride?: Partial<ApproveAccessRequestOverrides>,
-  options: AccessBaseOptions & WithLegacyJsonFlag = {},
-): Promise<DatasetWithId> {
-  const internalOptions = {
-    ...options,
-    fetch: options.fetch ?? (await getSessionFetch(options)),
-    updateAcr: options.updateAcr ?? true,
-  };
-
-  const internalGrantOptions = initializeGrantParameters(
-    typeof requestVc !== "undefined"
-      ? await getBaseAccess(
-          requestVc,
-          options,
-          solidVc.SolidAccessRequest,
-          gc.ConsentStatusRequested,
-        )
-      : undefined,
-    requestOverride,
-  );
-
-  const grantBody = getGrantBody({
-    access: internalGrantOptions.access,
-    requestor: internalGrantOptions.requestor,
-    resources: internalGrantOptions.resources,
-    requestorInboxUrl: internalGrantOptions.requestorInboxUrl,
-    purpose: internalGrantOptions.purpose,
-    issuanceDate: internalGrantOptions.issuanceDate,
-    expirationDate: internalGrantOptions.expirationDate ?? undefined,
-    status: gc.ConsentStatusExplicitlyGiven.value,
-    inherit: internalGrantOptions.inherit,
-  });
-
-  const grantedAccess = getAccessModesFromAccessGrant(grantBody);
-
-  if (internalOptions.updateAcr === true) {
-    await addVcMatcher(
-      grantBody.credentialSubject.providedConsent.forPersonalData,
-      grantedAccess,
-      internalOptions,
-    );
-  }
-
-  return issueAccessVc(grantBody, {
-    ...internalOptions,
-    returnLegacyJsonld: options.returnLegacyJsonld,
-    normalize: normalizeAccessGrant,
-  });
-}
-
 /**
  * Approve an access request. The content of the approved access request is provided
  * as a Verifiable Credential which properties may be overridden if necessary.
@@ -346,102 +277,59 @@ export async function approveAccessRequest(
   requestOverride: ApproveAccessRequestOverrides,
   options?: AccessBaseOptions & WithLegacyJsonFlag,
 ): Promise<DatasetWithId>;
-
-/**
- * @deprecated Please remove the `resourceOwner` parameter.
- * @hidden
- */
 export async function approveAccessRequest(
-  resourceOwner: WebId,
-  // If the VC is specified, all the overrides become optional
-  requestVc: DatasetWithId | VerifiableCredential | URL | UrlString,
+  requestVc: DatasetWithId | VerifiableCredential | URL | UrlString | undefined,
   requestOverride?: Partial<ApproveAccessRequestOverrides>,
-  options?: AccessBaseOptions & {
-    returnLegacyJsonld?: true;
-  },
-): Promise<AccessGrant>;
-/**
- * @deprecated Please remove the `resourceOwner` parameter.
- * @hidden
- */
-export async function approveAccessRequest(
-  resourceOwner: WebId,
-  // If the VC is specified, all the overrides become optional
-  requestVc: DatasetWithId | VerifiableCredential | URL | UrlString,
-  requestOverride?: Partial<ApproveAccessRequestOverrides>,
-  options?: AccessBaseOptions & WithLegacyJsonFlag,
-): Promise<DatasetWithId>;
-
-/**
- * @deprecated Please remove the `resourceOwner` parameter.
- * @hidden
- */
-export async function approveAccessRequest(
-  resourceOwner: WebId,
-  requestVc: undefined,
-  // If the VC is undefined, then some of the overrides become mandatory
-  requestOverride: ApproveAccessRequestOverrides,
-  options?: AccessBaseOptions & {
-    returnLegacyJsonld?: true;
-  },
-): Promise<AccessGrant>;
-/**
- * @deprecated Please remove the `resourceOwner` parameter.
- * @hidden
- */
-export async function approveAccessRequest(
-  resourceOwner: WebId,
-  requestVc: undefined,
-  // If the VC is undefined, then some of the overrides become mandatory
-  requestOverride: ApproveAccessRequestOverrides,
-  options?: AccessBaseOptions & WithLegacyJsonFlag,
-): Promise<DatasetWithId>;
-export async function approveAccessRequest(
-  resourceOwnerOrRequestVc:
-    | WebId
-    | DatasetWithId
-    | VerifiableCredential
-    | URL
-    | UrlString
-    | undefined,
-  requestVcOrOverride?:
-    | DatasetWithId
-    | VerifiableCredential
-    | URL
-    | UrlString
-    | Partial<ApproveAccessRequestOverrides>,
-  requestOverrideOrOptions?:
-    | Partial<ApproveAccessRequestOverrides>
-    | AccessBaseOptions,
-  options?: AccessBaseOptions & WithLegacyJsonFlag,
+  options: AccessBaseOptions & WithLegacyJsonFlag = {},
 ): Promise<DatasetWithId> {
-  let requestVc: DatasetWithId | VerifiableCredential | URL | UrlString;
-  let override: Partial<ApproveAccessRequestOverrides>;
-  let internalOptions: AccessBaseOptions & WithLegacyJsonFlag;
+  const internalOptions = {
+    ...options,
+    fetch: options.fetch ?? (await getSessionFetch(options)),
+    updateAcr: options.updateAcr ?? true,
+  };
 
-  if (typeof options === "object") {
-    // The deprecated signature is being used, so ignore the first parameter.
-    requestVc = requestVcOrOverride as DatasetWithId | URL | UrlString;
-    override =
-      requestOverrideOrOptions as Partial<ApproveAccessRequestOverrides>;
-    internalOptions = options;
-  } else {
-    requestVc = resourceOwnerOrRequestVc as
-      | DatasetWithId
-      | VerifiableCredential
-      | URL
-      | UrlString;
-    override = requestVcOrOverride as Partial<ApproveAccessRequestOverrides>;
-    internalOptions = requestOverrideOrOptions as AccessBaseOptions;
+  const internalGrantOptions = initializeGrantParameters(
+    typeof requestVc !== "undefined"
+      ? await getBaseAccess(
+          requestVc,
+          options,
+          solidVc.SolidAccessRequest,
+          gc.ConsentStatusRequested,
+        )
+      : undefined,
+    requestOverride,
+  );
+
+  const grantBody = getGrantBody({
+    access: internalGrantOptions.access,
+    requestor: internalGrantOptions.requestor,
+    resources: internalGrantOptions.resources,
+    requestorInboxUrl: internalGrantOptions.requestorInboxUrl,
+    purpose: internalGrantOptions.purpose,
+    issuanceDate: internalGrantOptions.issuanceDate,
+    expirationDate: internalGrantOptions.expirationDate ?? undefined,
+    status: gc.ConsentStatusExplicitlyGiven.value,
+    inherit: internalGrantOptions.inherit,
+  });
+
+  const grantedAccess = getAccessModesFromAccessGrant(grantBody);
+
+  if (internalOptions.updateAcr === true) {
+    await addVcMatcher(
+      grantBody.credentialSubject.providedConsent.forPersonalData,
+      grantedAccess,
+      internalOptions,
+    );
   }
 
-  const accessGrant = await internal_approveAccessRequest(
-    requestVc,
-    override,
-    internalOptions,
-  );
+  const accessGrant = await issueAccessVc(grantBody, {
+    ...internalOptions,
+    returnLegacyJsonld: options.returnLegacyJsonld,
+    normalize: normalizeAccessGrant,
+  });
+
   if (
-    internalOptions.returnLegacyJsonld !== false
+    options.returnLegacyJsonld !== false
       ? !isBaseAccessGrantVerifiableCredential(accessGrant) ||
         !isAccessGrant(accessGrant)
       : !isRdfjsBaseAccessGrantVerifiableCredential(accessGrant)