talawa-api icon indicating copy to clipboard operation
talawa-api copied to clipboard

Feature request: Private Events

Open palisadoes opened this issue 3 years ago • 2 comments

Is your feature request related to a problem? Please describe. The participants in all events are publicly visible to everyone. There needs to be options for more privacy.

Describe the solution you'd like

  1. The ability to make events private where only participants can see who is attending
  2. The optional ability to make private events invisible to persons not attending

Describe alternatives you've considered N/A

Approach to be followed (optional) N/A

Additional context Tied to Issue https://github.com/PalisadoesFoundation/talawa/issues/983

palisadoes avatar Sep 05 '21 19:09 palisadoes

This issue did not get any activity in the past 60 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

github-actions[bot] avatar Dec 29 '21 00:12 github-actions[bot]

This issue did not get any activity in the past 60 days and will be closed in 365 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

github-actions[bot] avatar Feb 28 '22 00:02 github-actions[bot]

@palisadoes I think this can be achieved by adding a isPrivate property to Event. If isPrivate === true, then in theregistrantsByEvent.ts we can add a check for the user if he/she is registered for the event. createEvent.ts and query.ts can be modified accordingly. Can you please share your views for this approach? Thanks!

beingnoble03 avatar Jan 28 '23 05:01 beingnoble03

  1. How would you enforce the requirements?
  2. How would this be integrated into the mobile app? What would the UX be?

palisadoes avatar Jan 28 '23 06:01 palisadoes

@palisadoes

  1. Can you please elaborate this point?
  2. The frontend won’t require much changes as we only want to ask the user to make the event private or not. According to me, it will only require a checkbox for asking "Private Event” while creating an event.

beingnoble03 avatar Jan 28 '23 10:01 beingnoble03

  1. How will you make sure that only authorized persons will have access to the event?
  2. What does access mean? Will uninvited people see the event in the calendar of others? If not, why? If so, what exactly will they see?
  3. What will be required in the mobile app, admin and API?

This cannot be thought of in isolation. It's not just a flag in a database.

I'm sure there are other questions.

palisadoes avatar Jan 28 '23 14:01 palisadoes

This issue did not get any activity in the past 14 days and will be closed in 365 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

github-actions[bot] avatar Feb 12 '23 00:02 github-actions[bot]

@tasneemkoushar @anwersayeed This is what private events was about

palisadoes avatar Feb 19 '23 17:02 palisadoes

This issue did not get any activity in the past 10 days and will be closed in 365 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

github-actions[bot] avatar Mar 06 '23 00:03 github-actions[bot]

Implementing privacy on event attendants to Talawa Suite of Apps.

by @SiddheshKukade

Private Events:

Mostly Private here means that the others can't see the list of people who will be attending that event(s).

Suggested Changes for:

Talawa API:

  • Adding isGuestListPrivate to Event model.

  • I checked there is already userAttendeSchema present in the event model. image

  • It is being used in the Event model. image

  • Using that property would help to manage the visibility of the attendants. As only the users present in userAttendeeSchema would be able to view that list.

  • We can enforce this in the API itself for more security

Talawa:

  • As talawa is meant for the users of the Organization. Some UI changes would be implemented like below:
    • Adding a generous message if attendee list is not received in the UI Join the event to see the attendees.
  • While creating an event, dding a additional checkbox for keeping the guest list private or not.

Talawa-Admin

  • As admin is meant for the the org admins who can also create events. There should be a option add a check for privateGuestList on the talawa API.

@palisadoes Sir, Please let me know your thoughts on these suggestions. If they are valid according to you. I can create detailed issues on admin and API to keep track of them there. I would like to work on this. Please assign.

SiddheshKukade avatar Mar 13 '23 18:03 SiddheshKukade

Is this in keeping with the desired event functionality listed here?

  • https://docs.talawa.io/docs/applications/overview

@xoldyckk Any thoughts on this?

palisadoes avatar Mar 14 '23 04:03 palisadoes

To discuss this issue further,

The event schema already has a field named isPublic

  isPublic: {
    type: Boolean,
    required: true,
  },

When an event is created with the above variable as false it means that it is a private event.

How we can make only registered participants see attending participants of the event:

We can have an association model. This model will associate an event with each participant.

e.g

