-
Notifications
You must be signed in to change notification settings - Fork 124
Add instance-managed OAuth providers #339
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| package oauth | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
| "strings" | ||
| ) | ||
|
|
||
| const ( | ||
| // GoogleOAuthClientIDEnv and GoogleOAuthClientSecretEnv configure the | ||
| // instance-managed Google OAuth application. | ||
| GoogleOAuthClientIDEnv = "AGENT_VAULT_OAUTH_GOOGLE_CLIENT_ID" | ||
| GoogleOAuthClientSecretEnv = "AGENT_VAULT_OAUTH_GOOGLE_CLIENT_SECRET" | ||
| ) | ||
|
|
||
| // ManagedProvider is an OAuth application configured by the instance operator. | ||
| // Vault users authorize their own accounts, but do not need to create or supply | ||
| // an OAuth client. | ||
| type ManagedProvider struct { | ||
| ID string | ||
| AuthorizationURL string | ||
| TokenURL string | ||
| ClientID string | ||
| ClientSecret string | ||
| TokenAuthMethod string | ||
| } | ||
|
|
||
| // LoadManagedProvidersFromEnv loads operator-managed OAuth applications. | ||
| // A partially configured provider fails closed instead of falling back to | ||
| // user-supplied client credentials unexpectedly. | ||
| func LoadManagedProvidersFromEnv() ([]ManagedProvider, error) { | ||
| googleClientID := strings.TrimSpace(os.Getenv(GoogleOAuthClientIDEnv)) | ||
| googleClientSecret := os.Getenv(GoogleOAuthClientSecretEnv) | ||
|
|
||
| if googleClientID == "" && googleClientSecret == "" { | ||
| return nil, nil | ||
| } | ||
| if googleClientID == "" || googleClientSecret == "" { | ||
| return nil, fmt.Errorf("%s and %s must be set together", GoogleOAuthClientIDEnv, GoogleOAuthClientSecretEnv) | ||
| } | ||
|
|
||
| // Keep the secret in process memory after startup, not in the inherited | ||
| // environment where child processes could read it. | ||
| _ = os.Unsetenv(GoogleOAuthClientSecretEnv) | ||
|
|
||
| return []ManagedProvider{{ | ||
| ID: "google", | ||
| AuthorizationURL: "https://accounts.google.com/o/oauth2/v2/auth?access_type=offline&prompt=consent", | ||
| TokenURL: "https://oauth2.googleapis.com/token", | ||
| ClientID: googleClientID, | ||
| ClientSecret: googleClientSecret, | ||
| TokenAuthMethod: "client_secret_post", | ||
| }}, nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| package oauth | ||
|
|
||
| import ( | ||
| "os" | ||
| "strings" | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestLoadManagedProvidersFromEnvDisabled(t *testing.T) { | ||
| t.Setenv(GoogleOAuthClientIDEnv, "") | ||
| t.Setenv(GoogleOAuthClientSecretEnv, "") | ||
|
|
||
| providers, err := LoadManagedProvidersFromEnv() | ||
| if err != nil { | ||
| t.Fatalf("LoadManagedProvidersFromEnv: %v", err) | ||
| } | ||
| if len(providers) != 0 { | ||
| t.Fatalf("providers = %d, want 0", len(providers)) | ||
| } | ||
| } | ||
|
|
||
| func TestLoadManagedProvidersFromEnvGoogle(t *testing.T) { | ||
| t.Setenv(GoogleOAuthClientIDEnv, " google-client-id ") | ||
| t.Setenv(GoogleOAuthClientSecretEnv, "google-client-secret") | ||
|
|
||
| providers, err := LoadManagedProvidersFromEnv() | ||
| if err != nil { | ||
| t.Fatalf("LoadManagedProvidersFromEnv: %v", err) | ||
| } | ||
| if len(providers) != 1 { | ||
| t.Fatalf("providers = %d, want 1", len(providers)) | ||
| } | ||
|
|
||
| got := providers[0] | ||
| if got.ID != "google" { | ||
| t.Errorf("ID = %q, want google", got.ID) | ||
| } | ||
| if got.ClientID != "google-client-id" { | ||
| t.Errorf("ClientID = %q, want trimmed client ID", got.ClientID) | ||
| } | ||
| if got.ClientSecret != "google-client-secret" { | ||
| t.Errorf("ClientSecret = %q, want configured secret", got.ClientSecret) | ||
| } | ||
| if !strings.Contains(got.AuthorizationURL, "access_type=offline") || !strings.Contains(got.AuthorizationURL, "prompt=consent") { | ||
| t.Errorf("AuthorizationURL = %q, want offline consent parameters", got.AuthorizationURL) | ||
| } | ||
| if _, ok := os.LookupEnv(GoogleOAuthClientSecretEnv); ok { | ||
| t.Errorf("%s remained in environment", GoogleOAuthClientSecretEnv) | ||
| } | ||
| } | ||
|
|
||
| func TestLoadManagedProvidersFromEnvRejectsPartialConfig(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| clientID string | ||
| clientSecret string | ||
| }{ | ||
| {name: "missing secret", clientID: "google-client-id"}, | ||
| {name: "missing client ID", clientSecret: "google-client-secret"}, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Setenv(GoogleOAuthClientIDEnv, tt.clientID) | ||
| t.Setenv(GoogleOAuthClientSecretEnv, tt.clientSecret) | ||
|
|
||
| if _, err := LoadManagedProvidersFromEnv(); err == nil { | ||
| t.Fatal("LoadManagedProvidersFromEnv succeeded with partial config") | ||
| } | ||
| }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,59 @@ | ||||||||||||||||||||||||||||
| package server | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||||
| "sort" | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| "github.com/Infisical/agent-vault/internal/oauth" | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // SetManagedOAuthProviders configures OAuth applications supplied by the | ||||||||||||||||||||||||||||
| // instance operator. It must be called before the server starts. | ||||||||||||||||||||||||||||
| func (s *Server) SetManagedOAuthProviders(providers []oauth.ManagedProvider) { | ||||||||||||||||||||||||||||
| s.managedOAuthProviders = make(map[string]oauth.ManagedProvider, len(providers)) | ||||||||||||||||||||||||||||
| for _, provider := range providers { | ||||||||||||||||||||||||||||
| if provider.ID == "" { | ||||||||||||||||||||||||||||
| continue | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| s.managedOAuthProviders[provider.ID] = provider | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (s *Server) managedOAuthProviderIDs() []string { | ||||||||||||||||||||||||||||
| ids := make([]string, 0, len(s.managedOAuthProviders)) | ||||||||||||||||||||||||||||
| for id := range s.managedOAuthProviders { | ||||||||||||||||||||||||||||
| ids = append(ids, id) | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| sort.Strings(ids) | ||||||||||||||||||||||||||||
| return ids | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (s *Server) applyManagedOAuthProvider(req *oauthConnectRequest) error { | ||||||||||||||||||||||||||||
| if req.Provider == "" { | ||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| provider, ok := s.managedOAuthProviders[req.Provider] | ||||||||||||||||||||||||||||
| if !ok { | ||||||||||||||||||||||||||||
| return fmt.Errorf("managed OAuth provider %q is not configured", req.Provider) | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| req.AuthorizationURL = provider.AuthorizationURL | ||||||||||||||||||||||||||||
| req.TokenURL = provider.TokenURL | ||||||||||||||||||||||||||||
| req.ClientID = provider.ClientID | ||||||||||||||||||||||||||||
| req.ClientSecret = provider.ClientSecret | ||||||||||||||||||||||||||||
| req.TokenAuthMethod = provider.TokenAuthMethod | ||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||
|
Comment on lines
+41
to
+46
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| func (s *Server) managedOAuthProviderForConfig(authorizationURL, tokenURL, clientID string) string { | ||||||||||||||||||||||||||||
| for _, id := range s.managedOAuthProviderIDs() { | ||||||||||||||||||||||||||||
| provider := s.managedOAuthProviders[id] | ||||||||||||||||||||||||||||
| if provider.AuthorizationURL == authorizationURL && | ||||||||||||||||||||||||||||
| provider.TokenURL == tokenURL && | ||||||||||||||||||||||||||||
| provider.ClientID == clientID { | ||||||||||||||||||||||||||||
| return id | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| return "" | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
googleClientIDis whitespace-trimmed (to tolerate copy-paste errors) butgoogleClientSecretis not. An operator who accidentally copies the secret with a trailing newline or space will get a secret stored verbatim — causing token-exchange failures that are hard to diagnose. Trim both for consistency.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!