Skip to content

Commit b5717bf

Browse files
authored
feat(auth): improve messaging for file system permissions issues (#3358)
## Problem Several users have reported problems related to permissions. This is probably because we use the user's home directory rather than a directory provided by VSC. ## Solution Show more info regarding permissions issues when they occur
1 parent 3c8f2b5 commit b5717bf

File tree

14 files changed

+387
-97
lines changed

14 files changed

+387
-97
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Feature",
3+
"description": "Improved error messages for file system permissions issues"
4+
}

buildspec/linuxTests.yml

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ version: 0.2
22

33
env:
44
variables:
5-
AWS_TOOLKIT_TEST_USER_DIR: '/tmp/'
65
AWS_TOOLKIT_TEST_NO_COLOR: '1'
76

87
phases:
@@ -32,9 +31,9 @@ phases:
3231

3332
build:
3433
commands:
35-
- npm run testCompile
36-
- npm run lint
37-
- xvfb-run npm test --silent
34+
- mkdir -p /home/codebuild-user
35+
- chown -R codebuild-user:codebuild-user /tmp /home/codebuild-user .
36+
- su codebuild-user -c "xvfb-run npm test --silent"
3837
- VCS_COMMIT_ID="${CODEBUILD_RESOLVED_SOURCE_VERSION}"
3938
- CI_BUILD_URL=$(echo $CODEBUILD_BUILD_URL | sed 's/#/%23/g') # Encode `#` in the URL because otherwise the url is clipped in the Codecov.io site
4039
- CI_BUILD_ID="${CODEBUILD_BUILD_ID}"

