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

feat(lightspeed): add permission support in lightspeed plugins #66

Merged
merged 7 commits into from
Nov 28, 2024

Conversation

karthikjeeyar
Copy link
Contributor

@karthikjeeyar karthikjeeyar commented Nov 22, 2024

Tickets:

https://issues.redhat.com/browse/RHDHPAI-33
https://issues.redhat.com/browse/RHDHPAI-34
https://issues.redhat.com/browse/RHDHPAI-35

Lightspeed Permission support

Add permission support for lightspeed plugins.

This PR adds three basic permissions lightspeed.conversations.read, lightspeed.conversations.create and lightspeed.conversations.delete. These permission are used to secure the UI and API endpoints.

Note:

To access lightspeed UI, then logged in user should be granted with below permissions.

lightspeed.conversations.read
lightspeed.conversations.create

API endpoints:

lightspeed.conversations.read - all GET requests are gated by this permission
lightspeed.conversations.create - all POST requests are gated by this permission
lightspeed.conversations.delete - all DELETE requests are gated by this permission

How to test this changes?

  1. Install and configure rbac-backend (Prerequiste)

  2. create a rbac-policy.csv file with below permission policies.

     p, role:default/team_a, lightspeed.conversations.read, read, allow
     p, role:default/team_a, lightspeed.conversations.create, create, allow
     p, role:default/team_a, lightspeed.conversations.delete, delete, allow
     
     g, user:default/<your-user-name>, role:default/team_a
    
  3. enable rbac and wire the policy file in the app-config.yaml file

    permission:
      enabled: true
      rbac:
        policies-csv-file: /your/absolute/path/rbac-policy.csv
        policyFileReload: true
    
  • run the application and modify the permission policy from allow to deny to see the permission required alert or Unauthorized error while accessing endpoints.

Screenshots

If one of the permissions (lightspeed.conversations.read or lightspeed.conversations.create) is denied for a user, then UI will show the missing permissions error state.

image

Delete button is disabled when lightspeed.conversations.delete is denied.

image

API endpoints

conversation_access_denied
models_api_access_denied

Unit test

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

@yangcao77
Copy link
Contributor

yangcao77 commented Nov 25, 2024

I have tried with the following rbac policy, basically removed the delete permission from the policy.

p, role:default/guests, lightspeed.conversations.read, read, allow
p, role:default/guests, lightspeed.conversations.create, create, allow
p, role:default/guests, lightspeed.conversations.delete, delete, deny


g, user:default/yangcao77, role:default/guests

I can chat with the lightspeed bot with no issue as expected, since I have read & create permission. However, I can also delete the conversation session. Please confirm if this is a bug, or I set it up wrong

UPDATE:
I updated to p, role:default/team_a, lightspeed.conversations.create, create, deny, and I was still able to chat with lightspeed bot

do I need to set up anything in my frontend/backend other than app-catalog?

@karthikjeeyar
Copy link
Contributor Author

karthikjeeyar commented Nov 26, 2024

@yangcao77 seems like your setup is incorrect.

Here are some troubleshooting steps:

  1. make sure you have installed and configured rbac backend
  2. check if you have commented or removed the @backstage/plugin-permission-backend/alpha and @backstage/plugin-permission-backend-module-allow-all-policy in backend/src/index.ts file as mentioned in the configuration docs. if allow-all-policy is not disabled then your permission framework will by default allow all the users.
  3. wire your rbac policy file via app-config (Note: Use full path to your reference your rbac policy file).
  4. If everything is wired properly, then you will not see any error in the terminal when you start the backstage app.

@Eswaraiahsapram
Copy link
Contributor

Pulled the changes locally and tested. Everything works as expected and looks good to me.

@yangcao77
Copy link
Contributor

@karthikjeeyar thanks for the instruction. confirmed I'm able to verify the rbac permissions.
Also all backend APIs still function as expected.

Copy link
Contributor

@yangcao77 yangcao77 left a comment

Choose a reason for hiding this comment

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

looks good to me from backend perspective.

also, do we need a changeset for this PR?

@karthikjeeyar
Copy link
Contributor Author

karthikjeeyar commented Nov 27, 2024

added changeset, and release both frontend and backend as 0.2.0. Since this is a private package, we might need to do a manual publishing to npm.

Edit: I made it public now, so it will auto publish to npm.

@karthikjeeyar karthikjeeyar force-pushed the lightspeed-rbac branch 3 times, most recently from 788d347 to 61b38d5 Compare November 27, 2024 09:21
@rhdh-gh-app
Copy link

rhdh-gh-app bot commented Nov 27, 2024

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-lightspeed-backend workspaces/lightspeed/plugins/lightspeed-backend minor v0.1.1
@red-hat-developer-hub/backstage-plugin-lightspeed-common workspaces/lightspeed/plugins/lightspeed-common patch v0.1.0
@red-hat-developer-hub/backstage-plugin-lightspeed workspaces/lightspeed/plugins/lightspeed minor v0.1.3

@rhdh-gh-app
Copy link

rhdh-gh-app bot commented Nov 27, 2024

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-lightspeed-backend workspaces/lightspeed/plugins/lightspeed-backend minor v0.1.1
@red-hat-developer-hub/backstage-plugin-lightspeed-common workspaces/lightspeed/plugins/lightspeed-common patch v0.1.0
@red-hat-developer-hub/backstage-plugin-lightspeed workspaces/lightspeed/plugins/lightspeed minor v0.1.4

@redhat-developer redhat-developer deleted a comment from rhdh-gh-app bot Nov 27, 2024
@redhat-developer redhat-developer deleted a comment from rhdh-gh-app bot Nov 27, 2024
@redhat-developer redhat-developer deleted a comment from rhdh-gh-app bot Nov 27, 2024
@redhat-developer redhat-developer deleted a comment from rhdh-gh-app bot Nov 27, 2024
@redhat-developer redhat-developer deleted a comment from rhdh-gh-app bot Nov 27, 2024
@redhat-developer redhat-developer deleted a comment from rhdh-gh-app bot Nov 27, 2024
@yangcao77
Copy link
Contributor

added changeset, and release both frontend and backend as 0.2.0. Since this is a private package, we might need to do a manual publishing to npm.

Edit: I made it public now, so it will auto publish to npm.

IIRC, in janus-idp repo, it requires version > 1.0 to be auto publish to npm. I'm not sure if rhdh-plugins is still the case.
better confirm with @nickboldt @christoph-jerolimov

@rohitkrai03
Copy link

added changeset, and release both frontend and backend as 0.2.0. Since this is a private package, we might need to do a manual publishing to npm.
Edit: I made it public now, so it will auto publish to npm.

IIRC, in janus-idp repo, it requires version > 1.0 to be auto publish to npm. I'm not sure if rhdh-plugins is still the case. better confirm with @nickboldt @christoph-jerolimov

@yangcao77 That was a limitation of semantic release. I think changesets do support releasing non v1 versions of pckages.

@rhdh-gh-app
Copy link

rhdh-gh-app bot commented Nov 27, 2024

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-lightspeed-backend workspaces/lightspeed/plugins/lightspeed-backend minor v0.1.1
@red-hat-developer-hub/backstage-plugin-lightspeed-common workspaces/lightspeed/plugins/lightspeed-common patch v0.1.0
@red-hat-developer-hub/backstage-plugin-lightspeed workspaces/lightspeed/plugins/lightspeed minor v0.1.4

Copy link

@rohitkrai03 rohitkrai03 left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm label Nov 28, 2024
@rohitkrai03 rohitkrai03 merged commit c113d1a into redhat-developer:main Nov 28, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants