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

fix: identity: use the provider user ID as primary key #1744

Merged
merged 3 commits into from
Feb 15, 2025
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
1 change: 1 addition & 0 deletions pkg/api/authn/noauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func (n *NoAuth) AuthenticateRequest(req *http.Request) (*authenticator.Response
req.Context(),
&types.Identity{
ProviderUsername: "nobody",
ProviderUserID: "nobody",
},
req.Header.Get("X-Obot-User-Timezone"),
types2.RoleAdmin,
Expand Down
1 change: 1 addition & 0 deletions pkg/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func (b *Bootstrap) AuthenticateRequest(req *http.Request) (*authenticator.Respo
req.Context(),
&types.Identity{
ProviderUsername: "bootstrap",
ProviderUserID: "bootstrap",
},
req.Header.Get("X-Obot-User-Timezone"),
types2.RoleAdmin,
Expand Down
1 change: 1 addition & 0 deletions pkg/gateway/client/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func (u UserDecorator) AuthenticateRequest(req *http.Request) (*authenticator.Re
AuthProviderName: firstValue(resp.User.GetExtra(), "auth_provider_name"),
AuthProviderNamespace: firstValue(resp.User.GetExtra(), "auth_provider_namespace"),
ProviderUsername: resp.User.GetName(),
ProviderUserID: resp.User.GetUID(),
}, req.Header.Get("X-Obot-User-Timezone"))
if err != nil {
return nil, false, err
Expand Down
32 changes: 29 additions & 3 deletions pkg/gateway/client/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package client
import (
"context"
"errors"
"fmt"

types2 "github.com/obot-platform/obot/apiclient/types"
"github.com/obot-platform/obot/pkg/gateway/types"
Expand Down Expand Up @@ -37,14 +38,39 @@ func (c *Client) EnsureIdentityWithRole(ctx context.Context, id *types.Identity,
func ensureIdentity(tx *gorm.DB, id *types.Identity, timezone string, role types2.Role) (*types.User, error) {
email := id.Email
if err := tx.First(id).Error; errors.Is(err, gorm.ErrRecordNotFound) {
if err = tx.Create(id).Error; err != nil {
// Before we try creating a new identity, we need to check if there is one that has not been fully migrated yet.
migratedIdentity := &types.Identity{
ProviderUsername: id.ProviderUsername,
ProviderUserID: fmt.Sprintf("OBOT_PLACEHOLDER_%s", id.ProviderUsername),
AuthProviderName: id.AuthProviderName,
AuthProviderNamespace: id.AuthProviderNamespace,
}
if err := tx.First(migratedIdentity).Error; errors.Is(err, gorm.ErrRecordNotFound) {
// If the identity does not exist, we can create it.
if err = tx.Create(id).Error; err != nil {
return nil, err
}
} else if err != nil {
return nil, err
} else {
// The migrated identity exists. We need to update it with the right provider_user_id.
if err := tx.Model(&migratedIdentity).Where("provider_user_id = ?", fmt.Sprintf("OBOT_PLACEHOLDER_%s", id.ProviderUsername)).Update("provider_user_id", id.ProviderUserID).Error; err != nil {
return nil, err
}

// Now we should be able to load the identity.
if err := tx.First(id).Error; err != nil {
return nil, err
}
}
} else if err != nil {
return nil, err
} else if id.Email != email {
}

// Check to see if the email got updated.
if id.Email != email {
id.Email = email
if err = tx.Updates(id).Error; err != nil {
if err := tx.Updates(id).Error; err != nil {
return nil, err
}
}
Expand Down
51 changes: 51 additions & 0 deletions pkg/gateway/db/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,57 @@ func (db *DB) AutoMigrate() (err error) {
}
}()

// Only run PostgreSQL-specific migrations if using PostgreSQL
if db.gormDB.Dialector.Name() == "postgres" {
// Check if the identities table exists
var exists bool
if err := tx.Raw(`
SELECT EXISTS (
SELECT 1
FROM information_schema.tables
WHERE table_name = 'identities'
)
`).Scan(&exists).Error; err != nil {
return err
}

if exists {
// The identities table needs to have auth_provider_namespace,auth_provider_name,provider_user_id as a primary key.
// It used to have auth_provider_namespace,auth_provider_name,provider_username as a primary key.

// Check if the migration is needed.
var needsIdentityMigration bool
if err := tx.Raw(`
SELECT COUNT(*) = 0 as needs_migration
FROM information_schema.key_column_usage
WHERE table_name = 'identities'
AND constraint_name = 'identities_pkey'
AND column_name = 'provider_user_id'
`).Scan(&needsIdentityMigration).Error; err != nil {
return err
}

if needsIdentityMigration {
// Add provider_user_id to identities table and update primary key.
if err := tx.Exec(`
-- Drop existing primary key
ALTER TABLE identities DROP CONSTRAINT identities_pkey;

-- Add provider_user_id column
ALTER TABLE identities ADD COLUMN provider_user_id text NOT NULL DEFAULT '';

-- Set placeholder values for existing records
UPDATE identities SET provider_user_id = 'OBOT_PLACEHOLDER_' || provider_username WHERE provider_user_id = '';

-- Add new primary key
ALTER TABLE identities ADD PRIMARY KEY (auth_provider_name, auth_provider_namespace, provider_user_id);
`).Error; err != nil {
return err
}
}
}
}

return tx.AutoMigrate(
types.AuthToken{},
types.TokenRequest{},
Expand Down
3 changes: 2 additions & 1 deletion pkg/gateway/types/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import "time"
type Identity struct {
AuthProviderName string `json:"authProviderName" gorm:"primaryKey;index:idx_user_auth_id"`
AuthProviderNamespace string `json:"authProviderNamespace" gorm:"primaryKey;index:idx_user_auth_id"`
ProviderUsername string `json:"providerUsername" gorm:"primaryKey"`
ProviderUsername string `json:"providerUsername"`
ProviderUserID string `json:"providerUserID" gorm:"primaryKey"`
Email string `json:"email"`
UserID uint `json:"userID" gorm:"index:idx_user_auth_id"`
IconURL string `json:"iconURL"`
Expand Down
1 change: 1 addition & 0 deletions pkg/services/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ func New(ctx context.Context, config Config) (*Services, error) {
// is enabled.
if err := gatewayClient.RemoveIdentity(ctx, &types.Identity{
ProviderUsername: "nobody",
ProviderUserID: "nobody",
}); err != nil {
return nil, fmt.Errorf(`failed to remove "nobody" user and identity from database: %w`, err)
}
Expand Down