node-casbin icon indicating copy to clipboard operation
node-casbin copied to clipboard

Extract out casbin.core.js, use it to generate wrappers like Node-Casbin, casbin.js, casbin-rn

Open hsluoyz opened this issue 3 years ago • 20 comments

The new Node-Casbin should have exactly 100% compatible API of the old code as now, except it's built on top of casbin.core.js: https://github.com/casbin/casbin.core.js

Here's the plan:

  1. A new project: https://github.com/casbin/casbin.core.js will be started. It will inherit all git histories of the old node-casbin, but with all file-related code removed by a commit, maybe a simple file-adapter is kept in the dev folder for running tests against model and policy files.
  2. The new node-casbin will still use this repo and inherit all GitHub stars. But most of code in this repo will be removed and replaced by depending on casbin.core.js via NPM package.
  3. The old https://github.com/casbin/casbin.js will be fully rewritten to build on top of the new casbin.core.js. It still uses its old repo.
  4. The new node-casbin and new casbin.js will only keep a small set of testsets, targeting for Node.js and browser envs only to prove it's working, like an adapter. Most of testcases like ACL, RBAC will be put in casbin.core.js

Does it sound OK?

@harveyappleton @nodece @Zxilly

hsluoyz avatar May 06 '21 11:05 hsluoyz

I'll prefer node-casbin-core. xxx.js looks like a frontend framework. node-casbin and casbin.js will both depend on it. It inherit all git histories of the old node-casbin.

The rest is no problem to me.

Zxilly avatar May 06 '21 12:05 Zxilly

@Zxilly Node uses JS, but JS is not Node only. So I don't think it's good to call node-casbin-core, as it's not for Node only. Actually it doesn't contain any Node-specific code.

hsluoyz avatar May 06 '21 12:05 hsluoyz

@hsluoyz I still feel this idea is complicated, we need to manager the casbin-core.js, casbin-rn, casbin.js and so on project. If we use memory adapter/model instead of file adapter/model, node-casbin can be used in any platform of javascript. Although we have brought breaking changes, but I think this is still the best way. We can continue to maintain v5 of node-casbin, and release a beta version for my idea.

nodece avatar May 07 '21 06:05 nodece

@nodece Node-Casbin is named for supporting all Node.js user cases. It doesn't make sense if it doesn't contain file operators. Because file is critical for Node.js

hsluoyz avatar May 07 '21 07:05 hsluoyz

@hsluoyz The name of the repository is not important, we released casbin package in NPM, this name does not seem to limit the environment.

nodece avatar May 07 '21 07:05 nodece

@nodece I'm OK with putting all code inside one repo if we can:

  1. Pack minimized required source code files into the package for an environment.
  2. Node-Casbin public API is not changed.

For example, the single repo contains three parts: A, B and C.

  1. A is the core code without file operations and can be built into casbin.js
  2. A + B can be built into Node-Casbin, B will provide the public API of Node-Casbin.
  3. A + C can be built into casbin-rn, C similar.

hsluoyz avatar May 07 '21 08:05 hsluoyz

Hmmph. NPM package pointing to node-casbin is actually quite confusing. I would probably just deprecate current npm libraries and make completely new naming scheme. Introducing a new version in a new npm/git repository could ease out the confusion.

Many libraries have done that so it would not be out of the ordinary (fullcalendar as an example).

casbin --> @casbin/core
casbin --> @casbin/file-adapter
casbin.js --> fully archived/deprecated, replaced with casbin core

Version 5 of casbin would then be kept in temporary maintenance mode and fully deprecated in April 2022 (at the same time of nodejs 12). Then just create migration guides where reference that you need to use adapters if you want to use the file system.

I think it should up to the user to decide what adapters they use.

Sefriol avatar May 07 '21 15:05 Sefriol

@Sefriol casbin.js will be named after @casbin/browser, it will also provide some browser only API. If we have completed @casbin/core, casbin in npm will be a virtual package pointed to @casbin/node.

Zxilly avatar May 08 '21 00:05 Zxilly

hi @hsluoyz, @Sefriol is the same idea as ours.

I plan to provide the following packages as first version:

  • @casbin/core: used to any javascript environment: node, browser, react native and so on
  • @casbin/node-file-adapter: a node file adapter

casbin org has too many names for JS project(example casbin.js, casbin.core.js, node-casbin), this can confuse many users, so I think we should continue to write code on https://github.com/casbin/node-casbin, Node.js is a superset of JS, it seems great.

nodece avatar May 08 '21 04:05 nodece

@nodece any form of breaking change for Node-Casbin is NOT OK.

hsluoyz avatar May 08 '21 07:05 hsluoyz

@Sefriol is on the money I think.

Make the core package work on ANY Js environment: node, browser, react native - whatever.

Then install environment specific adapters if you need them, eg file adapters for node.

