Skip to content

Commit

Permalink
Separate create / delete config of oauth providers
Browse files Browse the repository at this point in the history
ref DEV-1313
  • Loading branch information
louischan-oursky committed May 28, 2024
2 parents 4683c25 + 4946e0e commit 3f93144
Show file tree
Hide file tree
Showing 31 changed files with 352 additions and 375 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ require (
)

require (
github.com/authgear/oauthrelyingparty v1.2.0
github.com/authgear/oauthrelyingparty v1.3.0
github.com/go-jose/go-jose/v3 v3.0.3
)

Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ github.com/alicebob/miniredis/v2 v2.31.0 h1:ObEFUNlJwoIiyjxdrYF0QIDE7qXcLc7D3WpS
github.com/alicebob/miniredis/v2 v2.31.0/go.mod h1:UB/T2Uztp7MlFSDakaX1sTXUv5CASoprx0wulRT6HBg=
github.com/authgear/graphql-go-relay v0.0.0-20201016065100-df672205b892 h1:OIPk6DEk51wL35vsCYfLacc7pZ0ev8TKvmcp+EKOnPU=
github.com/authgear/graphql-go-relay v0.0.0-20201016065100-df672205b892/go.mod h1:SxJGRjo7+1SFr8pfMghiCM17WWFSswRwMvtv6t1aDO8=
github.com/authgear/oauthrelyingparty v1.2.0 h1:YGlrFgz0xDukuMS6oUOwcTeoq+Exv0Lx9wKBg0QUFB4=
github.com/authgear/oauthrelyingparty v1.2.0/go.mod h1:0UcO0p5eS9BPLulX6K7TvkXkuPeTn3QhAJ/UQg4fmiQ=
github.com/authgear/oauthrelyingparty v1.3.0 h1:OqHMh4OPXIldC94zXV4GYtTesqMcMveNVhLQiFPfPTs=
github.com/authgear/oauthrelyingparty v1.3.0/go.mod h1:0UcO0p5eS9BPLulX6K7TvkXkuPeTn3QhAJ/UQg4fmiQ=
github.com/aws/aws-sdk-go v1.47.9 h1:rarTsos0mA16q+huicGx0e560aYRtOucV5z2Mw23JRY=
github.com/aws/aws-sdk-go v1.47.9/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk=
github.com/beevik/etree v1.1.0/go.mod h1:r8Aw8JqVegEf0w2fDnATrX9VpkMcyFeM0FhwO62wh+A=
Expand Down
4 changes: 2 additions & 2 deletions pkg/auth/webapp/wechat_redirect_uri_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (m *WeChatRedirectURIMiddleware) Handle(next http.Handler) http.Handler {

func (m *WeChatRedirectURIMiddleware) isWechatEnabled() bool {
for _, providerConfig := range m.IdentityConfig.OAuth.Providers {
if providerConfig.Type() == oauthrelyingpartywechat.Type {
if providerConfig.AsProviderConfig().Type() == oauthrelyingpartywechat.Type {
return true
}
}
Expand All @@ -83,7 +83,7 @@ func (m *WeChatRedirectURIMiddleware) populateWechatRedirectURI(
// Validate x_wechat_redirect_uri
valid := false
for _, providerConfig := range m.IdentityConfig.OAuth.Providers {
if providerConfig.Type() == oauthrelyingpartywechat.Type {
if providerConfig.AsProviderConfig().Type() == oauthrelyingpartywechat.Type {
for _, allowed := range oauthrelyingpartywechat.ProviderConfig(providerConfig).WechatRedirectURIs() {
if weChatRedirectURI == allowed {
valid = true
Expand Down
4 changes: 2 additions & 2 deletions pkg/lib/authenticationflow/declarative/data_identification.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ func NewIdentificationOptionLoginID(i config.AuthenticationFlowIdentification) I
func NewIdentificationOptionsOAuth(oauthConfig *config.OAuthSSOConfig, oauthFeatureConfig *config.OAuthSSOProvidersFeatureConfig) []IdentificationOption {
output := []IdentificationOption{}
for _, p := range oauthConfig.Providers {
if !identity.IsOAuthSSOProviderTypeDisabled(p, oauthFeatureConfig) {
if !identity.IsOAuthSSOProviderTypeDisabled(p.AsProviderConfig(), oauthFeatureConfig) {
output = append(output, IdentificationOption{
Identification: config.AuthenticationFlowIdentificationOAuth,
ProviderType: p.Type(),
ProviderType: p.AsProviderConfig().Type(),
Alias: p.Alias(),
WechatAppType: wechat.ProviderConfig(p).AppType(),
})
Expand Down
11 changes: 6 additions & 5 deletions pkg/lib/authn/identity/candidate.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,19 @@ const (
CandidateKeyDeleteDisabled = "delete_disabled"
)

func NewOAuthCandidate(cfg oauthrelyingparty.ProviderConfig) Candidate {
func NewOAuthCandidate(cfg config.OAuthSSOProviderConfig) Candidate {
return Candidate{
CandidateKeyIdentityID: "",
CandidateKeyType: string(model.IdentityTypeOAuth),
CandidateKeyProviderType: string(cfg.Type()),
CandidateKeyProviderType: string(cfg.AsProviderConfig().Type()),
CandidateKeyProviderAlias: cfg.Alias(),
CandidateKeyProviderSubjectID: "",
CandidateKeyProviderAppType: string(wechat.ProviderConfig(cfg).AppType()),
CandidateKeyDisplayID: "",
CandidateKeyCreateDisabled: cfg.ModifyDisabled(),
CandidateKeyUpdateDisabled: cfg.ModifyDisabled(),
CandidateKeyDeleteDisabled: cfg.ModifyDisabled(),
CandidateKeyCreateDisabled: cfg.CreateDisabled(),
// Update is not supported
CandidateKeyUpdateDisabled: true,
CandidateKeyDeleteDisabled: cfg.DeleteDisabled(),
}
}

Expand Down
20 changes: 6 additions & 14 deletions pkg/lib/authn/identity/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"fmt"
"time"

"github.com/authgear/oauthrelyingparty/pkg/api/oauthrelyingparty"

"github.com/authgear/authgear-server/pkg/api/model"
"github.com/authgear/authgear-server/pkg/lib/config"
)
Expand Down Expand Up @@ -281,9 +279,9 @@ func (i *Info) findLoginIDConfig(c *config.IdentityConfig) (*config.LoginIDKeyCo
return keyConfig, ok
}

func (i *Info) findOAuthConfig(c *config.IdentityConfig) (oauthrelyingparty.ProviderConfig, bool) {
func (i *Info) findOAuthConfig(c *config.IdentityConfig) (config.OAuthSSOProviderConfig, bool) {
alias := i.OAuth.ProviderAlias
var providerConfig oauthrelyingparty.ProviderConfig
var providerConfig config.OAuthSSOProviderConfig
var ok bool = false
for _, pc := range c.OAuth.Providers {
pcAlias := pc.Alias()
Expand All @@ -309,8 +307,7 @@ func (i *Info) CreateDisabled(c *config.IdentityConfig) bool {
if !ok {
return true
}
// TODO(tung): Change to use create_disabled
return providerConfig.ModifyDisabled()
return providerConfig.CreateDisabled()
case model.IdentityTypeAnonymous:
fallthrough
case model.IdentityTypeBiometric:
Expand Down Expand Up @@ -339,8 +336,7 @@ func (i *Info) DeleteDisabled(c *config.IdentityConfig) bool {
if !ok {
return true
}
// TODO(tung): Change to use delete_disabled
return providerConfig.ModifyDisabled()
return providerConfig.DeleteDisabled()
case model.IdentityTypeAnonymous:
fallthrough
case model.IdentityTypeBiometric:
Expand All @@ -365,12 +361,8 @@ func (i *Info) UpdateDisabled(c *config.IdentityConfig) bool {
}
return *keyConfig.UpdateDisabled
case model.IdentityTypeOAuth:
providerConfig, ok := i.findOAuthConfig(c)
if !ok {
return true
}
// TODO(tung): Change to use update_disabled
return providerConfig.ModifyDisabled()
// Update is not supported for oauth identity
return false
case model.IdentityTypeAnonymous:
fallthrough
case model.IdentityTypeBiometric:
Expand Down
2 changes: 1 addition & 1 deletion pkg/lib/authn/identity/oauth/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (p *Provider) New(

alias := ""
for _, providerConfig := range p.IdentityConfig.OAuth.Providers {
providerID := providerConfig.ProviderID()
providerID := providerConfig.AsProviderConfig().ProviderID()
if providerID.Equal(i.ProviderID) {
alias = providerConfig.Alias()
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/lib/authn/identity/oauth/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (s *Store) scan(scn db.Scanner) (*identity.OAuth, error) {

alias := ""
for _, providerConfig := range s.IdentityConfig.OAuth.Providers {
providerID := providerConfig.ProviderID()
providerID := providerConfig.AsProviderConfig().ProviderID()
if providerID.Equal(i.ProviderID) {
alias = providerConfig.Alias()
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/lib/authn/identity/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -846,10 +846,10 @@ func (s *Service) listOAuthCandidates(oauths []*identity.OAuth) []identity.Candi
out := []identity.Candidate{}
for _, providerConfig := range s.Identity.OAuth.Providers {
pc := providerConfig
if identity.IsOAuthSSOProviderTypeDisabled(pc, s.IdentityFeatureConfig.OAuth.Providers) {
if identity.IsOAuthSSOProviderTypeDisabled(pc.AsProviderConfig(), s.IdentityFeatureConfig.OAuth.Providers) {
continue
}
configProviderID := pc.ProviderID()
configProviderID := pc.AsProviderConfig().ProviderID()
candidate := identity.NewOAuthCandidate(pc)
matched := false
for _, iden := range oauths {
Expand All @@ -861,7 +861,7 @@ func (s *Service) listOAuthCandidates(oauths []*identity.OAuth) []identity.Candi
}
}
canAppend := true
if providerConfig.ModifyDisabled() && !matched {
if providerConfig.DeleteDisabled() && !matched {
canAppend = false
}
if canAppend {
Expand Down
19 changes: 11 additions & 8 deletions pkg/lib/authn/identity/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,12 @@ func TestProviderListCandidates(t *testing.T) {

Convey("oauth", func() {
p.Authentication.Identities = []model.IdentityType{model.IdentityTypeOAuth}
p.Identity.OAuth.Providers = []oauthrelyingparty.ProviderConfig{
p.Identity.OAuth.Providers = []config.OAuthSSOProviderConfig{
{
"alias": "google",
"type": google.Type,
"modify_disabled": false,
"create_disabled": false,
"delete_disabled": false,
},
}

Expand All @@ -76,7 +77,7 @@ func TestProviderListCandidates(t *testing.T) {
"provider_subject_id": "",
"provider_app_type": "",
"create_disabled": false,
"update_disabled": false,
"update_disabled": true,
"delete_disabled": false,
},
})
Expand Down Expand Up @@ -112,11 +113,12 @@ func TestProviderListCandidates(t *testing.T) {
})

Convey("respect authentication", func() {
p.Identity.OAuth.Providers = []oauthrelyingparty.ProviderConfig{
p.Identity.OAuth.Providers = []config.OAuthSSOProviderConfig{
{
"alias": "google",
"type": google.Type,
"modify_disabled": false,
"create_disabled": false,
"delete_disabled": false,
},
}
p.Identity.LoginID.Keys = []config.LoginIDKeyConfig{
Expand Down Expand Up @@ -182,11 +184,12 @@ func TestProviderListCandidates(t *testing.T) {
userID := "a"

p.Authentication.Identities = []model.IdentityType{model.IdentityTypeOAuth}
p.Identity.OAuth.Providers = []oauthrelyingparty.ProviderConfig{
p.Identity.OAuth.Providers = []config.OAuthSSOProviderConfig{
{
"alias": "google",
"type": google.Type,
"modify_disabled": false,
"create_disabled": false,
"delete_disabled": false,
},
}

Expand Down Expand Up @@ -217,7 +220,7 @@ func TestProviderListCandidates(t *testing.T) {
"provider_subject_id": "[email protected]",
"provider_app_type": "",
"create_disabled": false,
"update_disabled": false,
"update_disabled": true,
"delete_disabled": false,
},
})
Expand Down
10 changes: 4 additions & 6 deletions pkg/lib/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"sigs.k8s.io/yaml"

"github.com/authgear/authgear-server/pkg/api/model"
liboauthrelyingparty "github.com/authgear/authgear-server/pkg/lib/oauthrelyingparty"
"github.com/authgear/authgear-server/pkg/util/validation"
)

Expand Down Expand Up @@ -153,11 +152,10 @@ func (c *AppConfig) validateOAuthProvider(ctx *validation.Context) {
}
oauthProviderAliases[alias] = struct{}{}

// Validate provider config if it is a builin provider.
provider := providerConfig.MustGetProvider()
if builtinProvider, ok := provider.(liboauthrelyingparty.BuiltinProvider); ok {
builtinProvider.ValidateProviderConfig(childCtx, providerConfig)
}
// Validate provider config
provider := providerConfig.AsProviderConfig().MustGetProvider()
schema := OAuthSSOProviderConfigSchemaBuilder(validation.SchemaBuilder(provider.GetJSONSchema())).ToSimpleSchema()
childCtx.AddError(schema.Validator().ValidateValue(providerConfig))
}
}

Expand Down
57 changes: 55 additions & 2 deletions pkg/lib/config/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/authgear/oauthrelyingparty/pkg/api/oauthrelyingparty"

"github.com/authgear/authgear-server/pkg/api/model"
"github.com/authgear/authgear-server/pkg/util/validation"
)

var _ = Schema.Add("IdentityConfig", `
Expand Down Expand Up @@ -274,15 +275,67 @@ var _ = Schema.Add("OAuthSSOConfig", `
}
`)

func OAuthSSOProviderConfigSchemaBuilder(builder validation.SchemaBuilder) validation.SchemaBuilder {
builder.Properties().
Property("alias", validation.SchemaBuilder{}.Type(validation.TypeString)).
Property("modify_disabled", validation.SchemaBuilder{}.Type(validation.TypeBoolean)).
Property("create_disabled", validation.SchemaBuilder{}.Type(validation.TypeBoolean)).
Property("delete_disabled", validation.SchemaBuilder{}.Type(validation.TypeBoolean))
builder.AddRequired("alias")
return builder
}

type OAuthSSOProviderConfig oauthrelyingparty.ProviderConfig

func (c OAuthSSOProviderConfig) SetDefaults() {
if _, ok := c["modify_disabled"].(bool); !ok {
c["modify_disabled"] = false
}

if _, ok := c["create_disabled"].(bool); !ok {
c["create_disabled"] = c["modify_disabled"].(bool)
}

if _, ok := c["delete_disabled"].(bool); !ok {
c["delete_disabled"] = c["modify_disabled"].(bool)
}

c.AsProviderConfig().SetDefaults()

// Cleanup deprecated fields
delete(c, "modify_disabled")
}

func (c OAuthSSOProviderConfig) AsProviderConfig() oauthrelyingparty.ProviderConfig {
return oauthrelyingparty.ProviderConfig(c)
}

func (c OAuthSSOProviderConfig) Alias() string {
alias, ok := c["alias"].(string)
if ok {
return alias
}
// This method is called in validateOAuthProvider which is part of the validation process
// So it is possible that alias is an invalid value
return ""
}

func (c OAuthSSOProviderConfig) CreateDisabled() bool {
return c["create_disabled"].(bool)
}
func (c OAuthSSOProviderConfig) DeleteDisabled() bool {
return c["delete_disabled"].(bool)
}

type OAuthSSOConfig struct {
Providers []oauthrelyingparty.ProviderConfig `json:"providers,omitempty"`
Providers []OAuthSSOProviderConfig `json:"providers,omitempty"`
}

func (c *OAuthSSOConfig) GetProviderConfig(alias string) (oauthrelyingparty.ProviderConfig, bool) {
for _, conf := range c.Providers {
if conf.Alias() == alias {
cc := conf
return cc, true
return cc.AsProviderConfig(), true
}
}
return nil, false
Expand Down
10 changes: 5 additions & 5 deletions pkg/lib/config/testdata/config_tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ name: missing-oauth-provider-alias
error: |-
invalid configuration:
/identity/oauth/providers/0: required
map[actual:[claims client_id modify_disabled type] expected:[alias client_id type] missing:[alias]]
map[actual:[claims client_id create_disabled delete_disabled type] expected:[alias client_id type] missing:[alias]]
config:
id: test
http:
Expand Down Expand Up @@ -269,7 +269,7 @@ name: oauth-provider-apple
error: |-
invalid configuration:
/identity/oauth/providers/0: required
map[actual:[alias claims client_id modify_disabled type] expected:[alias client_id key_id team_id type] missing:[key_id team_id]]
map[actual:[alias claims client_id create_disabled delete_disabled type] expected:[alias client_id key_id team_id type] missing:[key_id team_id]]
config:
id: test
http:
Expand All @@ -286,7 +286,7 @@ name: oauth-provider-azureadv2
error: |-
invalid configuration:
/identity/oauth/providers/0: required
map[actual:[alias claims client_id modify_disabled type] expected:[alias client_id tenant type] missing:[tenant]]
map[actual:[alias claims client_id create_disabled delete_disabled type] expected:[alias client_id tenant type] missing:[tenant]]
config:
id: test
http:
Expand All @@ -302,7 +302,7 @@ name: oauth-provider-azureadb2c
error: |-
invalid configuration:
/identity/oauth/providers/0: required
map[actual:[alias claims client_id modify_disabled type] expected:[alias client_id policy tenant type] missing:[policy tenant]]
map[actual:[alias claims client_id create_disabled delete_disabled type] expected:[alias client_id policy tenant type] missing:[policy tenant]]
config:
id: test
http:
Expand All @@ -319,7 +319,7 @@ name: oauth-provider-adfs
error: |-
invalid configuration:
/identity/oauth/providers/0: required
map[actual:[alias claims client_id modify_disabled type] expected:[alias client_id discovery_document_endpoint type] missing:[discovery_document_endpoint]]
map[actual:[alias claims client_id create_disabled delete_disabled type] expected:[alias client_id discovery_document_endpoint type] missing:[discovery_document_endpoint]]
config:
id: test
http:
Expand Down
Loading

0 comments on commit 3f93144

Please sign in to comment.