Skip to content

Commit

Permalink
Add VSCode extension configuration to disable KeyChain storage for cr…
Browse files Browse the repository at this point in the history
…edentials
  • Loading branch information
sagerb committed Jan 31, 2025
1 parent cc4a526 commit 855a659
Show file tree
Hide file tree
Showing 10 changed files with 103 additions and 14 deletions.
2 changes: 1 addition & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"request": "launch",
"mode": "auto",
"program": "${workspaceFolder}/cmd/publisher/main.go",
"args": ["ui", "--listen=localhost:9001", "-vv"],
"args": ["ui", "--listen=localhost:9001", "-vv", "--use-keychain=false"],
"cwd": "${workspaceFolder}/test/sample-content"
}
]
Expand Down
8 changes: 6 additions & 2 deletions cmd/publisher/commands/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,17 @@ package commands

import (
"github.com/posit-dev/publisher/internal/cli_types"
"github.com/posit-dev/publisher/internal/credentials"
"github.com/posit-dev/publisher/internal/events"
"github.com/posit-dev/publisher/internal/services/api"
"github.com/posit-dev/publisher/internal/util"
"github.com/r3labs/sse/v2"
)

type UICmd struct {
Path util.Path `help:"Sets the current working directory for the agent." arg:"" default:"."`
Listen string `help:"Network address to listen on." placeholder:"HOST[:PORT]" default:"localhost:0"`
Path util.Path `help:"Sets the current working directory for the agent." arg:"" default:"."`
Listen string `help:"Network address to listen on." placeholder:"HOST[:PORT]" default:"localhost:0"`
UseKeychain bool `help:"Use Keychain services to store/manage credentials." default:"true"`
}

func (cmd *UICmd) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContext) error {
Expand All @@ -31,6 +33,8 @@ func (cmd *UICmd) Run(args *cli_types.CommonArgs, ctx *cli_types.CLIContext) err
return err
}

credentials.UseKeychain = cmd.UseKeychain

// Auto-initialize if needed. This will be replaced by an API call from the UI
// for better error handling and startup performance.
svc := api.NewService(
Expand Down
5 changes: 5 additions & 0 deletions extensions/vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@
"markdownDescription": "Verify TLS certificates for connections. Only **disable** this setting if you experience certificate verification errors and the Posit Connect server is using a self-signed, expired, or otherwise misconfigured certificate.",
"type": "boolean",
"default": true
},
"positPublisher.useKeyChainCredentialStorage": {
"markdownDescription": "Enable Key Chain storage when retrieving and storing credentials. On systems which do not support a Key Chain facility, this configuration option is ignored. When the Key Chain facility is not used, a YAML file is used instead. (`~/.connect_credentials`). After changing this value, you must restart the extension to have the setting go into effect.",
"type": "boolean",
"default": true
}
}
}
Expand Down
12 changes: 11 additions & 1 deletion extensions/vscode/src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,20 @@ export const create = async (
context: ExtensionContext,
path: string,
port: number,
useKeyChain: boolean,
subcommand: string = "ui",
): Promise<[string, string[]]> => {
const executable = await getExecutableBinary(context);
return [executable, [subcommand, "-vv", `--listen=${HOST}:${port}`, path]];
return [
executable,
[
subcommand,
"-vv",
`--listen=${HOST}:${port}`,
`--use-keychain=${useKeyChain}`,
path,
],
];
};

