GALAglobal / TAPICC-API-implementation

TAPICC API implementation using node.js framework sails.js
Other
6 stars 1 forks source link

Permissions - proposal #52

Closed Alino closed 5 years ago

Alino commented 6 years ago

Each of these data models (Job, Asset, Task, Webhook) should have ownerId property. Which will be pointing to the Account id, of the owner of these resources.

Based on this value, we would be able to implement permission check, to display Jobs, Assets, Tasks and Webhooks only to the owner, if he is authenticated.


Do you agree, or would you do it differently?

ysavourel commented 6 years ago

This does not sounds right to me. Having an ownerId in the TAPICC models seems to go in the direction to have prescribed ways to implement the backend.

So far we have kept away from having any kind of customer/user information attached to the TAPICC objects because there is the assumption it is handled by the implementations. And those implementations may have very different ways to handle such things.

It seems the token is enough for linking a TAPICC call to permissions/ownership/etc. in the backend. The token is “simply” associated with a user and its metadata (e.g. CustomerId, roles, etc.)

For example, when a client does a GET /jobs/{jobId} the underlying implementation would do something like this:

  1. Get the token from the request.
  2. Check that it is a valid token and retrieve the metadata information associated with it.
  3. Retrieve the implementation's job data, based on jobId and the other data, opaque to TAPICC (e.g. a CustomerId)
  4. Build the TAPICC Job response object with only the relevant fields.

For instance, the implementation may store the jobs in a table with:

I think we are having sometimes difficulty to separate the TAPICC-API-Implementation (a specific implementation) and TAPICC itself, because it is very easy to unintentionally blend both together.

That said, I'm probably also bias to my own view of how I would implement TAPICC with our system. So, it'd be nice to get other inputs on this.

Alino commented 6 years ago

So far we have kept away from having any kind of customer/user information attached to the TAPICC objects because there is the assumption it is handled by the implementations. And those implementations may have very different ways to handle such things.

I agree.

Another way how it could be done is to create a new collection, let's call it "JobPermission" It would be a table which would look like this

id jobId accountId
1 1 2
2 2 2
3 3 3

Everytime a request is made to some resource, it would check for the jobId inside this table to see if the current api_key holder has access to it.

This would enable us to have multiple accounts access to the same resource. It would be also independent from the swagger definitions data models.

rmraya commented 6 years ago

If you want to manage permissions in TAPICC, then you also have to manage users in TAPICC.

Alino commented 6 years ago

we are managing system accounts already in this implementation, independently from TAPICC specification.

rmraya commented 6 years ago

You need a user id to set ownership.

If user accounts must be independent from TAPICC, then permissions should also be independent.

Alino commented 6 years ago

Yes, the user id in this implementation is called accountId as mentioned in the table above. Permissions will be independent from TAPICC specification, they are not mentioned in swagger definition at all, and there are no plans to do so.

Whole user/account management is independent from TAPICC spec.