For the use cases I want to use Casbin for, I do not need file adapters at all, as models and policies will be loaded dynamically from either API or database. And I don't think that's particularly unusual use case.

This also means core package will be smaller; which is good.

I think deprecating and doing a whole new naming scheme would be best bet.

Eg @casbin/core @casbin/node-file-adapter @casbin/frontend (if you wanted to. But to be honest, I think it's simpler if the same core library is used on both backend and frontend - same docs, same code, isomorphic. Eases confusion)

harveyappleton avatar May 08 '21 08:05 harveyappleton

@casbin/frontend (if you wanted to. But to be honest, I think it's simpler if the same core library is used on both backend and frontend - same docs, same code, isomorphic. Eases confusion)

Less maintenance as well. That's why I wanted casbin.js to be fully deprecated. Name is already confusing and I would assume that node-casbin uses casbin.js in the background if I didn't know anything about the project.

I see no problem in making breaking changes to casbin if deprecation warnings are put in place asap and users are given enough time to adjust (i.e. almost an year).

We can have node-casbin as separate package, but I do not really see a point of having a git repository for something that includes pretty much 2 imports.

Sefriol avatar May 08 '21 09:05 Sefriol

@Sefriol No breaking changes for Node-Casbin NPM package. This is mandatory. If you view GitHub stars, we can easily know that Node-Casbin doesn't rank first among all Node or JS authorization libraries. We cannot afford breaking changes now to diverge our users.

hsluoyz avatar May 08 '21 09:05 hsluoyz

Hey @hsluoyz, just wondering what is best way forward with this then? Perhaps keeping Node-Casbin package as is then, and maybe creating another JS casbin package that is meant to be used on clients eg React Native and React Web frontend? Wonder what your thoughts are on best way forward? :)

harveyappleton avatar Jun 03 '21 08:06 harveyappleton

@harveyappleton As for now, we chose to make a cross-platform core library, and then applied other API to it to create browser or node version.

Zxilly avatar Jun 03 '21 14:06 Zxilly

Hello @Sefriol, @harveyappleton, casbin.js@next has been released on NPM, you can use yarn add casbin.js@next or npm install casbin.js@next --save to add this to your project, how to use:

import { newEnforcer, newModel, MemoryAdapter } from 'casbin.js';

const model = newModel(`
[request_definition]
r = sub, obj, act

[policy_definition]
p = sub, obj, act

[role_definition]
g = _, _

[policy_effect]
e = some(where (p.eft == allow))

[matchers]
m = g(r.sub, p.sub) && r.obj == p.obj && r.act == p.act
`);

const adapter = new MemoryAdapter(`
p, alice, data1, read
p, bob, data2, write
p, data2_admin, data2, read
p, data2_admin, data2, write

g, alice, data2_admin
`);


const enforcer = await newEnforcer(model, adapter);

nodece avatar Aug 22 '21 08:08 nodece

I reopen this issue because I thought this issue once, and I don't suggest that we make a new repo to provide a pure JavaScript project, although we've made a repository https://github.com/casbin/casbin-core

  1. The node-casbin adapters and watchers don't apply to casbin-core.
  2. We need to maintain two sets of codebase.

I know there's a lot of breaking change, it's inevitable.

Please let me know what you think. Thank you!

nodece avatar Sep 14 '22 14:09 nodece

So the plan is to make node-casbin platform independent and deprecate all functions that use core nodejs libraries like fs?

Sefriol avatar Sep 14 '22 21:09 Sefriol

So the plan is to make node-casbin platform independent and deprecate all functions that use core nodejs libraries like fs?

@Sefriol Yes, you are right.

nodece avatar Sep 15 '22 01:09 nodece

Looks node-casbin@beta has been platform independent

Zxilly avatar Sep 15 '22 04:09 Zxilly

Looks node-casbin@beta has been platform independent

Right! I think we should release a stable version, but this version with breaking changes, so @hsluoyz want to use a new repo and a new package instead of this repo and this package.

nodece avatar Sep 24 '22 11:09 nodece

Any update on this? We do not need the fs as we are doing everything in memory so would be nice to use a polymorphic library for both frontend and backend.

nsainaney avatar Apr 28 '23 20:04 nsainaney

@nsainaney node-casbin has a v6 version can be used in browser.

Zxilly avatar Apr 29 '23 03:04 Zxilly

Could you try casbin@latest? It can be run on the Web, Node.js, and so on.

Examples: https://github.com/node-casbin/casbin-examples

nodece avatar Apr 30 '23 15:04 nodece

Could you try casbin@latest? It can be run on the Web, Node.js, and so on.

Examples: https://github.com/node-casbin/casbin-examples

it can't use in browser of v-5.28.0, it need Buffer support.

router.js?v=b2d7efec:59  ReferenceError: Buffer is not defined

=========================

use globle to void it in nuxtjs

import { Buffer } from 'buffer'

globalThis.Buffer = Buffer

transtone avatar Jan 03 '24 23:01 transtone