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.

Sign up for GitHub

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

Jump to bottom

Separate responsibilities of Redshift connection types #1562

Open
dlpzx opened this issue Sep 18, 2024 · 0 comments
Open

Separate responsibilities of Redshift connection types #1562

dlpzx opened this issue Sep 18, 2024 · 0 comments
Assignees
@dlpzx
Labels
effort: medium priority: high type: enhancement Feature enhacement

Comments

@dlpzx
Copy link
Contributor

dlpzx commented Sep 18, 2024 •
edited
Loading

Is your idea related to a problem? Please describe.

Current connection model
Right now we have 2 types of connections DATA_USER and ADMIN and connections are owned by a single Team (IdP group)

  • DATA_USER connections are fine. They are owned by data producers which are the ones publishing the data
  • ADMIN connections at the moment are owned by data consumer teams and needed as a requisite to create a share → wrong! they should be owned by Redshift admin teams. They can also be used to publish data.

New proposed connection model

Change ownership model of ADMIN connections

1️⃣ OPTION1: All teams in the Environment can use the connection-details

  • Modify listConnection API backend to retrieve all connections in user Environments
  • Add checks in createShareObject for Redshift principals, the groups must be in the Environments

The main issue with this option is that Redshift admins effectively lose control over who can create share requests for any namespace in the environment. There is no way to restrict create-share permissions to just one cluster.

2️⃣ DECIDED OPTION2: Connection admins can grant permissions to other groups to use the connections
Implementation: #1573

The ADMIN Connection owners correspond to the real admin team of a Redshift clusters. They have the power to decide which other teams can open share requests for the Connection namespace.

Some design considerations

  • Only the connection owners (redshift admins) will be able to grant/revoke/list group permissions to the connection.
  • Only connections of the type ADMIN will have grantable permissions (permission to be used to open a share request)
  • The design should be extensible to more types of permissions in the future

Implementation high-level steps

  • New FE window to grant permissions to teams to use the connection
  • New permission in permissions table - CREATE_REDSHIFT_SHARE_WITH_CONNECTION
  • New API calls to grant permissions, to list permissions, to remove team permissions
  • Modify listConnections API backend to retrieve user connections based on connection_permissions
  • Add checks in createShareObject for Redshift principals, the user must have permissions to the Connection
