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

#960 feat(UAC): change default ownership to admins for externally created resources #2137

Merged
merged 10 commits into from
Aug 19, 2018

Conversation

ricardona
Copy link
Contributor

@ricardona ricardona commented Aug 7, 2018

This change includes GUI, rest API, DB and migration

The default ownership will be:

Container <without ownership> --> Admin
Container <with ownership>    --> Public/Private

Closes #960

Ricardo Cardona Ramirez and others added 6 commits August 2, 2018 12:22
Deprecated AdministratorsOnly js and go backend
Update swagger definition changing AdministratorsOnly to Public
…tions

On stacks, containers networks, services , tasks and volumes.
 The administrator resources are deleted and Public resources are now managed by admins
Copy link
Member

@deviantony deviantony left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this @ricardona and sorry for the late review.

Small changes required and a question, see my comments.

api/bolt/migrator/migrator.go Outdated Show resolved Hide resolved
app/portainer/services/api/resourceControlService.js Outdated Show resolved Hide resolved
api/http/proxy/containers.go Show resolved Hide resolved
@deviantony deviantony added changes-required Waiting for an update of the contributor and removed contrib/func-review-in-progress labels Aug 13, 2018
Copy link
Member

@deviantony deviantony left a comment

Choose a reason for hiding this comment

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

@ricardona I've request other changes, mainly to make the codebase a bit more easier to read. Please have look at my comments.

api/http/handler/stacks/stack_delete.go Outdated Show resolved Hide resolved
api/http/handler/stacks/stack_file.go Outdated Show resolved Hide resolved
api/http/handler/stacks/stack_inspect.go Outdated Show resolved Hide resolved
api/http/handler/stacks/stack_migrate.go Outdated Show resolved Hide resolved
api/http/handler/stacks/stack_update.go Outdated Show resolved Hide resolved
api/http/proxy/access_control.go Outdated Show resolved Hide resolved
api/http/proxy/containers.go Show resolved Hide resolved
api/http/security/authorization.go Outdated Show resolved Hide resolved
api/http/security/authorization.go Outdated Show resolved Hide resolved
api/http/security/authorization.go Outdated Show resolved Hide resolved
@deviantony deviantony added contrib/tech-review-in-progress and removed changes-required Waiting for an update of the contributor labels Aug 15, 2018
Copy link
Member

@deviantony deviantony left a comment

Choose a reason for hiding this comment

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

LGTM

@deviantony deviantony added contrib/tests-in-progress breaking-changes This pull request introduces breaking changes and removed contrib/tech-review-in-progress labels Aug 15, 2018
@deviantony deviantony added this to the 1.19.2 milestone Aug 15, 2018
@deviantony
Copy link
Member

@ncresswell played with this a bit, looks OK. Thing to note: all public resources will be updated to administrator restricted (be it default policy or assigned by the user).

@deviantony deviantony added changes-required Waiting for an update of the contributor and removed contrib/tech-review-approved labels Aug 18, 2018
Copy link
Member

@deviantony deviantony left a comment

Choose a reason for hiding this comment

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

Found an issue with Swarm related resources, they seem to be declared as public in the table views (services, configs, secrets) whereas they have a different access control policy assigned.

See the screenshots below:

portainer 11

portainer 12

@deviantony deviantony added contrib/tech-review-in-progress and removed breaking-changes This pull request introduces breaking changes changes-required Waiting for an update of the contributor labels Aug 19, 2018
Copy link
Member

@deviantony deviantony left a comment

Choose a reason for hiding this comment

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

LGTM

@deviantony deviantony added contrib/ready-to-merge breaking-changes This pull request introduces breaking changes and removed contrib/tech-review-in-progress labels Aug 19, 2018
@deviantony deviantony merged commit e1e263d into portainer:develop Aug 19, 2018
@deviantony
Copy link
Member

Thanks @ricardona 👍

@ncresswell ncresswell added this to Portainer Release 1.19.2 in Roadmap Aug 20, 2018
@deviantony deviantony removed this from Portainer Release 1.19.2 in Roadmap Aug 20, 2018
@WTFKr0
Copy link
Contributor

WTFKr0 commented Oct 2, 2018

Hi

We just migrated to 1.19.2 and, as expected, all our resources have migrated to admin only instead of public (we deploy apps outside of portainer)
I look for a parameter to change this behavior (default to public)
Is there a such parameter ?
Our use of portainer is to have an only portainer instance managing many clusters
Admin may have access to all clusters, but users get access only to certain endpoints
Users accessing a endpoint have all access on that endpint, so public is fine

As a workaroud, i'll soon write a batch process to change admin ownership to public, so that newly deployed apps are set to public

Thanx

@deviantony
Copy link
Member

I look for a parameter to change this behavior (default to public)
Is there a such parameter ?

There is none.

Although, we're planning on introducing UAC related labels in the next release, see: #1257 (comment)

Would that be of any help to you?

@WTFKr0
Copy link
Contributor

WTFKr0 commented Oct 2, 2018

Yes it should be nice with that
The API allow me to be very efficient, I just code my batch in few minutes to get what I want
So it's fine for me for now, just waiting for labels 😃
Thanx

@ricardona
Copy link
Contributor Author

Our use of portainer is to have an only portainer instance managing many clusters
Admin may have access to all clusters, but users get access only to certain endpoints
Users accessing a endpoint have all access on that endpint, so public is fine

@WTFKr0 Your use case is similar to #2190 Segregate logically Portainer resources using projects and it will be included in the next release too. Could that help?

@deviantony #1257 and #2190 are trying to solve the same problem "declare access privileges in the compose file or from docker CLI". Does that mean that one should be discarded?

@deviantony
Copy link
Member

@ricardona I don't think so.

#1257 aims to provide the ability to set-up access control policies via labels.

#2190 aims to introduce a new project entity as well as enhance Portainer views to support project visualization.

xAt0mZ pushed a commit that referenced this pull request Aug 25, 2022
* EE-3915 ui fixes on docker container pages

* Update createcontainer.html

Update label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-changes This pull request introduces breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants