[MGNLDEMO-70] Eric, Peter & Tina should see less apps Created: 30/Jun/15  Updated: 15/Apr/16  Resolved: 16/Oct/15

Status: Closed
Project: Magnolia Demo Projects
Component/s: None
Affects Version/s: None
Fix Version/s: 0.8

Type: Bug Priority: Major
Reporter: Christopher Zimmermann Assignee: Antonín Juran
Resolution: Fixed Votes: 1
Labels: sales
Remaining Estimate: 3h 35m
Time Spent: 1h 25m
Original Estimate: 5h

Issue Links:
Relates
relates to MAGNOLIA-6407 Access permissions to apps for user r... Closed
relates to MGNLDEMO-106 Tina can't open Preview as visitor app Closed
relates to CNTCTSAPP-96 Not all user roles should have access... Closed
relates to MGNLDEMO-68 Share demo users between STK and trav... Closed
dependency
is depended upon by MGNLDEMO-98 AccessDeniedException is thrown when ... Closed
is depended upon by MGNLSITE-36 No site definition appears in Site Ap... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Epic Link: Travel Demo
Sprint: Kromeriz 14
Story Points: 2

 Description   

The demo should also restrict the availablity of apps that do not make sense for specific users - for example removing the entire Setup group and Web Dev group.

Eric, Eric-de and Peter should see:
Pages, Assets, Tours, TourCategories

They should not see:
Contacts App
Setup group
Web Dev group

Tina should see:
Tours App, Assets App

She should not see:
Pages App, Contacts App, TourCategories App
Setup Group
Webdev Group

A possible quick implementation would be to add permissions restrictions just to the Setup and Web Dev app groups - then it might not be necessary to change the permissions on many of the apps.

Ticket title was: "Apps should only be available to the users that have rights on their workspaces."
Currently some roles restrict which workspaces the users can edit - but the users can still see the app and open it - revealing app with no content or available actions. This is confusing & looks broken in demos. If a user has no access to the workspace, they should also not have access to the app.
This is important for good demos that show off the power of Magnolia security & apps.



 Comments   
Comment by Christopher Zimmermann [ 17/Sep/15 ]

Im not sure if the best way to fix this is simply to add the permissions nodes to all of the apps & groups - or if the app permissions feature needs to be expanded. For example - perhaps we should add a way to block access to a specific role, rather than only being able to grant to a specific role.
Or maybe an app should simply not be available if the user does not have access to the workspaces in the subapps contentconnectors. (This way configuration in the security app would be enough - one would not have to configure permissions to the app at all.)

Comment by Roman Kovařík [ 02/Oct/15 ]

Reopened:

  • TravelDemoModuleVersionHandler
    • the same tasks should run also on fresh install, not only on update
    • missing task name for NodeExistsDelegateTask
    • you can use contants for group/role names from SetupDemoRolesAndGroupsTask instead of Strings
  • info.magnolia.demo.travel.setup.SetupDemoRolesAndGroupsTaskTest#demoRolesCanAccessPagesApp: no need to create the path before the test
  • TravelDemoModuleVersionHandlerTest:
    • you can again use the constants for group/role names
    • doesn't it test the same thing as SetupDemoRolesAndGroupsTaskTest? If yes please remove.
  • ToursModuleVersionHandler: no need to add update tasks since the app is rebootstrapped by one of the tasks above.
Comment by Antonín Juran [ 06/Oct/15 ]

Will be finished after https://jira.magnolia-cms.com/browse/MAGNOLIA-6407.

Comment by Marvin Kerkhoff [ 09/Oct/15 ]

In our case we use a simple pattern. In every app there will be a permission. canAccessPagesApp, canAccessAssetsApp, canAccessXYApp. Then we define roles for all the apps in a subdirectory in the secure app. With this roles we can easily define who has access to which app. If you define roles like superuser or something else you will fail because you need to change the configs if you don't want that superuser can access the app.

We use also the same pattern vor accessing actions in the actionbar. For e.g. with the role canPublishPages you can define if a user can publish something in the pages app. We add also a second role. canPublish this defines that the user can publish in every app.

Comment by Roman Kovařík [ 14/Oct/15 ]
  1. Could you make a task (e.g MigrateAccessDefinitionToUseRoleBasedVoter)? There is 6x almost the same code.
  2. You can use Class.getName() instead of plain Strings (e.g. RoleBaseVoter.getName() instead of "info.magnolia.voting.voters.RoleBaseVoter")
  3. Same for true -> Boolean.TRUE.toString()
  4. And again constants for role names ("travel-demo-admincentral")
  5. Please make a method to check assertions so you don't need to copy them.
  6. Could you merge all the tests into one?
  7. Not related to this ticket: could you raise the <cloverCoverageThreshold> to 35? (mciwc)
Comment by Milan Divilek [ 16/Oct/15 ]

1. info.magnolia.demo.travel.setup.TravelDemoModuleVersionHandler - You are using same task in extra install tasks and during update. You can define task as final variable and use it for extra install tasks and during update. In that case you can remove the constants.

new SetupAccessDefinitionToUseRoleBaseVoter("Deny access permissions to apps", "Deny access permissions to Contacts app, Web Dev group, Set Up group for travel-demo-admincentral role", TRAVEL_DEMO_ADMINCENTRAL_ROLE, true, CONTACTS_APPS_CONTACTS_NODE_PATH, UIADMINCENTRAL_CONFIG_APPLAUNCH_GROUPS_STK_NODE_PATH, UIADMINCENTRAL_CONFIG_APPLAUNCH_GROUPS_MANAGE_NODE_PATH)

2. info.magnolia.demo.travel.setup.SetupAccessDefinitionToUseRoleBaseVoter - Please cascade for "path" constants and don't make constants protected just because of tests.

Generated at Mon Feb 12 05:15:59 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.