Skip to content
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

Remove the now-unused CreateGitHubOAuthProvider #3540

Merged
merged 1 commit into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 0 additions & 15 deletions internal/providers/github/service/mock/service.go

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

108 changes: 0 additions & 108 deletions internal/providers/github/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ import (
"github.com/google/go-github/v61/github"
"github.com/google/uuid"
"github.com/rs/zerolog"
"github.com/sqlc-dev/pqtype"
"golang.org/x/oauth2"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/stacklok/minder/internal/config/server"
"github.com/stacklok/minder/internal/controlplane/metrics"
Expand All @@ -48,9 +45,6 @@ import (

// GitHubProviderService encapsulates methods for creating and updating providers
type GitHubProviderService interface {
// CreateGitHubOAuthProvider creates a GitHub OAuth provider with an access token credential
CreateGitHubOAuthProvider(ctx context.Context, providerName string, providerClass db.ProviderClass,
token oauth2.Token, stateData db.GetProjectIDBySessionStateRow, state string) (*db.Provider, error)
// CreateGitHubAppProvider creates a GitHub App provider with an installation ID in a known project
CreateGitHubAppProvider(ctx context.Context, token oauth2.Token, stateData db.GetProjectIDBySessionStateRow,
installationID int64, state string) (*db.Provider, error)
Expand Down Expand Up @@ -116,108 +110,6 @@ func NewGithubProviderService(
}
}

// CreateGitHubOAuthProvider creates a GitHub OAuth provider with an access token credential
func (p *ghProviderService) CreateGitHubOAuthProvider(
ctx context.Context,
providerName string,
providerClass db.ProviderClass,
token oauth2.Token,
stateData db.GetProjectIDBySessionStateRow,
state string,
) (*db.Provider, error) {
tx, err := p.store.BeginTransaction()
if err != nil {
return nil, status.Errorf(codes.Internal, "error starting transaction: %v", err)
}
defer p.store.Rollback(tx)

qtx := p.store.GetQuerierWithTransaction(tx)

// Check if the provider exists
provider, err := qtx.GetProviderByName(ctx, db.GetProviderByNameParams{
Name: providerName,
Projects: []uuid.UUID{stateData.ProjectID},
})
if errors.Is(err, sql.ErrNoRows) {

// If the provider does not exist, create it
providerDef, err := providers.GetProviderClassDefinition(providerName)
if err != nil {
return nil, fmt.Errorf("error getting provider definition: %w", err)
}

createdProvider, err := qtx.CreateProvider(ctx, db.CreateProviderParams{
Name: providerName,
ProjectID: stateData.ProjectID,
Class: providerClass,
Implements: providerDef.Traits,
Definition: json.RawMessage(`{"github": {}}`),
AuthFlows: providerDef.AuthorizationFlows,
})
if err != nil {
return nil, fmt.Errorf("error creating provider: %w", err)
}
provider = createdProvider
} else if err != nil {
return nil, fmt.Errorf("error getting provider from DB: %w", err)
}

// Older enrollments may not have a RemoteUser stored; these should age out fairly quickly.
p.mt.AddTokenOpCount(ctx, "check", stateData.RemoteUser.Valid)
if stateData.RemoteUser.Valid {
credential := credentials.NewGitHubTokenCredential(token.AccessToken)
// owner is empty, as per original logic
_, delegate, err := p.ghClientFactory.BuildOAuthClient("", credential, "")
if err != nil {
return nil, fmt.Errorf("unable to create github client: %w", err)
}
if err := verifyProviderTokenIdentity(ctx, stateData.RemoteUser.String, delegate); err != nil {
return nil, ErrInvalidTokenIdentity
}
} else {
zerolog.Ctx(ctx).Warn().Msg("RemoteUser not found in session state")
}

ftoken := &oauth2.Token{
AccessToken: token.AccessToken,
TokenType: token.TokenType,
RefreshToken: "",
}

// encode token
encryptedToken, err := p.cryptoEngine.EncryptOAuthToken(ftoken)
if err != nil {
return nil, fmt.Errorf("error encoding token: %w", err)
}

serialized, err := encryptedToken.Serialize()
if err != nil {
return nil, err
}

_, err = qtx.UpsertAccessToken(ctx, db.UpsertAccessTokenParams{
ProjectID: stateData.ProjectID,
Provider: providerName,
OwnerFilter: stateData.OwnerFilter,
EnrollmentNonce: sql.NullString{
Valid: true,
String: state,
},
EncryptedAccessToken: pqtype.NullRawMessage{
RawMessage: serialized,
Valid: true,
},
})
if err != nil {
return nil, fmt.Errorf("error inserting access token: %w", err)
}
if err := p.store.Commit(tx); err != nil {

return nil, status.Errorf(codes.Internal, "error committing transaction: %v", err)
}
return &provider, nil
}

// CreateGitHubAppProvider creates a GitHub App provider with an installation ID
func (p *ghProviderService) CreateGitHubAppProvider(
ctx context.Context,
Expand Down
115 changes: 0 additions & 115 deletions internal/providers/github/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"crypto/rsa"
"crypto/x509"
"database/sql"
"encoding/base64"
"encoding/json"
"encoding/pem"
"errors"
Expand All @@ -42,7 +41,6 @@ import (

"github.com/stacklok/minder/internal/config/server"
"github.com/stacklok/minder/internal/controlplane/metrics"
"github.com/stacklok/minder/internal/crypto"
mockcrypto "github.com/stacklok/minder/internal/crypto/mock"
"github.com/stacklok/minder/internal/db"
"github.com/stacklok/minder/internal/db/embedded"
Expand Down Expand Up @@ -128,119 +126,6 @@ func testCreatePrivateKeyFile(t *testing.T) *os.File {
return tmpFile
}

func TestProviderService_CreateGitHubOAuthProvider(t *testing.T) {
t.Parallel()

const (
stateNonce = "test-oauth-nonce"
stateNonceUpdate = "test-oauth-nonce-update"
accountID = 12345
)
ctrl := gomock.NewController(t)
defer ctrl.Finish()

cfg := &server.ProviderConfig{}

delegate := mockgh.NewMockDelegate(ctrl)
delegate.EXPECT().
GetUserId(gomock.Any()).
Return(int64(accountID), nil)
clientFactory := mockclients.NewMockGitHubClientFactory(ctrl)
clientFactory.EXPECT().
BuildOAuthClient(gomock.Any(), gomock.Any(), gomock.Any()).
Return(nil, delegate, nil)

provSvc, mocks := testNewGitHubProviderService(t, ctrl, cfg, nil, clientFactory)
dbproj, err := mocks.fakeStore.CreateProject(context.Background(),
db.CreateProjectParams{
Name: "test",
Metadata: []byte(`{}`),
})
require.NoError(t, err)

encryptedValue := base64.StdEncoding.EncodeToString([]byte("my-encrypted-token"))
encryptedToken := crypto.NewBackwardsCompatibleEncryptedData(encryptedValue)
mocks.cryptoMocks.EXPECT().
EncryptOAuthToken(gomock.Any()).
Return(encryptedToken, nil)

dbProv, err := provSvc.CreateGitHubOAuthProvider(
context.Background(),
clients.Github,
db.ProviderClassGithub,
oauth2.Token{
AccessToken: "my-access",
TokenType: "Bearer",
},
db.GetProjectIDBySessionStateRow{
ProjectID: dbproj.ID,
OwnerFilter: sql.NullString{
Valid: true,
String: "testorg",
},
RemoteUser: sql.NullString{
Valid: true,
String: strconv.Itoa(accountID),
},
},
stateNonce)
require.NoError(t, err)
require.NotNil(t, dbProv)
require.Equal(t, dbProv.ProjectID, dbproj.ID)
require.Equal(t, dbProv.AuthFlows, clients.OAuthAuthorizationFlows)
require.Equal(t, dbProv.Implements, clients.OAuthImplements)

dbToken, err := mocks.fakeStore.GetAccessTokenByProvider(context.Background(), dbProv.Name)
require.NoError(t, err)
require.Len(t, dbToken, 1)
require.True(t, dbToken[0].EncryptedAccessToken.Valid)
deserialized, err := crypto.DeserializeEncryptedData(dbToken[0].EncryptedAccessToken.RawMessage)
require.NoError(t, err)
require.Equal(t, deserialized, encryptedToken)
require.Equal(t, dbToken[0].OwnerFilter, sql.NullString{String: "testorg", Valid: true})
require.Equal(t, dbToken[0].EnrollmentNonce, sql.NullString{String: stateNonce, Valid: true})

// test updating token
mocks.cryptoMocks.EXPECT().
EncryptOAuthToken(gomock.Any()).
Return(encryptedToken, nil)

dbProvUpdated, err := provSvc.CreateGitHubOAuthProvider(
context.Background(),
clients.Github,
db.ProviderClassGithub,
oauth2.Token{
AccessToken: "my-access2",
TokenType: "Bearer",
},
db.GetProjectIDBySessionStateRow{
ProjectID: dbproj.ID,
OwnerFilter: sql.NullString{
Valid: true,
String: "testorg",
},
RemoteUser: sql.NullString{
Valid: false,
},
},
stateNonceUpdate)
require.NoError(t, err)
require.NotNil(t, dbProv)
require.Equal(t, dbProvUpdated.ProjectID, dbProv.ProjectID)
require.Equal(t, dbProvUpdated.AuthFlows, dbProv.AuthFlows)
require.Equal(t, dbProvUpdated.Implements, dbProv.Implements)

dbTokenUpdate, err := mocks.fakeStore.GetAccessTokenByProvider(context.Background(), dbProv.Name)
require.NoError(t, err)
require.Len(t, dbTokenUpdate, 1)
require.True(t, dbToken[0].EncryptedAccessToken.Valid)
deserialized, err = crypto.DeserializeEncryptedData(dbToken[0].EncryptedAccessToken.RawMessage)
require.NoError(t, err)
require.Equal(t, deserialized, encryptedToken)
require.Equal(t, dbTokenUpdate[0].OwnerFilter, sql.NullString{String: "testorg", Valid: true})
require.Equal(t, dbTokenUpdate[0].EnrollmentNonce, sql.NullString{String: stateNonceUpdate, Valid: true})
}

func TestProviderService_VerifyProviderTokenIdentity(t *testing.T) {
t.Parallel()

Expand Down