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

Limit token access to certain API endpoints / parameters and HTTP methods #45

Open
stv0g opened this issue Nov 23, 2020 · 10 comments
Open

Comments

@stv0g
Copy link
Contributor

stv0g commented Nov 23, 2020

Allow the creation of tokens whose scope is limited to e.g.:

  • GETing /files/1234 (useful for sharing results)
  • PUTing /results/123124/file (useful for VILLAScontroller to upload results)
@stv0g
Copy link
Contributor Author

stv0g commented Nov 23, 2020

See #48 for an example use case.

Another use cases might be:

  • Sharing results / models by link
  • VILLAScontroller uploading results or downloading models with limited credentials

@stv0g
Copy link
Contributor Author

stv0g commented Nov 23, 2020

mentioned in issue web#145

@stv0g
Copy link
Contributor Author

stv0g commented Nov 23, 2020

I started working on it in the branch token-roles

After reviewing the code, I realized that it makes more sense to limit the token to a specific role rather than an API endpoint/route + method.

@stv0g
Copy link
Contributor Author

stv0g commented Nov 24, 2020

In GitLab by @skolen on Nov 24, 2020, 12:10

Could you push your branch so that I can have a look?

The role of a user defines its access rights to a great extent. We could introduce a default "SharingUser" with the role "Guest" and add an endpoint that returns an access token for the SharingUser instead of the requesting user. The rights of Guest users are tailored so that they cannot change anything in the DB. What do you think about this idea?

@stv0g
Copy link
Contributor Author

stv0g commented Nov 24, 2020

I briefly checked the code. It seems to me that there are only a handful of locations where the database.CtxUserId & database.CtxUserRole are used (mostly creation of resources which include the user who created the resource).

I like the idea of a "SharingUser". But I would rather call it "Service".

One thing I realized when looking at the code:

I would try to eliminate the CtxUserRole from the global context and replace it by permissions.
Currently the CRUD permissions are a bit limiting. I would extend the CRUD permissions to a more general concept of permissions.
E.g. "CanViewAllScenarios" could be a dedicated permission instead of checking against the "Admin" role.
That way all the route endpoint code would only check for permissions and have no clue about what roles are.
That would allow for a cleaner separation of the application logic and authentication system.
For examples, the new service tokens (for sharing, VILLAScontroller, etc..) are usually not tied to a role rather than a set of specific permissions.

What do you think?

@stv0g
Copy link
Contributor Author

stv0g commented Nov 24, 2020

Or we simply inherit the UserID claim from the user who generated the token.
Thats kind of like GitLab is doing it..

So we have service tokens with a limited reduced set of permissions which are derived from the user permissions.

@stv0g
Copy link
Contributor Author

stv0g commented Nov 24, 2020

So all users can create tokens as long as the new tokens don't gain additional permissions.

@stv0g
Copy link
Contributor Author

stv0g commented Dec 3, 2020

In GitLab by @skolen on Dec 3, 2020, 14:03

Maybe I don't fully get what you mean...

The role of a user is associated with a set of CRUD permissions (at least one set of permissions for each route package). Admin role has full access for example and guest role mostly only read access.
The endpoints check if the requesting user has a role with the CRUD permissions to perform the requested operation. We can add flexible sets of CRUD permissions for different purposes.

Currently, the user role (UserRoleCtx) is extracted from the gin context for this purpose (see function ValidateRole in package database). We could avoid using the UserRoleCtx here if we would use the UserIDCtx instead and query the database to determine the requesting user and its role from the data model.

We can add a "Service" role that has very limited CRUD permissions only for specific endpoints.

If I get your point, this is close to how you are suggesting to do it. Right?

@stv0g
Copy link
Contributor Author

stv0g commented Mar 29, 2021

mentioned in issue web#309

@stv0g
Copy link
Contributor Author

stv0g commented Nov 25, 2021

In GitLab by @skolen on Nov 25, 2021, 15:35

Is this still a feature that we should implement?
Or can we live with the existing possibilities?

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

No branches or pull requests

1 participant