Update IdP and OIDC Provider (#40)

* Update creating oidc provider * Validate partner and organization by checking in db tables. * Validate empty scopes * Fix oidc provider update * Fix application Host url in ACS URL of Idp
2026-08-24 15:47:19 +00:00 · 2022-03-07 19:06:57 +05:30
parent 274cfe7c4f
commit 7938f30668
4 changed files with 108 additions and 54 deletions
@@ -23,7 +23,7 @@ type OIDCProvider struct {
 	MapperFilename  string                 `bun:"mapper_filename"`
 	ClientId        string                 `bun:"client_id,notnull"`
 	ClientSecret    string                 `bun:"client_secret,notnull"`
-	Scopes          []string               `bun:"scopes,notnull"`
+	Scopes          []string               `bun:"scopes,array,notnull"`
 	IssuerURL       string                 `bun:"issuer_url,notnull"`
 	AuthURL         string                 `bun:"auth_url"`
 	TokenURL        string                 `bun:"token_url"`
@@ -160,10 +160,10 @@ func setup() {
 	viper.BindEnv(debugPortEnv)
 	viper.BindEnv(dbAddrEnv)
 	viper.BindEnv(dbNameEnv)
-	viper.BindEnv(dbPasswordEnv)
+	viper.BindEnv(dbUserEnv)
 	viper.BindEnv(dbPasswordEnv)
 	viper.BindEnv(devEnv)
-	viper.BindEnv(dbUserEnv)
+	viper.BindEnv(apiAddrEnv)
 	viper.BindEnv(kratosSchemeEnv)
 	viper.BindEnv(kratosAddrEnv)
 	viper.BindEnv(bootstrapKEKEnv)
@@ -185,6 +185,7 @@ func setup() {
 	dbName = viper.GetString(dbNameEnv)
 	dbUser = viper.GetString(dbUserEnv)
 	dbPassword = viper.GetString(dbPasswordEnv)
+	apiAddr = viper.GetString(apiAddrEnv)
 	dev = viper.GetBool(devEnv)
 	kratosScheme = viper.GetString(kratosSchemeEnv)
 	kratosAddr = viper.GetString(kratosAddrEnv)
@@ -245,7 +246,7 @@ func setup() {
 	gs = service.NewGroupService(db, as)
 	rs = service.NewRoleService(db, as)
 	rrs = service.NewRolepermissionService(db)
-	is = service.NewIdpService(db, apiAddrEnv)
+	is = service.NewIdpService(db, apiAddr)
 	oidcs = service.NewOIDCProviderService(db, kratosUrl)

 	//sentry related services
@@ -16,6 +16,7 @@ import (

 	"github.com/RafaySystems/rcloud-base/internal/models"
 	"github.com/RafaySystems/rcloud-base/internal/persistence/provider/pg"
+	"github.com/RafaySystems/rcloud-base/internal/utils"
 	commonv3 "github.com/RafaySystems/rcloud-base/proto/types/commonpb/v3"
 	systemv3 "github.com/RafaySystems/rcloud-base/proto/types/systempb/v3"
 	"github.com/google/uuid"
@@ -36,18 +37,20 @@ type IdpService interface {
 type idpService struct {
 	dao     pg.EntityDAO
 	appHost string
+	l       utils.Lookup
 }

 func NewIdpService(db *bun.DB, hostUrl string) IdpService {
 	return &idpService{
 		dao:     pg.NewEntityDAO(db),
 		appHost: hostUrl,
+		l:       utils.NewLookup(db),
 	}
 }

 func generateAcsURL(id string, hostUrl string) string {
 	b, _ := url.Parse(hostUrl)
-	return fmt.Sprintf("%s://%s/auth/v3/sso/acs/%s", b.Scheme, b.Host, id)
+	return fmt.Sprintf("%s/auth/v3/sso/acs/%s", b.String(), id)
 }

 // generateSpCert generates self signed certificate. Returns cert and
@@ -104,6 +107,20 @@ func generateSpCert(host string) (string, string, error) {
 	return string(cPEMBytes), string(privPEMBytes), nil
 }

+func (s *idpService) getPartnerOrganization(ctx context.Context, provider *systemv3.Idp) (uuid.UUID, uuid.UUID, error) {
+	partner := provider.GetMetadata().GetPartner()
+	org := provider.GetMetadata().GetOrganization()
+	partnerId, err := s.l.GetPartnerId(ctx, partner)
+	if err != nil {
+		return uuid.Nil, uuid.Nil, err
+	}
+	organizationId, err := s.l.GetOrganizationId(ctx, org)
+	if err != nil {
+		return partnerId, uuid.Nil, err
+	}
+	return partnerId, organizationId, nil
+}
+
 func (s *idpService) Create(ctx context.Context, idp *systemv3.Idp) (*systemv3.Idp, error) {
 	name := idp.Metadata.GetName()
 	domain := idp.Spec.GetDomain()
@@ -115,11 +132,23 @@ func (s *idpService) Create(ctx context.Context, idp *systemv3.Idp) (*systemv3.I
 	if len(domain) == 0 {
 		return &systemv3.Idp{}, fmt.Errorf("EMPTY DOMAIN")
 	}
-	e := &models.Idp{}
-	s.dao.GetByName(ctx, name, e)
-	if e.Name == name {
-		return &systemv3.Idp{}, fmt.Errorf("DUPLICATE NAME")
+
+	partnerId, organizationId, err := s.getPartnerOrganization(ctx, idp)
+	if err != nil {
+		return nil, fmt.Errorf("unable to get partner and org id")
 	}
+	i, _ := s.dao.GetIdByNamePartnerOrg(
+		ctx,
+		idp.GetMetadata().GetName(),
+		uuid.NullUUID{UUID: partnerId, Valid: true},
+		uuid.NullUUID{UUID: organizationId, Valid: true},
+		&models.Idp{},
+	)
+	if i != nil {
+		return nil, fmt.Errorf("Idp %q already exists", idp.GetMetadata().GetName())
+	}
+
+	e := &models.Idp{}
 	s.dao.GetX(ctx, "domain", domain, e)
 	if e.Domain == domain {
 		return &systemv3.Idp{}, fmt.Errorf("DUPLICATE DOMAIN")
@@ -129,6 +158,8 @@ func (s *idpService) Create(ctx context.Context, idp *systemv3.Idp) (*systemv3.I
 		Name:               name,
 		Description:        idp.Metadata.GetDescription(),
 		CreatedAt:          time.Now(),
+		PartnerId:          partnerId,
+		OrganizationId:     organizationId,
 		IdpName:            idp.Spec.GetIdpName(),
 		Domain:             domain,
 		SsoURL:             idp.Spec.GetSsoUrl(),
@@ -150,7 +181,7 @@ func (s *idpService) Create(ctx context.Context, idp *systemv3.Idp) (*systemv3.I
 		entity.SpCert = spcert
 		entity.SpKey = spkey
 	}
-	_, err := s.dao.Create(ctx, entity)
+	_, err = s.dao.Create(ctx, entity)
 	if err != nil {
 		return &systemv3.Idp{}, err
 	}
@@ -160,10 +191,8 @@ func (s *idpService) Create(ctx context.Context, idp *systemv3.Idp) (*systemv3.I
 		ApiVersion: apiVersion,
 		Kind:       "Idp",
 		Metadata: &commonv3.Metadata{
-			Name:         entity.Name,
-			Organization: entity.OrganizationId.String(),
-			Partner:      entity.PartnerId.String(),
-			Id:           entity.Id.String(),
+			Name: entity.Name,
+			Id:   entity.Id.String(),
 		},
 		Spec: &systemv3.IdpSpec{
 			IdpName:            entity.IdpName,
@@ -333,15 +362,13 @@ func (s *idpService) Update(ctx context.Context, idp *systemv3.Idp) (*systemv3.I
 		return &systemv3.Idp{}, err
 	}

-	acsURL := generateAcsURL(entity.Id.String(), s.appHost)
+	acsURL := generateAcsURL(idp.GetMetadata().GetId(), s.appHost)
 	rv := &systemv3.Idp{
 		ApiVersion: apiVersion,
 		Kind:       "Idp",
 		Metadata: &commonv3.Metadata{
-			Name:         entity.Name,
-			Organization: entity.OrganizationId.String(),
-			Partner:      entity.PartnerId.String(),
-			Id:           entity.Id.String(),
+			Name: entity.Name,
+			Id:   idp.GetMetadata().GetId(),
 		},
 		Spec: &systemv3.IdpSpec{
 			IdpName:            entity.IdpName,
@@ -10,6 +10,7 @@ import (

 	"github.com/RafaySystems/rcloud-base/internal/models"
 	"github.com/RafaySystems/rcloud-base/internal/persistence/provider/pg"
+	"github.com/RafaySystems/rcloud-base/internal/utils"
 	commonv3 "github.com/RafaySystems/rcloud-base/proto/types/commonpb/v3"
 	systemv3 "github.com/RafaySystems/rcloud-base/proto/types/systempb/v3"
 	"github.com/google/uuid"
@@ -31,12 +32,14 @@ type OIDCProviderService interface {
 type oidcProvider struct {
 	dao       pg.EntityDAO
 	kratosUrl string
+	l         utils.Lookup
 }

 func NewOIDCProviderService(db *bun.DB, kratosUrl string) OIDCProviderService {
 	return &oidcProvider{
 		dao:       pg.NewEntityDAO(db),
 		kratosUrl: kratosUrl,
+		l:         utils.NewLookup(db),
 	}
 }

@@ -50,16 +53,43 @@ func validateURL(rawURL string) error {
 	return err
 }

+func (s *oidcProvider) getPartnerOrganization(ctx context.Context, provider *systemv3.OIDCProvider) (uuid.UUID, uuid.UUID, error) {
+	partner := provider.GetMetadata().GetPartner()
+	org := provider.GetMetadata().GetOrganization()
+	partnerId, err := s.l.GetPartnerId(ctx, partner)
+	if err != nil {
+		return uuid.Nil, uuid.Nil, err
+	}
+	organizationId, err := s.l.GetOrganizationId(ctx, org)
+	if err != nil {
+		return partnerId, uuid.Nil, err
+	}
+	return partnerId, organizationId, nil
+}
+
 func (s *oidcProvider) Create(ctx context.Context, provider *systemv3.OIDCProvider) (*systemv3.OIDCProvider, error) {
-	// validate name
 	name := provider.Metadata.GetName()
 	if len(name) == 0 {
 		return &systemv3.OIDCProvider{}, fmt.Errorf("EMPTY NAME")
 	}
-	e := &models.OIDCProvider{}
-	s.dao.GetByName(ctx, name, e)
-	if e.Name == name {
-		return &systemv3.OIDCProvider{}, fmt.Errorf("DUPLICATE NAME")
+	scopes := provider.GetSpec().GetScopes()
+	if scopes == nil || len(scopes) == 0 {
+		return &systemv3.OIDCProvider{}, fmt.Errorf("EMPTY SCOPES")
+	}
+
+	partnerId, organizationId, err := s.getPartnerOrganization(ctx, provider)
+	if err != nil {
+		return nil, fmt.Errorf("unable to get partner and org id")
+	}
+	p, _ := s.dao.GetIdByNamePartnerOrg(
+		ctx,
+		provider.GetMetadata().GetName(),
+		uuid.NullUUID{UUID: partnerId, Valid: true},
+		uuid.NullUUID{UUID: organizationId, Valid: true},
+		&models.OIDCProvider{},
+	)
+	if p != nil {
+		return nil, fmt.Errorf("OIDC provider %q already exists", provider.GetMetadata().GetName())
 	}

 	mapUrl := provider.Spec.GetMapperUrl()
@@ -81,15 +111,17 @@ func (s *oidcProvider) Create(ctx context.Context, provider *systemv3.OIDCProvid
 	}

 	entity := &models.OIDCProvider{
-		Name:           name,
-		CreatedAt:      time.Time{},
-		ModifiedAt:     time.Time{},
-		ProviderName:   provider.Spec.GetProviderName(),
-		MapperURL:      mapUrl,
-		MapperFilename: provider.Spec.GetMapperFilename(),
-		ClientId:       provider.Spec.GetClientId(),
-		// ClientSecret is not returning to avoid leaking secret
-		// TODO: Use FieldMask on ClientSecret field
+		Name:            name,
+		Description:     provider.GetMetadata().GetDescription(),
+		CreatedAt:       time.Time{},
+		ModifiedAt:      time.Time{},
+		PartnerId:       partnerId,
+		OrganizationId:  organizationId,
+		ProviderName:    provider.Spec.GetProviderName(),
+		MapperURL:       mapUrl,
+		MapperFilename:  provider.Spec.GetMapperFilename(),
+		ClientId:        provider.Spec.GetClientId(),
+		ClientSecret:    provider.Spec.GetClientSecret(),
 		Scopes:          provider.Spec.GetScopes(),
 		IssuerURL:       issUrl,
 		AuthURL:         authUrl,
@@ -97,7 +129,7 @@ func (s *oidcProvider) Create(ctx context.Context, provider *systemv3.OIDCProvid
 		RequestedClaims: provider.Spec.GetRequestedClaims().AsMap(),
 		Predefined:      provider.Spec.GetPredefined(),
 	}
-	_, err := s.dao.Create(ctx, entity)
+	_, err = s.dao.Create(ctx, entity)
 	if err != nil {
 		return &systemv3.OIDCProvider{}, err
 	}
@@ -116,7 +148,6 @@ func (s *oidcProvider) Create(ctx context.Context, provider *systemv3.OIDCProvid
 			MapperUrl:       entity.MapperURL,
 			MapperFilename:  entity.MapperFilename,
 			ClientId:        entity.ClientId,
-			ClientSecret:    entity.ClientSecret,
 			Scopes:          entity.Scopes,
 			IssuerUrl:       entity.IssuerURL,
 			AuthUrl:         entity.AuthURL,
@@ -156,7 +187,6 @@ func (s *oidcProvider) GetByID(ctx context.Context, provider *systemv3.OIDCProvi
 			MapperUrl:       entity.MapperURL,
 			MapperFilename:  entity.MapperFilename,
 			ClientId:        entity.ClientId,
-			ClientSecret:    entity.ClientSecret,
 			Scopes:          entity.Scopes,
 			IssuerUrl:       entity.IssuerURL,
 			AuthUrl:         entity.AuthURL,
@@ -203,7 +233,6 @@ func (s *oidcProvider) GetByName(ctx context.Context, provider *systemv3.OIDCPro
 			MapperUrl:       entity.MapperURL,
 			MapperFilename:  entity.MapperFilename,
 			ClientId:        entity.ClientId,
-			ClientSecret:    entity.ClientSecret,
 			Scopes:          entity.Scopes,
 			IssuerUrl:       entity.IssuerURL,
 			AuthUrl:         entity.AuthURL,
@@ -267,9 +296,18 @@ func (s *oidcProvider) Update(ctx context.Context, provider *systemv3.OIDCProvid
 	if len(name) == 0 {
 		return &systemv3.OIDCProvider{}, status.Error(codes.InvalidArgument, "EMPTY NAME")
 	}
+	scopes := provider.GetSpec().GetScopes()
+	if scopes == nil || len(scopes) == 0 {
+		return &systemv3.OIDCProvider{}, fmt.Errorf("EMPTY SCOPES")
+	}
+
+	partnerId, organizationId, err := s.getPartnerOrganization(ctx, provider)
+	if err != nil {
+		return nil, fmt.Errorf("unable to get partner and org id")
+	}

 	existingP := &models.OIDCProvider{}
-	_, err := s.dao.GetByName(ctx, name, existingP)
+	_, err = s.dao.GetByName(ctx, name, existingP)
 	if err != nil {
 		if errors.Is(err, sql.ErrNoRows) {
 			return &systemv3.OIDCProvider{}, status.Errorf(codes.InvalidArgument, "OIDC PROVIDER %q NOT EXIST", name)
@@ -296,22 +334,11 @@ func (s *oidcProvider) Update(ctx context.Context, provider *systemv3.OIDCProvid
 		return &systemv3.OIDCProvider{}, fmt.Errorf("INVALID TOKEN URL")
 	}

-	orgId, err := uuid.Parse(provider.Metadata.GetOrganization())
-	if err != nil {
-		return &systemv3.OIDCProvider{}, status.Errorf(codes.InvalidArgument,
-			"ORG ID %q INCORRECT", provider.Metadata.GetOrganization())
-	}
-	partId, err := uuid.Parse(provider.Metadata.GetPartner())
-	if err != nil {
-		return &systemv3.OIDCProvider{}, status.Errorf(codes.InvalidArgument,
-			"PARTNER ID %q INCORRECT", provider.Metadata.GetPartner())
-	}
-
 	entity := &models.OIDCProvider{
 		Name:            provider.Metadata.GetName(),
 		Description:     provider.Metadata.GetDescription(),
-		OrganizationId:  orgId,
-		PartnerId:       partId,
+		OrganizationId:  organizationId,
+		PartnerId:       partnerId,
 		ModifiedAt:      time.Now(),
 		ProviderName:    provider.Spec.GetProviderName(),
 		MapperURL:       mapUrl,
@@ -337,21 +364,20 @@ func (s *oidcProvider) Update(ctx context.Context, provider *systemv3.OIDCProvid
 		Metadata: &commonv3.Metadata{
 			Name:        entity.Name,
 			Description: entity.Description,
-			Id:          entity.Id.String(),
+			Id:          provider.GetMetadata().GetId(),
 		},
 		Spec: &systemv3.OIDCProviderSpec{
 			ProviderName:    entity.ProviderName,
 			MapperUrl:       entity.MapperURL,
 			MapperFilename:  entity.MapperFilename,
 			ClientId:        entity.ClientId,
-			ClientSecret:    entity.ClientSecret,
 			Scopes:          entity.Scopes,
 			IssuerUrl:       entity.IssuerURL,
 			AuthUrl:         entity.AuthURL,
 			TokenUrl:        entity.TokenURL,
 			RequestedClaims: rclaims,
 			Predefined:      entity.Predefined,
-			CallbackUrl:     generateCallbackUrl(entity.Id.String(), s.kratosUrl),
+			CallbackUrl:     generateCallbackUrl(provider.GetMetadata().GetId(), s.kratosUrl),
 		},
 	}
 	return rv, nil