Add promotion persmission #2182

muralibasani · 2024-01-10T13:09:17Z

Linked issue

Resolves: #2121

What kind of change does this PR introduce?

New permission when topics are promoted

a config is added if new permission is required to promote topics
a new permission is added and this should be manually assigned to role by users
on topic promotion, apart from APPROVE_TOPICS, a check is added to look for permission APPROVE_TOPICS_PROMOTION if server config for extra permission is set to true. by default it is false.
Bug fix
New feature
[ X] Refactor
Docs update
CI update

What is the current behavior?

Describe the state of the application before this PR. Illustrations appreciated (videos, gifs, screenshots).
there is no extra permission to promote topics

What is the new behavior?

Describe the state of the application after this PR. Illustrations appreciated (videos, gifs, screenshots).
a new permission is added to check if the user has promote topics permission

Other information

Additional changes, explanations of the approach taken, unresolved issues, necessary follow ups, etc.

Requirements (all must be checked before review)

The pull request title follows our guidelines
Tests for the changes have been added (if relevant)
The latest changes from the main branch have been pulled
pnpm lint has been run successfully

Signed-off-by: muralibasani <[email protected]>

aindriu-aiven

A few comments

aindriu-aiven · 2024-01-10T13:41:16Z

core/src/main/java/io/aiven/klaw/service/DefaultDataService.java

