diff --git a/README.md b/README.md index 0c6cc073..49a03e39 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,8 @@ Available Commands: Flags: --app-id string The GitHub App to connect to. ($BATON_APP_ID) - --app-privatekey-path string Path to private key that is used to connect to the GitHub App ($BATON_APP_PRIVATEKEY_PATH) + --app-privatekey string Raw PEM contents of the private key used to connect to the GitHub App. Takes precedence over app-privatekey-path when both are set. ($BATON_APP_PRIVATEKEY) + --app-privatekey-path string Path to private key that is used to connect to the GitHub App. Ignored when app-privatekey is set. ($BATON_APP_PRIVATEKEY_PATH) --client-id string The client ID used to authenticate with ConductorOne ($BATON_CLIENT_ID) --client-secret string The client secret used to authenticate with ConductorOne ($BATON_CLIENT_SECRET) --enterprises strings Sync enterprise roles, must be an admin of the enterprise. ($BATON_ENTERPRISES) diff --git a/config_schema.json b/config_schema.json index ed33673b..035f781f 100644 --- a/config_schema.json +++ b/config_schema.json @@ -136,19 +136,22 @@ { "name": "app-privatekey-path", "displayName": "GitHub App private key (.pem)", - "description": "Path to private key that is used to connect to the GitHub App", - "isRequired": true, + "description": "Path to private key that is used to connect to the GitHub App. Ignored when app-privatekey is set.", "isSecret": true, "stringField": { - "rules": { - "isRequired": true - }, "type": "STRING_FIELD_TYPE_FILE_UPLOAD", "allowedExtensions": [ ".pem" ] } }, + { + "name": "app-privatekey", + "displayName": "GitHub App private key (PEM)", + "description": "Raw PEM contents of the private key used to connect to the GitHub App. Takes precedence over app-privatekey-path when both are set.", + "isSecret": true, + "stringField": {} + }, { "name": "org", "displayName": "Github App Organization", @@ -202,6 +205,7 @@ "fields": [ "app-id", "app-privatekey-path", + "app-privatekey", "org", "sync-secrets", "omit-archived-repositories", diff --git a/docs/connector.mdx b/docs/connector.mdx index 2286203a..2d2e0b8b 100644 --- a/docs/connector.mdx +++ b/docs/connector.mdx @@ -386,7 +386,10 @@ stringData: # GitHub credentials if configuring with a GitHub app BATON_APP_ID: + # Supply the private key one of two ways: BATON_APP_PRIVATEKEY_PATH: + # ...or pass the raw PEM contents directly (takes precedence when both are set): + # BATON_APP_PRIVATEKEY: BATON_ORGS: # Optional: include if you want C1 to provision access using this connector diff --git a/pkg/config/conf.gen.go b/pkg/config/conf.gen.go index 5ed99148..710e713a 100644 --- a/pkg/config/conf.gen.go +++ b/pkg/config/conf.gen.go @@ -10,6 +10,7 @@ type Github struct { InstanceUrl string `mapstructure:"instance-url"` AppId string `mapstructure:"app-id"` AppPrivatekeyPath []byte `mapstructure:"app-privatekey-path"` + AppPrivatekey string `mapstructure:"app-privatekey"` Org string `mapstructure:"org"` SyncSecrets bool `mapstructure:"sync-secrets"` OmitArchivedRepositories bool `mapstructure:"omit-archived-repositories"` diff --git a/pkg/config/config.go b/pkg/config/config.go index 2b9c5e63..5c676fac 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -40,13 +40,25 @@ var ( field.WithRequired(true), ) + // appPrivateKeyPath and appPrivateKey are two ways to supply the same GitHub + // App private key. Neither is marked required individually: providing either + // one satisfies the "app private key required" check, which is enforced in the + // connector's GitHub App constructor (see pkg/connector.appPrivateKeyPEM). + // A framework-level constraint can't express "required only for the GitHub App + // auth group", since constraints are evaluated globally across auth methods. appPrivateKeyPath = field.FileUploadField( "app-privatekey-path", []string{".pem"}, field.WithDisplayName("GitHub App private key (.pem)"), - field.WithDescription("Path to private key that is used to connect to the GitHub App"), + field.WithDescription("Path to private key that is used to connect to the GitHub App. Ignored when app-privatekey is set."), + field.WithIsSecret(true), + ) + + appPrivateKey = field.StringField( + "app-privatekey", + field.WithDisplayName("GitHub App private key (PEM)"), + field.WithDescription("Raw PEM contents of the private key used to connect to the GitHub App. Takes precedence over app-privatekey-path when both are set."), field.WithIsSecret(true), - field.WithRequired(true), ) syncSecrets = field.BoolField( @@ -90,6 +102,7 @@ var Config = field.NewConfiguration( instanceUrlField, appIDField, appPrivateKeyPath, + appPrivateKey, orgField, syncSecrets, omitArchivedRepositories, @@ -110,7 +123,7 @@ var Config = field.NewConfiguration( Name: GithubAppGroup, DisplayName: "GitHub app", HelpText: "Use a github app for authentication", - Fields: []field.SchemaField{appIDField, appPrivateKeyPath, orgField, syncSecrets, omitArchivedRepositories, directCollaboratorsOnly}, + Fields: []field.SchemaField{appIDField, appPrivateKeyPath, appPrivateKey, orgField, syncSecrets, omitArchivedRepositories, directCollaboratorsOnly}, Default: false, }, }), diff --git a/pkg/config/config_validation_test.go b/pkg/config/config_validation_test.go new file mode 100644 index 00000000..0c3bd1a8 --- /dev/null +++ b/pkg/config/config_validation_test.go @@ -0,0 +1,119 @@ +package config + +import ( + "testing" + + "github.com/conductorone/baton-sdk/pkg/field" + "github.com/spf13/viper" +) + +// validate runs the same field validation the SDK CLI performs at startup +// (pkg/cli.commands -> field.Validate with the selected auth-method group). +// An empty authMethod mirrors invoking the connector without an explicit +// --auth-method, which resolves to the default (personal-access-token) group. +func validate(t *testing.T, authMethod string, values map[string]string) error { + t.Helper() + v := viper.New() + for k, val := range values { + v.Set(k, val) + } + if authMethod == "" { + return field.Validate(Config, v) + } + return field.Validate(Config, v, field.WithAuthMethod(authMethod)) +} + +// TestConfigValidationAuthMethods guards the real connector Config against +// regressions in how the two auth methods validate. In particular it locks in +// that the personal-access-token path validates with only a token set — i.e. +// the "app private key required" rule must NOT leak onto the PAT auth method. +func TestConfigValidationAuthMethods(t *testing.T) { + cases := []struct { + name string + authMethod string + values map[string]string + wantErr bool + }{ + { + name: "default auth (PAT) with only token", + authMethod: "", + values: map[string]string{"token": "ghp_example"}, + wantErr: false, + }, + { + name: "explicit PAT auth with only token", + authMethod: GithubPersonalAccessTokenGroup, + values: map[string]string{"token": "ghp_example"}, + wantErr: false, + }, + { + name: "github app auth with app-privatekey-path", + authMethod: GithubAppGroup, + values: map[string]string{ + "app-id": "123", + "org": "my-org", + "app-privatekey-path": "/secrets/key.pem", + }, + wantErr: false, + }, + { + name: "github app auth with in-memory app-privatekey", + authMethod: GithubAppGroup, + values: map[string]string{ + "app-id": "123", + "org": "my-org", + "app-privatekey": "-----BEGIN PRIVATE KEY-----\n...\n-----END PRIVATE KEY-----", + }, + wantErr: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validate(t, tc.authMethod, tc.values) + if tc.wantErr && err == nil { + t.Fatalf("expected validation error, got nil") + } + if !tc.wantErr && err != nil { + t.Fatalf("unexpected validation error: %v", err) + } + }) + } +} + +// TestFrameworkAtLeastOneConstraintBreaksPATAuth documents WHY the "at least +// one of --app-privatekey / --app-privatekey-path" requirement is enforced in +// the connector's GitHub App constructor (see pkg/connector.appPrivateKeyPEM) +// rather than as a framework-level field.FieldsAtLeastOneUsed constraint. +// +// field.Validate filters the field *presence* map by the selected auth-method +// group, but field.validateConstraints evaluates every constraint on the +// Configuration globally — it has no notion of field groups, and +// SchemaFieldGroup cannot carry group-scoped constraints. So a global +// AtLeastOne(app-privatekey-path, app-privatekey) constraint fires whenever +// neither key is present, INCLUDING under the personal-access-token auth +// method (and the default no-auth-method case), which would reject the common +// `baton-github --token=...` invocation. +// +// This test builds a config that mirrors the real Config but adds that +// constraint, and asserts it breaks PAT auth — if a future SDK version makes +// constraints group-aware this test will start failing, signalling that the +// requirement can safely move to the framework. +func TestFrameworkAtLeastOneConstraintBreaksPATAuth(t *testing.T) { + cfg := field.NewConfiguration( + Config.Fields, + field.WithFieldGroups(Config.FieldGroups), + field.WithConstraints(field.FieldsAtLeastOneUsed(appPrivateKeyPath, appPrivateKey)), + ) + + v := viper.New() + v.Set("token", "ghp_example") + + // PAT auth with only a token — no app key is set on purpose. + err := field.Validate(cfg, v, field.WithAuthMethod(GithubPersonalAccessTokenGroup)) + if err == nil { + t.Fatalf("expected the global AtLeastOne constraint to (incorrectly) break PAT auth, " + + "but validation passed — the SDK may now scope constraints to field groups; " + + "if so, the app-privatekey requirement can move to a framework constraint") + } +} diff --git a/pkg/connector/app_privatekey_test.go b/pkg/connector/app_privatekey_test.go new file mode 100644 index 00000000..3e455fdc --- /dev/null +++ b/pkg/connector/app_privatekey_test.go @@ -0,0 +1,41 @@ +package connector + +import ( + "testing" + + cfg "github.com/conductorone/baton-github/pkg/config" + "github.com/stretchr/testify/require" +) + +func TestAppPrivateKeyPEM(t *testing.T) { + const ( + inlineKey = "-----BEGIN PRIVATE KEY-----\ninline\n-----END PRIVATE KEY-----" + pathKey = "-----BEGIN PRIVATE KEY-----\nfrom-path\n-----END PRIVATE KEY-----" + ) + + t.Run("prefers app-privatekey when both are set", func(t *testing.T) { + got, err := appPrivateKeyPEM(&cfg.Github{ + AppPrivatekey: inlineKey, + AppPrivatekeyPath: []byte(pathKey), + }) + require.NoError(t, err) + require.Equal(t, inlineKey, got) + }) + + t.Run("uses app-privatekey when only it is set", func(t *testing.T) { + got, err := appPrivateKeyPEM(&cfg.Github{AppPrivatekey: inlineKey}) + require.NoError(t, err) + require.Equal(t, inlineKey, got) + }) + + t.Run("falls back to app-privatekey-path when app-privatekey is empty", func(t *testing.T) { + got, err := appPrivateKeyPEM(&cfg.Github{AppPrivatekeyPath: []byte(pathKey)}) + require.NoError(t, err) + require.Equal(t, pathKey, got) + }) + + t.Run("errors when neither is set", func(t *testing.T) { + _, err := appPrivateKeyPEM(&cfg.Github{}) + require.Error(t, err) + }) +} diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index 2cdb5a62..b38ed4e8 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -349,8 +349,27 @@ func newWithGithubPAT(ctx context.Context, ghc *cfg.Github) (*GitHub, error) { }, nil } +// appPrivateKeyPEM returns the GitHub App private key PEM contents to use, +// preferring the in-memory app-privatekey flag over the on-disk +// app-privatekey-path. Providing either one satisfies the requirement; if +// neither is set an error is returned. +func appPrivateKeyPEM(ghc *cfg.Github) (string, error) { + if ghc.AppPrivatekey != "" { + return ghc.AppPrivatekey, nil + } + if len(ghc.AppPrivatekeyPath) > 0 { + return string(ghc.AppPrivatekeyPath), nil + } + return "", errors.New("github app authentication requires either --app-privatekey or --app-privatekey-path") +} + func newWithGithubApp(ctx context.Context, ghc *cfg.Github) (*GitHub, error) { - jwttoken, err := getJWTToken(ghc.AppId, string(ghc.AppPrivatekeyPath)) + privateKey, err := appPrivateKeyPEM(ghc) + if err != nil { + return nil, err + } + + jwttoken, err := getJWTToken(ghc.AppId, privateKey) if err != nil { return nil, err } @@ -382,7 +401,7 @@ func newWithGithubApp(ctx context.Context, ghc *cfg.Github) (*GitHub, error) { }, &appJWTTokenRefresher{ appID: ghc.AppId, - privateKey: string(ghc.AppPrivatekeyPath), + privateKey: privateKey, }, ) // Wrap the installation-token refresher in a refreshableTokenSource so the