topcoder-platform / forums

0 stars 0 forks source link

[$500] Client Manager - no navigation when embedded #652

Closed jmgasper closed 2 years ago

jmgasper commented 2 years ago

@atelomycterus - When embedded through the mfe embed, the Client Manager role should be restricted:

  1. No ability to start a new thread or post. They can only respond to posts created by members. They can add attachments in a response.
  2. No ability to announce
  3. No ability to click a handle to navigate to the user's profile
  4. No ability to navigate outside of the group page in the embed
  5. No Delete capability for messages
  6. No ability to unwatch
  7. No ability to navigate to the challenge via the challenge link.
jmgasper commented 2 years ago

Challenge https://www.topcoder.com/challenges/4f9fcc94-a37b-4981-861b-2117fa51be5e has been created for this ticket.

This is an automated message for ghostar via Topcoder X

jmgasper commented 2 years ago

Challenge https://www.topcoder.com/challenges/4f9fcc94-a37b-4981-861b-2117fa51be5e has been assigned to obog.

This is an automated message for ghostar via Topcoder X

atelomycterus commented 2 years ago

@jmgasper I've implemented almost all requirements but I have a few questions.

1.No ability to start a new thread or post. They can only respond to posts created by members. They can add attachments in a response.

2.No ability to announce - Fixed.

3.No ability to click a handle to navigate to the user's profile - Fixed.

4.No ability to navigate outside of the group page in the embed What about any links within body of a thread/comment? Should they be disabled?

5.No Delete capability for messages - Fixed.

6.No ability to unwatch - Fixed.

7.No ability to navigate to the challenge via the challenge link. Did you mean this link?.

image

What about voting buttons for threads/comments? image

PRs

Please apply PRs:

https://github.com/topcoder-platform/forums/pull/653 https://github.com/topcoder-platform/forums-groups-plugin/pull/98 https://github.com/topcoder-platform/forums-plugins/pull/108

https://github.com/topcoder-platform/forums-topcoder-editor-plugin/pull/28 - Fixed a minor issue (an image icon was displayed instead of an upload icon, this issue due to upgrading to EasyMde 2.15 ): image

Thanks!

Testing

Client Manager : the main site vs embedded

image

image

jmgasper commented 2 years ago

4 - No, we can leave normal links as-is for now, thanks

7 - Yes, and we also have this link, but not sure if that shows in the embed:

Screen_Shot_2022-01-12_at_8_31_54_am
atelomycterus commented 2 years ago

