fix: GoogleCalendar - Ensure that Delegation Credentials are able to reuse access_token #21389

hariombalhara · 2025-05-19T08:51:25Z

What does this PR do?

Fixes PRI-277

This PR unifies OAuth and JWT authentication by abstracting the existing JWT implementations to use OAuthManager as well. This makes JWT and OAuth approach quite similar and easier to understand.

It in-turn allows us to reuse the access token for JWT flow too which is used by Delegation Credential flow

Key Changes:

Unified Authentication: Created CalendarAuth class that handles both OAuth and JWT authentication through the same interface
Token Reuse: Extended OAuthManager to work with JWT, enabling access token reuse by creating credentials with tokens when needed
getCurrentTokenObject support in OAuthManager
Code Abstraction: Refactored existing OAuth and JWT code into a cleaner, shared architecture
Enhanced Token Management: JWT now benefits from the same token expiry checking and refresh mechanisms as OAuth
Better token Expiry handling - Earlier we were considering a token as expired when it was actually expired but now we consider it expired if it will expire in next 5 seconds.
Better error reporting from OAuthManager

Tests

Added more tests for CalendarService and moved auth based tests to separate CalendarService.auth.test.ts module. The tests in auth.test.ts dont mock OAuthManager and are better able to test auth flow
Added more tests for OAuthManager to test new functionality

Mandatory Tasks (DO NOT REMOVE)

I have self-reviewed the code (A decent size PR without self-review might be rejected).
I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

github-actions · 2025-05-19T08:51:38Z

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "persist-access-token-dwd". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

hariombalhara · 2025-05-19T08:51:40Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

vercel · 2025-05-19T08:51:42Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
cal	⬜️ Ignored (Inspect)	Visit Preview		May 26, 2025 8:50am

hariombalhara · 2025-05-20T06:57:23Z

packages/app-store/_utils/oauth/OAuthManager.test.ts

      expect(async () => {
        return auth.getTokenObjectOrFetch();
-      }).rejects.toThrowError("Invalid token response");
+      }).rejects.toThrowError("fetchNewTokenObject error");


Now we report the actual error

hariombalhara · 2025-05-20T07:29:08Z

packages/app-store/_utils/oauth/OAuthManager.ts

+    if (json && json.myFetchError) {
+      // Throw error back as it isn't a valid token response and we can't process it further
+      throw new Error(json.myFetchError);
+    }


We report the actual error now instead of going forward with this json and then being unable to parse the obviously invalid token and then throwing 'Invalid token response' error.

This was hiding the actual reason behind the error.

hariombalhara · 2025-05-20T07:30:10Z

packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts

Most of the helper functions are moved to utils.ts so that they can be shared with CalendarService.auth.test.ts as well

Also, all the auth testing related tests are moved to CalendarService.auth.test.ts where we don't mock OAuthManager and are more reliably to test Authentication.

hariombalhara · 2025-05-20T07:31:40Z

packages/app-store/googlecalendar/lib/__tests__/CalendarService.auth.test.ts

This test module has all the auth related tests moved from CalendarService.test.ts

github-actions · 2025-05-20T11:49:30Z

E2E results are ready!

Workflow #61631.3 latest results

hariombalhara · 2025-05-21T10:14:01Z

packages/lib/piiFreeData.ts

