Skip to content

Commit

Permalink
Merge pull request #456 from canonical/IAM-1047-provider-defaults-to-…
Browse files Browse the repository at this point in the history
…non-existant-schema

IAM-1047 provider defaults to non existant schema
  • Loading branch information
BarcoMasile authored Nov 14, 2024
2 parents b51b9ab + cde3d09 commit 8d43266
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 7 deletions.
12 changes: 6 additions & 6 deletions pkg/idp/third_party.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
15 changes: 15 additions & 0 deletions pkg/idp/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
56 changes: 56 additions & 0 deletions pkg/idp/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion ui/src/pages/providers/ProviderCreate.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Copyright 2024 Canonical Ltd.
// SPDX-License-Identifier: AGPL-3.0

import { FC } from "react";
import {
ActionButton,
Expand Down Expand Up @@ -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",
},
Expand Down

0 comments on commit 8d43266

Please sign in to comment.