const eventPivotSchema = new Schema({
  eventId: {
    type: String,
    required: true,
  },
  participantId: {
    type: String,
    required: true,
  },
.....

This way, they can only see event and event details that have their associated ID. Whether they belong to a Private or public event, they will only be able to see the event details they signed up for. So when a user is included to attend an event, we create a model that associates that user as a participant with the event.

If a participant is deleted, the eventPivotSchema will be modified to remove that participant.

The Talawa-Admin:

In the admin section, we can send query to the eventPivotSchema to populate the calendar with events a user is associated with.

The super admin will have an exception to see all events they belong to or not.

This is just a draft implementation that can be refined with good feedback.

kayceeDev avatar Mar 19 '23 11:03 kayceeDev

  1. If an event is public then everyone needs to see it.
  2. There will be events that don't require registration and therefore everyone will be able to see it too

How will you handle these cases?

palisadoes avatar Mar 19 '23 13:03 palisadoes

  1. If an event is public then everyone needs to see it.
  2. There will be events that don't require registration and therefore everyone will be able to see it too

How will you handle these cases?

In this case, if events is public and anybody can see, we can first have query to get all public event to populate the calendar even tho the user isn't a participant. And then for every event that is private and the user is a participant, we can populate it too on the calendar. So the first query goes to get public event that everyone can see and then the second query gets only user events which might be a private event. And for those private events, only if a user is associated with the event will the user see it. That's where the eventPivotSchema comes in.

kayceeDev avatar Mar 19 '23 16:03 kayceeDev

Why can't you just use one query and filter?

palisadoes avatar Mar 19 '23 16:03 palisadoes

Why can't you just use one query and filter?

Well we still can. This is my thought process: When a user is added as a participant to an event, the user details is added to the participants array. We could actually make that happen, for example, after we fetched events, when rendering the event in the admin, we can have a check that filters on event visibility. If the event if private, we will then check if the user id is in the list of participants. If it is present then we add the private event to calendar, else we don't add it. But if the event visibility is public then we don't need this check. We will just add it to the calendar.

I suggested the associative table because it will help avoid the time complexity of an array while mapping over the participants data to find if the user is present

kayceeDev avatar Mar 19 '23 16:03 kayceeDev

This way, we may not have to touch the API. Just modifying the logic in the admin of which events gets added to the Calendar. Then we can distinguish events with label on the calendar. More like a tooltip that tells private and public event on spot on

kayceeDev avatar Mar 19 '23 16:03 kayceeDev

We could also add a filter for users to filter events based on visibility. But this is optional. Superadmins can see all events as well as filter too. Admins can only see event in their organization and can filter with visibility as well. These are optional features if we want to take things further.

kayceeDev avatar Mar 19 '23 16:03 kayceeDev

OK, do you want to be assigned this task and create the resulting PR?

palisadoes avatar Mar 19 '23 17:03 palisadoes

OK, do you want to be assigned this task and create the resulting PR?

Yes definitely. I will love to work on it.

kayceeDev avatar Mar 19 '23 17:03 kayceeDev

@palisadoes I think first we will need to accurately define what exactly is a private event first. By private events, I am assuming you mean everything related to that event can only be accessed by its attendees.

@SiddheshKukade You are assuming only one aspect of private events which is the member list visibility. What about eventProjects and Tasks privacy how would that be handled in an optimized way? You can't just create a boolean field like that in their model to handle them.

@kayceeDev I think you are also only thinking about only the narrow point of userAttendees. What about those cases above mentioned?

Also, you cannot handle mongoDB collections using normalization as we do during BCNF for RDBMS it is a really bad idea and it sucks at JOINs. The best design pattern is believe it or not IS the array thing. Check this out if you have time.

https://www.mongodb.com/developer/products/mongodb/mongodb-schema-design-best-practices/

Also, the calendar thing is just one part of private events what about the data associated with an event? If you are maintaining the pivot collection to collect the event data you will have to have an additional query to get the data for the event(which is what populate function does internally) which will reduce the performance of the application.

@beingnoble03 Would you like to have a look at this?

kb-0311 avatar Mar 26 '23 13:03 kb-0311

@palisadoes I think first we will need to accurately define what exactly is a private event first. By private events, I am assuming you mean everything related to that event can only be accessed by its attendees.

@SiddheshKukade You are assuming only one aspect of private events which is the member list visibility. What about eventProjects and Tasks privacy how would that be handled in an optimized way? You can't just create a boolean field like that in their model to handle them.

@kayceeDev I think you are also only thinking about only the narrow point of userAttendees. What about those cases above mentioned?

Also, you cannot handle mongoDB collections using normalization as we do during BCNF for RDBMS it is a really bad idea and it sucks at JOINs. The best design pattern is believe it or not IS the array thing. Check this out if you have time.

https://www.mongodb.com/developer/products/mongodb/mongodb-schema-design-best-practices/

Also, the calendar thing is just one part of private events what about the data associated with an event? If you are maintaining the pivot collection to collect the event data you will have to have an additional query to get the data for the event(which is what populate function does internally) which will reduce the application's performance.

@beingnoble03 Would you like to have a look at this?

Thank you, Take a look at the merged PR and how I implemented the private event from the admin

I assume that tasks and eventProjects are associated with events by an id. When you can't access an event, you can't also access any other stuff attached to that particular event.

Another way to do it from the API is to also include a user role in a request to get an event. and then perform the necessary check to see if the user belongs to that event or if the user is an admin of that organization etc.

My frontend implementation shared above solves all these.

kayceeDev avatar Mar 26 '23 13:03 kayceeDev

@kayceeDev The access control checks should never ever be implemented on the client side. The error handling and appropriate checks with relaying data are the API's job , not the client's. And the checks will need to be implemented within the API itself with appropriate data sent back to the client based on role or attendance status. That's very basic stuff API for logic , Client to relay that logic.

What you did in that PR is not the right way to do so. Thanks for bringing that to my attention, that PR will need to be rolled back or a change will need to be made to remove those changes. We have been having problems with proper PR reviews for admin portal project because of a lot of activity so it must have gotten hasility merged. @palisadoes

My frontend implementation shared above solves all these. No. it does not.

If I use a different graphql playground or postman or something to make a query to access private event data directly to the API then the checks in the admin portal are useless (because I am not using the admin portal to access data at all )and I can easily access private sensitive data directly from the API. This is a security breach. There should be no RBAC or Logical checks like that in the frontend.

Another way to do it from the API is to also include a user role in a request to get an event. and then perform the necessary check to see if the user belongs to that event or if the user is an admin of that organization etc.

That should be the only way. Also do get familiar with the API as well, there are certain utilities like adminCheck you can go ahead a create a utility for each of the cases you mentioned if not already present in this PR. Also move the checks from frontend to this PR for the relevant queries.

I assume that tasks and eventProjects are associated with events by an id. When you can't access an event, you can't also access any other stuff attached to that particular event.

Think about how that can be applied in the API logic layer and not the frontend with the most optimal number of database queries and then do please discuss here what you came up with.

kb-0311 avatar Mar 26 '23 16:03 kb-0311

Not to dispute your statement above but what I did was filter the fetched event data according to user role in the optimal way possible. No Need changing the API or anything. Event data is returned, the data is passed through a function which filters that data according to logged in user type and then relevant data is then passed and rendered. This can be done through the API too. Buh the admin implementation are not wrong either. Also, we don't need to do multiple data fetching. The only extra data fetching done is getting the organization data to check if user is admin of that particular organization.

kayceeDev avatar Mar 26 '23 16:03 kayceeDev

@kayceeDev Read my comment again. I am not saying your checks are wrong I am saying they are completely in the wrong place. All checks should be in API.

I know what the logic was in the front end checks, I saw the PR. My issue has nothing to do with how they are rendered it has to do with how the access control has been enforced.

No Need to change the API or anything.

This is just objectively a wrong statement. When someone makes a private event they expect that their event's privacy is protected. Please do tell me how will your checks on the frontend protect against this ->

If I use a different client like graphql playground or postman or something to make a query to access private event data directly to the API then the checks in the admin portal are useless (because I am not using the admin portal to access data at all )and I can easily access private sensitive data directly from the API. This is a security breach. There should be no RBAC or Logical checks like that in the frontend. They should instead be in the backend.

They don't. You see why we need to add logical checks in the API and not in the front end. @xoldyckk pls do checkout this conversation it deals with the privacy of events and some form of access control check.

kb-0311 avatar Mar 26 '23 17:03 kb-0311

@kb-0311 I get your point now completely. The API implementing everything means the logic it's reusable across all frontend. Instead of just the admin for now. Thanks you for sticking with me. I appreciate This. Can we leave the Admin implementation for now while working on the API? What do you think?

kayceeDev avatar Mar 26 '23 17:03 kayceeDev

@kayceeDev You are welcome.

After the checks are implemented in the API ,What would be the use of the admin implementation? It would only take up unnecessary computation and complexity,I think it should be removed after you have implemented it in the backend.

kb-0311 avatar Mar 26 '23 17:03 kb-0311

Yes I just mean for now till the API is implemented. We should definitely revert the Admin later and reimplement it with the right API. That's just my suggestion

kayceeDev avatar Mar 26 '23 17:03 kayceeDev

@kayceeDev Sure.

kb-0311 avatar Mar 27 '23 06:03 kb-0311

@palisadoes I think first we will need to accurately define what exactly is a private event first. By private events, I am assuming you mean everything related to that event can only be accessed by its attendees.

@SiddheshKukade You are assuming only one aspect of private events which is the member list visibility. What about eventProjects and Tasks privacy how would that be handled in an optimized way? You can't just create a boolean field like that in their model to handle them.

@kayceeDev I think you are also only thinking about only the narrow point of userAttendees. What about those cases above mentioned?

Also, you cannot handle mongoDB collections using normalization as we do during BCNF for RDBMS it is a really bad idea and it sucks at JOINs. The best design pattern is believe it or not IS the array thing. Check this out if you have time.

https://www.mongodb.com/developer/products/mongodb/mongodb-schema-design-best-practices/

Also, the calendar thing is just one part of private events what about the data associated with an event? If you are maintaining the pivot collection to collect the event data you will have to have an additional query to get the data for the event(which is what populate function does internally) which will reduce the performance of the application.

@beingnoble03 Would you like to have a look at this?

  1. The purpose of private events are defined here: https://docs.talawa.io/docs/introduction/core-concepts
  2. If you have any further questions, we can discuss it here and add it to this document.

palisadoes avatar Mar 27 '23 21:03 palisadoes