From 105788afa020e37dc3cfb39e7acbaf4ef7109bf5 Mon Sep 17 00:00:00 2001 From: CR29-22-2805 <266851988+CR29-22-2805@users.noreply.github.com> Date: Thu, 9 Jul 2026 10:06:22 -0500 Subject: [PATCH 1/2] Add maintainer role for Devvit apps --- packages/cli/src/commands/publish.ts | 13 ++-- packages/cli/src/commands/upload.ts | 8 ++- .../authorization/appAuthorization.test.ts | 62 +++++++++++++++++++ .../util/authorization/appAuthorization.ts | 55 ++++++++++++++++ 4 files changed, 131 insertions(+), 7 deletions(-) create mode 100644 packages/cli/src/util/authorization/appAuthorization.test.ts create mode 100644 packages/cli/src/util/authorization/appAuthorization.ts diff --git a/packages/cli/src/commands/publish.ts b/packages/cli/src/commands/publish.ts index 9bd424df..713a48cb 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'; @@ -205,8 +210,8 @@ 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, username); + if (!canWriteAppVersion(appAuthorizationRole)) { if (flags['employee-update']) { const isEmployee = await isCurrentUserEmployee(token); if (!isEmployee) { @@ -214,9 +219,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..61bf6d59 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 { @@ -143,8 +147,8 @@ 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, username); + if (!canWriteAppVersion(appAuthorizationRole)) { shouldCreateNewApp = true; // Unless... if (flags['employee-update'] || flags['just-do-it']) { 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..703fd62f --- /dev/null +++ b/packages/cli/src/util/authorization/appAuthorization.test.ts @@ -0,0 +1,62 @@ +// eslint-disable-next-line no-restricted-imports +import type { FullAppInfo } from '@devvit/protos/types/devvit/dev_portal/app/app.js'; +import { describe, expect, test } from 'vitest'; + +import { + canWriteAppVersion, + getAppAuthorizationErrorMessage, + getAppAuthorizationRole, +} from './appAuthorization.js'; + +describe('appAuthorization', () => { + test('allows the app owner to write app versions', () => { + const appInfo = { + app: { + owner: { displayName: 'OwnerName' }, + }, + }; + + const role = getAppAuthorizationRole(appInfo as FullAppInfo, 'ownername'); + + expect(role).toBe('owner'); + expect(canWriteAppVersion(role)).toBe(true); + }); + + test('allows a designated app maintainer to write app versions', () => { + const appInfo = { + app: { + owner: { displayName: 'OwnerName' }, + maintainers: [{ displayName: 'MaintainerName' }], + }, + }; + + const role = getAppAuthorizationRole(appInfo as FullAppInfo, 'maintainername'); + + expect(role).toBe('maintainer'); + expect(canWriteAppVersion(role)).toBe(true); + }); + + test('does not allow unrelated developers to write app versions', () => { + const appInfo = { + app: { + owner: { displayName: 'OwnerName' }, + maintainers: [{ displayName: 'MaintainerName' }], + }, + }; + + const role = getAppAuthorizationRole(appInfo as FullAppInfo, 'OtherDeveloper'); + + expect(role).toBe('unauthorized'); + expect(canWriteAppVersion(role)).toBe(false); + }); + + test('uses a maintainer-aware error message', () => { + expect( + getAppAuthorizationErrorMessage( + { app: { owner: { displayName: 'OwnerName' } } } as FullAppInfo, + '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..8975f42d --- /dev/null +++ b/packages/cli/src/util/authorization/appAuthorization.ts @@ -0,0 +1,55 @@ +// eslint-disable-next-line no-restricted-imports +import type { FullAppInfo } from '@devvit/protos/types/devvit/dev_portal/app/app.js'; + +export type AppAuthorizationRole = 'owner' | 'maintainer' | 'unauthorized'; +export type AppWriteAction = 'upload' | 'publish'; + +type DeveloperDisplayName = { + displayName?: string; +}; + +type AppWithMaintainers = NonNullable & { + maintainers?: readonly DeveloperDisplayName[]; +}; + +function normalizeDisplayName(displayName: string | undefined): string | undefined { + return displayName?.trim().toLowerCase(); +} + +export function getAppAuthorizationRole( + appInfo: FullAppInfo | undefined, + username: string +): AppAuthorizationRole { + const app = appInfo?.app as AppWithMaintainers | undefined; + const normalizedUsername = normalizeDisplayName(username); + + if (!app || !normalizedUsername) { + return 'unauthorized'; + } + + if (normalizeDisplayName(app.owner?.displayName) === normalizedUsername) { + return 'owner'; + } + + if ( + app.maintainers?.some( + (maintainer) => normalizeDisplayName(maintainer.displayName) === normalizedUsername + ) + ) { + return 'maintainer'; + } + + return 'unauthorized'; +} + +export function canWriteAppVersion(role: AppAuthorizationRole): boolean { + return role === 'owner' || role === 'maintainer'; +} + +export function getAppAuthorizationErrorMessage( + appInfo: FullAppInfo, + appName: string, + action: AppWriteAction +): string { + return `You are not authorized to ${action} the app "${appName}". Please check that you are logged in as the owner (${appInfo.app?.owner?.displayName ?? ''}) or as a designated maintainer.`; +} From bd0cfdc3671f7b095668f366053ea48509de3487 Mon Sep 17 00:00:00 2001 From: CR29-22-2805 <266851988+CR29-22-2805@users.noreply.github.com> Date: Tue, 14 Jul 2026 16:41:06 -0500 Subject: [PATCH 2/2] Strengthen maintainer authorization prototype --- packages/cli/src/commands/playtest.ts | 19 ++- packages/cli/src/commands/publish.ts | 8 +- packages/cli/src/commands/upload.ts | 14 +- .../authorization/appAuthorization.test.ts | 86 ++++++++++-- .../util/authorization/appAuthorization.ts | 131 +++++++++++++++--- 5 files changed, 214 insertions(+), 44 deletions(-) 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 713a48cb..5f53b70e 100644 --- a/packages/cli/src/commands/publish.ts +++ b/packages/cli/src/commands/publish.ts @@ -183,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(); @@ -210,7 +211,10 @@ export default class Publish extends DevvitCommand { const shouldCreatePlaytestSubreddit = !this.project.getSubreddit('Dev'); - const appAuthorizationRole = getAppAuthorizationRole(appInfo, username); + const appAuthorizationRole = getAppAuthorizationRole(appInfo, { + id: userInfo.id, + displayName: username, + }); if (!canWriteAppVersion(appAuthorizationRole)) { if (flags['employee-update']) { const isEmployee = await isCurrentUserEmployee(token); diff --git a/packages/cli/src/commands/upload.ts b/packages/cli/src/commands/upload.ts index 61bf6d59..108ae6c8 100644 --- a/packages/cli/src/commands/upload.ts +++ b/packages/cli/src/commands/upload.ts @@ -53,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({ @@ -129,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(); @@ -147,14 +148,19 @@ export default class Upload extends DevvitCommand { let shouldCreateNewApp = false; let shouldCreatePlaytestSubreddit = !this.project.getSubreddit('Dev'); - const appAuthorizationRole = getAppAuthorizationRole(appInfo, username); + 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 index 703fd62f..de4c7ff1 100644 --- a/packages/cli/src/util/authorization/appAuthorization.test.ts +++ b/packages/cli/src/util/authorization/appAuthorization.test.ts @@ -1,5 +1,3 @@ -// eslint-disable-next-line no-restricted-imports -import type { FullAppInfo } from '@devvit/protos/types/devvit/dev_portal/app/app.js'; import { describe, expect, test } from 'vitest'; import { @@ -9,34 +7,69 @@ import { } from './appAuthorization.js'; describe('appAuthorization', () => { - test('allows the app owner to write app versions', () => { + test('recognizes the app owner by stable account ID', () => { const appInfo = { app: { - owner: { displayName: 'OwnerName' }, + owner: { id: 't2_owner', displayName: 'OriginalOwnerName' }, }, }; - const role = getAppAuthorizationRole(appInfo as FullAppInfo, 'ownername'); + const role = getAppAuthorizationRole(appInfo, { + id: 't2_owner', + displayName: 'RenamedOwner', + }); expect(role).toBe('owner'); expect(canWriteAppVersion(role)).toBe(true); }); - test('allows a designated app maintainer to write app versions', () => { + test('does not fall back to a matching display name when stable IDs disagree', () => { const appInfo = { app: { - owner: { displayName: 'OwnerName' }, - maintainers: [{ displayName: 'MaintainerName' }], + 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 as FullAppInfo, 'maintainername'); + const role = getAppAuthorizationRole(appInfo, { + id: 't2_maintainer', + displayName: 'RenamedMaintainer', + }); expect(role).toBe('maintainer'); expect(canWriteAppVersion(role)).toBe(true); }); - test('does not allow unrelated developers to write app versions', () => { + 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' }, @@ -44,16 +77,45 @@ describe('appAuthorization', () => { }, }; - const role = getAppAuthorizationRole(appInfo as FullAppInfo, 'OtherDeveloper'); + 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' } } } as FullAppInfo, + { app: { owner: { displayName: 'OwnerName' } } }, 'example-app', 'publish' ) diff --git a/packages/cli/src/util/authorization/appAuthorization.ts b/packages/cli/src/util/authorization/appAuthorization.ts index 8975f42d..83ee7983 100644 --- a/packages/cli/src/util/authorization/appAuthorization.ts +++ b/packages/cli/src/util/authorization/appAuthorization.ts @@ -1,39 +1,126 @@ -// eslint-disable-next-line no-restricted-imports -import type { FullAppInfo } from '@devvit/protos/types/devvit/dev_portal/app/app.js'; - export type AppAuthorizationRole = 'owner' | 'maintainer' | 'unauthorized'; -export type AppWriteAction = 'upload' | 'publish'; +export type AppWriteAction = 'upload' | 'publish' | 'playtest'; -type DeveloperDisplayName = { +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); +} -type AppWithMaintainers = NonNullable & { - maintainers?: readonly DeveloperDisplayName[]; -}; +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 { - return displayName?.trim().toLowerCase(); + 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: FullAppInfo | undefined, - username: string + appInfo: unknown, + currentUser: string | DeveloperIdentity ): AppAuthorizationRole { - const app = appInfo?.app as AppWithMaintainers | undefined; - const normalizedUsername = normalizeDisplayName(username); + const authorization = readAppAuthorizationData(appInfo); - if (!app || !normalizedUsername) { - return 'unauthorized'; + // 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; } - if (normalizeDisplayName(app.owner?.displayName) === normalizedUsername) { + const currentUserIdentity: DeveloperIdentity = + typeof currentUser === 'string' ? { displayName: currentUser } : currentUser; + + if (isSameDeveloper(authorization.owner, currentUserIdentity)) { return 'owner'; } if ( - app.maintainers?.some( - (maintainer) => normalizeDisplayName(maintainer.displayName) === normalizedUsername + authorization.maintainers.some((maintainer) => + isSameDeveloper(maintainer, currentUserIdentity) ) ) { return 'maintainer'; @@ -47,9 +134,11 @@ export function canWriteAppVersion(role: AppAuthorizationRole): boolean { } export function getAppAuthorizationErrorMessage( - appInfo: FullAppInfo, + appInfo: unknown, appName: string, action: AppWriteAction ): string { - return `You are not authorized to ${action} the app "${appName}". Please check that you are logged in as the owner (${appInfo.app?.owner?.displayName ?? ''}) or as a designated maintainer.`; + 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.`; }