Skip to content

Commit

Permalink
Add provider configuration to the session store (#3348)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
jhrozek authored May 16, 2024
1 parent bc0b4ff commit 2f83c54
Show file tree
Hide file tree
Showing 11 changed files with 2,165 additions and 2,058 deletions.
19 changes: 19 additions & 0 deletions database/migrations/000058_session_store_config.down.sql
Original file line number Diff line number Diff line change
@@ -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;
19 changes: 19 additions & 0 deletions database/migrations/000058_session_store_config.up.sql
Original file line number Diff line number Diff line change
@@ -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;
4 changes: 2 additions & 2 deletions database/query/session_store.sql
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
2 changes: 2 additions & 0 deletions docs/docs/ref/proto.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

48 changes: 32 additions & 16 deletions internal/controlplane/handlers_oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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")
Expand All @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down
5 changes: 4 additions & 1 deletion internal/controlplane/handlers_oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions internal/db/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 18 additions & 13 deletions internal/db/session_store.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions pkg/api/openapi/minder/v1/minder.swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 2f83c54

Please sign in to comment.