const getExecutableBinary = (context: ExtensionContext): string => {
Expand Down
14 changes: 12 additions & 2 deletions extensions/vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ export async function activate(context: ExtensionContext) {
const stream = new EventStream(port);
context.subscriptions.push(stream);

service = new Service(context, port, useExternalAgent);
const useKeyChain = extensionSettings.useKeyChainCredentialStorage();

service = new Service(context, port, useExternalAgent, useKeyChain);
service.start();

const watchers = new WatcherManager();
Expand Down Expand Up @@ -119,10 +121,18 @@ export async function deactivate() {

export const extensionSettings = {
verifyCertificates(): boolean {
// set value from extension configuration - defaults to true
// get value from extension configuration - defaults to true
const configuration = workspace.getConfiguration("positPublisher");
const value: boolean | undefined =
configuration.get<boolean>("verifyCertificates");
return value !== undefined ? value : true;
},
useKeyChainCredentialStorage(): boolean {
// get value from extension configuration - defaults to true
const configuration = workspace.getConfiguration("positPublisher");
const value: boolean | undefined = configuration.get<boolean>(
"useKeyChainCredentialStorage",
);
return value !== undefined ? value : true;
},
};
11 changes: 9 additions & 2 deletions extensions/vscode/src/servers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ import * as workspaces from "src/workspaces";
export class Server implements Disposable {
readonly port: number;
readonly outputChannel: OutputChannel;
readonly useKeyChain: boolean;

process: ChildProcessWithoutNullStreams | undefined = undefined;

constructor(port: number) {
constructor(port: number, useKeyChain: boolean) {
this.port = port;
this.outputChannel = window.createOutputChannel(`Posit Publisher`);
this.useKeyChain = useKeyChain;
}

/**
Expand All @@ -38,7 +40,12 @@ export class Server implements Disposable {
// todo - make this configurable
const path = workspaces.path();
// Create command to send to terminal stdin
const [command, args] = await commands.create(context, path!, this.port);
const [command, args] = await commands.create(
context,
path!,
this.port,
this.useKeyChain,
);
// Spawn child process
this.process = spawn(command, args);
// Handle error output
Expand Down
5 changes: 4 additions & 1 deletion extensions/vscode/src/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,20 @@ export class Service implements Disposable {
private server: Server;
private agentURL: string;
private useExternalAgent: boolean;
private useKeyChain: boolean;

constructor(
context: ExtensionContext,
port: number,
useExternalAgent: boolean,
useKeyChain: boolean,
) {
this.context = context;
this.useExternalAgent = useExternalAgent;
this.useKeyChain = useKeyChain;

this.agentURL = `http://${HOST}:${port}/api`;
this.server = new Server(port);
this.server = new Server(port, this.useKeyChain);
initApi(this.isUp(), this.agentURL);
}

Expand Down
17 changes: 13 additions & 4 deletions internal/credentials/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ type CredentialRecord struct {

type CredentialTable = map[string]CredentialRecord

var UseKeychain bool = true

// ToCredential converts a CredentialRecord to a Credential based on its version.
func (cr *CredentialRecord) ToCredential() (*Credential, error) {
switch cr.Version {
Expand Down Expand Up @@ -93,12 +95,19 @@ type CredentialsService interface {
// The main credentials service constructor that determines if the system's keyring is available to be used,
// if not, returns a file based credentials service.
func NewCredentialsService(log logging.Logger) (CredentialsService, error) {
krService := NewKeyringCredentialsService(log)
if krService.IsSupported() {
return krService, nil
var fcService *fileCredentialsService = nil

if UseKeychain {
krService := NewKeyringCredentialsService(log)
if krService.IsSupported() {
log.Debug("Using keychain for credential storage.")
return krService, nil
}
log.Debug("Fallback to file managed credentials service due to unavailable system keyring.")
} else {
log.Debug("Configuration has disabled keychain credentials. Using file managed credentials instead.")
}

log.Debug("Fallback to file managed credentials service due to unavailable system keyring")
fcService, err := NewFileCredentialsService(log)
if err != nil {
return fcService, err
Expand Down
38 changes: 37 additions & 1 deletion internal/credentials/credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ func (s *CredentialsServiceTestSuite) TestCredentialRecord_VersionErr() {

func (s *CredentialsServiceTestSuite) TestNewCredentialsService_KeyringOK() {
keyring.MockInit()

// Don't set credentials.UseKeychain to any value, let it default

s.log.On("Debug", "Using keychain for credential storage.").Return()

credservice, err := NewCredentialsService(s.log)
s.NoError(err)
s.Implements((*CredentialsService)(nil), credservice)
Expand All @@ -118,7 +123,38 @@ func (s *CredentialsServiceTestSuite) TestNewCredentialsService_KeyringErrFallba
keyring.MockInitWithError(keyringErr)

s.log.On("Debug", "System keyring service is not available", "error", "failed to load credentials: this is a teapot, unsupported system").Return()
s.log.On("Debug", "Fallback to file managed credentials service due to unavailable system keyring").Return()
s.log.On("Debug", "Fallback to file managed credentials service due to unavailable system keyring.").Return()

credservice, err := NewCredentialsService(s.log)
s.NoError(err)
s.Implements((*CredentialsService)(nil), credservice)
}

func (s *CredentialsServiceTestSuite) TestNewCredentialsService_WithUseKeyringTrue() {
keyring.MockInit()

// set credentials.UseKeychain to true
UseKeychain = true

s.log.On("Debug", "Using keychain for credential storage.").Return()

credservice, err := NewCredentialsService(s.log)
s.NoError(err)
s.Implements((*CredentialsService)(nil), credservice)
}

func (s *CredentialsServiceTestSuite) TestNewCredentialsService_WithUseKeyringFalse() {
// Use an in memory filesystem for this test
// avoiding to manipulate users ~/.connect-credentials
fsys = afero.NewMemMapFs()
defer func() { fsys = afero.NewOsFs() }()

keyring.MockInit()

s.log.On("Debug", "Configuration has disabled keychain credentials. Using file managed credentials instead.").Return()

// set credentials.UseKeychain to true
UseKeychain = false

credservice, err := NewCredentialsService(s.log)
s.NoError(err)
Expand Down
5 changes: 5 additions & 0 deletions internal/logging/loggingtest/mock_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,8 @@ func (m *MockLogger) WithArgs(args ...any) logging.Logger {
mockArgs := m.Called(args...)
return mockArgs.Get(0).(logging.Logger)
}

func (m *MockLogger) Debug(msg string, args ...any) {
mockArgs := append([]any{msg}, args...)
m.Called(mockArgs...)
}

0 comments on commit 855a659

Please sign in to comment.