Potential wrong invalidation of filters with sub-attributes after value paths

[LucasRouckhout]

Note: This might technically be an issue with the following underlying dependency. So let me know if I have to open an issue there instead. But the maintainer of that package also works on this project and I'm assuming I will reach more interested people here. Also, fixing this might require a version bump of the dependency in this project.

Problem

Filters of the following form emails[type eq "work"].value eq "[email protected]" are considered invalid and result in a 400 error response with following error body

{
  "schemas": [
    "urn:ietf:params:scim:api:messages:2.0:Error"
  ],
  "scimType": "invalidFilter",
  "detail": "The specified filter syntax was invalid, or the specified attribute and filter comparison combination is not supported.",
  "status": "400"
}

You can reproduce this by adding the following test case in filter_test file:

func Test_User_Filter(t *testing.T) {
	s := newTestServerForFilter()

	tests := []struct {
		name             string
		filter           string
		expectedUserName string
	}{
		{name: "Happy path", filter: "userName eq \"testUser\"", expectedUserName: "testUser"},
		{name: "Happy path with plus sign", filter: "userName eq \"testUser+test\"", expectedUserName: "testUser+test"},
		// While this test should fail by not finding a user it fails with an invalidFilter error
		{name: "Fails with invalidFilter", filter: "emails[type eq \"work\"].value eq \"[email protected]\"", expectedUserName: "testUser+test"},
	}

While this test should fail by not finding the user it fails because it considers to filter to be invalid instead. I also see this happening in a running instance of our SCIM server.

Questions

• Ought these filters to be considered valid according to the RFC in your opinion?
• If they are valid filters, could I get some pointers on where in the dependency this would need to be fixed so I could potentially open a PR in the dependency and a bump of the version in this project? It looks like filter-parser is a simple recursive decent parsers based on the ABNF present in the RFC. But have not really gotten any deeper than that.

Details

I'm not really sure whether this ought to be a valid filter or not. If I take a look at the ABNF spec it seems like this is actually not valid. Since I can't seem to produce such a filter given the production rules, but I might be mistaken.

The philosophical discussion about whether this is actually a valid filter according to the RFC aside. Some clients in the wild seem to already expect that these kind of filters are valid implicitly.

For example: in Azure Active Directory the work email attribute get's hardcoded to the following SCIM attribute emails[type eq "work"].value. Customers of Azure AD can chose what attribute to use to match the User in Active Directory with the User in the target Service Provider by choosing a so called matcher attribute. The Azure SCIM client seems to perform a lookup on the User resource to check whether the User resource already exists in the target Service Provider by using the following simple string interpolation to generate the filter

/Users?filter=<ATTRIBUTE> eq "<VALUE>"

And since they hardcoded the attribute for email to be emails[type eq "work"].value they generate filters of the kind causing the issue. This is how I initially stumbled upon the issue.

I also went looking for other parsers to see if they consider the filter to be valid and in my search I stumbled upon this commit thomaspoignant/scim2-parse-filter@f03e832 in a JS parser that explicitly added support for these kinds of filters.