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

Key Manager Visibility in Developer portal #2118

Closed
11 tasks done
tgtshanika opened this issue Aug 22, 2023 · 7 comments · Fixed by wso2/carbon-apimgt#12163 or wso2/apim-apps#504
Closed
11 tasks done

Key Manager Visibility in Developer portal #2118

tgtshanika opened this issue Aug 22, 2023 · 7 comments · Fixed by wso2/carbon-apimgt#12163 or wso2/apim-apps#504
Assignees
Milestone

Comments

@tgtshanika
Copy link
Contributor

tgtshanika commented Aug 22, 2023

Problem

Currently, the key manager visibility cannot be controlled or restricted for different user groups(roles) of API consumers.

Solution

Introducing the capability of specifying role-based visibility restriction rules(Deny/Allow) for key manager definition via admin portal.

Affected Component

APIM

Version

4.3.0

Implementation

  • Familiarise with Key managers in API manager
  • Design the DB with schema changes
  • Design the UI
  • Implement the database layer with schema changes
  • Implement API changes in the backend
  • Implement UI for the feature
  • Restrict Key generation for unauthorized key managers
  • Write UI Integration test
  • Write Integration test
  • Migration to APIM-4.3.0
  • Documentation for the feature

Related Issues

No response

Suggested Labels

No response

@Kirishikesan
Copy link

Kirishikesan commented Sep 4, 2023

Status Update

  1. Design Review : Completed
    #Database design was finalised
    #UI design needs some improvement. There will be a short UI design meeting after the backend implementation is completed

  2. Database layer : Completed

  3. Backend Implementation : Ongoing

@Kirishikesan
Copy link

Kirishikesan commented Sep 11, 2023

Summary of Design Review

  • API Schema changes : Adding an extra field "permissions" of type "permission" in keymanager requests to reflect the permissions that the keymanager has.
    The following is the api level schema of KeyManagerPermission
KeyManagerPermission:
  title: Key Manager Permission
  type: object
  properties:
    permissionType:
      type: string
    role:
      type: string
  • Database Schema changes : The following table was proposed.
  CREATE TABLE IF NOT EXISTS AM_KEY_MANAGER_PERMISSIONS (
      UUID VARCHAR(50) NULL,
      PERMISSIONS_TYPE VARCHAR(50) NULL,
      ROLES VARCHAR(512) NULL,
      PRIMARY KEY (UUID)
      FOREIGN KEY (UUID) REFERENCES AM_KEY_MANAGER(UUID));

It was proposed to change the database schema as follows to store the roles in different records.

  CREATE TABLE IF NOT EXISTS AM_KEY_MANAGER_PERMISSIONS (
    KEY_MANAGER_PERMISSION_ID INT NOT NULL AUTO_INCREMENT,
    KEY_MANAGER_UUID VARCHAR(50) NULL,
    PERMISSIONS_TYPE VARCHAR(50) NULL,
    ROLE VARCHAR(255) NULL,
    PRIMARY KEY (KEY_MANAGER_PERMISSION_ID),
    FOREIGN KEY (KEY_MANAGER_UUID) REFERENCES AM_KEY_MANAGER(UUID));

@Kirishikesan
Copy link

Kirishikesan commented Sep 11, 2023

Status of Backend Implementation

  • API level schema changes were done by adding an extra permissions of type permission in the keymanager create, update and get calls.
    Therefore there will be an array of permissions added to the keymanager api schema
{ 
    ...
    permissions : [
          { 
                permission Type :  <permissionType>,
                role : <role>
          },
          { 
                permission Type :  <permissionType>,
                role : <role>
          }
    ]
}
  • API layer implementation was complete for create, update and get calls.
  • It was proposed to change the API level schema as the following, to remove the redundant usage of permissionType property in the API schema of keymanagerpermissions.
    The keymanager api schema will now have
{ 
    ...
    permissions : {
       permissionType : <permissionType>
       roles : []
    }
}

@Kirishikesan
Copy link

Kirishikesan commented Sep 18, 2023

Hi team,

Status of the Backend Implementation

  1. CRUD operations for the key manager permissions - Implemented
  2. Filter key managers by role in devportal - Implemented

Starting the UI Implementation

Thanks & Regards,
Kirishikesan

@Kirishikesan
Copy link

Kirishikesan commented Sep 25, 2023

Hi team,

Progress update

  1. UI Implementation in the admin portal - Completed
  2. Restriction of keygeneration for unauthorized users - Completed.

With this the implementation is completed and the code review is scheduled on 26.09.2023.

The Integration tests, documentation and the migration tasks is due for this week.

Thanks & Regards,
Kirishikesan

@Kirishikesan
Copy link

Hi team,

The code review was completed on 27.09.2023 and the following decisions were taken.

  1. Remove publisher side information related to role restricted key managers - As they are partial information and the publisher role does not need to concern with the relevant business use case.
  2. Resident key managers should also be affected by this feature.
  3. Change the DB level schema to the following,
CREATE TABLE IF NOT EXISTS AM_KEY_MANAGER_PERMISSIONS (
  KEY_MANAGER_UUID VARCHAR(50) NULL,
  PERMISSIONS_TYPE VARCHAR(50) NULL,
  ROLE VARCHAR(255) NULL,
  PRIMARY KEY (KEY_MANAGER_UUID, ROLE),
  FOREIGN KEY (KEY_MANAGER_UUID) REFERENCES AM_KEY_MANAGER(UUID));
  1. SQL queries can be optimised by combining the queries. Therefore the queries should be changed as to do it in one transaction.
  2. Include javadoc for the functions

Meeting notes : https://docs.google.com/document/d/1_N6QNuRyGTvc_4fQg1v8CDKeLdzlpzAcL9tC_jgYVZ0/edit#heading=h.53k25yfmsand

Thanks & Regards,
Kirishikesan

@Kirishikesan
Copy link

Hi team,

The doc PR has been sent and the PRs are sent for Review.
There are failures in the integration test for this issue. This issue will be complete as soon as that issue is resolved and the PRs merged.

Thanks & Regards,
Kirishikesan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment