diff --git a/packages/cli/src/commands/playtest.ts b/packages/cli/src/commands/playtest.ts index 3f0a4b22..d810ec6b 100644 --- a/packages/cli/src/commands/playtest.ts +++ b/packages/cli/src/commands/playtest.ts @@ -50,6 +50,10 @@ import { toLowerCaseArgParser } from '../util/commands/DevvitCommand.js'; import { DevvitCommand } from '../util/commands/DevvitCommand.js'; import { getSubredditNameWithoutPrefix } from '../util/common-actions/getSubredditNameWithoutPrefix.js'; import { installOnSubreddit } from '../util/common-actions/installOnSubreddit.js'; +import { + canWriteAppVersion, + getAppAuthorizationRole, +} from '../util/authorization/appAuthorization.js'; import { waitUntilVersionBuildComplete } from '../util/common-actions/waitUntilVersionBuildComplete.js'; import { getAppBySlug } from '../util/getAppBySlug.js'; import { killProcessTree, killProcessTreeSync } from '../util/kill-process-tree.js'; @@ -433,10 +437,13 @@ export default class Playtest extends DevvitCommand { } this.project.app.defaultPlaytestSubredditId = this.#appInfo.app.defaultPlaytestSubredditId; - const isOwner = this.#appInfo?.app?.owner?.id === userInfo.id; + const appAuthorizationRole = getAppAuthorizationRole(this.#appInfo, { + id: userInfo.id, + displayName: userInfo.username, + }); await this.#checkIfUserAllowedToPlaytestThisApp( this.project.name, - isOwner, + canWriteAppVersion(appAuthorizationRole), token, flags['employee-update'] ); @@ -869,11 +876,11 @@ export default class Playtest extends DevvitCommand { async #checkIfUserAllowedToPlaytestThisApp( appName: string, - isOwner: boolean, + isAuthorizedToWriteApp: boolean, token: StoredToken, employeeUpdateFlag: boolean ): Promise { - if (isOwner) { + if (isAuthorizedToWriteApp) { return; } @@ -888,7 +895,9 @@ export default class Playtest extends DevvitCommand { // Check if the app name is available, implying this is a first run const appExists = await checkAppNameAvailability(this.#appClient, appName); if (appExists.exists) { - this.error(`That app already exists, and you can't playtest someone else's app!`); + this.error( + `That app already exists, and you are not its owner or a designated maintainer.` + ); } // App doesn't exist - tell the user to run `devvit upload` first diff --git a/packages/cli/src/commands/publish.ts b/packages/cli/src/commands/publish.ts index 9bd424df..5f53b70e 100644 --- a/packages/cli/src/commands/publish.ts +++ b/packages/cli/src/commands/publish.ts @@ -55,6 +55,11 @@ import { DevvitCommand } from '../util/commands/DevvitCommand.js'; import { installOnSubreddit } from '../util/common-actions/installOnSubreddit.js'; import { waitUntilVersionBuildComplete } from '../util/common-actions/waitUntilVersionBuildComplete.js'; import { DEVVIT_PORTAL_URL } from '../util/config.js'; +import { + canWriteAppVersion, + getAppAuthorizationErrorMessage, + getAppAuthorizationRole, +} from '../util/authorization/appAuthorization.js'; import { getAppBySlug } from '../util/getAppBySlug.js'; import { getAppSourceZip } from '../util/getAppSourceZip.js'; import { readLine } from '../util/input-util.js'; @@ -178,7 +183,8 @@ export default class Publish extends DevvitCommand { const token = await getAccessTokenAndLoginIfNeeded( flags['copy-paste'] ? 'CopyPaste' : 'LocalSocket' ); - const username = await this.getUserDisplayName(token); + const userInfo = await this.getUserInfo(token); + const username = userInfo.username; await this.checkDeveloperAccount(); @@ -205,8 +211,11 @@ export default class Publish extends DevvitCommand { const shouldCreatePlaytestSubreddit = !this.project.getSubreddit('Dev'); - const isOwner = appInfo.app.owner?.displayName === username; - if (!isOwner) { + const appAuthorizationRole = getAppAuthorizationRole(appInfo, { + id: userInfo.id, + displayName: username, + }); + if (!canWriteAppVersion(appAuthorizationRole)) { if (flags['employee-update']) { const isEmployee = await isCurrentUserEmployee(token); if (!isEmployee) { @@ -214,9 +223,7 @@ export default class Publish extends DevvitCommand { } this.warn(`Overriding ownership check because you're an employee and told me to!`); } else { - this.error( - `You are not the owner of the app "${this.project.name}". Please check that you are logged in as the correct user (${appInfo.app.owner?.displayName ?? ''}).` - ); + this.error(getAppAuthorizationErrorMessage(appInfo, this.project.name, 'publish')); } } diff --git a/packages/cli/src/commands/upload.ts b/packages/cli/src/commands/upload.ts index 244eb32e..108ae6c8 100644 --- a/packages/cli/src/commands/upload.ts +++ b/packages/cli/src/commands/upload.ts @@ -38,6 +38,10 @@ import { import { DevvitCommand } from '../util/commands/DevvitCommand.js'; import { installOnSubreddit } from '../util/common-actions/installOnSubreddit.js'; import { DEVVIT_PORTAL_URL } from '../util/config.js'; +import { + canWriteAppVersion, + getAppAuthorizationRole, +} from '../util/authorization/appAuthorization.js'; import { getAppBySlug } from '../util/getAppBySlug.js'; import { sendEvent } from '../util/metrics.js'; import { @@ -49,7 +53,7 @@ import { } from './init.js'; export default class Upload extends DevvitCommand { - static override description = `Upload the app to the App Directory. Uploaded apps are only visible to you (the app owner) and can only be installed to a small test subreddit with less than ${MAX_ALLOWED_SUBSCRIBER_COUNT} subscribers`; + static override description = `Upload the app to the App Directory. Uploaded apps are visible to the app owner and designated maintainers and can only be installed to a small test subreddit with less than ${MAX_ALLOWED_SUBSCRIBER_COUNT} subscribers`; static override flags = { bump: Flags.custom({ @@ -125,7 +129,8 @@ export default class Upload extends DevvitCommand { const token = await getAccessTokenAndLoginIfNeeded( flags['copy-paste'] ? 'CopyPaste' : 'LocalSocket' ); - const username = await this.getUserDisplayName(token); + const userInfo = await this.getUserInfo(token); + const username = userInfo.username; await this.checkDeveloperAccount(); @@ -143,14 +148,19 @@ export default class Upload extends DevvitCommand { let shouldCreateNewApp = false; let shouldCreatePlaytestSubreddit = !this.project.getSubreddit('Dev'); - const isOwner = appInfo?.app?.owner?.displayName === username; - if (!isOwner) { + const appAuthorizationRole = getAppAuthorizationRole(appInfo, { + id: userInfo.id, + displayName: username, + }); + if (!canWriteAppVersion(appAuthorizationRole)) { shouldCreateNewApp = true; // Unless... if (flags['employee-update'] || flags['just-do-it']) { const isEmployee = await isCurrentUserEmployee(token); if (!isEmployee) { - this.error(`You're not an employee, so you can't playtest someone else's app.`); + this.error( + `You're not an employee, so you can't upload a version of someone else's app.` + ); } // Else, we're an employee, so we can update someone else's app this.warn(`Overriding ownership check because you're an employee and told me to!`); diff --git a/packages/cli/src/util/authorization/appAuthorization.test.ts b/packages/cli/src/util/authorization/appAuthorization.test.ts new file mode 100644 index 00000000..de4c7ff1 --- /dev/null +++ b/packages/cli/src/util/authorization/appAuthorization.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, test } from 'vitest'; + +import { + canWriteAppVersion, + getAppAuthorizationErrorMessage, + getAppAuthorizationRole, +} from './appAuthorization.js'; + +describe('appAuthorization', () => { + test('recognizes the app owner by stable account ID', () => { + const appInfo = { + app: { + owner: { id: 't2_owner', displayName: 'OriginalOwnerName' }, + }, + }; + + const role = getAppAuthorizationRole(appInfo, { + id: 't2_owner', + displayName: 'RenamedOwner', + }); + + expect(role).toBe('owner'); + expect(canWriteAppVersion(role)).toBe(true); + }); + + test('does not fall back to a matching display name when stable IDs disagree', () => { + const appInfo = { + app: { + owner: { id: 't2_owner', displayName: 'SharedName' }, + }, + }; + + expect( + getAppAuthorizationRole(appInfo, { + id: 't2_other', + displayName: 'SharedName', + }) + ).toBe('unauthorized'); + }); + + test('recognizes a maintainer from typed authorization metadata by account ID', () => { + const appInfo = { + app: { + owner: { id: 't2_owner', displayName: 'OwnerName' }, + authorization: { + maintainers: [{ id: 't2_maintainer', displayName: 'MaintainerName' }], + }, + }, + }; + + const role = getAppAuthorizationRole(appInfo, { + id: 't2_maintainer', + displayName: 'RenamedMaintainer', + }); + + expect(role).toBe('maintainer'); + expect(canWriteAppVersion(role)).toBe(true); + }); + + test('accepts a backend-computed current-user role', () => { + const appInfo = { + app: { + authorization: { + currentUserRole: 'maintainer', + }, + }, + }; + + expect(getAppAuthorizationRole(appInfo, '')).toBe('maintainer'); + }); + + test('temporarily supports the prototype maintainer list by display name', () => { + const appInfo = { + app: { + owner: { displayName: 'OwnerName' }, + maintainers: [{ displayName: 'MaintainerName' }], + }, + }; + + expect(getAppAuthorizationRole(appInfo, ' maintainername ')).toBe('maintainer'); + }); + + test('does not authorize an unrelated developer', () => { + const appInfo = { + app: { + owner: { id: 't2_owner', displayName: 'OwnerName' }, + authorization: { + maintainers: [{ id: 't2_maintainer', displayName: 'MaintainerName' }], + }, + }, + }; + + const role = getAppAuthorizationRole(appInfo, { + id: 't2_other', + displayName: 'OtherDeveloper', + }); + + expect(role).toBe('unauthorized'); + expect(canWriteAppVersion(role)).toBe(false); + }); + + test.each(['owner', 'maintainer'] as const)( + 'allows the %s role to upload, publish, and playtest app versions', + (role) => { + expect(canWriteAppVersion(role)).toBe(true); + } + ); + + test('denies app-version writes when authorization metadata is missing', () => { + expect(getAppAuthorizationRole(undefined, 'OwnerName')).toBe('unauthorized'); + expect(getAppAuthorizationRole({ app: null }, 'OwnerName')).toBe('unauthorized'); + expect(getAppAuthorizationRole({ app: {} }, '')).toBe('unauthorized'); + }); + + test('uses a maintainer-aware error message', () => { + expect( + getAppAuthorizationErrorMessage( + { app: { owner: { displayName: 'OwnerName' } } }, + 'example-app', + 'publish' + ) + ).toContain('owner (OwnerName) or as a designated maintainer'); + }); +}); diff --git a/packages/cli/src/util/authorization/appAuthorization.ts b/packages/cli/src/util/authorization/appAuthorization.ts new file mode 100644 index 00000000..83ee7983 --- /dev/null +++ b/packages/cli/src/util/authorization/appAuthorization.ts @@ -0,0 +1,144 @@ +export type AppAuthorizationRole = 'owner' | 'maintainer' | 'unauthorized'; +export type AppWriteAction = 'upload' | 'publish' | 'playtest'; + +export type DeveloperIdentity = Readonly<{ + id?: string; + displayName?: string; +}>; + +type AppAuthorizationData = Readonly<{ + owner?: DeveloperIdentity; + maintainers: readonly DeveloperIdentity[]; + currentUserRole?: AppAuthorizationRole; +}>; + +type UnknownRecord = Record; + +function asRecord(value: unknown): UnknownRecord | undefined { + return typeof value === 'object' && value !== null ? (value as UnknownRecord) : undefined; +} + +function readString(value: unknown): string | undefined { + return typeof value === 'string' ? value : undefined; +} + +function readIdentity(value: unknown): DeveloperIdentity | undefined { + const record = asRecord(value); + if (!record) return undefined; + + const identity = { + id: readString(record.id), + displayName: readString(record.displayName), + } satisfies DeveloperIdentity; + + return identity.id || identity.displayName ? identity : undefined; +} + +function readIdentities(value: unknown): readonly DeveloperIdentity[] { + if (!Array.isArray(value)) return []; + + return value + .map(readIdentity) + .filter((identity): identity is DeveloperIdentity => identity !== undefined); +} + +function readAuthorizationRole(value: unknown): AppAuthorizationRole | undefined { + return value === 'owner' || value === 'maintainer' || value === 'unauthorized' + ? value + : undefined; +} + +/** + * Compatibility boundary for authorization metadata returned by the app API. + * + * The generated FullAppInfo type does not yet expose maintainer authorization. + * This parser accepts unknown input and supports both a future + * app.authorization payload and the prototype app.maintainers field. Replace + * this parser with the generated type once the backend/protobuf contract lands. + */ +function readAppAuthorizationData(appInfo: unknown): AppAuthorizationData { + const app = asRecord(asRecord(appInfo)?.app); + if (!app) return { maintainers: [] }; + + const authorization = asRecord(app.authorization); + const typedMaintainers = readIdentities(authorization?.maintainers); + + return { + owner: readIdentity(app.owner), + maintainers: typedMaintainers.length > 0 ? typedMaintainers : readIdentities(app.maintainers), + currentUserRole: readAuthorizationRole( + authorization?.currentUserRole ?? app.currentUserRole + ), + }; +} + +function normalizeId(id: string | undefined): string | undefined { + const normalized = id?.trim(); + return normalized || undefined; +} + +function normalizeDisplayName(displayName: string | undefined): string | undefined { + const normalized = displayName?.trim().toLowerCase(); + return normalized || undefined; +} + +function isSameDeveloper( + candidate: DeveloperIdentity | undefined, + currentUser: DeveloperIdentity +): boolean { + if (!candidate) return false; + + const candidateId = normalizeId(candidate.id); + const currentUserId = normalizeId(currentUser.id); + if (candidateId && currentUserId) { + return candidateId === currentUserId; + } + + const candidateDisplayName = normalizeDisplayName(candidate.displayName); + const currentUserDisplayName = normalizeDisplayName(currentUser.displayName); + return !!candidateDisplayName && candidateDisplayName === currentUserDisplayName; +} + +export function getAppAuthorizationRole( + appInfo: unknown, + currentUser: string | DeveloperIdentity +): AppAuthorizationRole { + const authorization = readAppAuthorizationData(appInfo); + + // Prefer a backend-computed role when available. The server must remain the + // authoritative enforcement boundary for all app-version write operations. + if (authorization.currentUserRole) { + return authorization.currentUserRole; + } + + const currentUserIdentity: DeveloperIdentity = + typeof currentUser === 'string' ? { displayName: currentUser } : currentUser; + + if (isSameDeveloper(authorization.owner, currentUserIdentity)) { + return 'owner'; + } + + if ( + authorization.maintainers.some((maintainer) => + isSameDeveloper(maintainer, currentUserIdentity) + ) + ) { + return 'maintainer'; + } + + return 'unauthorized'; +} + +export function canWriteAppVersion(role: AppAuthorizationRole): boolean { + return role === 'owner' || role === 'maintainer'; +} + +export function getAppAuthorizationErrorMessage( + appInfo: unknown, + appName: string, + action: AppWriteAction +): string { + const ownerDisplayName = readAppAuthorizationData(appInfo).owner?.displayName ?? ''; + + return `You are not authorized to ${action} the app "${appName}". Please check that you are logged in as the owner (${ownerDisplayName}) or as a designated maintainer.`; +}