+  return {
+    ...credential,
+    key: booleanKeyStatus,
+    delegatedTo: !!credential.delegatedTo,


Ensure that delegatedTo isn't logged fully.

hariombalhara · 2025-05-21T10:14:23Z

packages/lib/delegationCredential/server.ts

+    safeStringify({
+      enrichedHosts: enrichedHosts.map((host) => {
+        return {
+          userId: host.user.id,


Don't log anything more than what we need.

hariombalhara · 2025-05-21T10:15:15Z

packages/app-store/googlecalendar/lib/__mocks__/googleapis.ts

+vi.mock("googleapis-common", async () => {
+  const actual = await vi.importActual("googleapis-common");
+  return {
+    ...actual,
+    OAuth2Client: vi.fn().mockImplementation((...args: [string, string, string]) => {
+      lastCreatedOAuth2Client = {
+        type: "oauth2",
+        args,
+        setCredentials: setCredentialsMock,
+        isTokenExpiring: vi.fn().mockReturnValue(true),
+        refreshToken: vi.fn().mockResolvedValue({
+          res: {
+            data: MOCK_OAUTH2_TOKEN,
+            status: 200,
+            statusText: "OK",
+          },
+        }),
+      };
+      return lastCreatedOAuth2Client;
+    }),
+    JWT: vi.fn().mockImplementation((config: MockJWT["config"]) => {
+      lastCreatedJWT = {
+        type: "jwt",
+        config,
+        authorize: vi.fn().mockResolvedValue(undefined),
+      };
+      return lastCreatedJWT;
+    }),
+  };
+});
+vi.mock("@googleapis/admin", () => adminMock);
+vi.mock("@googleapis/calendar", () => calendarMock);


Moved these here from CalendarService.test.ts

hariombalhara · 2025-05-21T10:15:38Z

packages/app-store/googlecalendar/lib/__mocks__/features.repository.ts

Moved here fromCalendarService.test.ts

hariombalhara · 2025-05-21T10:15:44Z

packages/app-store/googlecalendar/lib/__mocks__/getGoogleAppKeys.ts

Moved here fromCalendarService.test.ts

linear · 2025-05-23T11:05:07Z

PRI-277

graphite-app · 2025-05-23T11:05:11Z

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (05/23/25)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add foundation team as reviewer" took an action on this PR • (05/25/25)

1 reviewer was added to this PR based on Keith Williams's automation.

cubic-dev-ai

cubic found 12 issues across 20 files. Review them in cubic.dev

_{React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.}

packages/app-store/googlecalendar/lib/__tests__/utils.ts

cubic-dev-ai · 2025-05-23T11:08:09Z

packages/app-store/_utils/oauth/updateTokenObject.ts

+    });
+
+    // If no delegation-credential is updated, create one
+    if (updated.count === 0) {


Creating a new delegation-credential after an updateMany that returned 0 is vulnerable to a classic race condition. If two parallel requests run this code, both can see count === 0 and each create a new row, leading to duplicate credentials. This should be handled with an atomic upsert or a Prisma transaction.

An upsert isn't possible at the moment because userId and delegationCredentialId isn't unique

packages/app-store/googlecalendar/lib/CalendarAuth.ts

packages/prisma/schema.prisma

packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts

packages/app-store/googlecalendar/lib/__tests__/CalendarService.auth.test.ts

scripts/prepare-local-for-delegation-credentials-testing.js

packages/app-store/_utils/oauth/updateTokenObject.ts

packages/lib/server/repository/credential.ts

packages/app-store/googlecalendar/lib/CalendarAuth.ts

Udit-takkar · 2025-05-26T08:44:26Z

E2E tests are failing @hariombalhara

hariombalhara · 2025-05-26T11:16:56Z

@Udit-takkar Passing now !! It was build issue from github

keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels May 19, 2025

vercel bot deployed to Preview – dev May 19, 2025 09:20 View deployment

hariombalhara force-pushed the 05-19-persist-access-token-dwd branch from 557e870 to 99a913a Compare May 19, 2025 11:40

vercel bot deployed to Preview – dev May 19, 2025 11:58 View deployment

vercel bot deployed to Preview – dev May 20, 2025 06:00 View deployment

hariombalhara commented May 20, 2025

View reviewed changes

vercel bot deployed to Preview – dev May 20, 2025 09:30 View deployment

hariombalhara force-pushed the 05-19-persist-access-token-dwd branch from 38c2dbc to 8a75800 Compare May 20, 2025 10:17

hariombalhara changed the title ~~persist-access-token-dwd~~ fix: Ensure that Delegation Credential are able to reuse access_token May 20, 2025

hariombalhara force-pushed the 05-19-persist-access-token-dwd branch from 8a75800 to 66a4889 Compare May 20, 2025 10:29

vercel bot deployed to Preview – dev May 20, 2025 10:52 View deployment

hariombalhara added the ready-for-e2e label May 20, 2025

hariombalhara commented May 21, 2025

View reviewed changes

packages/app-store/googlecalendar/lib/__mocks__/features.repository.ts

Copy link

Member Author

hariombalhara May 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved here fromCalendarService.test.ts

hariombalhara commented May 21, 2025

View reviewed changes

packages/app-store/googlecalendar/lib/__mocks__/getGoogleAppKeys.ts

Copy link

Member Author

hariombalhara May 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved here fromCalendarService.test.ts

vercel bot deployed to Preview – dev May 22, 2025 04:03 View deployment

hariombalhara changed the base branch from main to graphite-base/21389 May 22, 2025 09:04

hariombalhara force-pushed the 05-19-persist-access-token-dwd branch from adf5260 to 64b5781 Compare May 22, 2025 09:04

hariombalhara changed the base branch from graphite-base/21389 to 05-22-fix_error_in_selected-calendars_cron_for_delegation_credential May 22, 2025 09:04

hariombalhara mentioned this pull request May 22, 2025

fix: Error in selected-calendars cron of Delegation Credentials due to plus based emails #21465

Merged

hariombalhara requested a review from zomars May 23, 2025 11:07

hariombalhara self-assigned this May 23, 2025

hariombalhara added the High priority Created by Linear-GitHub Sync label May 23, 2025

cubic-dev-ai bot reviewed May 23, 2025

View reviewed changes

hariombalhara changed the title ~~fix: GoogleCalendar - Ensure that Delegation Credential are able to reuse access_token~~ fix: GoogleCalendar - Ensure that Delegation Credentials are able to reuse access_token May 23, 2025

vercel bot deployed to Preview – dev May 23, 2025 11:38 View deployment

zomars enabled auto-merge (squash) May 23, 2025 20:50

zomars previously approved these changes May 23, 2025

View reviewed changes

vercel bot deployed to Preview – dev May 23, 2025 21:47 View deployment

vercel bot deployed to Preview – dev May 23, 2025 23:42 View deployment

vercel bot deployed to Preview – dev May 24, 2025 14:08 View deployment

persist-access-token-dwd

d778249

hariombalhara dismissed zomars’s stale review via d778249 May 25, 2025 03:42

hariombalhara force-pushed the 05-19-persist-access-token-dwd branch from 93a047b to d778249 Compare May 25, 2025 03:42

graphite-app bot requested a review from a team May 25, 2025 03:42

github-actions bot added the ❗️ migrations contains migration files label May 25, 2025

vercel bot deployed to Preview – dev May 25, 2025 04:15 View deployment

vercel bot deployed to Preview – dev May 26, 2025 06:48 View deployment

Add missing appId

ef45c63

hariombalhara force-pushed the 05-19-persist-access-token-dwd branch from 36efbb3 to ef45c63 Compare May 26, 2025 07:11

vercel bot deployed to Preview – dev May 26, 2025 07:42 View deployment

Merge branch 'main' into 05-19-persist-access-token-dwd

0c13e1b

vercel bot deployed to Preview – dev May 26, 2025 09:20 View deployment

emrysal approved these changes May 26, 2025

View reviewed changes

zomars merged commit 30060f0 into main May 26, 2025
84 of 88 checks passed

zomars deleted the 05-19-persist-access-token-dwd branch May 26, 2025 14:19

fix: GoogleCalendar - Ensure that Delegation Credentials are able to reuse access_token #21389

fix: GoogleCalendar - Ensure that Delegation Credentials are able to reuse access_token #21389

Uh oh!

Conversation

hariombalhara commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Tests

Mandatory Tasks (DO NOT REMOVE)

How should this be tested?

Uh oh!

github-actions bot commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hariombalhara commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

E2E results are ready!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linear bot commented May 23, 2025

Uh oh!

graphite-app bot commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Graphite Automations

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai bot May 23, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Udit-takkar commented May 26, 2025

Uh oh!

hariombalhara commented May 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

hariombalhara commented May 19, 2025 •

edited

Loading

github-actions bot commented May 19, 2025 •

edited

Loading

hariombalhara commented May 19, 2025 •

edited

Loading

vercel bot commented May 19, 2025 •

edited

Loading

github-actions bot commented May 20, 2025 •

edited

Loading

graphite-app bot commented May 23, 2025 •

edited

Loading