Skip to content

Issue 582 market rbac integration#597

Merged
jamiehewitt15 merged 40 commits into
mainfrom
issue-582-market-rbac-integration
Jun 10, 2021
Merged

Issue 582 market rbac integration#597
jamiehewitt15 merged 40 commits into
mainfrom
issue-582-market-rbac-integration

Conversation

@jamiehewitt15

@jamiehewitt15 jamiehewitt15 commented May 14, 2021

Copy link
Copy Markdown
Contributor

Closes: #582

Changes proposed in this PR:

  • Adding util function for calling the RBAC server
  • Creating new environmental variable for the RBAC server address
  • Creating Permissions component to wrap elements that should be restricted due to access roles (browse, consume, publish)
  • By default access is restricted if a URL for the RBAC server is defined in the environmental variables.
  • If a URL for the RBAC server is not defined then the permissions will default to allow access and this PR won;t change the user experience. For markets (such as Ocean Market) that don't have any role based restrictions, the URL env should be left blank.

Steps for testing:

  1. Clone the RBAC server:
git clone https://github.com/oceanprotocol/RBAC-Server.git
  1. Create an .env file and add thefollowing URLs:
KEYCLOAK_URL="https://keycloak-int.data-marketplace.io/auth/realms/marketplace/protocol/openid-connect/userinfo"
KEYCLOAK_ADDRESS_URL="https://keycloak-int.data-marketplace.io/auth/realms/marketplace/custom-api/wallet"
DEFAULT_AUTH_SERVICE = "keycloak"
  1. Run the RBAC server locally:
cd RBAC-Server
npm install
npm run build
npm run start
  1. Run this branch of the market locally:
git checkout issue-582-market-rbac-integration
npm run start
  1. In order to correct test the permissions you will need a role assigned to your Ethereum address within keycloak. Speak to the Daimler team about having your wallet address added to keycloak

Testing (without any permission restrictions):

In ocean market and other marketplace forks, there will be no need to have any permission based restrictions. This is made possible in this PR. If a RBAC_URL is not defined then the permission response will always be true and there will be no change in the user experience.

To test:

  1. Remove or comment out the RBAC_URL from .env.development:
    RBAC_URL="http://localhost:3000"
  2. Run the market with npm run start and use it as per normal

@vercel

vercel Bot commented May 14, 2021

Copy link
Copy Markdown

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/oceanprotocol/market/BqACwKygAByfhGX76d4z6b5FnBaT
✅ Preview: https://market-git-issue-582-market-rbac-integration-oceanprotocol.vercel.app

@kremalicious

kremalicious commented Jun 4, 2021

Copy link
Copy Markdown
Contributor

If the RBAC_URL file is not defined the permission will always return true which means there is no change to the user experience from before, no wallet is required

I am looking at the Vercel preview as that's what would be deployed to live. And there I get the screen posted above so I guess Vercel picks up .env files although it did not pick up the network variable. Which is why we usually don't commit them and gitignore them as we set env vars via Vercel/Netlify interface. Also no need to complicate things and just use the one .env so add to .env.example

@jamiehewitt15

Copy link
Copy Markdown
Contributor Author

I am looking at the Vercel preview as that's what would be deployed to live. And there I get the screen posted above so I guess Vercel picks up .env files although it did not pick up the network variable. Which is why we usually don't commit them and gitignore them as we set env vars via Vercel/Netlify interface. Also no need to complicate things and just use the one .env so add to .env.example

Ok cool, I've removed the .env.development and updated .env.example

@jamiehewitt15

Copy link
Copy Markdown
Contributor Author

The latest vercel deployment doesn't require the wallet to be connected now

@qlty-cloud-legacy

Copy link
Copy Markdown

Code Climate has analyzed commit 10b0136 and detected 0 issues on this pull request.

View more on Code Climate.

@mihaisc

mihaisc commented Jun 7, 2021

Copy link
Copy Markdown
Contributor

Did some quick tests with/without wallet, current functionality seems to be ok

@jamiehewitt15

Copy link
Copy Markdown
Contributor Author

Thanks @mihaisc

@jamiehewitt15 jamiehewitt15 merged commit e26ed0e into main Jun 10, 2021
@jamiehewitt15 jamiehewitt15 deleted the issue-582-market-rbac-integration branch June 10, 2021 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrate Market with RBAC server

4 participants