@@ -47,6 +49,8 @@ public class DefaultDataService {
 @Value("${klaw.clusterapi.url:http://localhost:9343}")
 private String clusterApiUrl;

+ private boolean extraPermissionForTopicsPromotion;


This doesnt look like it is used.

aindriu-aiven · 2024-01-10T13:41:56Z

core/src/main/java/io/aiven/klaw/service/DefaultDataService.java

+
+ KwProperties kwProperties38 =
+ new KwProperties(
+ KLAW_EXTRA_PERMISSION_TOPIC_PROMOTION_KEY,


I would probably call this KLAW_OPTIONAL_PERMISSION_TOPIC_PROMOTION_KEY

aindriu-aiven · 2024-01-10T13:42:47Z

core/src/main/java/io/aiven/klaw/service/DefaultDataService.java

+ KLAW_EXTRA_PERMISSION_TOPIC_PROMOTION_KEY,
+ tenantId,
+ "false",
+ "Need of an extra permission APPROVE_TOPICS_PROMOTION to promote topics");


"Optional permission APPROVE_TOPICS_PROMOTION to allow users to promote topics but not create topics"

aindriu-aiven · 2024-01-10T13:44:29Z

core/src/main/java/io/aiven/klaw/service/TopicControllerService.java

@@ -708,6 +709,16 @@ public ApiResponse approveTopicRequests(String topicId) throws KlawException {
 int tenantId = commonUtilsService.getTenantId(userName);
 TopicRequest topicRequest = getTopicRequestFromTopicId(Integer.parseInt(topicId), tenantId);

+ String needOfExtraPermissionForPromote =


maybe "isOptionalExtraPermissionForPromote" as it is a boolean?

aindriu-aiven · 2024-01-10T13:47:08Z

core/src/main/java/io/aiven/klaw/service/TopicControllerService.java

@@ -708,6 +709,16 @@ public ApiResponse approveTopicRequests(String topicId) throws KlawException {
 int tenantId = commonUtilsService.getTenantId(userName);
 TopicRequest topicRequest = getTopicRequestFromTopicId(Integer.parseInt(topicId), tenantId);

+ String needOfExtraPermissionForPromote =
+ manageDatabase.getKwPropertyValue(KLAW_EXTRA_PERMISSION_TOPIC_PROMOTION_KEY, tenantId);
+ if (topicRequest.getRequestOperationType().equals(RequestOperationType.PROMOTE.value)


This means that if a user also has the CreateTopic Permission they will also require the optional promote permission.

Should it be in the check that they have either the Create or Promote version?

This is just a question though on the UX I don't mind which way its implemented as long as we are happy.

Valid question. Approve_topics permission is always required. But if they set this server config property to true, then approve_topics_promotion check is validated

aindriu-aiven · 2024-01-10T14:01:28Z

core/src/main/java/io/aiven/klaw/dao/migration/MigrateData2x8x0.java

+ allUserInfo.stream().map(UserInfo::getTenantId).collect(Collectors.toSet());
+
+ for (int tenantId : tenantIds) {
+ List<KwProperties> kwPropertiesList = selectDataJdbc.selectAllKwPropertiesPerTenant(tenantId);


Sorry just realised it's probably worth putting a check in here to only update it if it does not already exist?

Signed-off-by: muralibasani <[email protected]>

aindriu-aiven · 2024-01-11T15:29:31Z

core/src/main/java/io/aiven/klaw/dao/migration/MigrateData2x8x0.java

+ public boolean migrate() {
+ log.info(
+ "Start to migrate 2.8.0 data. Add new server property klaw.extra.permission.topic.promote.");
+


minor correction klaw.extra.permission.topic.promote. ->klaw.extra.permission.topic.create

aindriu-aiven · 2024-01-11T15:52:29Z

core/src/main/java/io/aiven/klaw/dao/migration/MigrateData2x8x0.java

+ KLAW_OPTIONAL_PERMISSION_NEW_TOPIC_CREATION_KEY,
+ tenantId,
+ "false",
+ "Need of an extra permission " + APPROVE_TOPICS_CREATE + " to create new topics");


Suggested change

"Need of an extra permission " + APPROVE_TOPICS_CREATE + " to create new topics");

"Enforce extra permission " + APPROVE_TOPICS_CREATE + " to create new topics");

aindriu-aiven

Functionally looks good, just a couple of small comments

Signed-off-by: muralibasani <[email protected]>

…sion

…promote-permission

Signed-off-by: muralibasani <[email protected]>

aindriu-aiven

LGTM

Signed-off-by: muralibasani <[email protected]>

aindriu-aiven

LGTM

Add promotion persmission

97e9156

Signed-off-by: muralibasani <[email protected]>

aindriu-aiven reviewed Jan 10, 2024

View reviewed changes

muralibasani added 6 commits January 10, 2024 16:22

Validate checks

1f46863

Signed-off-by: muralibasani <[email protected]>

Updating permission validation

37c412b

Signed-off-by: muralibasani <[email protected]>

Validation for topic create requests

e9b8461

Signed-off-by: muralibasani <[email protected]>

Validation for topic create requests

cdbdef5

Signed-off-by: muralibasani <[email protected]>

Validation for topic create requests

7dae8ac

Signed-off-by: muralibasani <[email protected]>

Validation for topic create requests

46cc7bb

Signed-off-by: muralibasani <[email protected]>

aindriu-aiven reviewed Jan 11, 2024

View reviewed changes

aindriu-aiven and others added 7 commits January 11, 2024 16:24

Merge branch 'main' into promote-permission

15ec5c3

Update text

f42a297

Signed-off-by: muralibasani <[email protected]>

Merge branch 'main' of github.com:Aiven-Open/klaw into promote-permis…

8d9c59f

…sion

Merge branch 'promote-permission' of github.com:Aiven-Open/klaw into …

a5305a8

…promote-permission

Update text

072f69c

Signed-off-by: muralibasani <[email protected]>

Limiting users on FE as well

83ebfbf

Signed-off-by: muralibasani <[email protected]>

Button disabled with hover text

9717ab9

Signed-off-by: muralibasani <[email protected]>

aindriu-aiven previously approved these changes Jan 22, 2024

View reviewed changes

Fix conflicts

45eb392

Signed-off-by: muralibasani <[email protected]>

muralibasani dismissed aindriu-aiven’s stale review via 45eb392 January 22, 2024 13:47

aindriu-aiven approved these changes Jan 22, 2024

View reviewed changes

muralibasani merged commit 0ccca58 into main Jan 22, 2024
24 checks passed

muralibasani deleted the promote-permission branch January 22, 2024 15:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add promotion persmission #2182

Add promotion persmission #2182

muralibasani commented Jan 10, 2024

aindriu-aiven left a comment

aindriu-aiven Jan 10, 2024

aindriu-aiven Jan 10, 2024

aindriu-aiven Jan 10, 2024

aindriu-aiven Jan 10, 2024

aindriu-aiven Jan 10, 2024

muralibasani Jan 10, 2024

aindriu-aiven Jan 10, 2024

aindriu-aiven Jan 11, 2024

aindriu-aiven Jan 11, 2024

aindriu-aiven left a comment

aindriu-aiven left a comment

aindriu-aiven left a comment

	"Need of an extra permission " + APPROVE_TOPICS_CREATE + " to create new topics");
	"Enforce extra permission " + APPROVE_TOPICS_CREATE + " to create new topics");

Add promotion persmission #2182

Add promotion persmission #2182

Conversation

muralibasani commented Jan 10, 2024

Linked issue

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

Other information

Requirements (all must be checked before review)

aindriu-aiven left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aindriu-aiven left a comment

Choose a reason for hiding this comment

aindriu-aiven left a comment

Choose a reason for hiding this comment

aindriu-aiven left a comment

Choose a reason for hiding this comment