src/codecatalyst/tools.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,9 @@ async function verifySSHHost({
253253

254254
const sshConfigPath = getSshConfigPath()
255255
try {
256-
await fs.mkdirp(path.dirname(sshConfigPath))
257-
await fs.appendFile(sshConfigPath, section)
256+
await fs.ensureDir(path.dirname(path.dirname(sshConfigPath)), { mode: 0o755 })
257+
await fs.mkdir(path.dirname(sshConfigPath), 0o700)
258+
await fs.appendFile(sshConfigPath, section, { mode: 0o600 })
258259
} catch (e) {
259260
const message = localize(
260261
'AWS.codecatalyst.error.writeFail',

src/credentials/activation.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
import * as vscode from 'vscode'
77
import { AwsContext } from '../shared/awsContext'
8-
import { Settings } from '../shared/settings'
98
import { Auth } from './auth'
109
import { LoginManager } from './loginManager'
1110
import { fromString } from './providers/credentials'
@@ -15,7 +14,6 @@ import { registerCommandsWithVSCode } from '../shared/vscode/commands2'
1514
export async function initialize(
1615
extensionContext: vscode.ExtensionContext,
1716
awsContext: AwsContext,
18-
settings: Settings,
1917
loginManager: LoginManager
2018
): Promise<void> {
2119
Auth.instance.onDidChangeActiveConnection(conn => {

src/credentials/sso/cache.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@
44
*/
55

66
import * as crypto from 'crypto'
7-
import { homedir } from 'os'
8-
import { join } from 'path'
7+
import * as path from 'path'
98
import { getLogger } from '../../shared/logger/logger'
109
import { createDiskCache, KeyedCache, mapCache } from '../../shared/utilities/cacheUtils'
1110
import { stripUndefined } from '../../shared/utilities/collectionUtils'
1211
import { hasProps, selectFrom } from '../../shared/utilities/tsUtils'
1312
import { SsoToken, ClientRegistration } from './model'
13+
import { SystemUtilities } from '../../shared/systemUtilities'
14+
import { DevSettings } from '../../shared/settings'
1415

1516
interface RegistrationKey {
1617
readonly region: string
@@ -29,16 +30,17 @@ export interface SsoCache {
2930
readonly registration: KeyedCache<ClientRegistration, RegistrationKey>
3031
}
3132

32-
const cacheDir = join(homedir(), '.aws', 'sso', 'cache')
33+
const defaultCacheDir = path.join(SystemUtilities.getHomeDirectory(), '.aws', 'sso', 'cache')
34+
export const getCacheDir = () => DevSettings.instance.get('ssoCacheDirectory', defaultCacheDir)
3335

34-
export function getCache(directory = cacheDir): SsoCache {
36+
export function getCache(directory = getCacheDir()): SsoCache {
3537
return {
3638
token: getTokenCache(directory),
3739
registration: getRegistrationCache(directory),
3840
}
3941
}
4042

41-
export function getRegistrationCache(directory = cacheDir): KeyedCache<ClientRegistration, RegistrationKey> {
43+
export function getRegistrationCache(directory = getCacheDir()): KeyedCache<ClientRegistration, RegistrationKey> {
4244
const hashScopes = (scopes: string[]) => {
4345
const shasum = crypto.createHash('sha256')
4446
scopes.forEach(s => shasum.update(s))
@@ -47,7 +49,7 @@ export function getRegistrationCache(directory = cacheDir): KeyedCache<ClientReg
4749

4850
const getTarget = (key: RegistrationKey) => {
4951
const suffix = `${key.region}${key.scopes && key.scopes.length > 0 ? `-${hashScopes(key.scopes)}` : ''}`
50-
return join(directory, `aws-toolkit-vscode-client-id-${suffix}.json`)
52+
return path.join(directory, `aws-toolkit-vscode-client-id-${suffix}.json`)
5153
}
5254

5355
// Compatability for older Toolkit versions (format on disk is unchanged)
@@ -61,7 +63,7 @@ export function getRegistrationCache(directory = cacheDir): KeyedCache<ClientReg
6163
return mapCache(cache, read, write)
6264
}
6365

64-
export function getTokenCache(directory = cacheDir): KeyedCache<SsoAccess> {
66+
export function getTokenCache(directory = getCacheDir()): KeyedCache<SsoAccess> {
6567
// Older specs do not store the registration
6668
type MaybeRegistration = Partial<Omit<ClientRegistration, 'expiresAt'> & { readonly registrationExpiresAt: string }>
6769

@@ -123,7 +125,7 @@ export function getTokenCache(directory = cacheDir): KeyedCache<SsoAccess> {
123125
shasum.update(encoded) // lgtm[js/weak-cryptographic-algorithm]
124126
const hashedUrl = shasum.digest('hex') // lgtm[js/weak-cryptographic-algorithm]
125127

126-
return join(directory, `${hashedUrl}.json`)
128+
return path.join(directory, `${hashedUrl}.json`)
127129
}
128130

129131
const logger = (message: string) => getLogger().debug(`SSO token cache: ${message}`)

src/dev/beta.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ async function checkBetaUrl(vsixUrl: string): Promise<void> {
8787
try {
8888
await promptInstallToolkit(betaPath, latestBetaInfo.version, vsixUrl)
8989
} finally {
90-
await SystemUtilities.remove(tmpFolder)
90+
await SystemUtilities.delete(tmpFolder, { recursive: true })
9191
}
9292
} else {
9393
await updateBetaToolkitData(vsixUrl, {

src/extension.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ export async function activate(context: vscode.ExtensionContext) {
124124
const settings = Settings.instance
125125
const experiments = Experiments.instance
126126

127-
await initializeCredentials(context, awsContext, settings, loginManager)
127+
await initializeCredentials(context, awsContext, loginManager)
128128
await activateTelemetry(context, awsContext, settings)
129129

130130
experiments.onDidChange(({ key }) => {

src/integrationTest/globalSetup.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ export async function mochaGlobalSetup(this: Mocha.Runner) {
2626
this.on('hook', hook => setRunnableTimeout(hook, maxTestDuration))
2727
this.on('test', test => setRunnableTimeout(test, maxTestDuration))
2828

29+
// Mocha won't show the full error chain
30+
this.on('fail', (test, err) => {
31+
getLogger().error(`test "${test.title}" failed: %s`, err)
32+
})
33+
2934
// Set up a listener for proxying login requests
3035
patchWindow()
3136

src/shared/errors.ts

Lines changed: 92 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
import { Uri } from 'vscode'
6+
import * as vscode from 'vscode'
77
import { AWSError } from 'aws-sdk'
88
import { ServiceException } from '@aws-sdk/smithy-client'
99
import { isThrottlingError, isTransientError } from '@aws-sdk/service-error-classification'
1010
import { Result } from './telemetry/telemetry'
1111
import { CancellationError } from './utilities/timeoutUtils'
12-
import { isNonNullable } from './utilities/tsUtils'
12+
import { hasKey, isNonNullable } from './utilities/tsUtils'
13+
import type * as fs from 'fs'
14+
import type * as os from 'os'
1315

1416
export const errorCode = {
1517
invalidConnection: 'InvalidConnection',
@@ -81,7 +83,7 @@ export interface ErrorInformation {
8183
*
8284
* TODO: implement this
8385
*/
84-
readonly documentationUri?: Uri
86+
readonly documentationUri?: vscode.Uri
8587
}
8688

8789
/**
@@ -397,3 +399,90 @@ export function getRequestId(error: unknown): string | undefined {
397399
return error.$metadata.requestId
398400
}
399401
}
402+
403+
export function isFileNotFoundError(err: unknown): boolean {
404+
if (err instanceof vscode.FileSystemError) {
405+
return err.code === vscode.FileSystemError.FileNotFound().code
406+
} else if (isNonNullable(err) && typeof err === 'object' && hasKey(err, 'code')) {
407+
return err.code === 'ENOENT'
408+
}
409+
410+
return false
411+
}
412+
413+
export function isNoPermissionsError(err: unknown): boolean {
414+
if (err instanceof vscode.FileSystemError) {
415+
return (
416+
err.code === vscode.FileSystemError.NoPermissions().code ||
417+
// The code _should_ be `NoPermissions`, maybe this is a bug?
418+
(err.code === 'Unknown' && err.message.includes('EACCES: permission denied'))
419+
)
420+
} else if (isNonNullable(err) && typeof err === 'object' && hasKey(err, 'code')) {
421+
return err.code === 'EACCES'
422+
}
423+
424+
return false
425+
}
426+
427+
const modeToString = (mode: number) =>
428+
Array.from('rwxrwxrwx')
429+
.map((c, i, a) => ((mode >> (a.length - (i + 1))) & 1 ? c : '-'))
430+
.join('')
431+
432+
function getEffectivePerms(uid: number, gid: number, stats: fs.Stats) {
433+
const mode = stats.mode
434+
const isOwner = uid === stats.uid
435+
const isGroup = gid === stats.gid && !isOwner
436+
437+
// Many unix systems support multiple groups but we only know the primary
438+
// The user can still have group permissions despite not having the same `gid`
439+
// These situations are ambiguous, so the effective permissions are the
440+
// intersection of the two bitfields
441+
if (!isOwner && !isGroup) {
442+
return {
443+
isAmbiguous: true,
444+
effective: mode & 0o007 & ((mode & 0o070) >> 3),
445+
}
446+
}
447+
448+
const ownerMask = isOwner ? 0o700 : 0
449+
const groupMask = isGroup ? 0o070 : 0
450+
451+
return {
452+
isAmbiguous: false,
453+
effective: ((mode & groupMask) >> 3) | ((mode & ownerMask) >> 6),
454+
}
455+
}
456+
457+
// The wildcard (`*`) symbol is non-standard. It's used to represent "don't cares" and takes
458+
// on the actual flag once known.
459+
export type PermissionsTriplet = `${'r' | '-' | '*'}${'w' | '-' | '*'}${'x' | '-' | '*'}`
460+
export class PermissionsError extends ToolkitError {
461+
public readonly actual: string // This is a resolved triplet, _not_ the full bits
462+
463+
public constructor(
464+
public readonly uri: vscode.Uri,
465+
public readonly stats: fs.Stats,
466+
public readonly userInfo: os.UserInfo<string>,
467+
public readonly expected: PermissionsTriplet
468+
) {
469+
const mode = `${stats.isDirectory() ? 'd' : '-'}${modeToString(stats.mode)}`
470+
const owner = stats.uid === userInfo.uid ? userInfo.username : stats.uid
471+
const { effective, isAmbiguous } = getEffectivePerms(userInfo.uid, userInfo.gid, stats)
472+
const actual = modeToString(effective).slice(-3)
473+
const resolvedExpected = Array.from(expected)
474+
.map((c, i) => (c === '*' ? actual[i] : c))
475+
.join('')
476+
const actualText = !isAmbiguous ? actual : `${mode.slice(-6, -3)} & ${mode.slice(-3)} (ambiguous)`
477+
478+
super(`${uri.fsPath} has incorrect permissions. Expected ${resolvedExpected}, found ${actualText}.`, {
479+
code: 'InvalidPermissions',
480+
details: {
481+
isOwner: stats.uid === -1 ? 'unknown' : userInfo.uid == stats.uid,
482+
mode: `${mode}${stats.uid === -1 ? '' : ` ${owner}`}${stats.gid === -1 ? '' : ` ${stats.gid}`}`,
483+
},
484+
})
485+
486+
this.actual = actual
487+
}
488+
}

src/shared/settings.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,7 @@ const devSettings = {
588588
renderDebugDetails: Boolean,
589589
endpoints: Record(String, String),
590590
cawsStage: String,
591+
ssoCacheDirectory: String,
591592
}
592593
type ResolvedDevSettings = FromDescriptor<typeof devSettings>
593594
type AwsDevSetting = keyof ResolvedDevSettings

0 commit comments

Comments
 (0)