@dlpzx dlpzx added this to v2.7.0 Sep 18, 2024
@github-project-automation github-project-automation bot moved this to Nominated in v2.7.0 Sep 18, 2024
@dlpzx dlpzx moved this from Nominated to In progress in v2.7.0 Sep 18, 2024
@dlpzx dlpzx self-assigned this Sep 18, 2024
@dlpzx dlpzx mentioned this issue Sep 20, 2024
Merged
dlpzx added a commit that referenced this issue Sep 23, 2024
@dlpzx
Add Redshift connection tooltips and info + restrict to DATA_USER con…
95e278c 
…nections for import Redshift Dataset (#1565)

### Feature or Bugfix
- Feature: enhancement

### Detail
This feature is an enhancement suggested by Redshift experts on #955,
which is well explained in #1562.
This PR:
- adds more info and tooltips that explain details about Redshift
Connections on the UI
- restricts the type of connection that can be used to import a dataset:
ONLY DATA_USER CONNECTIONS CAN BE USED TO IMPORT DATASETS. It implements
this logic both in the frontend and backend

FIRST VERSION:
<img width="1126" alt="image"
src="https://github.com/user-attachments/assets/14bb5a85-9868-4e8d-b7aa-1c84feb2a681">

UPDATED:

![image](https://github.com/user-attachments/assets/1b199dba-d6ee-471f-9cd7-d74e70b8dd4b)


### Relates
#1562 

### Security
Please answer the questions below briefly where applicable, or write
`N/A`. Based on
[OWASP 10](https://owasp.org/Top10/en/).

- Does this PR introduce or modify any input fields or queries - this
includes
fetching data from storage outside the application (e.g. a database, an
S3 bucket)?
  - Is the input sanitized?
- What precautions are you taking before deserializing the data you
consume?
  - Is injection prevented by parametrizing queries?
  - Have you ensured no `eval` or similar functions are used?
- Does this PR introduce any functionality or component that requires
authorization?
- How have you ensured it respects the existing AuthN/AuthZ mechanisms?
  - Are you logging failed auth attempts?
- Are you using or adding any cryptographic features?
  - Do you use a standard proven implementations?
  - Are the used keys controlled by the customer? Where are they stored?
- Are you introducing any new policies/roles/users?
  - Have you used the least-privilege principle? How?


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
@dlpzx dlpzx mentioned this issue Sep 24, 2024
Merged
9 tasks
SofiaSazonova pushed a commit that referenced this issue Oct 2, 2024
@dlpzx
New permission model for Redshift ADMIN connections (#1573)
34d5651 
### Feature or Bugfix
- Feature

### Detail
Implements new way of transferring connection permissions as explained
in #1562

This PR introduces a new frontend window to deal with connection
permissions.
- [X] Add button in column in the Connections tab - disabled for
DATA_USER connections
- [X] Create new window where admins can grant other groups permissions
to the connection.
   - add group premissions
   - remove group permissions
- groupOptions is updated depending on the environment groups that still
do not have permissions to the connection

At the moment it only allows to select the CREATE_SHARE_REQUEST
permission, which is hardcoded in the UI view. If in the future we
introduce more permissions we can "easily" read those grantable
permissions from the backend and update the FE view and replace the
current chip for a dropdown of permissions.,

4 new APIs to manage the permissions to the Connection:
- [X] add permissions for a group to a connection
- [X] delete permissions for a group to a connection
- [X] list group permissions for a connection
- [x] list groups in an environment without any permissions to a
connection

New permission in connection_permissions + new db functions for the
above.
- MANAGE_CONNECTION_PERMISSIONS - to separate from GET_CONNECTION
permissions which is very generic and could be granted to other teams.
- CREATE_SHARE_REQUEST

In the share request view there are no changes in the frontend, but in
the backend the listConnections API db query has been modified to return
also the connections of a user if the user belongs to a group with
permissions.
- [x] Ensure in the backend that only users with permissions to a
connection can open a share request for that connection



In addition we need to:
- [X] Add unit tests
- [x] Update documentation

### Coverage of unit tests for Redshift
For the `redshift_datasets` module:

<img width="648" alt="image"
src="https://github.com/user-attachments/assets/12b66038-e2b7-4321-94dc-80fcc4a24022">


For the `redshift_datasets_shares` module:


<img width="598" alt="image"
src="https://github.com/user-attachments/assets/9024cad4-05f6-422c-9d5a-a972107f987b">

## Testing
- tested locally
- tested CICD pipeline completes, UI view for connections updated!
### Relates
- #1562 

### Security
Please answer the questions below briefly where applicable, or write
`N/A`. Based on
[OWASP 10](https://owasp.org/Top10/en/).

- Does this PR introduce or modify any input fields or queries - this
includes
fetching data from storage outside the application (e.g. a database, an
S3 bucket)?
  - Is the input sanitized?
- What precautions are you taking before deserializing the data you
consume?
  - Is injection prevented by parametrizing queries?
  - Have you ensured no `eval` or similar functions are used?
- Does this PR introduce any functionality or component that requires
authorization?
- How have you ensured it respects the existing AuthN/AuthZ mechanisms?
  - Are you logging failed auth attempts?
- Are you using or adding any cryptographic features?
  - Do you use a standard proven implementations?
  - Are the used keys controlled by the customer? Where are they stored?
- Are you introducing any new policies/roles/users?
  - Have you used the least-privilege principle? How?


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dlpzx dlpzx

Labels
effort: medium priority: high type: enhancement Feature enhacement
Projects
v2.7.0
Status: Done
Milestone
No milestone
Development

No branches or pull requests
1 participant
@dlpzx