Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Authorization requirement for Space.collaboration #3438

Closed
wants to merge 1 commit into from

Conversation

ccanos
Copy link
Contributor

@ccanos ccanos commented Nov 9, 2023

This fixes #5158 (client) but I'm not fully sure if this is a proper fix or if it is dangerous in any way.

  • I have realized that the resolver for community doesn't have @AuthorizationAgentPrivilege either.

  • With this change I can request authorization without errors

{
space(ID:"eco1") {
  collaboration{
    id
    authorization{
      myPrivileges
    }
  }
}}

image

which is what I need, with authorization.myPrivileges I have enough.

And I have confirmed that the other fields: relations, callouts, tagsetTemplates, timeline throw an error and come set to null if I request them.

But I am concerned about that privilege CREATE_RELATION, having that permission may be a sign of another different bug.

@valentinyanakiev
Copy link
Member

Conceptually, that is a no-go. I have investigated thoroughly and the whole need for querying collaboration on Space comes from this PR. CollaborationPrivileges are added on SpaceContext and later those privileges are used in CalloutTypeSelect. I believe that is very very wrong and probably has multiple bugs in it.

  1. Collaboraiton can be on any journey, and CalloutTypeSelect can be executed on any level, as that is, essentially, where the Callout would be.
  2. At the moment CalloutTypeSelect gets the permissions from the Space.
  3. That will lead to a mess on Challenge and Opportunity. I believe code similar to useCalloutCreation is needed (maybe this can be refactored into a provider if it is reused in multiple spaces).
  4. Then we can remove the Collaboration from the spaceProvider queries (which were never intend to, logically, have anything to do with Collaboration - Callouts on conceptual level).

So I believe this needs re-design in the approach, it's not a simple straightforward fix.

@ccanos
Copy link
Contributor Author

ccanos commented Nov 10, 2023

Ok, understood. I'm going to extract the authorization check from useCalloutCreation and use it for this. it's a refactor but not too big. This PR can be closed then

@ccanos ccanos closed this Nov 10, 2023
@ccanos ccanos deleted the client-5158 branch November 10, 2023 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants