diff --git a/scripts/print-scopes.ts b/scripts/print-scopes.ts new file mode 100644 index 0000000..4d7377c --- /dev/null +++ b/scripts/print-scopes.ts @@ -0,0 +1,19 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Prints the OAuth scopes that should be registered on the GCP consent + * screen, one per line. Sourced from FEATURE_GROUPS so the registration + * list and the runtime request list cannot drift. + * + * Used by scripts/setup-gcp.sh. + */ + +import { getAllPossibleScopes } from '../workspace-server/src/features/feature-config'; + +for (const scope of getAllPossibleScopes()) { + console.log(scope); +} diff --git a/scripts/setup-gcp.sh b/scripts/setup-gcp.sh index 639850e..059fafc 100755 --- a/scripts/setup-gcp.sh +++ b/scripts/setup-gcp.sh @@ -91,19 +91,31 @@ echo -e " 4. Under ${GREEN}Test users${NC}, add the email addresses of anyone" echo -e " who will use this extension (required while in Testing mode)" echo "" -SCOPES=( - "https://www.googleapis.com/auth/documents" - "https://www.googleapis.com/auth/drive" - "https://www.googleapis.com/auth/calendar" - "https://www.googleapis.com/auth/chat.spaces" - "https://www.googleapis.com/auth/chat.messages" - "https://www.googleapis.com/auth/chat.memberships" - "https://www.googleapis.com/auth/userinfo.profile" - "https://www.googleapis.com/auth/gmail.modify" - "https://www.googleapis.com/auth/directory.readonly" - "https://www.googleapis.com/auth/presentations.readonly" - "https://www.googleapis.com/auth/spreadsheets.readonly" -) +# Single source of truth: scopes are computed from FEATURE_GROUPS in +# workspace-server/src/features/feature-config.ts. See issue #323. +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +SCOPES_OUTPUT=$(cd "$REPO_ROOT" && npx --no-install ts-node --transpile-only scripts/print-scopes.ts 2>&1) +if [ $? -ne 0 ]; then + echo -e "${RED}Error: Failed to compute OAuth scopes from feature-config.ts.${NC}" + echo -e "${RED}Did you run 'npm install' at the repo root?${NC}" + echo "$SCOPES_OUTPUT" + exit 1 +fi + +SCOPES=() +# Filter to https:// lines so any Node/ts-node warnings written to stderr +# (captured via 2>&1 above so we can surface them on failure) don't end up +# in the user-visible scope list. +while IFS= read -r line; do + [[ "$line" == https://* ]] && SCOPES+=("$line") +done <<< "$SCOPES_OUTPUT" + +if [ ${#SCOPES[@]} -eq 0 ]; then + echo -e "${RED}Error: print-scopes.ts produced no scope output.${NC}" + echo "$SCOPES_OUTPUT" + exit 1 +fi for scope in "${SCOPES[@]}"; do echo -e " ${GREEN}$scope${NC}" diff --git a/workspace-server/src/__tests__/features/feature-config.test.ts b/workspace-server/src/__tests__/features/feature-config.test.ts index f304664..ce83667 100644 --- a/workspace-server/src/__tests__/features/feature-config.test.ts +++ b/workspace-server/src/__tests__/features/feature-config.test.ts @@ -4,8 +4,16 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { execSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + import { describe, it, expect } from '@jest/globals'; -import { FEATURE_GROUPS, featureGroupKey } from '../../features/feature-config'; +import { + FEATURE_GROUPS, + featureGroupKey, + getAllPossibleScopes, +} from '../../features/feature-config'; describe('feature-config', () => { it('should have unique feature group keys', () => { @@ -57,3 +65,64 @@ describe('feature-config', () => { expect(timeRead!.scopes).toEqual([]); }); }); + +describe('getAllPossibleScopes (issue #323)', () => { + it('should include both write and readonly scopes for paired groups', () => { + const scopes = getAllPossibleScopes(); + // Both must be registered on the consent screen — users may flip + // .write off, which causes .readonly to be requested. + expect(scopes).toContain('https://www.googleapis.com/auth/drive'); + expect(scopes).toContain('https://www.googleapis.com/auth/drive.readonly'); + expect(scopes).toContain('https://www.googleapis.com/auth/gmail.modify'); + expect(scopes).toContain('https://www.googleapis.com/auth/gmail.readonly'); + expect(scopes).toContain('https://www.googleapis.com/auth/calendar'); + expect(scopes).toContain( + 'https://www.googleapis.com/auth/calendar.readonly', + ); + }); + + it('should exclude default-OFF group scopes that are not in any default-ON group', () => { + const scopes = getAllPossibleScopes(); + // tasks.* are default-OFF and their scopes are not shared with any + // default-ON group, so they shouldn't be in the registration list. + expect(scopes).not.toContain('https://www.googleapis.com/auth/tasks'); + expect(scopes).not.toContain( + 'https://www.googleapis.com/auth/tasks.readonly', + ); + }); + + it('should be sorted and deduplicated', () => { + const scopes = getAllPossibleScopes(); + const sortedUnique = [...new Set(scopes)].sort(); + expect(scopes).toEqual(sortedUnique); + }); + + it('print-scopes.ts should emit the same list (drift guard for setup-gcp.sh)', () => { + // setup-gcp.sh shells out to scripts/print-scopes.ts; if this test + // fails, the consent screen registration list will drift from + // FEATURE_GROUPS — which is the bug in issue #323. + const repoRoot = join(__dirname, '..', '..', '..', '..'); + // execSync (not execFileSync) so Windows can resolve npx.cmd via the + // shell. Tests run on ubuntu/macos/windows. + const output = execSync( + 'npx --no-install ts-node --transpile-only scripts/print-scopes.ts', + { cwd: repoRoot, encoding: 'utf8' }, + ); + const printed = output.trim().split(/\r?\n/); + expect(printed).toEqual(getAllPossibleScopes()); + }); + + it('setup-gcp.sh should not contain a hardcoded SCOPES list (drift guard)', () => { + // If someone re-inlines the list, this test catches it. + const repoRoot = join(__dirname, '..', '..', '..', '..'); + const setupScript = readFileSync( + join(repoRoot, 'scripts', 'setup-gcp.sh'), + 'utf8', + ); + // A hardcoded list would have a literal scope URL inside SCOPES=( ... ). + const hardcodedMatch = setupScript.match( + /SCOPES=\(\s*"https:\/\/www\.googleapis\.com\//, + ); + expect(hardcodedMatch).toBeNull(); + }); +}); diff --git a/workspace-server/src/__tests__/features/feature-resolver.test.ts b/workspace-server/src/__tests__/features/feature-resolver.test.ts index c8390c4..fa473f3 100644 --- a/workspace-server/src/__tests__/features/feature-resolver.test.ts +++ b/workspace-server/src/__tests__/features/feature-resolver.test.ts @@ -196,4 +196,58 @@ describe('resolveFeatures', () => { const unique = new Set(requiredScopes); expect(requiredScopes.length).toBe(unique.size); }); + + describe('read/write scope dedup (issue #323)', () => { + const READONLY_PAIRS: Array<[string, string]> = [ + ['drive.readonly', 'drive'], + ['calendar.readonly', 'calendar'], + ['chat.spaces.readonly', 'chat.spaces'], + ['chat.messages.readonly', 'chat.messages'], + ['chat.memberships.readonly', 'chat.memberships'], + ['gmail.readonly', 'gmail.modify'], + ]; + const fullUrl = (s: string) => `https://www.googleapis.com/auth/${s}`; + + it.each(READONLY_PAIRS)( + 'should not request %s when paired write scope is enabled (defaults)', + (readonlyScope, writeScope) => { + const { requiredScopes } = resolveFeatures(); + expect(requiredScopes).not.toContain(fullUrl(readonlyScope)); + expect(requiredScopes).toContain(fullUrl(writeScope)); + }, + ); + + it.each([ + ['drive.write:off', 'drive.readonly'], + ['calendar.write:off', 'calendar.readonly'], + ['gmail.write:off', 'gmail.readonly'], + ['chat.write:off', 'chat.spaces.readonly'], + ])( + 'should request readonly scope when write group disabled (%s)', + (override, readonlyScope) => { + const { requiredScopes } = resolveFeatures(undefined, override); + expect(requiredScopes).toContain(fullUrl(readonlyScope)); + }, + ); + + it('should not affect tool registration — read tools stay enabled when write is on', () => { + const { enabledTools } = resolveFeatures(); + expect(enabledTools.has('drive.search')).toBe(true); + expect(enabledTools.has('gmail.search')).toBe(true); + expect(enabledTools.has('calendar.list')).toBe(true); + expect(enabledTools.has('chat.listSpaces')).toBe(true); + }); + + it('should still include read scopes for services without a write group (people)', () => { + const { requiredScopes } = resolveFeatures(); + expect(requiredScopes).toContain(fullUrl('directory.readonly')); + expect(requiredScopes).toContain(fullUrl('userinfo.profile')); + }); + + it('should still include readonly scopes for default-OFF write groups (slides, sheets)', () => { + const { requiredScopes } = resolveFeatures(); + expect(requiredScopes).toContain(fullUrl('presentations.readonly')); + expect(requiredScopes).toContain(fullUrl('spreadsheets.readonly')); + }); + }); }); diff --git a/workspace-server/src/features/feature-config.ts b/workspace-server/src/features/feature-config.ts index e3a2cb1..5b4e810 100644 --- a/workspace-server/src/features/feature-config.ts +++ b/workspace-server/src/features/feature-config.ts @@ -255,3 +255,23 @@ export const FEATURE_GROUPS: readonly FeatureGroup[] = [ defaultEnabled: false, }, ] as const satisfies readonly FeatureGroup[]; + +/** + * Every scope that any default-enabled feature group could request. + * + * This is the registration list for the OAuth consent screen — broader than + * the runtime request set, because users can disable individual write groups + * via WORKSPACE_FEATURE_OVERRIDES, which causes the paired read group's + * `.readonly` scope to be requested. Both must already be registered, or + * unverified apps hit "This app is blocked." + * + * Returned sorted for stable diffs in `setup-gcp.sh`. + */ +export function getAllPossibleScopes(): string[] { + const set = new Set(); + for (const fg of FEATURE_GROUPS) { + if (!fg.defaultEnabled) continue; + for (const scope of fg.scopes) set.add(scope); + } + return [...set].sort(); +} diff --git a/workspace-server/src/features/feature-resolver.ts b/workspace-server/src/features/feature-resolver.ts index 6551a91..ec9712d 100644 --- a/workspace-server/src/features/feature-resolver.ts +++ b/workspace-server/src/features/feature-resolver.ts @@ -145,7 +145,13 @@ export function resolveFeatures( } } - // Collect enabled tools and scopes + // Collect enabled tools and scopes. + // + // Scope dedup: when a service's `.write` group is enabled alongside its + // `.read` group, the write scope already grants read access at the API + // level, so we skip the read group's scopes. Avoids prompting the user + // for both `drive` and `drive.readonly` (and equivalents) on consent. + // Tools are unaffected — read tools still get registered. const enabledTools = new Set(); const scopeSet = new Set(); @@ -153,9 +159,17 @@ export function resolveFeatures( const key = featureGroupKey(fg); if (!groupEnabled.get(key)) continue; - // Add scopes for this enabled group - for (const scope of fg.scopes) { - scopeSet.add(scope); + const writeKey = `${fg.service}.write`; + const subsumedByWrite = + fg.group === 'read' && + writeKey !== key && + groupIndex.has(writeKey) && + groupEnabled.get(writeKey) === true; + + if (!subsumedByWrite) { + for (const scope of fg.scopes) { + scopeSet.add(scope); + } } // Add tools (minus individually disabled ones) diff --git a/workspace-server/src/features/index.ts b/workspace-server/src/features/index.ts index c94cd04..ea418e5 100644 --- a/workspace-server/src/features/index.ts +++ b/workspace-server/src/features/index.ts @@ -4,7 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -export { FEATURE_GROUPS, featureGroupKey } from './feature-config'; +export { + FEATURE_GROUPS, + featureGroupKey, + getAllPossibleScopes, +} from './feature-config'; export type { FeatureGroup } from './feature-config'; export { resolveFeatures, parseOverrides } from './feature-resolver'; export type { ResolvedFeatures } from './feature-resolver';