Skip to content

Commit

Permalink
refactor: replace oidc jsonschema with jsonnet
Browse files Browse the repository at this point in the history
This patch replaces the previous methodology of merging OIDC data which used JSON Schema with Extensions and JSON Path in favor of a much easier to use approach with JSONNet.

Closes #380

BREAKING CHANGE: This is a breaking change as previous OIDC configurations will not work. Please consult the newly written documentation on OpenID Connect to learn how to use OIDC in your login and registration flows. Since the OIDC feature was not publicly broadcasted yet we have chosen not to provide an upgrade path. If you have issues, please reach out on the forums or slack.
  • Loading branch information
aeneasr committed May 5, 2020
1 parent d901354 commit 2b45e79
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 156 deletions.
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ require (
github.com/gobuffalo/packr/v2 v2.8.0
github.com/gobuffalo/pop/v5 v5.0.11
github.com/gobuffalo/uuid v2.0.5+incompatible
github.com/gobwas/glob v0.2.3 // indirect
github.com/gofrs/uuid v3.2.0+incompatible
github.com/golang/gddo v0.0.0-20190904175337-72a348e765d2
github.com/golang/mock v1.3.1
github.com/google/go-github/v27 v27.0.1
github.com/google/go-jsonnet v0.15.1-0.20200415122941-8a0084e64395
github.com/google/uuid v1.1.1
github.com/gorilla/context v1.1.1
github.com/gorilla/sessions v1.1.3
Expand All @@ -54,7 +56,7 @@ require (
github.com/ory/mail/v3 v3.0.0
github.com/ory/sdk/swagutil v0.0.0-20200425113349-92ce176f501e
github.com/ory/viper v1.7.4
github.com/ory/x v0.0.116
github.com/ory/x v0.0.117
github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2
github.com/pkg/errors v0.9.1
github.com/shurcooL/go v0.0.0-20180423040247-9e1955d9fb6e
Expand Down
10 changes: 8 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,8 @@ github.com/gobuffalo/validate/v3 v3.2.0 h1:Zrpkz2kuZ4rGXLaO3IHVlwX512/cUWRvNjw46
github.com/gobuffalo/validate/v3 v3.2.0/go.mod h1:PrhDOdDHxtN8KUgMvF3TDL0r1YZXV4sQnyFX/EmeETY=
github.com/gobuffalo/x v0.0.0-20181003152136-452098b06085/go.mod h1:WevpGD+5YOreDJznWevcn8NTmQEW5STSBgIkpkjzqXc=
github.com/gobuffalo/x v0.0.0-20181007152206-913e47c59ca7/go.mod h1:9rDPXaB3kXdKWzMc4odGQQdG2e2DIEmANy5aSJ9yesY=
github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y=
github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJAkT8=
github.com/gofrs/uuid v3.1.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
github.com/gofrs/uuid v3.2.0+incompatible h1:y12jRkkFxsd7GpqdSZ+/KCs/fJbqpEXSGd4+jfEaewE=
github.com/gofrs/uuid v3.2.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
Expand Down Expand Up @@ -585,6 +587,10 @@ github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg=
github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
github.com/google/go-github/v27 v27.0.1 h1:sSMFSShNn4VnqCqs+qhab6TS3uQc+uVR6TD1bW6MavM=
github.com/google/go-github/v27 v27.0.1/go.mod h1:/0Gr8pJ55COkmv+S/yPKCczSkUPIM/LnFyubufRNIS0=
github.com/google/go-jsonnet v0.15.0 h1:lEUXTDnVsHu+CLLzMeWAdWV4JpCgkJeDqdVNS8RtyuY=
github.com/google/go-jsonnet v0.15.0/go.mod h1:ex9QcU8vzXQUDeNe4gaN1uhGQbTYpOeZ6AbWdy6JbX4=
github.com/google/go-jsonnet v0.15.1-0.20200415122941-8a0084e64395 h1:PftVLaNFPyiHId46033ADWFgXAWIwSDK9ESNRIKdj1Q=
github.com/google/go-jsonnet v0.15.1-0.20200415122941-8a0084e64395/go.mod h1:sOcuej3UW1vpPTZOr8L7RQimqai1a57bt5j22LzGZCw=
github.com/google/go-querystring v1.0.0 h1:Xkwi/a1rcvNg1PPYe5vI8GbeBY/jrVuDX5ASuANWTrk=
github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck=
github.com/google/martian v2.1.0+incompatible h1:/CP5g8u/VJHijgedC/Legn3BAbAaWPgecwXBIDzw5no=
Expand Down Expand Up @@ -915,8 +921,8 @@ github.com/ory/x v0.0.85/go.mod h1:s44V8t3xyjWZREcU+mWlp4h302rTuM4aLXcW+y5FbQ8=
github.com/ory/x v0.0.88/go.mod h1:wrnJRjIfYXFY/AUiuUlcIUpLBDxFtWc+8x6toAeLZXU=
github.com/ory/x v0.0.93/go.mod h1:lfcTaGXpTZs7IEQAW00r9EtTCOxD//SiP5uWtNiz31g=
github.com/ory/x v0.0.110/go.mod h1:DJfkE3GdakhshNhw4zlKoRaL/ozg/lcTahA9OCih2BE=
github.com/ory/x v0.0.116 h1:gq47UBzFe9l8n4CToLFMAkjNwqTR+oq1JZYxhA0T5dM=
github.com/ory/x v0.0.116/go.mod h1:ImFneVZHXPCeI1EYXLzRylIkOUMQnWT9Xwuasd8QHxw=
github.com/ory/x v0.0.117 h1:Ronk2N5NwgPn7OytRlS9rrGJCyg+MbJDH6xLlXCHQAo=
github.com/ory/x v0.0.117/go.mod h1:ImFneVZHXPCeI1EYXLzRylIkOUMQnWT9Xwuasd8QHxw=
github.com/parnurzeal/gorequest v0.2.15/go.mod h1:3Kh2QUMJoqw3icWAecsyzkpY7UzRfDhbRdTjtNwNiUE=
github.com/pborman/uuid v1.2.0 h1:J7Q5mO4ysT1dv8hyrUGHb9+ooztCXu1D8MY8DZYsu3g=
github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
Expand Down
42 changes: 0 additions & 42 deletions selfservice/strategy/oidc/extension.go

This file was deleted.

52 changes: 0 additions & 52 deletions selfservice/strategy/oidc/extension_test.go

This file was deleted.

6 changes: 4 additions & 2 deletions selfservice/strategy/oidc/form.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"github.com/tidwall/sjson"

"github.com/ory/x/decoderx"

"github.com/ory/kratos/identity"
)

func decoderRegistration(ref string) (decoderx.HTTPDecoderOption, error) {
Expand All @@ -28,9 +30,9 @@ func decoderRegistration(ref string) (decoderx.HTTPDecoderOption, error) {

// merge merges the userFormValues (extracted from the initial POST request) prefixed with `traits` (encoded) with the
// values coming from the OpenID Provider (openIDProviderValues).
func merge(userFormValues string, openIDProviderValues json.RawMessage, option decoderx.HTTPDecoderOption) (json.RawMessage, error) {
func merge(userFormValues string, openIDProviderValues json.RawMessage, option decoderx.HTTPDecoderOption) (identity.Traits, error) {
if userFormValues == "" {
return openIDProviderValues, nil
return identity.Traits(openIDProviderValues), nil
}

var decodedForm struct {
Expand Down
4 changes: 3 additions & 1 deletion selfservice/strategy/oidc/provider_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ type Configuration struct {
// Scope specifies optional requested permissions.
Scope []string `json:"scope"`

SchemaURL string `json:"schema_url"`
// Normalize specifies the JSONNet code snippet which uses the OpenID Connect Provider's data (e.g. GitHub or Google
// profile information) to hydrate the identity's data.
Normalize string `json:"normalizer"`
}

func (p Configuration) Redir(public *url.URL) string {
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/oidc/provider_generic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func makeAuthCodeURL(t *testing.T, r *login.Request) string {
ClientID: "client",
ClientSecret: "secret",
IssuerURL: "https://accounts.google.com",
SchemaURL: "file://./stub/hydra.schema.json",
Normalize: "file://./stub/hydra.schema.json",
}, public)
c, err := p.OAuth2(context.TODO())
require.NoError(t, err)
Expand Down
48 changes: 20 additions & 28 deletions selfservice/strategy/oidc/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@ import (
"strings"

"github.com/gofrs/uuid"
"github.com/google/go-jsonnet"
"github.com/julienschmidt/httprouter"
"github.com/pkg/errors"

"github.com/ory/x/errorsx"
"github.com/ory/x/fetcher"

"github.com/ory/x/jsonx"

"github.com/ory/herodot"
"github.com/ory/x/stringsx"
"github.com/ory/x/urlx"

"github.com/ory/kratos/selfservice/flow/login"
Expand Down Expand Up @@ -85,6 +86,7 @@ type dependencies interface {
type Strategy struct {
c configuration.Provider
d dependencies
f *fetcher.Fetcher
validator *schema.Validator
}

Expand Down Expand Up @@ -117,6 +119,7 @@ func NewStrategy(
return &Strategy{
c: c,
d: d,
f: fetcher.NewFetcher(),
validator: schema.NewValidator(),
}
}
Expand Down Expand Up @@ -379,57 +382,46 @@ func (s *Strategy) processRegistration(w http.ResponseWriter, r *http.Request, a
return
}

i := identity.NewIdentity(configuration.DefaultIdentityTraitsSchemaID)
runner, err := schema.NewExtensionRunner(schema.ExtensionRunnerOIDCMetaSchema, NewValidationExtensionRunner(i))
jn, err := s.f.Fetch(provider.Config().Normalize)
if err != nil {
s.handleError(w, r, a.GetID(), nil, err)
return
}

var doc bytes.Buffer
if err := json.NewEncoder(&doc).Encode(claims); err != nil {
var jsonClaims bytes.Buffer
if err := json.NewEncoder(&jsonClaims).Encode(claims); err != nil {
s.handleError(w, r, a.GetID(), nil, err)
return
}

// Validate the claims first (which will also copy the values around based on the schema)
if err := s.validator.Validate(
stringsx.Coalesce(
provider.Config().SchemaURL,
),
doc.Bytes(),
schema.WithExtensionRunner(runner),
); err != nil {
s.d.Logger().
WithField("provider", provider.Config().ID).
WithField("schema_url", provider.Config().SchemaURL).
WithField("claims", fmt.Sprintf("%+v", claims)).
Error("Unable to validate claims against provider schema. Your schema should work regardless of these values.")
// Force a system error because this can not be resolved by the user.
s.handleError(w, r, a.GetID(), nil, errors.WithStack(herodot.ErrInternalServerError.WithTrace(err).WithReasonf("%s", err)))
vm := jsonnet.MakeVM()
vm.ExtCode("claims", jsonClaims.String())
snippetTraits, err := vm.EvaluateSnippet(provider.Config().Normalize, jn.String())
if err != nil {
s.handleError(w, r, a.GetID(), nil, err)
return
}

i := identity.NewIdentity(configuration.DefaultIdentityTraitsSchemaID)
i.Traits = identity.Traits(snippetTraits)
option, err := decoderRegistration(s.c.DefaultIdentityTraitsSchemaURL().String())
if err != nil {
s.handleError(w, r, a.GetID(), nil, err)
return
}

traits, err := merge(
i.Traits, err = merge(
x.SessionGetStringOr(r, s.d.CookieManager(), sessionName, sessionFormState, ""),
json.RawMessage(i.Traits), option,
json.RawMessage(snippetTraits), option,
)
if err != nil {
s.handleError(w, r, a.GetID(), nil, err)
return
}

i.Traits = identity.Traits(traits)

// Validate the identity itself
if err := s.d.IdentityValidator().Validate(i); err != nil {
s.handleError(w, r, a.GetID(), traits, err)
s.handleError(w, r, a.GetID(), i.Traits, err)
return
}

Expand All @@ -440,7 +432,7 @@ func (s *Strategy) processRegistration(w http.ResponseWriter, r *http.Request, a
Provider: provider.Config().ID,
},
}); err != nil {
s.handleError(w, r, a.GetID(), traits, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to encode password options to JSON: %s", err)))
s.handleError(w, r, a.GetID(), i.Traits, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to encode password options to JSON: %s", err)))
return
}

Expand All @@ -451,7 +443,7 @@ func (s *Strategy) processRegistration(w http.ResponseWriter, r *http.Request, a
})

if err := s.d.RegistrationExecutor().PostRegistrationHook(w, r, identity.CredentialsTypeOIDC, a, i); err != nil {
s.handleError(w, r, a.GetID(), traits, err)
s.handleError(w, r, a.GetID(), i.Traits, err)
return
}
}
Expand Down Expand Up @@ -533,7 +525,7 @@ func (s *Strategy) provider(id string) (Provider, error) {
}
}

func (s *Strategy) handleError(w http.ResponseWriter, r *http.Request, rid uuid.UUID, traits json.RawMessage, err error) {
func (s *Strategy) handleError(w http.ResponseWriter, r *http.Request, rid uuid.UUID, traits []byte, err error) {
if x.IsZeroUUID(rid) {
s.d.SelfServiceErrorManager().Forward(r.Context(), w, r, err)
return
Expand Down
4 changes: 2 additions & 2 deletions selfservice/strategy/oidc/strategy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,15 +126,15 @@ func TestStrategy(t *testing.T) {
ClientID: "client",
ClientSecret: "secret",
IssuerURL: remotePublic + "/",
SchemaURL: "file://./stub/hydra.schema.json",
Normalize: "file://./stub/oidc.hydra.jsonnet",
},
{
Provider: "generic",
ID: "invalid-issuer",
ClientID: "client",
ClientSecret: "secret",
IssuerURL: strings.Replace(remotePublic, "127.0.0.1", "localhost", 1) + "/",
SchemaURL: "file://./stub/hydra.schema.json",
Normalize: "file://./stub/oidc.hydra.jsonnet",
},
},
},
Expand Down
25 changes: 0 additions & 25 deletions selfservice/strategy/oidc/stub/hydra.schema.json

This file was deleted.

11 changes: 11 additions & 0 deletions selfservice/strategy/oidc/stub/oidc.hydra.jsonnet
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
local subject = std.toString(std.extVar('claims').sub);

if std.length(subject) == 0 then error 'claim sub not set'

{
identity: {
traits: {
subject: subject,
},
},
}

0 comments on commit 2b45e79

Please sign in to comment.