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

Migrate Console Role Permissions #6352

Closed
wants to merge 125 commits into from

Conversation

shashimalcse
Copy link
Contributor

sadilchamishka
sadilchamishka previously approved these changes Jan 24, 2025
List<Permission> deletedPermissions, String audience, String audienceId,
String tenantDomain) throws IdentityRoleManagementException {

if (APPLICATION.equals(audience)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use same isConsoleRole(roleId, tenantDomain) method.

public void postGetRole(Role role, String roleId, String tenantDomain) throws IdentityRoleManagementException {


if (isConsoleRole(roleId, tenantDomain)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using common method, let's write convenient logics.

if (!"CONSOLE".equals()role.getAudienceName() || !"application".equalsIgnoreCase(role.getAudience())) {
return;
}

Comment on lines 88 to 94
boolean isConsoleRole = false;
for (String roleId : roleIds) {
if (isConsoleRole(roleId, tenantDomain)) {
isConsoleRole = true;
break;
}
}
Copy link
Contributor

@sadilchamishka sadilchamishka Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By looking at the permissions also we can decide a console role. A console role exist permissions starts with console: which is not allowed for the roles.

List<Permission> systemPermissions = getSystemPermission(tenantDomain);
List<APIResourceCollection> apiResourceCollections = getAPIResourceCollections(tenantDomain);
List<Permission> consoleFeaturePermissions = getConsoleFeaturePermissions(rolePermissions);
if (!consoleFeaturePermissions.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we add scopes to an existing role (old role). Then its corresponding scopes will be saved console:abc:view, console:abc:edit.
When getting the scopes of the role, it will have new formatted scopes and old role related scopes. But the logic might skip for processing old roles related scopes.

}
}

private List<Permission> getUpdatedPermissions(List<Permission> rolePermissions, String tenantDomain)
Copy link
Contributor

@sadilchamishka sadilchamishka Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A Suggession to organize the code.

HashMap
console:app:edit -> { s1, s2....}
console:app:view - > {s1,..}

Iterate over collection:     ex: console:app, console:idp,.....
      1. if console:app:edit exist in permission list.     // first iteration is console:app
           Add all respective scopes. 
           Continue.
      2. if console:app:view exist in permission list.
           Add respective view scopes. 
           Continue.
      // backward compatible logic.
      x = Fetch previous console:app edit related scopes set.
      3. if x contains in permission list
            Add all respective scopes. 
            Continue.
     
      4. Add respective view scopes. 

Copy link
Contributor Author

@shashimalcse shashimalcse Jan 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 this will improve the logic

shashimalcse and others added 23 commits January 26, 2025 01:56
@shashimalcse
Copy link
Contributor Author

close with this : #6372

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.