fix: remove non-existent colums in getMembershipFromQuery queries

[doncicuto]

In issue #7841 @mahmoodfathy commented an issue when the API call for Listing My ZITADEL Manager Roles is called with any kind of query (orgQuery, projectQuery, projectGrantQuery...). A column XXXXXX does not exist (SQLSTATE 42703) error is thrown.

The issue was focused in getMembershipFromQuery where filtering queries functions are called: prepareOrgMember, prepareIAMMember, prepareProjectMember and prepareProjectGrantMember

Those functions allow queries for columns that are not members of the table to be queried so I've added a conditional clause to avoid using the queries that cannot be called.

For example, for prepareOrgMember, member.id, member.project_id and member.grant_id columns are not added to the filter queries

for _, q := range query.Queries {
		if q.Col().table.name == membershipAlias.name &&
			!slices.Contains([]string{membershipIAMID.name, membershipProjectID.name, membershipGrantID.name}, q.Col().name) {
			builder = q.toQuery(builder)
		}
	}
	return builder.MustSql()

Here I show one screenshot where the error "column XXXXXX does not exist (SQLSTATE 42703)" is no longer thrown using an orgQuery.

Should close #7841

Definition of Ready

• I am happy with the code
• Short description of the feature/issue is added in the pr description
• PR is linked to the corresponding user story
• Acceptance criteria are met
• All open todos and follow ups are defined in a new ticket and justified
• Deviations from the acceptance criteria and design are agreed with the PO and documented.
• No debug or dead code
• My code has no repetitions
• Critical parts are tested automatically
• Where possible E2E tests are implemented
• Documentation/examples are up-to-date
• All non-functional requirements are met
• Functionality of the acceptance criteria is checked manually on the dev system.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 8, 2024 11:51am

[stebenz]

@doncicuto I'm a bit on the fence on this, in my opinion the correct way to solve this would be a bit different, so that you query for the specific columns of the sub-tables.

Would that not be solvable if you use for example

func NewMembershipOrgIDQuery(value string) (SearchQuery, error) {
	return NewTextQuery(OrgMemberOrgID, value, TextEquals)
}

instead of

func NewMembershipOrgIDQuery(value string) (SearchQuery, error) {
	return NewTextQuery(membershipOrgID, value, TextEquals)
}

and then

if q.Col().table.name == membershipAlias.name || q.Col().table.name == orgMemberTable.name

so that you have all shared columns like membershipUserID but also the column of the specific table, instead of

if q.Col().table.name == membershipAlias.name

Does that make sense?

[doncicuto]

Hi @stebenz, I've tried to fix the code with your suggestion.

I've tried to run some manual tests against the API and all seems fine:

I hope I've understood the suggestion correctly, and my apologies as I'll need a lot of time to understand how the backend works so thanks for your time.

[stebenz]

@doncicuto No problem at all.
The idea was that if columns are only filled in specific sub-selects, as they are here for each sub-table, the respective query on the sub table only has the where condition.
Anyway, could you please also do some tests for combined queries like projectID and orgID in the same query?

[doncicuto]

Sure @stebenz

Just to confirm, as defined by proto file, you can only use one query method in getMembershipFromQuery so I'm not sure how could I add a test like that to user_membership_test.go to check that proto error is returned:

Docs seem to imply that you can use several queries for project, org... but I don't know how can descriptions like "one of type use iam, org id, project id or project grant id" can be added to docs

If you can give me a hint to implement those tests I'd appreciate it

[stebenz]

@doncicuto The query would be like so:

{
  "query": {
    "offset": "0",
    "limit": 100,
    "asc": true
  },
  "queries": [
    {
      "orgQuery": {
        "orgId": "69629023906488334"
      },
    },
    {
      "projectQuery": {
        "projectId": "69629023906488334"
      }
    }
  ]
}

Unfortunately, the docs are wrongly generated, but that was already in issue #5352.

[doncicuto]

I see @stebenz, but my doubt was that the grpc definition for MembershipQuery says that it uses oneof and therefore if I try to use several queries (one OrgQuery and one ProjectQuery) as the example you propose I get an error from GRPC specifying that I can't use more than one query

message MembershipQuery {
    oneof query {
        option (validate.required) = true;

        MembershipOrgQuery org_query = 1;
        MembershipProjectQuery project_query = 2;
        MembershipProjectGrantQuery project_grant_query = 3;
        MembershipIAMQuery iam_query = 4;
    }
}

In summary you wanted me to test combined queries like projectID and orgID but I don't know how I can test that if GRPC says that I can't use combined queries due to the oneof specification

Thanks @stebenz

[eliobischof]

I see @stebenz, but my doubt was that the grpc definition for MembershipQuery says that it uses oneof and therefore if I try to use several queries (one OrgQuery and one ProjectQuery) as the example you propose I get an error from GRPC specifying that I can't use more than one query

message MembershipQuery {
    oneof query {
        option (validate.required) = true;

        MembershipOrgQuery org_query = 1;
        MembershipProjectQuery project_query = 2;
        MembershipProjectGrantQuery project_grant_query = 3;
        MembershipIAMQuery iam_query = 4;
    }
}

In summary you wanted me to test combined queries like projectID and orgID but I don't know how I can test that if GRPC says that I can't use combined queries due to the oneof specification

Thanks @stebenz

@doncicuto Maybe the PR #8002 I just created helps.
It explains how to actually use the queries array https://docs-git-docs-please-help-zitadel.vercel.app/docs/apis/known_docs_issues#generated-examples-for-oneof-fields

[stebenz]

Hm, but the queries are a repeated list on the request:

message ListUserMembershipsRequest {
    //list limitations and ordering
    string user_id = 1 [(validate.rules).string = {min_len: 1, max_len: 200}];
    //the field the result is sorted
    zitadel.v1.ListQuery query = 2;
    //criteria the client is looking for
    repeated zitadel.user.v1.MembershipQuery queries = 3;
}
message MembershipQuery {
    oneof query {
        option (validate.required) = true;

        MembershipOrgQuery org_query = 1;
        MembershipProjectQuery project_query = 2;
        MembershipProjectGrantQuery project_grant_query = 3;
        MembershipIAMQuery iam_query = 4;
    }
}

The idea here is that you can have a list of query parameters, where each one is a oneof so that you only can query with specific types.

[doncicuto]

Ok I think I understand you but I don't explain myself so I feel a little dumb here.

My point, you want me to try a test like this:

{
  "query": {
    "offset": "0",
    "limit": 100,
    "asc": true
  },
  "queries": [
    {
      "orgQuery": {
        "orgId": "69629023906488334"
      },
    },
    {
      "projectQuery": {
        "projectId": "69629023906488334"
      }
    }
  ]
}

But if I execute that query against my code I get a GRPC error parsing the query not accepting it, and that error I assume is because the GRPC doesn't like that I include more than one query type and that error is not part of my code.

As always I don't want to waste your time so if you prefer it I can close this PR as I don't know how I can fix that GRPC error to fullfil your requirements.

Thanks!

[stebenz]

The screenshot you sent here, should that contain the example I provided?
As I see it you use, with 1 object and 2 attributes "orgQuery" and "projectQuery", where these 2 are the oneof and why you get the error:

{
  "queries": [
    { 
      "orgQuery": { "orgId": "69629023906488334" },
      "projectQuery": { "projectId": "69629023906488334" }
    }
  ]
}

And not use different objects in the array:

{
  "queries": [
      { "orgQuery": { "orgId": "69629023906488334" }},
      { "projectQuery": { "projectId": "69629023906488334" }}
  ]
}

[doncicuto]

Thanks @stebenz, it's clear that I'm dumb enough