@jmgasper I did some tweaks for a group page (don't duplicate Group name/Category name in a breadcrumb when embedded, etc). Please apply PRs: https://github.com/topcoder-platform/forums-groups-plugin/pull/99 https://github.com/topcoder-platform/forums/pull/654

Thanks!

  1. The group description field is stored in Markdown format.
    image

The body of discussions/comments are also stored in Markdown format. It's formatted to html before sending to a client. We can fix it different ways:

User can add any links, use @topcoderHandler (which is clickable) in any post and can navigate outside of the group page/thread page. image

How strictly do you want to control user behavior and prevent navigating outside of a group page/thread page? If it's fixed on the client side, user can open the console and see a link, copy it and navigate outside.

jmgasper commented 2 years ago

@atelomycterus - I'm not quite sure how fine-grained we want to go. Let's deploy this and get it QA'd and we'll see what further tweaks Topcoder wants to make, thanks.

atelomycterus commented 2 years ago

@jmgasper Agree, let's deploy and get a feedback from Topcoder. Thanks!

jmgasper commented 2 years ago

@atelomycterus - This is deployed now:

https://platform.topcoder-dev.com/self-service/work-items/85dfab33-5e0d-4abb-b479-e1b93b032b78

I'm still working on testing with a Client Manager role. Hope to have that done by tomorrow, thanks.

atelomycterus commented 2 years ago

@jmgasper 1. Now one Vanilla access token is used for all Vanilla REST API calls. This is incorrect. I generated it for nursoltan-s to help testing MFE. The access tokens’ permissions are reflective of the permissions of the user who generated the token. This token should be revoked for security reasons.

VANILLA_ACCESS_TOKEN is public and hard-codded on all config files (PROD, DEV): https://github.com/topcoder-platform/micro-frontends-self-service-app/blob/develop/config/dev.js, https://github.com/topcoder-platform/micro-frontends-self-service-app/blob/develop/config/prod.js.

We don't allow Vanilla members to generate Vanilla access tokens and remove this link from User profile in Vanilla Forums. We need to support calls Vanilla Rest API (/api/v2) with Topcoder access token.
Now Vanilla REST API works only with Vanilla access_token. Before that, we used it only with a challenge forum processor.

Let me know if you have any questions.

  1. Which account with 'Client Manager' role can I use for testing?

Thanks!

jmgasper commented 2 years ago

@atelomycterus - I added TCConnCopilot as Client Manager to dev challenge 85dfab33-5e0d-4abb-b479-e1b93b032b78

jmgasper commented 2 years ago

@atelomycterus - I also added jgasperMobile12 as a Client Manager, with login password in dev Appirio123

jmgasper commented 2 years ago

@atelomycterus - Also jcori / appirio123

jmgasper commented 2 years ago

How do we handle accounts with multiple roles? For instance, jcori has a bunch of roles on that test challenge, and seems to be able to access links that a client manager shouldn't

atelomycterus commented 2 years ago

@jmgasper I fixed one issue. Sorry about that. Please apply PR- https://github.com/topcoder-platform/forums-plugins/pull/109 and deploy it.

I'll test it.

How do we handle accounts with multiple roles? For instance, jcori has a bunch of roles on that test challenge, and seems to be able to access links that a client manager shouldn't

I check only ' Client Manager'. When embedded through the mfe embed, the Client Manager role should be restricted.

Thanks!

jmgasper commented 2 years ago

@atelomycterus - Thanks. We seem to have an issue on the self-service app and I'm debugging with some help. Hopefully we can get that addressed ASAP and I'll test

jmgasper commented 2 years ago

Ok - fixed now.

jmgasper commented 2 years ago

@atelomycterus - Ok, on https://platform.topcoder-dev.com/self-service/work-items/85dfab33-5e0d-4abb-b479-e1b93b032b78, mess is a Client Manager, and solely that role, but when I test, I can still click handles and challenge links and navigate off the page.

jmgasper commented 2 years ago

Does this have to do with their role in Vanilla as well?

Here's what the roles look like in OR:

Screen Shot 2022-01-18 at 5 21 50 pm
atelomycterus commented 2 years ago

@jmgasper Thanks! I checked the roles , they are ok in Vanilla. The issue with a Vanilla Dispatcher and a group page only, other pages are rendered as expected: image

I logged in as jcori, both views should be identical, the same view and controller are used but they are rendered differently. So I am debugging it: https://vanilla.topcoder-dev.com/group/3124/?embed_type=mfe https://vanilla.topcoder-dev.com/group/85dfab33-5e0d-4abb-b479-e1b93b032b78/?embed_type=mfe image

atelomycterus commented 2 years ago

@jmgasper This issue ☝️ fixed. Please apply PRs: https://github.com/topcoder-platform/forums-plugins/pull/110. https://github.com/topcoder-platform/forums-groups-plugin/pull/100 Thanks!

Quick test embedded content vs the main site:

jmgasper commented 2 years ago

@atelomycterus - Looks good in my testing, thanks! Just one small issue to fix up with the frame sizing.

jmgasper commented 2 years ago

Payment task has been updated: https://www.topcoder.com/challenges/4f9fcc94-a37b-4981-861b-2117fa51be5e Payments Complete Winner: obog Copilot: ghostar Challenge 4f9fcc94-a37b-4981-861b-2117fa51be5e has been paid and closed.

This is an automated message for ghostar via Topcoder X

atelomycterus commented 2 years ago

@jmgasper Yes, it works now. The issue with frame sizing should be fixed in MFE. Please don't forget about the task mentioned in https://github.com/topcoder-platform/forums/issues/652#issuecomment-1013822071. Thanks!

jmgasper commented 2 years ago

@atelomycterus - Thanks for that. For the token issue - should we generate a separate token only for the MFE that can only access that single API that is needed? Is that possible?