diff --git a/path_config.go b/path_config.go index 2fd95718..0b3c2682 100644 --- a/path_config.go +++ b/path_config.go @@ -89,6 +89,14 @@ func pathConfig(b *jwtAuthBackend) *framework.Path { Name: "Provider Config", }, }, + "namespace_in_state": { + Type: framework.TypeBool, + Description: "Pass namespace in the OIDC state parameter instead of as a separate query parameter. With this setting, the allowed redirect URL(s) in Vault and on the provider side should not contain a namespace query parameter. This means only one redirect URL entry needs to be maintained on the provider side for all vault namespaces that will be authenticating against it. Defaults to true for new configs.", + DisplayAttrs: &framework.DisplayAttributes{ + Name: "Namespace in OIDC state", + Value: true, + }, + }, }, Operations: map[logical.Operation]framework.OperationHandler{ @@ -166,6 +174,7 @@ func (b *jwtAuthBackend) pathConfigRead(ctx context.Context, req *logical.Reques "jwks_ca_pem": config.JWKSCAPEM, "bound_issuer": config.BoundIssuer, "provider_config": config.ProviderConfig, + "namespace_in_state": config.NamespaceInState, }, } @@ -189,6 +198,23 @@ func (b *jwtAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Reque ProviderConfig: d.Get("provider_config").(map[string]interface{}), } + // Check if the config already exists, to determine if this is a create or + // an update, since req.Operation is always 'update' in this handler, and + // there's no existence check defined. + existingConfig, err := b.config(ctx, req.Storage) + if err != nil { + return nil, err + } + if nsInState, ok := d.GetOk("namespace_in_state"); ok { + config.NamespaceInState = nsInState.(bool) + } else if existingConfig == nil { + // new configs default to true + config.NamespaceInState = true + } else { + // maintain the existing value + config.NamespaceInState = existingConfig.NamespaceInState + } + // Run checks on values methodCount := 0 if config.OIDCDiscoveryURL != "" { @@ -349,6 +375,7 @@ type jwtConfig struct { BoundIssuer string `json:"bound_issuer"` DefaultRole string `json:"default_role"` ProviderConfig map[string]interface{} `json:"provider_config"` + NamespaceInState bool `json:"namespace_in_state"` ParsedJWTPubKeys []interface{} `json:"-"` } diff --git a/path_config_test.go b/path_config_test.go index 443aa84c..45730a1d 100644 --- a/path_config_test.go +++ b/path_config_test.go @@ -28,6 +28,7 @@ func TestConfig_JWT_Read(t *testing.T) { "jwks_ca_pem": "", "bound_issuer": "http://vault.example.com/", "provider_config": map[string]interface{}{}, + "namespace_in_state": false, } req := &logical.Request{ @@ -136,6 +137,7 @@ func TestConfig_JWT_Write(t *testing.T) { OIDCResponseTypes: []string{}, BoundIssuer: "http://vault.example.com/", ProviderConfig: map[string]interface{}{}, + NamespaceInState: true, } conf, err := b.(*jwtAuthBackend).config(context.Background(), storage) @@ -172,6 +174,7 @@ func TestConfig_JWKS_Update(t *testing.T) { "jwt_supported_algs": []string{}, "bound_issuer": "", "provider_config": map[string]interface{}{}, + "namespace_in_state": false, } req := &logical.Request{ @@ -342,6 +345,7 @@ func TestConfig_OIDC_Write(t *testing.T) { OIDCResponseTypes: []string{}, OIDCDiscoveryURL: "https://team-vault.auth0.com/", ProviderConfig: map[string]interface{}{}, + NamespaceInState: true, } conf, err := b.(*jwtAuthBackend).config(context.Background(), storage) @@ -433,6 +437,7 @@ func TestConfig_OIDC_Write_ProviderConfig(t *testing.T) { "provider": "azure", "extraOptions": "abound", }, + NamespaceInState: true, } conf, err := b.(*jwtAuthBackend).config(context.Background(), storage) @@ -489,6 +494,7 @@ func TestConfig_OIDC_Write_ProviderConfig(t *testing.T) { OIDCResponseTypes: []string{}, OIDCDiscoveryURL: "https://team-vault.auth0.com/", ProviderConfig: map[string]interface{}{}, + NamespaceInState: true, } conf, err := b.(*jwtAuthBackend).config(context.Background(), storage) @@ -502,6 +508,188 @@ func TestConfig_OIDC_Write_ProviderConfig(t *testing.T) { }) } +func TestConfig_OIDC_Create_Namespace(t *testing.T) { + type testCase struct { + create map[string]interface{} + expected jwtConfig + } + tests := map[string]testCase{ + "namespace_in_state not specified": { + create: map[string]interface{}{ + "oidc_discovery_url": "https://team-vault.auth0.com/", + }, + expected: jwtConfig{ + OIDCDiscoveryURL: "https://team-vault.auth0.com/", + NamespaceInState: true, + OIDCResponseTypes: []string{}, + JWTSupportedAlgs: []string{}, + JWTValidationPubKeys: []string{}, + ProviderConfig: map[string]interface{}{}, + }, + }, + "namespace_in_state true": { + create: map[string]interface{}{ + "oidc_discovery_url": "https://team-vault.auth0.com/", + "namespace_in_state": true, + }, + expected: jwtConfig{ + OIDCDiscoveryURL: "https://team-vault.auth0.com/", + NamespaceInState: true, + OIDCResponseTypes: []string{}, + JWTSupportedAlgs: []string{}, + JWTValidationPubKeys: []string{}, + ProviderConfig: map[string]interface{}{}, + }, + }, + "namespace_in_state false": { + create: map[string]interface{}{ + "oidc_discovery_url": "https://team-vault.auth0.com/", + "namespace_in_state": false, + }, + expected: jwtConfig{ + OIDCDiscoveryURL: "https://team-vault.auth0.com/", + NamespaceInState: false, + OIDCResponseTypes: []string{}, + JWTSupportedAlgs: []string{}, + JWTValidationPubKeys: []string{}, + ProviderConfig: map[string]interface{}{}, + }, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + b, storage := getBackend(t) + + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: configPath, + Storage: storage, + Data: test.create, + } + resp, err := b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + conf, err := b.(*jwtAuthBackend).config(context.Background(), storage) + assert.NoError(t, err) + assert.Equal(t, &test.expected, conf) + }) + } + +} + +func TestConfig_OIDC_Update_Namespace(t *testing.T) { + type testCase struct { + existing map[string]interface{} + update map[string]interface{} + expected jwtConfig + } + tests := map[string]testCase{ + "existing false, update to true": { + existing: map[string]interface{}{ + "oidc_discovery_url": "https://team-vault.auth0.com/", + "namespace_in_state": false, + }, + update: map[string]interface{}{ + "oidc_discovery_url": "https://team-vault.auth0.com/", + "namespace_in_state": true, + }, + expected: jwtConfig{ + OIDCDiscoveryURL: "https://team-vault.auth0.com/", + NamespaceInState: true, + OIDCResponseTypes: []string{}, + JWTSupportedAlgs: []string{}, + JWTValidationPubKeys: []string{}, + ProviderConfig: map[string]interface{}{}, + }, + }, + "existing false, update something else": { + existing: map[string]interface{}{ + "oidc_discovery_url": "https://team-vault.auth0.com/", + "namespace_in_state": false, + }, + update: map[string]interface{}{ + "oidc_discovery_url": "https://team-vault.auth0.com/", + "default_role": "ui", + }, + expected: jwtConfig{ + OIDCDiscoveryURL: "https://team-vault.auth0.com/", + NamespaceInState: false, + DefaultRole: "ui", + OIDCResponseTypes: []string{}, + JWTSupportedAlgs: []string{}, + JWTValidationPubKeys: []string{}, + ProviderConfig: map[string]interface{}{}, + }, + }, + "existing true, update to false": { + existing: map[string]interface{}{ + "oidc_discovery_url": "https://team-vault.auth0.com/", + "namespace_in_state": true, + }, + update: map[string]interface{}{ + "oidc_discovery_url": "https://team-vault.auth0.com/", + "namespace_in_state": false, + }, + expected: jwtConfig{ + OIDCDiscoveryURL: "https://team-vault.auth0.com/", + NamespaceInState: false, + OIDCResponseTypes: []string{}, + JWTSupportedAlgs: []string{}, + JWTValidationPubKeys: []string{}, + ProviderConfig: map[string]interface{}{}, + }, + }, + "existing true, update something else": { + existing: map[string]interface{}{ + "oidc_discovery_url": "https://team-vault.auth0.com/", + "namespace_in_state": true, + }, + update: map[string]interface{}{ + "oidc_discovery_url": "https://team-vault.auth0.com/", + "default_role": "ui", + }, + expected: jwtConfig{ + OIDCDiscoveryURL: "https://team-vault.auth0.com/", + NamespaceInState: true, + DefaultRole: "ui", + OIDCResponseTypes: []string{}, + JWTSupportedAlgs: []string{}, + JWTValidationPubKeys: []string{}, + ProviderConfig: map[string]interface{}{}, + }, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + b, storage := getBackend(t) + + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: configPath, + Storage: storage, + Data: test.existing, + } + resp, err := b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + req.Data = test.update + resp, err = b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + conf, err := b.(*jwtAuthBackend).config(context.Background(), storage) + assert.NoError(t, err) + assert.Equal(t, &test.expected, conf) + }) + } + +} + const ( testJWTPubKey = `-----BEGIN PUBLIC KEY----- MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEEVs/o5+uQbTjL3chynL4wXgUg2R9 diff --git a/path_oidc.go b/path_oidc.go index 36345600..88cd95c4 100644 --- a/path_oidc.go +++ b/path_oidc.go @@ -371,6 +371,24 @@ func (b *jwtAuthBackend) authURL(ctx context.Context, req *logical.Request, d *f return logical.ErrorResponse("role %q could not be found", roleName), nil } + // If namespace will be passed around in state, and it has been provided as + // a redirectURI query parameter, remove it from redirectURI, and append it + // to the state (later in this function) + namespace := "" + if config.NamespaceInState { + inputURI, err := url.Parse(redirectURI) + if err != nil { + return resp, nil + } + qParam := inputURI.Query() + namespace = qParam.Get("namespace") + if len(namespace) > 0 { + qParam.Del("namespace") + inputURI.RawQuery = qParam.Encode() + redirectURI = inputURI.String() + } + } + if !validRedirect(redirectURI, role.AllowedRedirectURIs) { logger.Warn("unauthorized redirect_uri", "redirect_uri", redirectURI) return resp, nil @@ -408,6 +426,10 @@ func (b *jwtAuthBackend) authURL(ctx context.Context, req *logical.Request, d *f logger.Warn("error generating OAuth state", "error", err) return resp, nil } + if config.NamespaceInState && len(namespace) > 0 { + // embed namespace in state in the auth_url + stateID = fmt.Sprintf("%s,ns=%s", stateID, namespace) + } authCodeOpts := []oauth2.AuthCodeOption{ oidc.Nonce(nonce), diff --git a/path_oidc_test.go b/path_oidc_test.go index fafc1321..2bb8f2f0 100644 --- a/path_oidc_test.go +++ b/path_oidc_test.go @@ -208,6 +208,163 @@ func TestOIDC_AuthURL(t *testing.T) { }) } +func TestOIDC_AuthURL_namespace(t *testing.T) { + + type testCase struct { + namespaceInState string + allowedRedirectURIs []string + incomingRedirectURI string + expectedStateRegEx string + expectedRedirectURI string + expectFail bool + } + + tests := map[string]testCase{ + "namespace as query parameter": { + namespaceInState: "false", + allowedRedirectURIs: []string{"https://example.com?namespace=test"}, + incomingRedirectURI: "https://example.com?namespace=test", + expectedStateRegEx: `\w{27}`, + expectedRedirectURI: `https://example.com?namespace=test`, + }, + "namespace as query parameter, bad allowed redirect": { + namespaceInState: "false", + allowedRedirectURIs: []string{"https://example.com"}, + incomingRedirectURI: "https://example.com?namespace=test", + expectedStateRegEx: `\w{27}`, + expectedRedirectURI: `https://example.com?namespace=test`, + expectFail: true, + }, + "namespace in state": { + namespaceInState: "true", + allowedRedirectURIs: []string{"https://example.com"}, + incomingRedirectURI: "https://example.com?namespace=test", + expectedStateRegEx: `\w{27},ns=test`, + expectedRedirectURI: `https://example.com`, + }, + "namespace in state, bad allowed redirect": { + namespaceInState: "true", + allowedRedirectURIs: []string{"https://example.com?namespace=test"}, + incomingRedirectURI: "https://example.com?namespace=test", + expectFail: true, + }, + "nested namespace in state": { + namespaceInState: "true", + allowedRedirectURIs: []string{"https://example.com"}, + incomingRedirectURI: "https://example.com?namespace=org4321/dev", + expectedStateRegEx: `\w{27},ns=org4321/dev`, + expectedRedirectURI: `https://example.com`, + }, + "namespace as query parameter, no namespaces": { + namespaceInState: "false", + allowedRedirectURIs: []string{"https://example.com"}, + incomingRedirectURI: "https://example.com", + expectedStateRegEx: `\w{27}`, + expectedRedirectURI: `https://example.com`, + }, + "namespace in state, no namespaces": { + namespaceInState: "true", + allowedRedirectURIs: []string{"https://example.com"}, + incomingRedirectURI: "https://example.com", + expectedStateRegEx: `\w{27}`, + expectedRedirectURI: `https://example.com`, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + b, storage := getBackend(t) + + // Configure backend + data := map[string]interface{}{ + "oidc_discovery_url": "https://team-vault.auth0.com/", + "oidc_discovery_ca_pem": "", + "oidc_client_id": "abc", + "oidc_client_secret": "def", + "default_role": "test", + "bound_issuer": "http://vault.example.com/", + "namespace_in_state": test.namespaceInState, + } + + // basic configuration + req := &logical.Request{ + Operation: logical.UpdateOperation, + Path: configPath, + Storage: storage, + Data: data, + } + + resp, err := b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v\n", err, resp) + } + + // set up test role + rolePayload := map[string]interface{}{ + "user_claim": "email", + "bound_audiences": "vault", + "allowed_redirect_uris": test.allowedRedirectURIs, + } + + req = &logical.Request{ + Operation: logical.CreateOperation, + Path: "role/test", + Storage: storage, + Data: rolePayload, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v\n", err, resp) + } + + authURLPayload := map[string]interface{}{ + "role": "test", + "redirect_uri": test.incomingRedirectURI, + } + req = &logical.Request{ + Operation: logical.UpdateOperation, + Path: "oidc/auth_url", + Storage: storage, + Data: authURLPayload, + } + + resp, err = b.HandleRequest(context.Background(), req) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v\n", err, resp) + } + + rawAuthURL := resp.Data["auth_url"].(string) + if test.expectFail && len(rawAuthURL) > 0 { + t.Fatalf("Expected auth_url to fail (empty), but got %s", rawAuthURL) + } + if test.expectFail && len(rawAuthURL) == 0 { + return + } + + authURL, err := url.Parse(rawAuthURL) + if err != nil { + t.Fatal(err) + } + qParams := authURL.Query() + redirectURI := qParams.Get("redirect_uri") + if test.expectedRedirectURI != redirectURI { + t.Fatalf("expected redirect_uri to match: %s, %s", test.expectedRedirectURI, redirectURI) + } + + state := qParams.Get("state") + matchState, err := regexp.MatchString(test.expectedStateRegEx, state) + if err != nil { + t.Fatal(err) + } + if !matchState { + t.Fatalf("expected state to match regex: %s, %s", test.expectedStateRegEx, state) + } + + }) + } +} + func TestOIDC_Callback(t *testing.T) { t.Run("successful login", func(t *testing.T) {