From 81196252a52862d1f452aec3546fb43d45e451b7 Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Tue, 18 Aug 2026 05:10:23 -0400 Subject: [PATCH 1/2] fix: request the identity:handle OAuth scope Changing an AT Protocol handle failed in production with "Missing required scope identity:handle". The scope was never added to the default OAuth scope list, so the PDS refused com.atproto.identity.updateHandle. This only bites users whose session comes from OAuth. A custodial session is built from stored credentials and has full account access, so the handle change worked for them. Once a user takes ownership of their own PDS account, the same call returns a 403. Taking ownership of your account should not cost you the ability to rename it. Existing sessions do not gain a newly added scope. The API already compares granted scopes against the configured list and reports the difference on /auth/me, and the profile page already shows a Reconnect banner, so affected users are prompted to re-authorize. The unit test on the default scope list is a guard. This was reported once before and closed against a change that fixed something else. --- src/utils/bluesky.spec.ts | 33 ++++++++++++++++++++++++++++++++- src/utils/bluesky.ts | 2 +- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/utils/bluesky.spec.ts b/src/utils/bluesky.spec.ts index d934ac01..a366e56f 100644 --- a/src/utils/bluesky.spec.ts +++ b/src/utils/bluesky.spec.ts @@ -1,4 +1,5 @@ -import { createPlcFallbackFetch } from './bluesky'; +import { ConfigService } from '@nestjs/config'; +import { createPlcFallbackFetch, getAtprotoConfiguredScopes } from './bluesky'; describe('createPlcFallbackFetch', () => { let mockGlobalFetch: jest.Mock; @@ -183,3 +184,33 @@ describe('createPlcFallbackFetch', () => { }); }); }); + +describe('getAtprotoConfiguredScopes', () => { + const configWith = (scopes?: string) => + ({ get: () => scopes }) as unknown as ConfigService; + + it('should use the configured scopes when ATPROTO_OAUTH_SCOPES is set', () => { + const result = getAtprotoConfiguredScopes(configWith('atproto repo:a.b.c')); + expect(result).toBe('atproto repo:a.b.c'); + }); + + it('should fall back to the defaults when ATPROTO_OAUTH_SCOPES is unset', () => { + const scopes = getAtprotoConfiguredScopes(configWith(undefined)).split(' '); + expect(scopes).toEqual( + expect.arrayContaining([ + 'atproto', + 'account:email', + 'repo:community.lexicon.calendar.event', + 'repo:community.lexicon.calendar.rsvp', + ]), + ); + }); + + // Without this scope the PDS rejects com.atproto.identity.updateHandle with a + // 403 for anyone whose session is OAuth rather than custodial - so taking + // ownership of your own account costs you the ability to rename it. + it('should request identity:handle so users can change their own handle', () => { + const scopes = getAtprotoConfiguredScopes(configWith(undefined)).split(' '); + expect(scopes).toContain('identity:handle'); + }); +}); diff --git a/src/utils/bluesky.ts b/src/utils/bluesky.ts index bd04d7b9..e1ba548e 100644 --- a/src/utils/bluesky.ts +++ b/src/utils/bluesky.ts @@ -16,7 +16,7 @@ import { createRequestLock } from '../auth-bluesky/stores/redlock'; * Default OAuth scopes if ATPROTO_OAUTH_SCOPES env var is not set. */ const DEFAULT_ATPROTO_OAUTH_SCOPES = - 'atproto account:email rpc:app.bsky.actor.getProfile?aud=did:web:api.bsky.app#bsky_appview repo:community.lexicon.calendar.event repo:community.lexicon.calendar.rsvp'; + 'atproto account:email identity:handle rpc:app.bsky.actor.getProfile?aud=did:web:api.bsky.app#bsky_appview repo:community.lexicon.calendar.event repo:community.lexicon.calendar.rsvp'; /** * Get the configured ATProto OAuth scopes from env var or default. From 363c37f6c4c31a7204df70d8a9059735b318eb4a Mon Sep 17 00:00:00 2001 From: Tom Scanlan Date: Wed, 19 Aug 2026 08:23:52 -0400 Subject: [PATCH 2/2] fix: do not flag external PDS users for the handle scope Adding identity:handle to the default scope list means every existing OAuth session is now missing a scope. The stale scope check measured every non-custodial identity against the same global list, so users hosted on an external PDS get the "New permissions are available, reconnect" banner too. That prompt is wrong for them. updateHandle rejects any identity that is not on our PDS, so identity:handle can never be used for those accounts. The banner also contradicts the one right next to it, which tells the same user that account changes must be made through their own PDS provider. Following the prompt grants us a handle rename permission we never use. The fix is the yardstick, not the result. getRequiredScopesForIdentity builds the scope list a given identity is expected to hold, and both places that report a mismatch now compare against that instead of the global list. Users on our PDS are unaffected, so people who take ownership of their account still get prompted to reconnect, which is the case the scope was added for. A scope that turns on some other condition adds a field and a case in one place. The scope itself is unchanged. It stays in the default list and in the OAuth client metadata. Narrowing what we request at authorization time is a separate change, and it can call the same function. --- .../atproto-identity.controller.spec.ts | 86 +++++++++++++++++++ .../atproto-identity.controller.ts | 17 +++- src/auth/auth.service.spec.ts | 85 ++++++++++++++++++ src/auth/auth.service.ts | 12 ++- src/utils/check-scope-mismatch.spec.ts | 71 ++++++++++++++- src/utils/check-scope-mismatch.ts | 32 +++++++ 6 files changed, 296 insertions(+), 7 deletions(-) diff --git a/src/atproto-identity/atproto-identity.controller.spec.ts b/src/atproto-identity/atproto-identity.controller.spec.ts index 8c9ed035..79bfe9c3 100644 --- a/src/atproto-identity/atproto-identity.controller.spec.ts +++ b/src/atproto-identity/atproto-identity.controller.spec.ts @@ -31,6 +31,8 @@ describe('AtprotoIdentityController', () => { let pdsAccountService: PdsAccountService; let pdsSessionService: PdsSessionService; let blueskyService: BlueskyService; + let authBlueskyService: AuthBlueskyService; + let configService: ConfigService; const mockIdentityEntity: Partial = { id: 1, @@ -179,6 +181,8 @@ describe('AtprotoIdentityController', () => { pdsAccountService = module.get(PdsAccountService); pdsSessionService = module.get(PdsSessionService); blueskyService = module.get(BlueskyService); + authBlueskyService = module.get(AuthBlueskyService); + configService = module.get(ConfigService); }); it('should be defined', () => { @@ -1268,4 +1272,86 @@ describe('AtprotoIdentityController', () => { ).rejects.toThrow(BadRequestException); }); }); + + describe('getIdentity - identity:handle is only required for our PDS', () => { + // identity:handle authorizes a handle rename. updateHandle rejects any + // identity that is not on our PDS, so an external identity is never + // required to hold it and must not be asked to reconnect for it. + const ourPdsNonCustodial: Partial = { + ...mockIdentityEntity, + pdsCredentials: null, + isCustodial: false, + }; + + beforeEach(() => { + // Both cases hold a live OAuth session; only the host PDS differs. + jest.spyOn(blueskyService, 'tryResumeSession').mockResolvedValue({ + sessionManager: {}, + } as never); + jest.spyOn(configService, 'get').mockImplementation((key: string) => { + if (key === 'pds.url') return 'https://pds.openmeet.net'; + if (key === 'pds.serviceHandleDomains') return '.opnmt.me'; + if (key === 'ATPROTO_OAUTH_SCOPES') + return 'atproto account:email identity:handle'; + return null; + }); + }); + + it('should report identity:handle missing for an our-PDS identity', async () => { + // Arrange + jest + .spyOn(identityService, 'findByUserUlid') + .mockResolvedValue(ourPdsNonCustodial as UserAtprotoIdentityEntity); + jest + .spyOn(authBlueskyService, 'getScopeMismatch') + .mockResolvedValue(['identity:handle']); + + // Act + const result = await controller.getIdentity(mockRequest); + + // Assert + expect(result?.isOurPds).toBe(true); + expect(result?.scopeMismatch).toBe(true); + expect(result?.missingScopes).toEqual(['identity:handle']); + }); + + it('should not report identity:handle missing for an external-PDS identity', async () => { + // Arrange + jest + .spyOn(identityService, 'findByUserUlid') + .mockResolvedValue( + mockNonCustodialIdentity as UserAtprotoIdentityEntity, + ); + jest + .spyOn(authBlueskyService, 'getScopeMismatch') + .mockResolvedValue(['identity:handle']); + + // Act + const result = await controller.getIdentity(mockRequest); + + // Assert + expect(result?.isOurPds).toBe(false); + expect(result?.scopeMismatch).toBe(false); + expect(result?.missingScopes).toEqual([]); + }); + + it('should still report other missing scopes for an external-PDS identity', async () => { + // Arrange: stored flag holds identity:handle alongside a real gap + jest + .spyOn(identityService, 'findByUserUlid') + .mockResolvedValue( + mockNonCustodialIdentity as UserAtprotoIdentityEntity, + ); + jest + .spyOn(authBlueskyService, 'getScopeMismatch') + .mockResolvedValue(['identity:handle', 'account:email']); + + // Act + const result = await controller.getIdentity(mockRequest); + + // Assert + expect(result?.scopeMismatch).toBe(true); + expect(result?.missingScopes).toEqual(['account:email']); + }); + }); }); diff --git a/src/atproto-identity/atproto-identity.controller.ts b/src/atproto-identity/atproto-identity.controller.ts index 264cd503..0f2a8a42 100644 --- a/src/atproto-identity/atproto-identity.controller.ts +++ b/src/atproto-identity/atproto-identity.controller.ts @@ -43,6 +43,8 @@ import { NullableType } from '../utils/types/nullable.type'; import { AllConfigType } from '../config/config.type'; import { UserAtprotoIdentityEntity } from '../user-atproto-identity/infrastructure/persistence/relational/entities/user-atproto-identity.entity'; import { AuthBlueskyService } from '../auth-bluesky/auth-bluesky.service'; +import { getRequiredScopesForIdentity } from '../utils/check-scope-mismatch'; +import { getAtprotoConfiguredScopes } from '../utils/bluesky'; @ApiTags('AT Protocol Identity') @Controller({ @@ -543,6 +545,7 @@ export class AtprotoIdentityController { tenantId: string, ): Promise { const ourPdsUrl = this.configService.get('pds.url', { infer: true }); + const isOurPds = identity.pdsUrl === ourPdsUrl; const serviceHandleDomains = this.configService.get('pds.serviceHandleDomains', { infer: true }) || ''; const validHandleDomains = serviceHandleDomains @@ -599,8 +602,16 @@ export class AtprotoIdentityController { identity.did, ); if (mismatchData && mismatchData.length > 0) { - scopeMismatch = true; - missingScopes = mismatchData; + // The stored list was measured against the global scope list, so + // narrow it to what this identity is actually required to hold. + const requiredScopes = getRequiredScopesForIdentity( + getAtprotoConfiguredScopes(this.configService), + { isOurPds }, + ).split(/\s+/); + missingScopes = mismatchData.filter((scope) => + requiredScopes.includes(scope), + ); + scopeMismatch = missingScopes.length > 0; } } catch (error) { this.logger.warn('Failed to check scope mismatch', { @@ -615,7 +626,7 @@ export class AtprotoIdentityController { handle: identity.handle, pdsUrl: identity.pdsUrl, isCustodial: identity.isCustodial, - isOurPds: identity.pdsUrl === ourPdsUrl, + isOurPds, hasActiveSession, scopeMismatch, missingScopes, diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts index 912d318e..89d4688a 100644 --- a/src/auth/auth.service.spec.ts +++ b/src/auth/auth.service.spec.ts @@ -981,6 +981,91 @@ describe('AuthService', () => { expect(result?.atprotoIdentity?.scopeMismatch).toBe(false); expect(result?.atprotoIdentity?.missingScopes).toEqual([]); }); + + describe('identity:handle is only required for our PDS', () => { + // identity:handle authorizes a handle rename. updateHandle rejects any + // identity that is not on our PDS, so an external identity is never + // required to hold it and must not be asked to reconnect for it. + const ourPdsIdentity = { + ...baseIdentity, + did: 'did:plc:ourpds456', + handle: 'test-user.opnmt.me', + pdsUrl: 'https://pds.openmeet.net', + }; + + const sessionGranting = (scope: string) => ({ + sessionManager: { + getTokenInfo: jest.fn().mockResolvedValue({ scope }), + }, + }); + + beforeEach(() => { + mockConfigService.get.mockImplementation((key: string) => { + if (key === 'pds.url') return 'https://pds.openmeet.net'; + if (key === 'pds.serviceHandleDomains') return '.opnmt.me'; + if (key === 'ATPROTO_OAUTH_SCOPES') + return 'atproto identity:handle repo:community.lexicon.calendar.fake'; + return null; + }); + }); + + it('should report identity:handle missing for an our-PDS identity', async () => { + // Arrange: took ownership, session predates the identity:handle scope + mockUserAtprotoIdentityService.findByUserUlid.mockResolvedValue( + ourPdsIdentity, + ); + mockBlueskyService.tryResumeSession.mockResolvedValue( + sessionGranting('atproto repo:community.lexicon.calendar.fake'), + ); + + // Act + const result = await authService.me(jwtPayload); + + // Assert + expect(result?.atprotoIdentity?.isOurPds).toBe(true); + expect(result?.atprotoIdentity?.scopeMismatch).toBe(true); + expect(result?.atprotoIdentity?.missingScopes).toEqual([ + 'identity:handle', + ]); + }); + + it('should not report identity:handle missing for an external-PDS identity', async () => { + // Arrange: same stale session shape, but hosted on bsky.social + mockUserAtprotoIdentityService.findByUserUlid.mockResolvedValue( + baseIdentity, + ); + mockBlueskyService.tryResumeSession.mockResolvedValue( + sessionGranting('atproto repo:community.lexicon.calendar.fake'), + ); + + // Act + const result = await authService.me(jwtPayload); + + // Assert + expect(result?.atprotoIdentity?.isOurPds).toBe(false); + expect(result?.atprotoIdentity?.scopeMismatch).toBe(false); + expect(result?.atprotoIdentity?.missingScopes).toEqual([]); + }); + + it('should still report other missing scopes for an external-PDS identity', async () => { + // Arrange: external identity missing identity:handle AND a real scope + mockUserAtprotoIdentityService.findByUserUlid.mockResolvedValue( + baseIdentity, + ); + mockBlueskyService.tryResumeSession.mockResolvedValue( + sessionGranting('atproto'), + ); + + // Act + const result = await authService.me(jwtPayload); + + // Assert + expect(result?.atprotoIdentity?.scopeMismatch).toBe(true); + expect(result?.atprotoIdentity?.missingScopes).toEqual([ + 'repo:community.lexicon.calendar.fake', + ]); + }); + }); }); describe('validateSocialLogin - PDS Account Auto-Creation', () => { diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index e7811c06..025960e8 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -61,7 +61,10 @@ import { MeResponse } from './dto/me-response.dto'; import { AtprotoIdentityDto } from '../atproto-identity/dto/atproto-identity.dto'; import { ElastiCacheService } from '../elasticache/elasticache.service'; import { getTenantConfig } from '../utils/tenant-config'; -import { checkScopeMismatch } from '../utils/check-scope-mismatch'; +import { + checkScopeMismatch, + getRequiredScopesForIdentity, +} from '../utils/check-scope-mismatch'; import { getAtprotoConfiguredScopes } from '../utils/bluesky'; import { OAuthSession } from '@atproto/oauth-client'; import { CreateLoginLinkResponseDto } from './dto/create-login-link-response.dto'; @@ -670,6 +673,7 @@ export class AuthService { if (identity) { // Map to DTO, explicitly excluding pdsCredentials const ourPdsUrl = this.configService.get('pds.url', { infer: true }); + const isOurPds = identity.pdsUrl === ourPdsUrl; const serviceHandleDomains = this.configService.get('pds.serviceHandleDomains', { infer: true, @@ -706,7 +710,9 @@ export class AuthService { this.configService, ); scopeMissingScopes = checkScopeMismatch( - configuredScopes, + getRequiredScopesForIdentity(configuredScopes, { + isOurPds, + }), tokenInfo.scope, ); scopeHasMismatch = scopeMissingScopes.length > 0; @@ -726,7 +732,7 @@ export class AuthService { handle: identity.handle, pdsUrl: identity.pdsUrl, isCustodial: identity.isCustodial, - isOurPds: identity.pdsUrl === ourPdsUrl, + isOurPds, hasActiveSession, scopeMismatch: scopeHasMismatch, missingScopes: scopeMissingScopes, diff --git a/src/utils/check-scope-mismatch.spec.ts b/src/utils/check-scope-mismatch.spec.ts index dbfdd9ec..764904c1 100644 --- a/src/utils/check-scope-mismatch.spec.ts +++ b/src/utils/check-scope-mismatch.spec.ts @@ -1,4 +1,7 @@ -import { checkScopeMismatch } from './check-scope-mismatch'; +import { + checkScopeMismatch, + getRequiredScopesForIdentity, +} from './check-scope-mismatch'; describe('checkScopeMismatch', () => { it('should return empty array when scopes are the same', () => { @@ -72,3 +75,69 @@ describe('checkScopeMismatch', () => { ).toEqual([]); }); }); + +describe('getRequiredScopesForIdentity', () => { + const configured = 'atproto account:email identity:handle repo:x.y.event'; + + it('should require identity:handle for an identity on our PDS', () => { + expect( + getRequiredScopesForIdentity(configured, { isOurPds: true }), + ).toEqual('atproto account:email identity:handle repo:x.y.event'); + }); + + it('should not require identity:handle for an external PDS identity', () => { + expect( + getRequiredScopesForIdentity(configured, { isOurPds: false }), + ).toEqual('atproto account:email repo:x.y.event'); + }); + + it('should keep every other scope for an external PDS identity', () => { + const required = getRequiredScopesForIdentity(configured, { + isOurPds: false, + }).split(' '); + expect(required).toEqual( + expect.arrayContaining(['atproto', 'account:email', 'repo:x.y.event']), + ); + expect(required).toHaveLength(3); + }); + + it('should preserve the configured order', () => { + expect( + getRequiredScopesForIdentity('identity:handle atproto', { + isOurPds: true, + }), + ).toEqual('identity:handle atproto'); + }); + + it('should tolerate extra whitespace in the configured list', () => { + expect( + getRequiredScopesForIdentity(' atproto identity:handle ', { + isOurPds: false, + }), + ).toEqual('atproto'); + }); + + it('should return an empty string when nothing is configured', () => { + expect(getRequiredScopesForIdentity('', { isOurPds: true })).toEqual(''); + }); + + it('should feed checkScopeMismatch so an external identity has no gap', () => { + // The session was granted under the scope list that predates + // identity:handle. Our PDS still needs it; an external PDS does not. + const granted = 'atproto account:email repo:x.y.event'; + + expect( + checkScopeMismatch( + getRequiredScopesForIdentity(configured, { isOurPds: true }), + granted, + ), + ).toEqual(['identity:handle']); + + expect( + checkScopeMismatch( + getRequiredScopesForIdentity(configured, { isOurPds: false }), + granted, + ), + ).toEqual([]); + }); +}); diff --git a/src/utils/check-scope-mismatch.ts b/src/utils/check-scope-mismatch.ts index 5c8c3ae7..54a72ba5 100644 --- a/src/utils/check-scope-mismatch.ts +++ b/src/utils/check-scope-mismatch.ts @@ -18,3 +18,35 @@ export function checkScopeMismatch( return [...configured].filter((scope) => !granted.has(scope)); } + +/** + * The scopes a session for this identity is expected to carry. + * + * Most configured scopes apply to every identity. `identity:handle` does not. + * It authorizes com.atproto.identity.updateHandle, and updateHandle rejects + * any identity that is not on our PDS. Measuring an external identity against + * it reports a gap that can never close, and asks that user to grant a handle + * rename permission we never use for them. + * + * The scope stays in the configured list and in the OAuth client metadata. + * Users who take ownership of an account on our PDS sign in over OAuth, and + * they do need it. + * + * Build the yardstick with this before comparing against granted scopes, + * rather than filtering the comparison afterwards. A scope that turns on some + * other condition adds a field to `identity` and a case here. + * + * @param configuredScopes - Space-delimited scopes the app is configured with + * @param identity.isOurPds - Whether the identity is hosted on OpenMeet's PDS + * @returns Space-delimited scopes required for this identity + */ +export function getRequiredScopesForIdentity( + configuredScopes: string, + identity: { isOurPds: boolean }, +): string { + return configuredScopes + .split(/\s+/) + .filter((scope) => scope.length > 0) + .filter((scope) => (scope === 'identity:handle' ? identity.isOurPds : true)) + .join(' '); +}