From 5826d76276a38257af43710c8617ab9393659147 Mon Sep 17 00:00:00 2001 From: barco Date: Tue, 12 Nov 2024 12:49:45 +0100 Subject: [PATCH 1/3] refactor: fix default value for `mapper_url` in ProviderCreate.tsx --- ui/src/pages/providers/ProviderCreate.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/src/pages/providers/ProviderCreate.tsx b/ui/src/pages/providers/ProviderCreate.tsx index 9578b019e..f493814e9 100644 --- a/ui/src/pages/providers/ProviderCreate.tsx +++ b/ui/src/pages/providers/ProviderCreate.tsx @@ -1,3 +1,6 @@ +// Copyright 2024 Canonical Ltd. +// SPDX-License-Identifier: AGPL-3.0 + import { FC } from "react"; import { ActionButton, @@ -31,7 +34,7 @@ const ProviderCreate: FC = () => { id: "", client_id: "", client_secret: "", - mapper_url: "file:///etc/config/kratos/okta_schema.jsonnet", + mapper_url: "", scope: "email", subject_source: "userinfo", }, From f42102acbd4074e5ae1dfd64e02b3fd29433d7a4 Mon Sep 17 00:00:00 2001 From: barco Date: Tue, 12 Nov 2024 17:14:26 +0100 Subject: [PATCH 2/3] refactor: allow for empty AuthURL and TokenURL if IssueURL is provided, allow Mapper to be empty since Kratos will use default Mapper if its value is empty and will try to use OIDC discovery when IssuerURL is provided --- pkg/idp/third_party.go | 12 ++++++------ pkg/idp/validation.go | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/pkg/idp/third_party.go b/pkg/idp/third_party.go index e8828d486..459e4f731 100644 --- a/pkg/idp/third_party.go +++ b/pkg/idp/third_party.go @@ -50,19 +50,19 @@ type Configuration struct { // IssuerURL is the OpenID Connect Server URL. You can leave this empty if `provider` is not set to `generic`. // If set, neither `auth_url` nor `token_url` are required. // validate that this field is required only when Provider field == "generic" - IssuerURL string `json:"issuer_url" yaml:"issuer_url" validate:"required_if=Provider generic"` + IssuerURL string `json:"issuer_url" yaml:"issuer_url"` // AuthURL is the authorize url, typically something like: https://example.org/oauth2/auth // Should only be used when the OAuth2 / OpenID Connect server is not supporting OpenID Connect Discovery and when // `provider` is set to `generic`. - // validate that this field is required only when Provider field == "generic" - AuthURL string `json:"auth_url" yaml:"auth_url" validate:"required_if=Provider generic"` + // validate that this field is required only when Provider field == "generic" and IssuerURL is empty + AuthURL string `json:"auth_url" yaml:"auth_url"` // TokenURL is the token url, typically something like: https://example.org/oauth2/token // Should only be used when the OAuth2 / OpenID Connect server is not supporting OpenID Connect Discovery and when // `provider` is set to `generic`. - // validate that this field is required only when Provider field == "generic" - TokenURL string `json:"token_url" yaml:"token_url" validate:"required_if=Provider generic"` + // validate that this field is required only when Provider field == "generic" and IssuerURL is empty + TokenURL string `json:"token_url" yaml:"token_url"` // Tenant is the Azure AD Tenant to use for authentication, and must be set when `provider` is set to `microsoft`. // Can be either `common`, `organizations`, `consumers` for a multitenant application or a specific tenant like @@ -103,7 +103,7 @@ type Configuration struct { // profile information) to hydrate the identity's data. // // It can be either a URL (file://, http(s)://, base64://) or an inline JSONNet code snippet. - Mapper string `json:"mapper_url" yaml:"mapper_url" validate:"required"` + Mapper string `json:"mapper_url" yaml:"mapper_url"` // RequestedClaims string encoded json object that specifies claims and optionally their properties which should be // included in the id_token or returned from the UserInfo Endpoint. diff --git a/pkg/idp/validation.go b/pkg/idp/validation.go index 46a548c27..f1b2b616b 100644 --- a/pkg/idp/validation.go +++ b/pkg/idp/validation.go @@ -23,9 +23,24 @@ type PayloadValidator struct { logger logging.LoggerInterface } +func genericIssuerOAuth2URLsValidation(sl validator.StructLevel) { + configuration := sl.Current().Interface().(Configuration) + + if configuration.Provider != "generic" { + return + } + + // Kratos will try OIDC discovery, so if IssuerURL is not empty, AuthURL and TokenURL could be empty + // if IssuerURL is empty, then we need both AuthURL and TokenURL + if configuration.IssuerURL == "" && (configuration.AuthURL == "" || configuration.TokenURL == "") { + sl.ReportError(configuration.IssuerURL, "issuer_url", "IssuerURL", "issuer_urls", "") + } +} + func (p *PayloadValidator) setupValidator() { // validate Provider to be one of the supported ones p.validator.RegisterAlias("supported_provider", fmt.Sprintf("oneof=%s", SUPPORTED_PROVIDERS)) + p.validator.RegisterStructValidation(genericIssuerOAuth2URLsValidation, Configuration{}) } func (p *PayloadValidator) NeedsValidation(r *http.Request) bool { From cde3d097fe23f593f79da24092a951d6bb0104d3 Mon Sep 17 00:00:00 2001 From: barco Date: Tue, 12 Nov 2024 17:14:45 +0100 Subject: [PATCH 3/3] test: add tests for changed spec --- pkg/idp/validation_test.go | 56 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/pkg/idp/validation_test.go b/pkg/idp/validation_test.go index c401ffa48..110558453 100644 --- a/pkg/idp/validation_test.go +++ b/pkg/idp/validation_test.go @@ -119,6 +119,45 @@ func TestValidate(t *testing.T) { expectedResult: nil, expectedError: nil, }, + { + name: "CreateIdPSuccessWithOIDCDiscovery", + method: http.MethodPost, + endpoint: "", + body: func() []byte { + conf := new(Configuration) + conf.ID = "google_generic" + conf.Provider = "generic" + conf.IssuerURL = "mock-url" + conf.SubjectSource = "me" + conf.Scope = []string{} + conf.Mapper = "mock-url" + + marshal, _ := json.Marshal(conf) + return marshal + }, + expectedResult: nil, + expectedError: nil, + }, + { + name: "CreateIdPSuccessWithoutOIDCDiscovery", + method: http.MethodPost, + endpoint: "", + body: func() []byte { + conf := new(Configuration) + conf.ID = "google_generic" + conf.Provider = "generic" + conf.AuthURL = "mock-url" + conf.TokenURL = "mock-url" + conf.SubjectSource = "me" + conf.Scope = []string{} + conf.Mapper = "mock-url" + + marshal, _ := json.Marshal(conf) + return marshal + }, + expectedResult: nil, + expectedError: nil, + }, { name: "PartialUpdateIdPSuccess", method: http.MethodPatch, @@ -165,6 +204,23 @@ func TestValidate(t *testing.T) { expectedResult: validator.ValidationErrors{}, expectedError: nil, }, + { + name: "CreateIdPEmptyURLsAndIssuerValidationError", + method: http.MethodPost, + endpoint: "", + body: func() []byte { + conf := new(Configuration) + conf.Provider = "generic" + conf.SubjectSource = "me" + conf.Scope = []string{"profile"} + conf.Mapper = "mock-url" + + marshal, _ := json.Marshal(conf) + return marshal + }, + expectedResult: validator.ValidationErrors{}, + expectedError: nil, + }, { name: "PartialUpdateIdPValidationError", method: http.MethodPatch,