Skip to content

Stop using TokenPath for API key in Assist #26671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
justinas merged 2 commits into from
May 24, 2023
Merged

Stop using TokenPath for API key in Assist #26671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c25bd49
Stop using TokenPath for API key in Assist
May 22, 2023
58d554d
Merge remote-tracking branch 'origin/master' into justinas/plugin-rem…
May 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions api/proto/teleport/legacy/types/types.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5195,13 +5195,8 @@ message PluginBearerTokenCredentials {
// Token is the literal bearer token to be submitted to the 3rd-party API provider.
string token = 1;

// TokenFile is a path to the local file containing a bearer token.
Copy link
Copy Markdown
Contributor

@capnspacehook capnspacehook May 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure we want reserve this and not deprecate it instead?

Copy link
Copy Markdown
Contributor Author

@justinas justinas May 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we usually deprecate first, then remove the field + reserve.
85c426b

Since this field is not deeply entrenched in the codebase yet, and we're removing all usages immediately, it seems that reserved might be more fitting (can't accidentally use this field again in new code).

// This takes precedence over Token, and is currently only used by
// OpenAI plugin in Cloud, where by default a Teleport-owned key is used.
// This avoids exposing the token itself to the cluster.
// The file must exist and be readable on whatever runs the plugin
// (in the OpenAI case, it is the Proxy).
string token_file = 2;
reserved 2; // token_file
reserved "token_file";
}

// PluginList represents a list of plugin resources
Expand Down
8 changes: 4 additions & 4 deletions api/types/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ func (p *PluginV1) CheckAndSetDefaults() error {
if bearer == nil {
return trace.BadParameter("openai plugin must be used with the bearer token credential type")
}
if (bearer.Token == "") == (bearer.TokenFile == "") {
return trace.BadParameter("exactly one of Token and TokenFile must be specified")
if bearer.Token == "" {
return trace.BadParameter("Token must be specified")
}
case *PluginSpecV1_Jamf:
if settings.Jamf.JamfSpec.ApiEndpoint == "" {
Expand Down Expand Up @@ -153,8 +153,8 @@ func (p *PluginV1) CheckAndSetDefaults() error {
if bearer == nil {
return trace.BadParameter("okta plugin must be used with the bearer token credential type")
}
if (bearer.Token == "") == (bearer.TokenFile == "") {
return trace.BadParameter("exactly one of Token and TokenFile must be specified")
if bearer.Token == "" {
return trace.BadParameter("Token must be specified")
}
default:
return trace.BadParameter("settings are not set or have an unknown type")
Expand Down
31 changes: 0 additions & 31 deletions api/types/plugin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,19 +99,6 @@ func TestPluginOpenAIValidation(t *testing.T) {
require.NoError(t, err)
},
},
{
name: "valid credentials (token file)",
creds: &PluginCredentialsV1{
Credentials: &PluginCredentialsV1_BearerToken{
BearerToken: &PluginBearerTokenCredentials{
TokenFile: "/var/lib/secrets/openai_token",
},
},
},
assertErr: func(t require.TestingT, err error, args ...any) {
require.NoError(t, err)
},
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -201,24 +188,6 @@ func TestPluginOktaValidation(t *testing.T) {
require.NoError(t, err)
},
},
{
name: "valid credentials (token file)",
settings: &PluginSpecV1_Okta{
Okta: &PluginOktaSettings{
OrgUrl: "https://test.okta.com",
},
},
creds: &PluginCredentialsV1{
Credentials: &PluginCredentialsV1_BearerToken{
BearerToken: &PluginBearerTokenCredentials{
TokenFile: "/var/lib/secrets/okta_token",
},
},
},
assertErr: func(t require.TestingT, err error, args ...any) {
require.NoError(t, err)
},
},
}

for _, tc := range testCases {
Expand Down
Loading