Add provider configuration to the session store (#3348)

* Extend the GetAuthorizationURLRequest protobuf with a config

When GetAuthorizationURLRequest is called, it also creates the session
state for the provider about to be created. Let's extend the message to
include the config.

* Add providerClass to GetAuthorizationURLRequest

At the moment the session state treats the name and the class the same,
which works because we only have github. Let's pass the name and the
state separately.

For backwards compatibility, we still use the provider name as the class
unless set explicitly.

* Pass the config from GetAuthorizationURL to a newly added database column in the session_store table

Store the configuration passed through the GetAuthorizationURL to the
database so it can be retrieved when actually creating the provider.

* Bump database migration version

* Fix tests

* Fix lint

* Bump the migrations again

* Regenerate sqlc after new make bootstrap

database/migrations/000058_session_store_config.down.sql

@@ -0,0 +1,19 @@
+-- Copyright 2024 Stacklok, Inc
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--      http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+BEGIN;
+
+ALTER TABLE session_store DROP COLUMN IF EXISTS provider_config;
+
+COMMIT;

database/migrations/000058_session_store_config.up.sql

@@ -0,0 +1,19 @@
+-- Copyright 2024 Stacklok, Inc
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--      http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+BEGIN;
+
+ALTER TABLE session_store ADD COLUMN provider_config BYTEA;
+
+COMMIT;

database/query/session_store.sql

@@ -1,8 +1,8 @@
 -- name: CreateSessionState :one
-INSERT INTO session_store (provider, project_id, remote_user, session_state, owner_filter, redirect_url) VALUES ($1, $2, $3, $4, $5, $6) RETURNING *;
+INSERT INTO session_store (provider, project_id, remote_user, session_state, owner_filter, provider_config, redirect_url) VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING *;
 
 -- name: GetProjectIDBySessionState :one
-SELECT provider, project_id, remote_user, owner_filter, redirect_url FROM session_store WHERE session_state = $1;
+SELECT provider, project_id, remote_user, owner_filter, provider_config, redirect_url FROM session_store WHERE session_state = $1;
 
 -- name: DeleteSessionStateByProjectID :exec
 DELETE FROM session_store WHERE provider = $1 AND project_id = $2;

internal/controlplane/handlers_oauth.go

@@ -55,19 +55,26 @@ func (s *Server) GetAuthorizationURL(ctx context.Context,
 	entityCtx := engine.EntityFromContext(ctx)
 	projectID := entityCtx.Project.ID
 
-	var provider string
+	var providerName string
 	if req.GetContext().GetProvider() == "" {
-		provider = defaultProvider
+		providerName = defaultProvider
 	} else {
-		provider = req.GetContext().GetProvider()
+		providerName = req.GetContext().GetProvider()
+	}
+
+	var providerClass string
+	if req.GetProviderClass() == "" {
+		providerClass = providerName
+	} else {
+		providerClass = req.GetProviderClass()
 	}
 
 	// Telemetry logging
-	logger.BusinessRecord(ctx).Provider = provider
+	logger.BusinessRecord(ctx).Provider = providerName
 	logger.BusinessRecord(ctx).Project = projectID
 
 	// get provider info
-	providerDef, err := providers.GetProviderClassDefinition(provider)
+	providerDef, err := providers.GetProviderClassDefinition(providerClass)
 	if err != nil {
 		return nil, providerError(err)
 	}
@@ -82,7 +89,7 @@ func (s *Server) GetAuthorizationURL(ctx context.Context,
 	// trace call to AuthCodeURL
 	span := trace.SpanFromContext(ctx)
 	span.SetName("server.GetAuthorizationURL")
-	span.SetAttributes(attribute.Key("provider").String(provider))
+	span.SetAttributes(attribute.Key("provider").String(providerName))
 	defer span.End()
 
 	user, _ := auth.GetUserClaimFromContext[string](ctx, "gh_id")
@@ -97,7 +104,7 @@ func (s *Server) GetAuthorizationURL(ctx context.Context,
 
 	// Delete any existing session state for the project
 	err = s.store.DeleteSessionStateByProjectID(ctx, db.DeleteSessionStateByProjectIDParams{
-		Provider:  provider,
+		Provider:  providerName,
 		ProjectID: projectID})
 	if err != nil && !errors.Is(err, sql.ErrNoRows) {
 		return nil, status.Errorf(codes.Unknown, "error deleting session state: %s", err)
@@ -118,22 +125,31 @@ func (s *Server) GetAuthorizationURL(ctx context.Context,
 		redirectUrl = sql.NullString{Valid: true, String: encryptedRedirectUrl.EncodedData}
 	}
 
+	var confBytes []byte
+	if conf := req.GetConfig(); conf != nil {
+		confBytes, err = conf.MarshalJSON()
+		if err != nil {
+			return nil, status.Errorf(codes.InvalidArgument, "error marshalling config: %s", err)
+		}
+	}
+
 	// Insert the new session state into the database along with the user's project ID
 	// retrieved from the JWT token
 	_, err = s.store.CreateSessionState(ctx, db.CreateSessionStateParams{
-		Provider:     provider,
-		ProjectID:    projectID,
-		RemoteUser:   sql.NullString{Valid: user != "", String: user},
-		SessionState: state,
-		OwnerFilter:  owner,
-		RedirectUrl:  redirectUrl,
+		Provider:       providerName,
+		ProjectID:      projectID,
+		RemoteUser:     sql.NullString{Valid: user != "", String: user},
+		SessionState:   state,
+		OwnerFilter:    owner,
+		RedirectUrl:    redirectUrl,
+		ProviderConfig: confBytes,
 	})
 	if err != nil {
 		return nil, status.Errorf(codes.Unknown, "error inserting session state: %s", err)
 	}
 
 	// Create a new OAuth2 config for the given provider
-	oauthConfig, err := s.providerAuthFactory(provider, req.Cli)
+	oauthConfig, err := s.providerAuthFactory(providerClass, req.Cli)
 	if err != nil {
 		return nil, err
 	}
@@ -394,9 +410,9 @@ func (s *Server) getValidSessionState(ctx context.Context, state string) (db.Get
 	return stateData, nil
 }
 
-func (s *Server) exchangeCodeForToken(ctx context.Context, providerName string, code string) (*oauth2.Token, error) {
+func (s *Server) exchangeCodeForToken(ctx context.Context, providerClass string, code string) (*oauth2.Token, error) {
 	// generate a new OAuth2 config for the given provider
-	oauthConfig, err := s.providerAuthFactory(providerName, true)
+	oauthConfig, err := s.providerAuthFactory(providerClass, true)
 	if err != nil {
 		return nil, fmt.Errorf("error creating OAuth config: %w", err)
 	}

internal/controlplane/handlers_oauth_test.go

@@ -29,6 +29,8 @@ import (
 	"net/url"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/google/uuid"
 	"github.com/lestrrat-go/jwx/v2/jwt/openid"
 	"github.com/stretchr/testify/assert"
@@ -833,7 +835,8 @@ func (p partialDbParamsMatcher) Matches(x interface{}) bool {
 
 	typedX.SessionState = ""
 
-	return typedX == p.value
+	return cmp.Equal(typedX, p.value,
+		cmpopts.IgnoreFields(db.CreateSessionStateParams{}, "ProviderConfig"))
 }
 
 func (m partialDbParamsMatcher) String() string {