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

A Model for top-level LTS Permission #832

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

bdu-birhanu
Copy link
Contributor

@bdu-birhanu bdu-birhanu commented Oct 8, 2024

Pull Request

Overview

Proposed Changes

  • Added a section about top-level LTS permissions.
  • Added a table on permissions and responsibilities in shared LTS allocations.
  • Added a table on permissions and responsibilities in individual LTS allocations.

Related Issues

Fixes #654
Fixes #671

@bdu-birhanu bdu-birhanu marked this pull request as draft October 8, 2024 20:19
@bdu-birhanu bdu-birhanu marked this pull request as ready for review October 11, 2024 19:03
@wwarriner
Copy link
Contributor

Three main pieces of information needed to fully manage an LTS allocation:

  1. Access key (one per person, per allocation)
  2. Secret key (same)
  3. Allocation IAM (one per allocation)

1 + 2 grant full control of an allocation
3 can be used in policies

What does a steward have that can't be granted by policy? Bucket management.

@wwarriner
Copy link
Contributor

wwarriner commented Oct 15, 2024

Policies are treated like objects, to modify, must delete and put a new version. Must do a get, modify, put cycle to change even a small typo.

@bdu-birhanu bdu-birhanu marked this pull request as draft October 25, 2024 13:27
@wwarriner
Copy link
Contributor

Each person (identity) can be involved with managing multiple allocations, and will need a key set for each one. Each key set may be granted for different roles, depending on the person's functional title (PI, Core Director, everyone else) and their role in the allocation.

  • PI owner of a shared allocation
  • Core Director owner of a shared allocation
  • Steward of a shared allocation, regardless of title
  • Owner of an individual allocation, regardless of title

@wwarriner
Copy link
Contributor

wwarriner commented Oct 30, 2024

Let's arrange the text into sections like this:

  • Terminology

  • Shared Allocations

    • Functional Roles (Owner, Steward, Member)
      • Make a table of permissions and responsibilities
      • Roles are columns
      • Permissions are rows:
        • responsible for allocation (owner only)
        • create bucket, delete bucket, rename bucket (owner and steward)
        • all other operations (owner and steward get mark, "member" gets text like "per bucket policy")
    • Who can have what role?
      • Owners must be Core Directors or a Lab PI.
      • Stewards may be anyone designated by an owner.
      • Members may be anyone with access to the system, i.e., has created an individual allocation and a Cheaha account.
    • How do I gain access to my role?
      • Owners and stewards have key sets for the allocation(s) they manage. These key sets are distinct, one per person per allocation, and separate from the key set they use for their individual allocations.
      • Members are granted access by bucket policy for each bucket they need access to.
  • Individual Allocations

@bdu-birhanu bdu-birhanu marked this pull request as ready for review November 1, 2024 16:30
@bdu-birhanu bdu-birhanu marked this pull request as draft November 7, 2024 20:58
@bdu-birhanu bdu-birhanu marked this pull request as ready for review November 8, 2024 22:37
Copy link
Contributor

@wwarriner wwarriner left a comment

Choose a reason for hiding this comment

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

I think this is good, it needs a bit of tweaking, and I raised a question.

My question: should we merge this with "Bucket Permissions" and call that page "Access Control"? What are your thoughts?

docs/data_management/lts/top_level_lts_permission.md Outdated Show resolved Hide resolved
docs/data_management/lts/top_level_lts_permission.md Outdated Show resolved Hide resolved
docs/data_management/lts/top_level_lts_permission.md Outdated Show resolved Hide resolved
mkdocs.yml Outdated Show resolved Hide resolved
@wwarriner wwarriner added the pr: needs changes Review complete, needs changes label Nov 12, 2024
@bdu-birhanu bdu-birhanu marked this pull request as draft November 13, 2024 15:11
@bdu-birhanu bdu-birhanu marked this pull request as ready for review November 13, 2024 21:58
@bdu-birhanu bdu-birhanu added pr: review PR is ready for review and removed pr: needs changes Review complete, needs changes labels Nov 15, 2024
Copy link
Contributor

@wwarriner wwarriner left a comment

Choose a reason for hiding this comment

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

This is great, let's pivot to Identity and Access Management (IAM) instead of Access Control or Permissons. Don't use the term "Access Control" nor "Permissions" since those are S3 concepts we aren't documenting yet.

docs/data_management/lts/policies.md Outdated Show resolved Hide resolved
docs/data_management/lts/policies.md Outdated Show resolved Hide resolved
docs/data_management/lts/policies.md Show resolved Hide resolved
mkdocs.yml Outdated Show resolved Hide resolved
@wwarriner wwarriner added pr: needs changes Review complete, needs changes and removed pr: review PR is ready for review labels Nov 20, 2024
@bdu-birhanu bdu-birhanu added pr: review PR is ready for review and removed pr: needs changes Review complete, needs changes labels Dec 3, 2024
Copy link
Contributor

@wwarriner wwarriner left a comment

Choose a reason for hiding this comment

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

Great work! I think this is the final round of review, probably.

In addition to specific comments, let's be consistent with capitalizing "Core" when referring to the concept of a "Research Core".

docs/data_management/lts/iam_and_policies.md Outdated Show resolved Hide resolved
docs/data_management/lts/iam_and_policies.md Outdated Show resolved Hide resolved
docs/data_management/lts/iam_and_policies.md Outdated Show resolved Hide resolved
docs/data_management/lts/iam_and_policies.md Outdated Show resolved Hide resolved
docs/data_management/lts/iam_and_policies.md Outdated Show resolved Hide resolved
@wwarriner wwarriner added pr: needs changes Review complete, needs changes and removed pr: review PR is ready for review labels Dec 11, 2024
@bdu-birhanu bdu-birhanu added pr: review PR is ready for review and removed pr: needs changes Review complete, needs changes labels Dec 13, 2024
Copy link
Contributor

@wwarriner wwarriner left a comment

Choose a reason for hiding this comment

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

One typo then ready to go.

docs/data_management/lts/iam_and_policies.md Outdated Show resolved Hide resolved
@wwarriner wwarriner added pr: needs changes Review complete, needs changes and removed pr: review PR is ready for review labels Dec 20, 2024
@bdu-birhanu bdu-birhanu added pr: review PR is ready for review and removed pr: needs changes Review complete, needs changes labels Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: review PR is ready for review
Projects
None yet
2 participants