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

16815 authz design #17094

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

16815 authz design #17094

wants to merge 11 commits into from

Conversation

jalbinson
Copy link
Collaborator

This PR documents a new design for authorizing endpoints in ReportStream.

Linked Issues

@jalbinson jalbinson requested a review from a team as a code owner January 17, 2025 19:54
Copy link
Contributor

github-actions bot commented Jan 17, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Manifest Files

@jalbinson
Copy link
Collaborator Author

Copy link
Contributor

github-actions bot commented Jan 17, 2025

Test Results

1 274 tests  ±0   1 270 ✅ ±0   7m 40s ⏱️ +21s
  167 suites ±0       4 💤 ±0 
  167 files   ±0       0 ❌ ±0 

Results for commit 6103dc2. ± Comparison against base commit 90921c8.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jan 17, 2025

Integration Test Results

 60 files   60 suites   44m 5s ⏱️
428 tests 418 ✅ 10 💤 0 ❌
431 runs  421 ✅ 10 💤 0 ❌

Results for commit 6103dc2.

♻️ This comment has been updated with latest results.

prime-router/docs/authz/authz-design.md Outdated Show resolved Hide resolved
prime-router/docs/authz/authz-design.md Outdated Show resolved Hide resolved
prime-router/docs/authz/authz-design.md Show resolved Hide resolved
prime-router/docs/authz/authz-design.md Outdated Show resolved Hide resolved
…/prime-reportstream into platform/jamie/16815-authz-design
Copy link

@tsurya tsurya left a comment

Choose a reason for hiding this comment

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

LGTM

| org:write | Able to update organizations | ReportStream-OrgAdmin |
| org:read | Able to access read-only information related to organizations | ReportStream-User |
| submit | Able to submit reports | ReportStream-Submit |
| elims | Special `elims` specific actions (see below) | ReportStream-Elims |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm I don't really like the idea of this, it's coupled to a particular customer's use case. Instead we should look to try to understand the use case in a "generic" way. For example, what would a generic name for this scope be? I think there wouldn't be one because its a mishmash to serve a weird edge case?

I think I would prefer we don't support this and instead force users to create new accounts if they want a particular role for a particular organization? So that means:

If a user has a "scope group" those scopes will apply to all "org groups". If we want a user to have different "scope groups" for different "org groups" that will not be supported and instead a new user account should be created. This makes sense to me because from an organizational perspective that IS a different user.

For example, if I worked for CDC and Palantir, I would have two different accounts, one for each organization, and if the two companies work closely together and communicate via slack, I could log in to slack with either of my two different accounts and have different permissions depending on what account I am signed in under.

Copy link
Collaborator Author

@jalbinson jalbinson Jan 24, 2025

Choose a reason for hiding this comment

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

For example, what would a generic name for this scope be? I think there wouldn't be one because its a mishmash to serve a weird edge case?

We could break it out into something like the following if it would be more clear.

submissions
daily-data

If a user has a "scope group" those scopes will apply to all "org groups". If we want a user to have different "scope groups" for different "org groups" that will not be supported and instead a new user account should be created. This makes sense to me because from an organizational perspective that IS a different user.

I'm a little confused on this. Okta groups are serving multiple purposes in this design. The ReportStream-* groups serve as a way to show what users are able to request certain scopes. The org groups are the more standard md-phd type showing what organization your scope will apply to.

For example, if I worked for CDC and Palantir, I would have two different accounts, one for each organization, and if the two companies work closely together and communicate via slack, I could log in to slack with either of my two different accounts and have different permissions depending on what account I am signed in under.

I didn't particularly like this idea but it greatly simplifies authz setup. I also think it wouldn't be terribly common as most users are tied to a specific organization.

Copy link
Collaborator

@arnejduranovic arnejduranovic Jan 24, 2025

Choose a reason for hiding this comment

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

Okay I think I figured out at least part of what's not sitting right with me. I think we are actually trying to combine user/group permissions (scopes) with organization settings. For example: If we want to give CDC_ELIMS organization permission to view NY's daily data page, then that's not a new scope, but a new organization setting (managed in RS settings database), like so:

NY_DPH
    - allow-daily-data-access: [CDC_ELIMS]

Copy link
Collaborator

Choose a reason for hiding this comment

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

UPDATE/TODO: Jamie, Surya, and I met on call and discussed this in more detail. We agreed that we should look into, and document, the possibility of splitting the "elims" scope into "organization settings". Concrete todo tasks:

  • Investigate how/where "organization settings" can be managed and evaluate pro/cons and document desired solution. Some possibilities we discussed were storing settings in:
    • Okta itself
    • RS Postgres DB
    • New DB/table in Azure
  • Think through what "organization settings" we would want and what use-case they would serve. For example:
    • allow-daily-data-access: [Organization_B] allows Organization_B access to the Daily Data page of the organization the setting belongs to. Review the mural linked in the ticket/epic to ensure we cover all documented needs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Update this section

| org:write | Able to update organizations | ReportStream-OrgAdmin |
| org:read | Able to access read-only information related to organizations | ReportStream-User |
| submit | Able to submit reports | ReportStream-Submit |
| elims | Special `elims` specific actions (see below) | ReportStream-Elims |
Copy link
Collaborator

Choose a reason for hiding this comment

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

UPDATE/TODO: Jamie, Surya, and I met on call and discussed this in more detail. We agreed that we should look into, and document, the possibility of splitting the "elims" scope into "organization settings". Concrete todo tasks:

  • Investigate how/where "organization settings" can be managed and evaluate pro/cons and document desired solution. Some possibilities we discussed were storing settings in:
    • Okta itself
    • RS Postgres DB
    • New DB/table in Azure
  • Think through what "organization settings" we would want and what use-case they would serve. For example:
    • allow-daily-data-access: [Organization_B] allows Organization_B access to the Daily Data page of the organization the setting belongs to. Review the mural linked in the ticket/epic to ensure we cover all documented needs.

auth/docs/setup.md Outdated Show resolved Hide resolved
prime-router/docs/authz/authz-design.md Outdated Show resolved Hide resolved
Copy link

sonarqubecloud bot commented Feb 3, 2025

| org:write | Able to update organizations | ReportStream-OrgAdmin |
| org:read | Able to access read-only information related to organizations | ReportStream-User |
| submit | Able to submit reports | ReportStream-Submit |
| submissions | Able to access submissions page | ReportStream-submissions |
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No longer required since both covered by org:read

```

At a code level:
- ensure the user contains the `daily-data` scope
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

simplify because daily-data page is only 1 receiver at a time

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

Successfully merging this pull request may close these issues.

Design and Document Modern and Secure AuthZ Strategy First Draft
3 participants